Fix bundler handling of 'without' #13351
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
While this still is flagged as a WIP, the changes up through 67911c8 LGTM and make sense with the bundler docs. I imagine that something about our subclass-of-bundler implementation is causing the set_local to not align with internal changes in bundler. I would prefer a test, somewhere, that allows us to verify that certain gems are (or are not) included in the generated artifact. |
robbavey
force-pushed
the
bundle_without
branch
from
b6de2d4
to
038e767
Compare
Prior to this change, the values set in `set_local` are ignored when invoking bundler via the command line, as is used with `invoke!`. This commit sets those values in `ENV` variables instead, fixing the functionality to not install development gems.
Co-authored-by: Ry Biesemeyer <ry.biesemeyer@elastic.co>
* logstash service needs to be installed * gem_vendored? needs to use full path to vendor files * use `stdout` from `cat` command to generate spec temporary file
robbavey
changed the title
|
@yaauie Ready for another round - tests are now working |
yaauie
approved these changes
Oct 26, 2021
There was a problem hiding this comment.
Actual changes LGTM.
It's not exactly clear to me why we need to both do a set_local and invoke with_env for "path", "gemfile", and "without", but the tests seem to speak for themselves.
Could be improved by backing off the complexity of the gem_vendored? helper I contributed, since we don't use the block-form anyway.
robbavey
force-pushed
the
bundle_without
branch
from
d5bb507
to
f3fba7a
Compare
robbavey
added a commit
to robbavey/logstash
that referenced
this pull request
Oct 26, 2021
Backport PR elastic#13351 to 7.16 branch. Original message: * Fix bundler handling of 'without' Prior to this change, the values set in `set_local` are ignored when invoking bundler via the command line, as is used with `invoke!`. This commit sets those values in `ENV` variables instead, fixing the functionality to not install development gems. * Update bundler spec to check ENV variable * Added test to ensure kramdown gem not vendored * Re-add set_local setting to play nice with `expand_logstash_mixin_dependencies` * logstash service needs to be installed * gem_vendored? needs to use full path to vendor files * use `stdout` from `cat` command to generate spec temporary file * Removed unnecessary support for supplying a block from #gem_vendored? Co-authored-by: Ry Biesemeyer <ry.biesemeyer@elastic.co>
robbavey
added a commit
to robbavey/logstash
that referenced
this pull request
Oct 26, 2021
…lastic#13351) Backport PR elastic#13351 to 7.16 branch. Original message: * Fix bundler handling of 'without' Prior to this change, the values set in `set_local` are ignored when invoking bundler via the command line, as is used with `invoke!`. This commit sets those values in `ENV` variables instead, fixing the functionality to not install development gems. * Update bundler spec to check ENV variable * Added test to ensure kramdown gem not vendored * Re-add set_local setting to play nice with `expand_logstash_mixin_dependencies` * logstash service needs to be installed * gem_vendored? needs to use full path to vendor files * use `stdout` from `cat` command to generate spec temporary file * Removed unnecessary support for supplying a block from #gem_vendored? Co-authored-by: Ry Biesemeyer <ry.biesemeyer@elastic.co>
robbavey
added a commit
to robbavey/logstash
that referenced
this pull request
Oct 26, 2021
Backport PR elastic#13351 to 7.15 branch. Original message: * Fix bundler handling of 'without' Prior to this change, the values set in `set_local` are ignored when invoking bundler via the command line, as is used with `invoke!`. This commit sets those values in `ENV` variables instead, fixing the functionality to not install development gems. * Update bundler spec to check ENV variable * Added test to ensure kramdown gem not vendored * Re-add set_local setting to play nice with `expand_logstash_mixin_dependencies` * logstash service needs to be installed * gem_vendored? needs to use full path to vendor files * use `stdout` from `cat` command to generate spec temporary file * Removed unnecessary support for supplying a block from #gem_vendored? Co-authored-by: Ry Biesemeyer <ry.biesemeyer@elastic.co>
robbavey
added a commit
to robbavey/logstash
that referenced
this pull request
Oct 26, 2021
Backport PR elastic#13351 to 7.16 branch. Original message: * Fix bundler handling of 'without' Prior to this change, the values set in `set_local` are ignored when invoking bundler via the command line, as is used with `invoke!`. This commit sets those values in `ENV` variables instead, fixing the functionality to not install development gems. * Update bundler spec to check ENV variable * Added test to ensure kramdown gem not vendored * Re-add set_local setting to play nice with `expand_logstash_mixin_dependencies` * logstash service needs to be installed * gem_vendored? needs to use full path to vendor files * use `stdout` from `cat` command to generate spec temporary file * Removed unnecessary support for supplying a block from #gem_vendored? Co-authored-by: Ry Biesemeyer <ry.biesemeyer@elastic.co>
This was referenced Oct 26, 2021
robbavey
added a commit
that referenced
this pull request
Oct 26, 2021
Backport PR #13351 to 7.15 branch. Original message: * Fix bundler handling of 'without' Prior to this change, the values set in `set_local` are ignored when invoking bundler via the command line, as is used with `invoke!`. This commit sets those values in `ENV` variables instead, fixing the functionality to not install development gems. * Update bundler spec to check ENV variable * Added test to ensure kramdown gem not vendored * Re-add set_local setting to play nice with `expand_logstash_mixin_dependencies` * logstash service needs to be installed * gem_vendored? needs to use full path to vendor files * use `stdout` from `cat` command to generate spec temporary file * Removed unnecessary support for supplying a block from #gem_vendored? Co-authored-by: Ry Biesemeyer <ry.biesemeyer@elastic.co>
robbavey
added a commit
that referenced
this pull request
Oct 26, 2021
Backport PR #13351 to 7.16 branch. Original message: * Fix bundler handling of 'without' Prior to this change, the values set in `set_local` are ignored when invoking bundler via the command line, as is used with `invoke!`. This commit sets those values in `ENV` variables instead, fixing the functionality to not install development gems. * Update bundler spec to check ENV variable * Added test to ensure kramdown gem not vendored * Re-add set_local setting to play nice with `expand_logstash_mixin_dependencies` * logstash service needs to be installed * gem_vendored? needs to use full path to vendor files * use `stdout` from `cat` command to generate spec temporary file * Removed unnecessary support for supplying a block from #gem_vendored? Co-authored-by: Ry Biesemeyer <ry.biesemeyer@elastic.co>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this change, the values set in
set_localare ignored when invokingbundler via the command line, as is used with
invoke!. This commit sets thosevalues in
ENVvariables instead, fixing the functionality to not installdevelopment gems.