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

Remove custom bundler #15066

Merged
merged 2 commits into from Jun 29, 2023
Merged

Remove custom bundler #15066

merged 2 commits into from Jun 29, 2023

Conversation

jsvd
Copy link
Member

@jsvd jsvd commented May 25, 2023

Instead of relying on external bundler because JRuby flutuated between including and not including bundler for a while, now that bundler is part of rubygems, unless there's a CVE afecting the shipped version of bundler there's no need to have a custom one any more.
This simplifies our code and bootstrapping tasks, so this PR removes the need to install bundler during bootstrap.

As side effect this PR reintroduces the older patches to not go OOM during plugin docs generation.

TODO: find a way to not require installing bundler for the QA tests. We can deal with that in a follow up PR

@jsvd jsvd marked this pull request as ready for review June 2, 2023 12:05
@jsvd
Copy link
Member Author

jsvd commented Jun 2, 2023

I have started a set of tests in CI for this remove_custom_bundler branch, that otherwise would only be run at merge time:

Copy link
Member

@yaauie yaauie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wary of running integration specs with a different bundler than we use at runtime, since the integration tests include coverage for installing and updating plugins which will use the bundler on the load path. This could lead to integration specs passing or failing independently of the actual behavior of our packaged artifacts.

spec/unit/bootstrap/bundler_spec.rb Show resolved Hide resolved
@jsvd jsvd mentioned this pull request Jun 15, 2023
9 tasks
@jsvd jsvd changed the title Experiment: Remove custom bundler Remove custom bundler Jun 15, 2023
@jsvd jsvd force-pushed the remove_custom_bundler branch 2 times, most recently from 060085f to b98846d Compare June 16, 2023 10:26
@jsvd jsvd requested a review from yaauie June 16, 2023 10:35
@jsvd
Copy link
Member Author

jsvd commented Jun 16, 2023

PR ready for a new review. I've removed the need to have a separate bundler for integration tests.
The PR now contains two commits:

  • afb6363 reintroduces the patches needed for bundler < 2.4 to not go OOM during docs generation, and uses the right bundler classes for tests
  • b98846d removes the need for an external bundler for all tasks by using JRuby's

@jsvd
Copy link
Member Author

jsvd commented Jun 16, 2023

jenkins test this please

Copy link
Member

@yaauie yaauie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic. I have left one note about guarding the DepProxy patch so that we remove or reimplement it when it no longer applies.

rakelib/plugins_docs_dependencies.rake Show resolved Hide resolved
@jsvd jsvd requested a review from yaauie June 20, 2023 18:24
@jsvd
Copy link
Member Author

jsvd commented Jun 28, 2023

jenkins test this please

Copy link
Member

@yaauie yaauie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic. LGTM 👍🏼

@jsvd jsvd merged commit 98c3d30 into main Jun 29, 2023
3 checks passed
@jsvd jsvd deleted the remove_custom_bundler branch June 29, 2023 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants