-
Notifications
You must be signed in to change notification settings - Fork 1k
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 Bundler v1 Deprecation and Unsupported Feature Flags and Deactivate Bundler v1 CI Tests #10796
Conversation
f8bc272
to
256dbd4
Compare
256dbd4
to
bf12637
Compare
@@ -15,8 +15,7 @@ jobs: | |||
fail-fast: false | |||
matrix: | |||
suite: | |||
- { path: bundler, name: bundler1, ecosystem: bundler } | |||
- { path: bundler, name: bundler2, ecosystem: bundler } | |||
- { path: bundler, name: bundler, ecosystem: bundler } | |||
- { path: cargo, name: cargo, ecosystem: cargo } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove bundler1 from test suites
exit 1 | ||
fi | ||
|
||
export MODULE="bundler" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are removing bundler v1 from specs and replace suite name as bundler
which is going to run specs for bundler v2 directly.
@@ -3,8 +3,7 @@ FROM ghcr.io/dependabot/dependabot-updater-core | |||
USER dependabot | |||
|
|||
COPY --chown=dependabot:dependabot bundler/helpers /opt/bundler/helpers | |||
RUN bash /opt/bundler/helpers/v1/build \ | |||
&& bash /opt/bundler/helpers/v2/build | |||
RUN bash /opt/bundler/helpers/v2/build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to build old bundler v1 code anymore
return V2 if Dependabot::Experiments.enabled?(:bundler_v1_unsupported_error) | ||
|
||
V1 | ||
V2 | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are going to always return bundler v2 except if version defined lower than version 2
. When version is lower than version 2 we assume it is going to be V1 and our unsupported error is going to throw error for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is failover_version
being used anywhere now? Maybe it can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is removed.
# shellcheck source=../helpers/v2/build | ||
DEPENDABOT_NATIVE_HELPERS_PATH="" source helpers/v2/build \ | ||
&& bundle exec rspec spec \ | ||
&& cd - |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only need to run specs for bundler v2 version.
end | ||
end | ||
|
||
describe "an untagged dependency", :bundler_v2_only do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need bundler v1 only specs anymore
end | ||
end | ||
|
||
describe "a github dependency", :bundler_v2_only do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need bundler v1 only specs anymore
end | ||
|
||
context "with a subdependency of a git source", :bundler_v2_only do | ||
context "with a subdependency of a git source" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need bundler v1 only specs anymore
}) | ||
end | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need bundler v1 only specs anymore
end | ||
|
||
it "does not touch the yanked gem", :bundler_v2_only do | ||
it "does not touch the yanked gem" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need bundler v1 only specs anymore
end | ||
|
||
it "preserves the BUNDLED WITH line in the lockfile", :bundler_v2_only do | ||
it "preserves the BUNDLED WITH line in the lockfile" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need bundler v1 only specs anymore
end | ||
|
||
it "returns the latest version", :bundler_v2_only do | ||
it "returns the latest version" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need bundler v1 only specs anymore
# Check if the feature flag for Bundler v1 unsupported error is enabled. | ||
return false unless name == "bundler" && Dependabot::Experiments.enabled?(:bundler_v1_unsupported_error) | ||
|
||
version < supported_versions.first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since bundler_v1 unsupported error is activated and stable we don't need feature flag to check it anymore.
expect(mock_error_handler).to receive(:handle_dependency_error).with(error: error, dependency: dependency) | ||
perform | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We removed this test because unsupported error is moved on parent file update file command instead of in operation. The unsupported error will be thrown before operation.perform is called.
expect(mock_error_handler).to receive(:handle_dependency_error).with(error: error, dependency: dependency) | ||
perform | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We removed this test because unsupported error is moved on parent file update file command instead of in operation. The unsupported error will be thrown before operation.perform is called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much easier to review now that the other changes were in a separate PR, thank you!
Thanks for the review and all the help. |
What are you trying to accomplish?
This PR removes the feature flags associated with the deprecation and unsupported status of Bundler v1 and deactivates Bundler v1 CI tests. With these changes, our CI pipeline focuses exclusively on Bundler v2 as the primary supported version. This is part of a gradual approach to phasing out Bundler v1 without immediately removing its code, helping to simplify the test suite by prioritizing Bundler v2.
Anything you want to highlight for special attention from reviewers?
This PR deactivates Bundler v1 CI tests and removes related feature flags, but it does not remove Bundler v1 code. This helps maintain a stable test environment with Bundler v2 set as the default in CI while marking Bundler v1 as inactive.
How will you know you've accomplished your goal?
Checklist