Remove Bundler v1 Code and Related Feature Flags #10775

kbukum1 · 2024-10-10T22:09:58Z

What are you trying to accomplish?

This PR removes all Bundler v1-specific code and deprecation feature flags (add_deprecation_warn_to_pr_message and bundler_v1_unsupported_error) as part of deprecating support for Bundler v1. This cleanup ensures that Dependabot exclusively supports Bundler v2 and higher versions moving forward, aligning with recent changes to our package manager structure.

Anything you want to highlight for special attention from reviewers?

Given the scope of changes, particularly removing legacy Bundler v1 feature flags and related code, this update may impact areas previously conditional on Bundler version checks. I chose to remove the v1-related code and feature flags in favor of a streamlined v2-only approach to simplify the codebase and prevent potential misconfigurations.

When reviewing this PR, please note that although there are changes in over 379 files, the substantial updates are limited to around 60 files. There's no need to go through each file in the following folders, as we are entirely removing Bundler v1 and have comprehensive tests in place for Bundler v2 functionality:

bundler/helpers/v1
bundler/spec/fixtures/projects/bundler1

How will you know you've accomplished your goal?

I will verify that all instances of Bundler v1-related code and feature flags are removed and that Bundler v2 is the only supported version.
Testing will confirm that attempts to use Bundler v1 configurations result in expected errors, reinforcing Bundler v2 support.

Checklist

I have run the complete test suite to ensure all tests and linters pass.
I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
I have written clear and descriptive commit messages.
I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
I have ensured that the code is well-documented and easy to understand.

abdulapopoola · 2024-10-10T22:26:31Z

I think we should rename the v2 folder too but that can be a follow up PR

abdulapopoola · 2024-10-10T22:27:21Z

Do we have plans for smoke tests removal and all that too?

abdulapopoola · 2024-10-11T02:33:56Z

bin/ci-test

