Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bundler Deprecation warnings #943

Closed
mayrsascha opened this issue Dec 31, 2019 · 10 comments
Closed

Bundler Deprecation warnings #943

mayrsascha opened this issue Dec 31, 2019 · 10 comments

Comments

@mayrsascha
Copy link

I am getting deprecation warnings when deploying this buildpack:

       Running: bundle install --without development:test --path vendor/bundle --binstubs vendor/bundle/bin -j4 --deployment
       [DEPRECATED] The `--deployment` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set deployment 'true'`, and stop using this flag
       [DEPRECATED] The `--path` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set path 'vendor/bundle'`, and stop using this flag
       [DEPRECATED] The `--without` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set without 'development:test'`, and stop using this flag
       [DEPRECATED] The --binstubs option will be removed in favor of `bundle binstubs`

Using Ruby 2.7.0, BUNDLED WITH 2.1.2, Rails 6.0.2

It's not really a blocker, as the build passes, I just thought to bring it up here

@schneems
Copy link
Contributor

schneems commented Jan 7, 2020

Thanks for opening the issue. I'm not sure the best way to move forward here. I'll list out my concerns and my options.

Performance / Build Time

Running these bundle commands individually adds extra time:

$ time bundle --help > /dev/null

real	0m0.211s
user	0m0.152s
sys	0m0.052s

4 bundle commands is about 800ish ms which is pretty dang close to an extra second of build time. People are constantly looking to shave their build times so adding an extra second to every deploy isn't ideal.

Those commands aren't local

$ bundle config set deployment 'true'
You are replacing the current global value of deployment, which is currently "blerg"

Some tests in the buildpack are run as unit tests including some bundler tests that were isolated by directory. This behavior makes those much more fragile, we'll have to clean up more or maybe decide to get rid of those unit tests.

I'll also need to figure out where those values are stored globally and properly persist them across builds. Since they're not in the local .bundle/config directory I'll have to figure out how to move those files in a way that persists those settings from build to runtime.

Options

  1. Run the commands given by the output.

Consequences: Need to figure out how to switch around the files it generates/touches. Added 1 second to every ruby deploy.

  1. Use the bundle env vars. I documented this feature here https://devcenter.heroku.com/articles/bundler-configuration#environment-variable-behavior.

Consequences: Env var management between runtime and build time can be confusing as it seems to happen "automatically". This might surprisingly be the most bulletproof option unless they also plan on deprecating or removing env var based config.

  1. Something else we've not yet found or implemented. It's not too late for me to potentially implement some feature that makes this problem go away.

Possible options:

  • Enable setting an env var like BUNDLER_STICKY_CONFIG=true to use the old behavior.

Consequences: Bundler team was probably looking forward to getting rid of that code path, now instead they've still got to maintain it indefinitely behind an env flag. If their change is motivated by the desire to "clean up" this wouldn't be accepted. If their change is motivated only by the desire to be less surprising to the end-user, then I think it's a pretty good option.

  • Add (or find) a command line command that lets us set the local value instead of the global value.

Consequences: This doesn't help our time issue from above at all, and since older bundler versions don't have such a local config set command Heroku would have to maintain two bundler codepaths for the rest of time and remember that whenever one is updated, the other would have to be updated as well. It's doable, but not ideal. I don't like this option.

  • Some other option yet to be discussed

Consequences: Unknown

  • Do nothing

Consequences: Customers apps break eventually, not really an option but listing it for completeness.

Moving forward

Pending additional information or work on Bundler core, the best path forward is probably going to be to set the env vars configs so they're sticky at runtime and build time.

@dentarg
Copy link

dentarg commented Jan 7, 2020

If you haven't seen @schneems, the deprecation of these and other flags are being discussed in rubygems/bundler#7531

@hone
Copy link
Member

hone commented Jan 10, 2020

I think we can do option 2 without too many downsides. We can display output to the user with an extra print line above bundle install, so users can still see the configuration being set.

@QuantumWaver
Copy link

There been any updates here? Will Heroku users need to set those bundle env vars as per the option 2 above, or will this be handled in an update to the Heroku ruby buildpack? I ask mainly because I see that capistrano-rails has recently released an update for it:
https://github.com/capistrano/rails/releases/tag/v1.6.0

Thanks!

@schneems
Copy link
Contributor

Will Heroku users need to set those bundle env vars as per the option 2 above, or will this be handled in an update to the Heroku ruby buildpack?

I'll handle this in the buildpack. I've been sidelined by a series of nasty bugs.

I ask mainly because I see that capistrano-rails has recently released an update for it:
https://github.com/capistrano/rails/releases/tag/v1.6.0

Thanks! I'll take a look at that reference soon.

@QuantumWaver
Copy link

@schneems Thanks for the response, much appreciated! Hope you feel better!

@schneems schneems changed the title Deprecation warnings Bundler Deprecation warnings Jul 22, 2020
schneems added a commit that referenced this issue Jul 22, 2020
Starting with Bundler 2.x deprecation warnings started appearing noting that flags will no longer persist config when running `bundle install`:

```
       Running: bundle install --without development:test --path vendor/bundle --binstubs vendor/bundle/bin -j4 --deployment
       [DEPRECATED] The `--deployment` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set deployment 'true'`, and stop using this flag
       [DEPRECATED] The `--path` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set path 'vendor/bundle'`, and stop using this flag
       [DEPRECATED] The `--without` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set without 'development:test'`, and stop using this flag
       [DEPRECATED] The --binstubs option will be removed in favor of `bundle binstubs`
```

This is generally a confusing part of running bundler locally where someone on a team might try running `--bundle_without` once and then be confused at future behavior.

To support persisting values, we can use the bundler env vars that start with `BUNDLE_`. These will be set at:

- Build time
  - exported for other buildpacks
- Runtime

In addition, they're being shown in the `bundle install` output so if a developer gets a failure on Heroku they can copy and paste the command to test locally.
schneems added a commit that referenced this issue Jul 24, 2020
Starting with Bundler 2.x deprecation warnings started appearing noting that flags will no longer persist config when running `bundle install`:

```
       Running: bundle install --without development:test --path vendor/bundle --binstubs vendor/bundle/bin -j4 --deployment
       [DEPRECATED] The `--deployment` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set deployment 'true'`, and stop using this flag
       [DEPRECATED] The `--path` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set path 'vendor/bundle'`, and stop using this flag
       [DEPRECATED] The `--without` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set without 'development:test'`, and stop using this flag
       [DEPRECATED] The --binstubs option will be removed in favor of `bundle binstubs`
```

This is generally a confusing part of running bundler locally where someone on a team might try running `--bundle_without` once and then be confused at future behavior.

To support persisting values, we can use the bundler env vars that start with `BUNDLE_`. These will be set at:

- Build time
  - exported for other buildpacks
- Runtime

In addition, they're being shown in the `bundle install` output so if a developer gets a failure on Heroku they can copy and paste the command to test locally.
schneems added a commit that referenced this issue Aug 12, 2020
Starting with Bundler 2.x deprecation warnings started appearing noting that flags will no longer persist config when running `bundle install`:

```
       Running: bundle install --without development:test --path vendor/bundle --binstubs vendor/bundle/bin -j4 --deployment
       [DEPRECATED] The `--deployment` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set deployment 'true'`, and stop using this flag
       [DEPRECATED] The `--path` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set path 'vendor/bundle'`, and stop using this flag
       [DEPRECATED] The `--without` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set without 'development:test'`, and stop using this flag
       [DEPRECATED] The --binstubs option will be removed in favor of `bundle binstubs`
```

This is generally a confusing part of running bundler locally where someone on a team might try running `--bundle_without` once and then be confused at future behavior.

To support persisting values, we can use the bundler env vars that start with `BUNDLE_`. These will be set at:

- Build time
  - exported for other buildpacks
- Runtime

In addition, they're being shown in the `bundle install` output so if a developer gets a failure on Heroku they can copy and paste the command to test locally.
schneems added a commit that referenced this issue Aug 13, 2020
Starting with Bundler 2.x deprecation warnings started appearing noting that flags will no longer persist config when running `bundle install`:

```
       Running: bundle install --without development:test --path vendor/bundle --binstubs vendor/bundle/bin -j4 --deployment
       [DEPRECATED] The `--deployment` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set deployment 'true'`, and stop using this flag
       [DEPRECATED] The `--path` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set path 'vendor/bundle'`, and stop using this flag
       [DEPRECATED] The `--without` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set without 'development:test'`, and stop using this flag
       [DEPRECATED] The --binstubs option will be removed in favor of `bundle binstubs`
```

This is generally a confusing part of running bundler locally where someone on a team might try running `--bundle_without` once and then be confused at future behavior.

To support persisting values, we can use the bundler env vars that start with `BUNDLE_`. These will be set at:

- Build time
  - exported for other buildpacks
- Runtime

In addition, they're being shown in the `bundle install` output so if a developer gets a failure on Heroku they can copy and paste the command to test locally.
schneems added a commit that referenced this issue Aug 19, 2020
schneems added a commit that referenced this issue Aug 19, 2020
@schneems schneems reopened this Aug 19, 2020
@schneems
Copy link
Contributor

Re-opening due to #1046 (comment)

Merging this behavior in caused slug size to double. It looks like using environment variables is not a drop-in replacement for the arguments.

@SampsonCrowley
Copy link

SampsonCrowley commented Sep 30, 2020

EDIT AGAIN: is there a timeline on releasing this updated buildpack?

@schneems am I missing an obvious reason why the buildpack can't just use inline variables in place of argument flags? (BUNDLE_WITHOUT="development:test" bundle ....) inline variables override anything exported for the current command only; completely gone by the next line

EDIT: nevermind, I was misinterpretting what you meant by ENV vars. After reading the issue you reopened this for, I see inline variables is what you mean, not setting a heroku config var

@schneems
Copy link
Contributor

schneems commented Oct 5, 2020

I'm looking to release tomorrow

@SampsonCrowley
Copy link

@schneems sounds great, thanks for the update

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

No branches or pull requests

6 participants