Long config options seem to be 'lost'

[Timbus]

So, bundler seems to be 'losing' my rather long build options whenever it updates the config:

$ bundle config thingy --asdf --fdsa --ty='oh man i hope this doesnt break bundler because that would suck' --ehhh='oh geez it looks like i might have broken bundler somehow' --very-important-option=DontDeleteRoot

$ bundle config thingy
Settings for `thingy` in order of priority. The top value will be used
Set for your local app (blah): "'--asdf --fdsa --ty=oh man i hope this doesnt break bundler because that would suck --ehhh=oh geez it looks like i might have broken bundler somehow"

$ bundle --version
Bundler version 1.12.0.rc

Contents of .bundle/config:

---
BUNDLE_THINGY: "--asdf --fdsa --ty=oh man i hope this doesnt break bundler because
  that would suck --ehhh=oh geez it looks like i might have broken bundler somehow
  --very-important-option=DontDeleteRoot"

Note that it can't seem to read the last line, specifically.

Annoyingly, it works if I manually fix up the config to only use one or two lines, but any time bundler tries to regenerate the config, it's broken again.

[segiddins]

That's very odd, as bundler uses the YAML module to serialize the config file

[Timbus]

Yeah, it looked like YAML output, but the reader seems to be doing something.. 'fancy'..

Especially since it's trying to show me the YAML pipe | operator if I add it to the config in an attempt to make it load more lines. Surely that should have been stripped..

[segiddins]

what's the output out ruby -ryaml -e 'p YAML.load(".bundle/config")' ?

[Timbus]

Sorry, didn't see this yesterday.

Yes the YAML module loads the config correctly, when using load_file.
{ "BUNDLE_THINGY"=>"--asdf --fdsa --ty=oh man i hope this doesnt break bundler because that would suck --ehhh=oh geez it looks like i might have broken bundler somehow --very-important-option=DontDeleteRoot" }

I have replicated this issue on an ubuntu machine and a red hat one, running different ruby and bundler versions. Are you not seeing the same issue?

[jbodah]

I was able to reproduce it on my machine. Problem seems to be related with the regex in Bundler::Settings#load_config

Changing to ^(BUNDLE_.+): (['"]?)(.*(?:(\n(?!BUNDLE).+)*)?)\2$ seems to work for the limited test sample I've tried. That thing is a bit of a beast though and it might be better to move away from regex

[Timbus]

Yeah, I actually fixed this for myself last weekend by changing load_config to:

def load_config(config_file)
  valid_file = config_file && config_file.exist? && !config_file.size.zero?
  if !ignore_config? && valid_file
    require "bundler/psyched_yaml"
    config = YAML.load_file(config_file)
    if config.is_a?(Hash)
      return config.select { |key|
        key.match(/^(BUNDLE_.+)/)
      }.map { |key, value|
        [convert_to_backward_compatible_key(key), value]
      }.to_h
    end
  end
  {}
end

I decided against making a pull request because I figured such a change might cause compatibility issues that I'm not really willing to put my name next to.

And I'm also pretty bad at ruby so, there's that too :P

[segiddins]

Yeah, we can't use the yaml module to read the config since it'd cause issues if the user has a different psych in their lockfile

[Timbus]

But you can use it to write the config? 😕

Well. Okay. The regex fix works well enough then.

[segiddins]

Yeah, since the config is never written to when using Bundler.setup, -rbundler/setup, or bundle exec

[Timbus]

I'm assuming your bundler/psyched_yaml module guarantees that your output format will always be the same, but I'm just having trouble reasoning why you bother saving it as YAML at all, if it can lead to strange errors like this. Why not just stick with a more basic format?

My 'bonus question' is.. why would an alternate psych version not be able to.. read YAML? Or is the problem something deeper than that?

[segiddins]

the problem is that we could activate the incorrect version of psych

[segiddins]

why you bother saving it as YAML at all

We have to, to be backwards compatible. There's a long history around this that has involved lots of pain for us, believe me :(

[Timbus]

the problem is that we could activate the incorrect version of psych

Ooooh. I get it. Well, carry on then. Thanks for fixing this.