remove forced dependency on old bundler #9395
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
rakelib/dependency.rake
Outdated
| @@ -1,7 +1,7 @@ | |||
|
|
|||
| namespace "dependency" do | |||
| task "bundler" do | |||
| Rake::Task["gem:require"].invoke("bundler", "~> 1.9.4") | |||
| Rake::Task["gem:require"].invoke("bundler", "~> 1") | |||
There was a problem hiding this comment.
@jsvd maybe just make this 1.16.0 exactly like we have in the integration tests? This isn't really a dependency that must be free floating (imo) ( ~> 1 seems wrong because we now changed the API calls in a way that will only work with newer Bundler?).
|
LGTM if it goes green :) |
|
As with most things bundler..a simple change always turns out to not be so simple...this seems to be breaking other parts of the bundler setup. checking. |
|
Thanks for this Joao! We really should move off that ancient version of bundler. LMK if you'd like one of us on core to take over this patch, I understand if you don't have the bandwidth. I'm glad to do it myself. |
|
jenkins, please test this |
jsvd
force-pushed
the
update_bundler
branch
2 times, most recently
from
07a6c4a
to
8937474
Compare
jsvd
commented
Nov 21, 2018
jsvd
force-pushed
the
update_bundler
branch
2 times, most recently
from
e3d44ee
to
c49b03c
Compare
|
Still broken. I've reduced the differences between a broken build and a good one to just bundler versions and a method alias (since []= doesn't exist in newer bundlers: |
|
all tests are now passing. Looking at the produced artifacts (like tar.gz) I noticed the development gems are being included (and weren't included before), I will investigate. Example: |
|
I believe all edge cases have been fixed. I can no longer find situations where the newer bundler breaks the development or production mode, so this PR is now ready for review. |
jsvd
commented
Nov 26, 2018
| @@ -67,7 +67,7 @@ def inject(gemfile_path, lockfile_path, dependencies) | |||
| gemfile = LogStash::Gemfile.new(File.new(gemfile_path, "r+")).load | |||
|
|
|||
| begin | |||
| @new_deps.each do |dependency| | |||
There was a problem hiding this comment.
this is simply due to a change in the name of the variable in recent bundlers:
- 1.17.1 uses
@depshttps://github.com/bundler/bundler/blob/v1.17.1/lib/bundler/injector.rb#L18 - 1.9.10 used
@new_depshttps://github.com/bundler/bundler/blob/v1.9.10/lib/bundler/injector.rb#L9
|
@jsvd 🎉 🎉 LGTM :) |
This commit required some tweaking of how we setup Bundler due to changes in reset behaviour, an internal variable name change, and the Bundler::Settings api changing.
yaauie
reviewed
Nov 27, 2018
There was a problem hiding this comment.
In general, this looks sensible.
I left a comment about a potential change that isn't really called out in this PR. We change both the implementation and a spec for the :without setting, but we don't change any callers. Does the new bundler accept an array value here where the old one required a colon-concatenated string?
Otherwise, LGTM
|
Jenkins test this please |
jsvd
added a commit
to jsvd/logstash
that referenced
this pull request
Dec 3, 2018
This reverts commit ab20b40. Due to failing tests like https://logstash-ci.elastic.co/job/elastic+logstash+master+multijob--ruby-unit-tests/86/
jsvd
added a commit
that referenced
this pull request
Dec 3, 2018
This reverts commit ab20b40. Due to failing tests like https://logstash-ci.elastic.co/job/elastic+logstash+master+multijob--ruby-unit-tests/86/
yaauie
added a commit
to yaauie/logstash
that referenced
this pull request
Jan 8, 2019
This reverts commit bef9841.
yaauie
added a commit
that referenced
this pull request
Jan 9, 2019
* Revert "Revert "remove forced dependency on old bundler (#9395)"" This reverts commit bef9841. * plugin management: update internal bundler to 1.17.x APIs * deps: update dev dependency webmock to version compatible with JRuby 9.2 * spec: update Pack fixture to include manticore version that doesn't conflict
yaauie
added a commit
to yaauie/logstash
that referenced
this pull request
Jan 20, 2019
* Revert "Revert "remove forced dependency on old bundler (elastic#9395)"" This reverts commit bef9841. * plugin management: update internal bundler to 1.17.x APIs * deps: update dev dependency webmock to version compatible with JRuby 9.2 * spec: update Pack fixture to include manticore version that doesn't conflict
yaauie
added a commit
to yaauie/logstash
that referenced
this pull request
Jan 22, 2019
* Revert "Revert "remove forced dependency on old bundler (elastic#9395)"" This reverts commit bef9841. * plugin management: update internal bundler to 1.17.x APIs * deps: update dev dependency webmock to version compatible with JRuby 9.2 * spec: update Pack fixture to include manticore version that doesn't conflict
jsvd
pushed a commit
to jsvd/logstash
that referenced
this pull request
Jan 22, 2019
* Revert "Revert "remove forced dependency on old bundler (elastic#9395)"" This reverts commit bef9841. * plugin management: update internal bundler to 1.17.x APIs * deps: update dev dependency webmock to version compatible with JRuby 9.2 * spec: update Pack fixture to include manticore version that doesn't conflict
yaauie
added a commit
to yaauie/logstash
that referenced
this pull request
Feb 4, 2019
* Revert "Revert "remove forced dependency on old bundler (elastic#9395)"" This reverts commit bef9841. * plugin management: update internal bundler to 1.17.x APIs * deps: update dev dependency webmock to version compatible with JRuby 9.2 * spec: update Pack fixture to include manticore version that doesn't conflict
yaauie
added a commit
that referenced
this pull request
Feb 5, 2019
* bump jruby to 9.2 * don't rely on logstash-base docker image * work around webmock ruby 2.5 support * ensure data folder exists in docker * change fixnum and bignum to integer * FileUtils.rmdir to rm_rf this is because from 2.3 to 2.5 FileUtils.rmdir will throw an exception if the directory isn't empty. On 2.3 the operation will just not delete the directory silently. * bump jruby to 9.2.5.0 and fix test * make rake default task since prepare pack needs it * Resolve compiler warnings (#10247) There are 3 types of compiler warnings that are either resolved or suppressed: 1. Rawtypes: In JRuby 9.2, `RubyArray` is a generic, so references throughout our codebase to the now "raw" type trigger warnings. In most cases we cannot actually resolve the issue, since the JRuby-provided methods for creating `RubyArray`s still return the raw type, so these have been suppressed. 2. Deprecations: - `RubyString#intern19()` -> `RubyString#intern()` - `RubyString#downcase19(ThreadContext)` -> `RubyString#downcase(ThreadContext)` - `NativeException`: remove import & reference directly; suppress usage warnings - `RaiseException()`: migrate to equivalent non-deprecated methods wherever possible; in some cases where we are using this in conjunction with the also-deprecated `NativeException` to preserve java stacktraces, there seems to be no non-deprecated path forward, so these cases have been suppressed. 3. Redundant Casts - Resolved * JRuby 9.2 bundler shenanigans (#10266) * Revert "Revert "remove forced dependency on old bundler (#9395)"" This reverts commit bef9841. * plugin management: update internal bundler to 1.17.x APIs * deps: update dev dependency webmock to version compatible with JRuby 9.2 * spec: update Pack fixture to include manticore version that doesn't conflict * build: update gradle to version that has Java 11 support * java11: resolve or suppress deprecation warnings * Remove superfluous flag opting into ParNew GC implementation When opting into CMS garbage collector with `XX:+UseConcMarkSweepGC`, the young generation collector ParNew has been the default since Java 8, making the `XX:+UseParNew` flag redundant; the flag was removed in Java 9, and should no longer be specified to work with modern Javas. https://bugs.openjdk.java.net/browse/JDK-8006478 https://openjdk.java.net/jeps/214 * spec: set thread name to example description for easier debugging * spec: prevent errors in testing specs by checking against skip list before using * no-op: remove use of `HashMap#computeIfAbsent` on single-threaded code > This method will, on a best-effort basis, throw a `ConcurrentModificationException` > if it is detected that the mapping function modifies this map during computation. > > -- https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/HashMap.html#computeIfAbsent(K,java.util.function.Function) * qa: by default, run integration against Elastic Stack 6.5.x To support development on Logstash on top of Java 11, default to testing against an Elastic Stack that is capable of running on Java 11. * qa: ignore deprecation warnings when comparing offline pack output * qa: add Java 9+ support to ChildProcess dev dependency this can safely be removed when the childprocess gem supports Java9+ enkessler/childprocess#141 * qa: allow connections to localhost in webmock * bump jrjackson version * fix filebeat integration tests * spec: ensure license compliance spec runs first The license compliance spec that validates the licenses of bundled plugins appears to not be compatible with the hooks that we inject into bundler for plugin management, and will fail in obscure ways when run after those hooks have been added. Since those hooks are not necessary for validating licenses, the easiest solution was to ensure that those specs run first, before the VM has been poluted. Since the gradle/junit/rspec bridge that is currently in place runs all specs in the same JVM, we also need to make sure that the rspec "world" is reset before a run, to ensure that it doesn't retain spec definitions from previous runs. Also updates the rake invocation, although I'm not sure it is used any more.
yaauie
added a commit
to yaauie/logstash
that referenced
this pull request
Feb 5, 2019
* bump jruby to 9.2 * don't rely on logstash-base docker image * work around webmock ruby 2.5 support * ensure data folder exists in docker * change fixnum and bignum to integer * FileUtils.rmdir to rm_rf this is because from 2.3 to 2.5 FileUtils.rmdir will throw an exception if the directory isn't empty. On 2.3 the operation will just not delete the directory silently. * bump jruby to 9.2.5.0 and fix test * make rake default task since prepare pack needs it * Resolve compiler warnings (elastic#10247) There are 3 types of compiler warnings that are either resolved or suppressed: 1. Rawtypes: In JRuby 9.2, `RubyArray` is a generic, so references throughout our codebase to the now "raw" type trigger warnings. In most cases we cannot actually resolve the issue, since the JRuby-provided methods for creating `RubyArray`s still return the raw type, so these have been suppressed. 2. Deprecations: - `RubyString#intern19()` -> `RubyString#intern()` - `RubyString#downcase19(ThreadContext)` -> `RubyString#downcase(ThreadContext)` - `NativeException`: remove import & reference directly; suppress usage warnings - `RaiseException()`: migrate to equivalent non-deprecated methods wherever possible; in some cases where we are using this in conjunction with the also-deprecated `NativeException` to preserve java stacktraces, there seems to be no non-deprecated path forward, so these cases have been suppressed. 3. Redundant Casts - Resolved * JRuby 9.2 bundler shenanigans (elastic#10266) * Revert "Revert "remove forced dependency on old bundler (elastic#9395)"" This reverts commit bef9841. * plugin management: update internal bundler to 1.17.x APIs * deps: update dev dependency webmock to version compatible with JRuby 9.2 * spec: update Pack fixture to include manticore version that doesn't conflict * build: update gradle to version that has Java 11 support * java11: resolve or suppress deprecation warnings * Remove superfluous flag opting into ParNew GC implementation When opting into CMS garbage collector with `XX:+UseConcMarkSweepGC`, the young generation collector ParNew has been the default since Java 8, making the `XX:+UseParNew` flag redundant; the flag was removed in Java 9, and should no longer be specified to work with modern Javas. https://bugs.openjdk.java.net/browse/JDK-8006478 https://openjdk.java.net/jeps/214 * spec: set thread name to example description for easier debugging * spec: prevent errors in testing specs by checking against skip list before using * no-op: remove use of `HashMap#computeIfAbsent` on single-threaded code > This method will, on a best-effort basis, throw a `ConcurrentModificationException` > if it is detected that the mapping function modifies this map during computation. > > -- https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/HashMap.html#computeIfAbsent(K,java.util.function.Function) * qa: by default, run integration against Elastic Stack 6.5.x To support development on Logstash on top of Java 11, default to testing against an Elastic Stack that is capable of running on Java 11. * qa: ignore deprecation warnings when comparing offline pack output * qa: add Java 9+ support to ChildProcess dev dependency this can safely be removed when the childprocess gem supports Java9+ enkessler/childprocess#141 * qa: allow connections to localhost in webmock * bump jrjackson version * fix filebeat integration tests * spec: ensure license compliance spec runs first The license compliance spec that validates the licenses of bundled plugins appears to not be compatible with the hooks that we inject into bundler for plugin management, and will fail in obscure ways when run after those hooks have been added. Since those hooks are not necessary for validating licenses, the easiest solution was to ensure that those specs run first, before the VM has been poluted. Since the gradle/junit/rspec bridge that is currently in place runs all specs in the same JVM, we also need to make sure that the rspec "world" is reset before a run, to ensure that it doesn't retain spec definitions from previous runs. Also updates the rake invocation, although I'm not sure it is used any more.
jsvd
pushed a commit
that referenced
this pull request
Feb 5, 2019
backport of #10279 to 6.x * bump jruby to 9.2 * don't rely on logstash-base docker image * work around webmock ruby 2.5 support * ensure data folder exists in docker * change fixnum and bignum to integer * FileUtils.rmdir to rm_rf this is because from 2.3 to 2.5 FileUtils.rmdir will throw an exception if the directory isn't empty. On 2.3 the operation will just not delete the directory silently. * bump jruby to 9.2.5.0 and fix test * make rake default task since prepare pack needs it * Resolve compiler warnings (#10247) There are 3 types of compiler warnings that are either resolved or suppressed: 1. Rawtypes: In JRuby 9.2, `RubyArray` is a generic, so references throughout our codebase to the now "raw" type trigger warnings. In most cases we cannot actually resolve the issue, since the JRuby-provided methods for creating `RubyArray`s still return the raw type, so these have been suppressed. 2. Deprecations: - `RubyString#intern19()` -> `RubyString#intern()` - `RubyString#downcase19(ThreadContext)` -> `RubyString#downcase(ThreadContext)` - `NativeException`: remove import & reference directly; suppress usage warnings - `RaiseException()`: migrate to equivalent non-deprecated methods wherever possible; in some cases where we are using this in conjunction with the also-deprecated `NativeException` to preserve java stacktraces, there seems to be no non-deprecated path forward, so these cases have been suppressed. 3. Redundant Casts - Resolved * JRuby 9.2 bundler shenanigans (#10266) * Revert "Revert "remove forced dependency on old bundler (#9395)"" This reverts commit bef9841. * plugin management: update internal bundler to 1.17.x APIs * deps: update dev dependency webmock to version compatible with JRuby 9.2 * spec: update Pack fixture to include manticore version that doesn't conflict * build: update gradle to version that has Java 11 support * java11: resolve or suppress deprecation warnings * Remove superfluous flag opting into ParNew GC implementation When opting into CMS garbage collector with `XX:+UseConcMarkSweepGC`, the young generation collector ParNew has been the default since Java 8, making the `XX:+UseParNew` flag redundant; the flag was removed in Java 9, and should no longer be specified to work with modern Javas. https://bugs.openjdk.java.net/browse/JDK-8006478 https://openjdk.java.net/jeps/214 * spec: set thread name to example description for easier debugging * spec: prevent errors in testing specs by checking against skip list before using * no-op: remove use of `HashMap#computeIfAbsent` on single-threaded code > This method will, on a best-effort basis, throw a `ConcurrentModificationException` > if it is detected that the mapping function modifies this map during computation. > > -- https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/HashMap.html#computeIfAbsent(K,java.util.function.Function) * qa: by default, run integration against Elastic Stack 6.5.x To support development on Logstash on top of Java 11, default to testing against an Elastic Stack that is capable of running on Java 11. * qa: ignore deprecation warnings when comparing offline pack output * qa: add Java 9+ support to ChildProcess dev dependency this can safely be removed when the childprocess gem supports Java9+ enkessler/childprocess#141 * qa: allow connections to localhost in webmock * bump jrjackson version * fix filebeat integration tests * spec: ensure license compliance spec runs first The license compliance spec that validates the licenses of bundled plugins appears to not be compatible with the hooks that we inject into bundler for plugin management, and will fail in obscure ways when run after those hooks have been added. Since those hooks are not necessary for validating licenses, the easiest solution was to ensure that those specs run first, before the VM has been poluted. Since the gradle/junit/rspec bridge that is currently in place runs all specs in the same JVM, we also need to make sure that the rspec "world" is reset before a run, to ensure that it doesn't retain spec definitions from previous runs. Also updates the rake invocation, although I'm not sure it is used any more.
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.
This is a byproduct of the closed PR #9362
We no longer need to pin bundler to an old version. Newer versions did break the internal Bundler.settings, also updated here.