Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Please do not deprecate "--without development test" flag #7531

Closed
DannyBen opened this issue Jan 1, 2020 · 18 comments
Closed

Please do not deprecate "--without development test" flag #7531

DannyBen opened this issue Jan 1, 2020 · 18 comments

Comments

@DannyBen
Copy link

DannyBen commented Jan 1, 2020

Removing this flag will complicate things in certain scenarios.

Consider this:

You have a docker-based Ruby application, and you are mounting the current folder to the container - as is a common practice when developing.

When the image is built, when using the bundle config without ... option, it creates the .bundle folder, not only in the docker container, but also in the host.

I believe that it would be less disruptive to keep these flags, and if you need to remove any complications in this flag that arise from it "remembering its state" - then do not remember its state. Let it generate the proper Gemfile.lock, without the requested groups, and the users should know that if they run it again, without these --without flags, they will get the full installation.

In other words - instead of removing the flag, make it non persistent.

@ioquatix
Copy link

ioquatix commented Jan 6, 2020

This equally applies to --deployment. So rather than creating a new issue, let's consider it part of the same issue.

@deivid-rodriguez
Copy link
Member

We're definitely open for feedback here, thanks!

Note that you can also use global configurations with bundle config --global without ... or environment variables like BUNDLE_WITHOUT (I think environment variables are a good fit for docker).

We definitely need to document all the options in the deprecation message, I'll work on this as soon as possible.

Do you think, even knowing these alternatives are available, that your suggestion will be less disruptive?

I believe that it would be less disruptive to keep these flags, and if you need to remove any complications in this flag that arise from it "remembering its state", " then do not remember its state.

Well, this is exactly what we're doing, but since things are going to break when we do this, we are deprecating the things that will break first and provide alternatives that won't break.

For example, if we stop remembering CLI flags, bundle install --path vendor/bundle && bundle exec foo will start failing because bundle exec will look for gems in the default location, not in vendor/bundle. If you instead explicitly tell bundler before hand where to look for gems, then things won't break.

@ioquatix
Copy link

ioquatix commented Jan 6, 2020

What is the motivation for this change?

@deivid-rodriguez
Copy link
Member

The motivation is that silently remembering CLI flags has historically caused a lot of issues for both beginner and experienced users of bundler. I think that's because this is not how CLI tools usually work, so it breaks user expectations. CLI flags are supposed to apply to the current command, nobody expects that they are remembered.

@DannyBen
Copy link
Author

DannyBen commented Jan 6, 2020

@deivid-rodriguez, thanks for the reply and additional details.

Of course, I cannot pretend like I know the nuances and complexities of bundler, and I am sure there are good reasons for these deprecations.

I did not consider the fact that bundler actually needs to know its "without" config for subsequent commands, coming after bundler install.

The environment variable is indeed a good option for docker, and other environment specific config, and it allows the user to configure without polluting the working directory or source control with possibly invalid configuration files.

Although bundler install --without development test is the more intuitive approach, I agree that if it "saves its state", then it is less ideal (and a little unconventional), and I understand that if it doesn't save its state, then it will cause other side effects.

@casperisfine
Copy link

casperisfine commented Jan 7, 2020

CLI flags are supposed to apply to the current command, nobody expects that they are remembered.

I think everyones agrees with that, and there's definitely a case for deprecating that remembering behavior.

However that doesn't make --deployment, --without etc useless.

On various CI or build system, being able to do the entire bundler thing in just one command rather than 4 or 5 is really important.

Is there any chance these flags would be kept, but simply not be remembered anymore ?

Edit: sorry looks like I missed part of one of your previous comments:

For example, if we stop remembering CLI flags, bundle install --path vendor/bundle && bundle exec foo will start failing because bundle exec will look for gems in the default location, not in vendor/bundle.

So I know understand why they can't be kept :/

It's really a shame.

@DannyBen
Copy link
Author

DannyBen commented Jan 7, 2020

On various CI or build system, being able to do the entire bundler thing in just one command rather than 4 or 5 is really important.

I totally agree that CI is a big part of my concerns as well, but why do you say 4 or 5 steps?
I think that the number would be 1.5 or 2 at most - am I missing something?

Old way

$ bundle install --without development test

New way

# temporary, for this command only
$ BUNDLER_WITHOUT="development test" bundle install
# similar to: RAILS_ENV=production rails server

# permanent, for the environment
$ export BUNDLER_WITHOUT="development test"
$ bundle install

New way in docker-land

ENV BUNDLER_WITHOUT development test
RUN bundle install

It looks like we are all in agreement that command line flags should be stateless, but we now also learn that bundler is stateful.

So these two notions, yield the conclusion that the minimum number of possible "moves" is 2:

  1. Set the environment
  2. Run bundler CLI

