Skip to content

Conversation

@jsvd
Copy link
Member

@jsvd jsvd commented Dec 2, 2025

No description provided.

donoghuc and others added 8 commits December 2, 2025 09:34
Bundler is used to manage a gem environment that is shipped with logstash
artifacts. By default, bundler will install newer/duplicate gems than shipped
with ruby distributions (in logstash's case jruby). Duplicate gems in the
shipped environment can cause issues with code loading with ambiguous gem specs
or gem activation issues. This commit adds a step to compute the duplicate gems
managed with bundler (and therefore direct/transitive dependencies of
logstash/plugins) and *removes* copies shipped with jruby. Note that there are
two locations to do the deduplication at. Both the stdlib gems as well as what
jruby refers to as "bundled" gems. The existing pattern for excluding files from
artifacts is used to implement the deduplication.
Deduplication should happen as a depenedency of installing default gems. In the
current workflow we have a top level gradle task for packaging which calls out
to rake. Rake then invokes a *separate* gradle process. When we modify the jruby
default, when the separate gradle process goes to check of jruby is installed,
it sees a modified jruby and tries to re-install. We work around this by
changing how gradle detects if jruby is required to be installed.
This commit adds the installDefaultGems task to the unit test tasks. This
ensures that the gem env tested at the unit level matches the deduplicated one
at the integration/acceptance level. Takes over #18330
This commit changes gemInstaller such that the centralized gem_home from
Logstash::Environment is used instead of hard coding in a fragile path. The
tests were the only consumer of the optional positional parameter in the
`install` class method.
After some deeeeeeeep diving into comparing the state of running logstash from a
compiled artifact vs the unit tests i finally figured out that the use of the
bundler `setup!` method in unit tests is imcompatible with a couple of tests.
Specifically that method puts bundler installed gems ahead of the standard lib
gems in the load path. This commit solves that by re-positioning the standarl
lib back to the front of the load path.
Ideally bundler will consider default/stdlib gems when doing dependency resolution
to avoid duplication in the first place. this seems to break the pluginmanager.
Verify this happens in CI...
lib/bootstrap/bundler.rb:
1. Added jruby_bundled_specs method to Source::Rubygems that returns specs from JRuby's specifications/ directory
2. Modified specs method to merge jruby_bundled_specs with higher precedence when prefer_local is true
3. Updated install patch to skip installation for gems that already exist in JRuby's bundled gems directory
4. Track @jruby_bundled_specs_dir alongside @jruby_default_specs_dir

lib/pluginmanager/command.rb:
1. Added explicit filter to not touch JRuby's bundled gems in remove_orphan_dependencies!

The behavior now:
- JRuby bundled gems are preferred over remote versions
- If a version constraint requires a newer version, it will be fetched from remote
- No unnecessary copying of already-available gems to vendor/bundle
@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2025

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
  • /run exhaustive tests : Run the exhaustive tests Buildkite pipeline.

@jsvd jsvd changed the title [Bundler local fix [IGNORE] Bundler local fix Dec 2, 2025
@mergify
Copy link
Contributor

mergify bot commented Dec 2, 2025

This pull request does not have a backport label. Could you fix it @jsvd? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
  • If no backport is necessary, please add the backport-skip label

@jsvd
Copy link
Member Author

jsvd commented Dec 2, 2025

/run exhaustive tests

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 2, 2025

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

Successfully merging this pull request may close these issues.

4 participants