@@ -11,7 +11,7 @@ function print_usage() {
 function handle_args() {
  export SUITE_NAME=$1
  case $SUITE_NAME in
-    bundler1 | bundler2)


do we still need the "in" check or can we simplify some more?

I think we can keep that since in future we may also add bundler v3 in case. I tried to keep the syntax of supporting multiple as it is just currently we have one version, v2 now.

While it's okay to keep it, I also suspect realistically bundler v3 is very unlikely to ship for a long, long time... if so, it might be simpler to just streamline entirely to remove confusion for developers who wonder "why is this?"... at the very least you'd want a code comment of:

in used here in case bundler v3 ever ships...

btw, @deivid-rodriguez any input on how long it might be til we see bundler v3?

No, not really.

The condition is removed and it is simplified.

abdulapopoola · 2024-10-11T02:34:21Z

common/spec/dependabot/notices_spec.rb

@@ -35,9 +35,6 @@ def deprecated?

  sig { override.returns(T::Boolean) }
  def unsupported?
-    # Check if the feature flag for Bundler v1 unsupported error is enabled.
-    return false unless Dependabot::Experiments.enabled?(:bundler_v1_unsupported_error)


let's make sure to remove the flag everywhere too :)

Yes, actually I am removing from everywhere and not only unsupported but deprecation flag as well. We are done now checking last issues. After core is done, I will also remove them feature flags from dependabot-api as well.

add_deprecation_warn_to_pr_message

bundler_v1_unsupported_error

The PR is created to remove from dependabot-api.

PR: dependabot-api#5816

…e forcefully since unsupported takes over deprecation.

kbukum1 · 2024-10-11T02:48:48Z

Do we have plans for smoke tests removal and all that too?

I think we don't need to do anything on smoke tests but I need to check later. We have simple removal job on dependabot-api for feature flags.

…code

kbukum1 · 2024-10-14T20:29:24Z

updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb

          else
            Dependabot.logger.info("Nothing to update for Dependency Group: '#{group.name}'")
          end

          dependency_change
+        rescue ToolVersionNotSupported => e
+          error_handler.handle_job_error(error: e, dependency_group: group)
+          nil


When we have unsupported error for any package manager, we are going to stop the group update and handle not supported error.

kbukum1 · 2024-10-14T20:31:32Z

updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb

          if target_dependencies.empty?
            record_security_update_dependency_not_found
          else
            target_dependencies.each { |dep| check_and_create_pr_with_error_handling(dep) }
          end
+        rescue StandardError => e
+          error_handler.handle_dependency_error(error: e, dependency: target_dependencies&.first)
        end


When we have unsupported error for the package manager, we don't want to go over each dependency, instead we want to raise unsupported error for the whole process and handle error properly.

kbukum1 · 2024-10-14T20:32:31Z

updater/lib/dependabot/updater/group_update_creation.rb

      sig { params(group: Dependabot::DependencyGroup).returns(T.nilable(Dependabot::DependencyChange)) }
      def compile_all_dependency_changes_for(group)
        prepare_workspace

+        # Raise an error if the package manager version is unsupported
+        dependency_snapshot.package_manager&.raise_if_unsupported!


When we have unsupported error for any package manager in the group, we are going to raise error before stating update process.

kbukum1 · 2024-10-14T20:32:54Z

updater/lib/dependabot/updater/operations/refresh_group_update_pull_request.rb

@@ -118,6 +118,8 @@ def perform # rubocop:disable Metrics/AbcSize

            upsert_pull_request_with_error_handling(T.must(dependency_change), job_group)
          end
+        rescue ToolVersionNotSupported => e
+          error_handler.handle_job_error(error: e, dependency_group: job_group)


When we have unsupported error for any package manager, we are going to stop the group update and handle not supported error.

kbukum1 · 2024-10-14T20:33:16Z

updater/lib/dependabot/updater/operations/update_all_versions.rb

          # Retrieve the list of initial notices from dependency snapshot
          @notices = dependency_snapshot.notices
          # More notices can be added during the update process

          dependencies.each { |dep| check_and_create_pr_with_error_handling(dep) }
+        rescue StandardError => e
+          error_handler.handle_dependency_error(error: e, dependency: dependencies.last)
        end


When we have unsupported error for the package manager, we don't want to go over each dependency, instead we want to raise unsupported error for the whole process and handle error properly.

kbukum1 · 2024-10-14T20:33:47Z

updater/lib/dependabot/updater/operations/update_all_versions.rb

@@ -52,11 +52,16 @@ def perform
          Dependabot.logger.info("Starting update job for #{job.source.repo}")
          Dependabot.logger.info("Checking all dependencies for version updates...")

+          # Raise an error if the package manager version is unsupported
+          dependency_snapshot.package_manager&.raise_if_unsupported!


When we have unsupported error for the package manager, we don't want to go over each dependency, instead we want to raise unsupported error for the whole process and handle error properly.

kbukum1 · 2024-10-14T20:34:05Z

updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb

+          target_dependencies = dependency_snapshot.job_dependencies
+
+          # Raise an error if the package manager version is unsupported
+          dependency_snapshot.package_manager&.raise_if_unsupported!


When we have unsupported error for the package manager, we don't want to go over each dependency, instead we want to raise unsupported error for the whole process and handle error properly.

It would be simpler to not modify these operations, and have DependencySnapshot::dependency_file_parser raise an error right when we calculate the "package_manager".

I think the problem is we are not catching errors on top of the operations. So when I do that I also need to error handling. I will check the feasibility of that.

@jakecoffman ,

is it ok to do that after cleaning-up the code. I think this will also need some tests and also after adding it to the DependencySnapshot::dependency_file_parser I need check error handling mechanism for this code the place where is called and handle error properly. Otherwise it is turning to unknown error.

Just for posterity I raised a PR #10794 which is what I was suggesting.

kbukum1 · 2024-10-14T20:34:24Z

updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb

          dependency = dependencies.last

+          # Raise an error if the package manager version is unsupported
+          dependency_snapshot.package_manager&.raise_if_unsupported!


When we have unsupported error for the package manager, we don't want to go over each dependency, instead we want to raise unsupported error for the whole process and handle error properly.

deivid-rodriguez

Thanks for doing this!

kbukum1 · 2024-10-14T20:38:17Z

Thanks for doing this!

Thank you for your support and feedback!

jakecoffman

I think this PR is difficult to review as it's doing two things:

Adding support for deprecating an ecosystem to the updater code
Deprecating and deleting bundler v1

jakecoffman · 2024-10-15T13:01:05Z

updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb

+          target_dependencies = dependency_snapshot.job_dependencies
+
+          # Raise an error if the package manager version is unsupported
+          dependency_snapshot.package_manager&.raise_if_unsupported!


It would be simpler to not modify these operations, and have DependencySnapshot::dependency_file_parser raise an error right when we calculate the "package_manager".

…code

kbukum1 · 2024-10-15T23:17:59Z

We are splitting PR in multiple PRs. We created a new PR for moving unsupporting error at the top of update operations.

raise when an unsupported package manager version is present #10794

kbukum1 · 2024-10-16T19:07:46Z

This PR is splitted into 3 PRs

(Deployed) raise when an unsupported package manager version is present #10794
(Deployed) Remove Bundler v1 Deprecation and Unsupported Feature Flags and Deactivate Bundler v1 CI Tests #10796
(In Progress) Remove Bundler v1 Code #10793

remove unsupported bundler v1 helper code

2946c40

github-actions bot added the L: ruby:bundler RubyGems via bundler label Oct 10, 2024

kbukum1 added 2 commits October 10, 2024 15:20

remove bundler v1 from dockerfile.

556fbc3

update ci-test

4f6347f

abdulapopoola approved these changes Oct 10, 2024

View reviewed changes

kbukum1 added 11 commits October 10, 2024 15:34

removed bundler v1 from spec helper

e45fb62

remove bundler v1 from spec helpers and some specs

1863bea

removed bundler1 fixtures

e7c8a8e

removed bundler1 related specs

c4fd07c

remove bundler v1 related code from dev environment.

8ccf388

remove bundler_v1_only enabled specs

2f392f5

revert back removed PackageManagerHelper

701121d

updated bundler versions

633340f

enable feature flag unsupported for bundler v1

45523d8

removed feature flag add_deprecation_warn_to_pr_message

f258fc4

removed bundler v1 deprecation and unsupported feature flags

816b980

abdulapopoola reviewed Oct 11, 2024

View reviewed changes

fixed bundle package manager spec issues

fded892

abdulapopoola reviewed Oct 11, 2024

View reviewed changes

kbukum1 added 2 commits October 10, 2024 19:37

when unsupported is false deprecation should take place

67a7ec9

when we want to check deprecation the unsupported? should return fals…

01a23fc

…e forcefully since unsupported takes over deprecation.

kbukum1 marked this pull request as ready for review October 11, 2024 02:45

kbukum1 requested a review from a team as a code owner October 11, 2024 02:45

kbukum1 requested review from jeffwidman, jakecoffman, honeyankit and jurre October 11, 2024 03:26

kbukum1 force-pushed the kamil/remove_unsupporter_bundler_v1_related_code branch 2 times, most recently from 21290bf to caab60b Compare October 11, 2024 20:44

kbukum1 added 9 commits October 11, 2024 13:46

move raising unsupported error to the beginning of update process.

b5c04ad

handle package manager unsupported error for operations

50eceb1

fixed unsupported issues

cb19ff9

update unsupported error for single operations

512089c

add/update unsupported spec for create group

cb42946

moved specs into #perform describe

d611d4f

update error handling for groups

0630084

update refresh group spec for unsupported error

5c2ae84

Merge branch 'main' into kamil/remove_unsupporter_bundler_v1_related_…

b78c977

…code

kbukum1 commented Oct 14, 2024

View reviewed changes

deivid-rodriguez reviewed Oct 14, 2024

View reviewed changes

github-actions bot added the L: javascript label Oct 14, 2024

kbukum1 force-pushed the kamil/remove_unsupporter_bundler_v1_related_code branch from 9c056ff to b78c977 Compare October 14, 2024 22:49

jakecoffman reviewed Oct 15, 2024

View reviewed changes

Merge branch 'main' into kamil/remove_unsupporter_bundler_v1_related_…

b4e0b9b

…code

kbukum1 closed this Oct 16, 2024

kbukum1 deleted the kamil/remove_unsupporter_bundler_v1_related_code branch October 21, 2024 20:32

kbukum1 self-assigned this Nov 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Bundler v1 Code and Related Feature Flags #10775

Remove Bundler v1 Code and Related Feature Flags #10775

kbukum1 commented Oct 10, 2024 •

edited

Loading

abdulapopoola commented Oct 10, 2024

abdulapopoola commented Oct 10, 2024

abdulapopoola Oct 11, 2024

kbukum1 Oct 11, 2024 •

edited

Loading

jeffwidman Oct 11, 2024 •

edited

Loading

deivid-rodriguez Oct 11, 2024

kbukum1 Oct 11, 2024 •

edited

Loading

abdulapopoola Oct 11, 2024

kbukum1 Oct 11, 2024

kbukum1 Oct 11, 2024 •

edited

Loading

kbukum1 commented Oct 11, 2024

kbukum1 Oct 14, 2024 •

edited

Loading

kbukum1 Oct 14, 2024

kbukum1 Oct 14, 2024

kbukum1 Oct 14, 2024

kbukum1 Oct 14, 2024

kbukum1 Oct 14, 2024

kbukum1 Oct 14, 2024

jakecoffman Oct 15, 2024

kbukum1 Oct 15, 2024

kbukum1 Oct 15, 2024

jakecoffman Oct 15, 2024

kbukum1 Oct 14, 2024

deivid-rodriguez left a comment

kbukum1 commented Oct 14, 2024

jakecoffman left a comment

jakecoffman Oct 15, 2024

kbukum1 commented Oct 15, 2024 •

edited

Loading

kbukum1 commented Oct 16, 2024

Remove Bundler v1 Code and Related Feature Flags #10775

Remove Bundler v1 Code and Related Feature Flags #10775

Conversation

kbukum1 commented Oct 10, 2024 • edited Loading

What are you trying to accomplish?

Anything you want to highlight for special attention from reviewers?

How will you know you've accomplished your goal?

Checklist

abdulapopoola commented Oct 10, 2024

abdulapopoola commented Oct 10, 2024

Choose a reason for hiding this comment

kbukum1 Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

jeffwidman Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

kbukum1 commented Oct 11, 2024

kbukum1 Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deivid-rodriguez left a comment

Choose a reason for hiding this comment

kbukum1 commented Oct 14, 2024

jakecoffman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 commented Oct 15, 2024 • edited Loading

kbukum1 commented Oct 16, 2024

kbukum1 commented Oct 10, 2024 •

edited

Loading

kbukum1 Oct 11, 2024 •

edited

Loading

jeffwidman Oct 11, 2024 •

edited

Loading

kbukum1 Oct 11, 2024 •

edited

Loading

kbukum1 Oct 11, 2024 •

edited

Loading

kbukum1 Oct 14, 2024 •

edited

Loading

kbukum1 commented Oct 15, 2024 •

edited

Loading