And therefore, the two proposed options - bundler config ... or BUNDLER_WITHOUT=... are actually the proper way to solve this.

No?

(Of course, there is that other option, to make bundler stateless - but that's a story for another "what if"...)

@casperisfine
Copy link

but why do you say 4 or 5 steps?
I think that the number would be 1.5 or 2 at most - am I missing something?

Because even though the issue mainly talk about --without, I'm using a bunch of other deprecated flags. e.g:

bundle install --clean --path /tmp/bundle --without development:test:debug.

So following the deprecation advice, that would be 3 bundle config ... followed by the final bundle install, so 4 commands.

And therefore, the two proposed options

Sure, it works. It's just that it's much more involved than the previous one command line.

On some system invoking multiple command is complicated, on other it's setting ENV that ain't super easy. And all that is much less discoverable than some flags in bundle install --help.

@DannyBen
Copy link
Author

DannyBen commented Jan 7, 2020

I totally agree that this should not take more than 2 commands at most, since it is a common need to have more than one bundler environments.

Perhaps the reasonable conclusion should then be, to have a single environment variable, that simply points to a config file?

I was skimming this manual page and didn't find anything.

Maybe:

$ export BUNDLE_CONFIG=bundler-production.conf
$ bundle install

and then, we can commit these configs to git repositories without harming anything, and still have 2 commands at most, and environment persistence?

This achieves all of the goals, and even provides improved clarity to beginners. No?

@masterkain
Copy link

masterkain commented Jan 18, 2020

I'm trying to adjust to the changes upgrading from 1.x to 2.1.x and here's some findings:

previous deployment flag:

bundle install --deployment --without development test --jobs 5 --retry 3

current:

bundle config set deployment 'true'
bundle config set without 'development test'
bundle install --jobs 5 --retry 3

the problem I see with this is that if this code runs in a ruby:2.6-alpine docker image (as part of a build step in a CI):

  • with the current version BUNDLE_PATH env variable will take precedence over the vendor/bundle one that is supposed to be set by the deployment flag/config, and the ruby image will have that set

  • with the previous version I had no issue

overriding BUNDLE_PATH manually in the image with the full path will make things work again, but in this case the flag is pointless

I also tried with bundle config set --local deployment 'true' but I still have the same issue, bundle will install in a BUNDLE_PATH if set

@deivid-rodriguez
Copy link
Member

Official docker images no longer set the BUNDLE_PATH, so this should be ok if you upgrade I think.

@ioquatix
Copy link

ioquatix commented Feb 2, 2020

I'm just trying to get my head around these changes.

koyoko% bundle config set deployment true
koyoko% ls -la
total 0
drwxr-xr-x  2 samuel users   40 Feb  2 20:22 .
drwxrwxrwt 23 root   root  1780 Feb  2 20:22 ..

Shouldn't some kind of config file be created?

@ioquatix
Copy link

ioquatix commented Feb 2, 2020

I figured out I need to add --local.

system("bundle", "config", "set", "--local", "deployment", "true")
system("bundle", "config", "set", "--local", "without", "development test")

This is what I added to my automated server setup script.

Does that seem right?

Then, I should change my post-update hook from:

bundle install --deployment --jobs=4 --retry=2 --quiet

to

bundle install --jobs=4 --retry=2 --quiet

@ioquatix
Copy link

ioquatix commented Feb 2, 2020

Now that I grok this change, I fully support it. I've updated my deployment scripts... so I no longer use the flag variant.

Wondering if I should also use config to set jobs and retry parameters?

@deivid-rodriguez
Copy link
Member

Yeah, if you want them to be applied to all bundle install invokations you should configure them. I didn't deprecate them because I considered that they do make sense in the context of a single command, and not persisting them anymore shouldn't affect other commands.

@ioquatix
Copy link

ioquatix commented Feb 2, 2020

how do you persist those flags? (--jobs=4 --retry=2)

@DannyBen
Copy link
Author

DannyBen commented Feb 2, 2020

how do you persist those flags? (--jobs=4 --retry=2)

Can we please keep this on topic, and not turn it into a support thread? People are getting notified with every post...

@indirect
Copy link
Member

indirect commented Feb 2, 2020

@ioquatix yes, you can persist settings for those flags those via bundle config or by exporting env vars. The full list of configuration keys and their env var names is located in the man page for bundle config: https://bundler.io/man/bundle-config.1.html#LIST-OF-AVAILABLE-KEYS.

@DannyBen asking questions about how to handle the deprecation is on topic in a thread discussing the deprecation, imo. Thanks for opening this issue, since the thread has helped make it clear that we'll need to write some more documentation explaining the reason for the change and making it clearer how to resolve the deprecation warning. Since we aren't planning to add stateful flags back to Bundler, I'm going to close this ticket.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants