Raise Error for Unsupported Bundler Version #10601

kbukum1 · 2024-09-13T20:19:01Z

What are you trying to accomplish?

This PR introduces logic to raise an error when the version of Bundler is unsupported. The change is controlled by the bundler_v1_unsupported_error feature flag, allowing the error to be activated only when the flag is enabled.

Anything you want to highlight for special attention from reviewers?

The feature flag bundler_v1_unsupported_error controls whether the error is raised for unsupported Bundler v1 versions.

How will you know you've accomplished your goal?

The goal is achieved if:

The error is raised correctly when the feature flag is enabled and Bundler v1 is unsupported.
The error will be displayed on the Dependabot Insights page and the Security Alerts page.
The behavior is properly controlled by the feature flag.

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.

kbukum1 · 2024-09-13T20:41:28Z

common/lib/dependabot/notices.rb

-        show_alert: true
-      )
-    end
-


This was originally created to follow the notice structure when implementing the deprecation feature. However, after implementing the unsupported error, I realized we don’t need a similar structure to the deprecation notice. We already have a specialized error (Dependabot::ToolVersionNotSupported) used in other cases, so we are generalizing that for unsupported versions.

Reference code: https://github.com/dependabot/dependabot-core/blob/main/common/lib/dependabot/errors.rb#L435

kbukum1 · 2024-09-13T20:41:46Z

common/lib/dependabot/notices.rb

-
-      generate_pm_unsupported_notice(package_manager)
-    end
-


This was originally created to follow the notice structure when implementing the deprecation feature. However, after implementing the unsupported error, I realized we don’t need a similar structure to the deprecation notice. We already have a specialized error (Dependabot::ToolVersionNotSupported) used in other cases, so we are generalizing that for unsupported versions.

Reference code: https://github.com/dependabot/dependabot-core/blob/main/common/lib/dependabot/errors.rb#L435

kbukum1 · 2024-09-13T20:53:57Z

common/lib/dependabot/package_manager.rb

      return true if unsupported_versions.include?(version)

      supported_versions = self.supported_versions
-      return version < supported_versions.first if supported_versions.any?


This method was added during the deprecation warning but wasn’t used. We're now refining it for the unsupported error, returning false and leave the implementation to the sub class package manager.

kbukum1 · 2024-09-13T20:54:45Z

common/lib/dependabot/package_manager.rb

+        supported_versions_message
+      )
+    end
+
    # Checks if the current version is unsupported.
    # Returns true if the version is in the unsupported_versions array; false otherwise.
    # @example
    #   package_manager.unsupported? #=> false
    sig { returns(T::Boolean) }
    def unsupported?


Just note that this method written in general however the implementation can override this so bundler v1 package manager currently implementing this.

kbukum1 · 2024-09-13T20:56:09Z

bundler/lib/dependabot/bundler/package_manager.rb

+        return false unless Dependabot::Experiments.enabled?(:bundler_v1_unsupported_error)
+
+        # Determine if the Bundler version is unsupported.
+        version < supported_versions.first


We expect the version to be lower then the first element of supported versions list if it is not supported. In other cases we are unable to determine if it is not supported.

kbukum1 · 2024-09-13T20:57:00Z

bundler/lib/dependabot/bundler/package_manager.rb

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


This feature flag will be used to activate the unsupported version error for Bundler v1 on the specified unsupported date.

kbukum1 · 2024-09-16T04:11:31Z

common/spec/dependabot/notices_spec.rb

@@ -96,86 +114,6 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [
    end
  end

-  describe ".generate_support_notice" do


Not used generate_support_notice method is removed after finding out that we don't need to create error notice for unsupported package manager. Instead we are using general error flow to raise unsupported error.

kbukum1 · 2024-09-16T04:12:08Z

common/spec/dependabot/notices_spec.rb

@@ -204,33 +142,4 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [
        })
    end
  end
-
-  describe ".generate_pm_unsupported_notice" do


Not used generate_pm_unsupported_notice method is removed after finding out that we don't need to create error notice for unsupported package manager. Instead we are using general error flow to raise unsupported error.

kbukum1 · 2024-09-16T17:01:33Z

bundler/lib/dependabot/bundler/package_manager.rb

-      Version.new("2")
-    ].freeze, T::Array[Dependabot::Version])
+    # Keep versions in ascending order
+    SUPPORTED_BUNDLER_VERSIONS = T.let([Version.new("2")].freeze, T::Array[Dependabot::Version])


Keeping the array in ascending order is important because we compare the first element of supported_versions with the current version (version < supported_versions.first) to determine if it is unsupported.

We added specs to check if this array is ordered, so we don't need to run sorting.

Assuming that we'd only support versions greater than X, we could remove the requirement of having developers manually sorting by checking that a version is greater than all versions in this collection.

That would fall apart if we supported, say PackageManager v1 and v3, but not v2. However, that would also fall apart here.

applied the change to compare with each of them.

kbukum1 · 2024-09-16T17:53:33Z

common/spec/dependabot/notices_spec.rb

+  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)


The feature flag going to be activated when we are not supporting the version anymore.

kbukum1 · 2024-09-16T20:38:55Z

Tested the operations locally.

When the feature flag is ON:

When the feature flag is OFF:

kbukum1 · 2024-09-16T22:09:08Z

updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb

@@ -15,23 +15,6 @@

 require "dependabot/bundler"

-# Stub PackageManagerBase
-class StubPackageManager < Dependabot::PackageManagerBase


Moved into DummyPkgHelpers

kbukum1 · 2024-09-16T22:09:21Z

updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb

@@ -16,23 +16,6 @@

 require "dependabot/bundler"

-# Stub PackageManagerBase
-class StubPackageManager < Dependabot::PackageManagerBase


Moved into DummyPkgHelpers

kbukum1 · 2024-09-16T22:09:32Z

updater/spec/dependabot/updater/operations/update_all_versions_spec.rb

@@ -18,23 +18,6 @@

 require "dependabot/bundler"

-# Stub PackageManagerBase
-class StubPackageManager < Dependabot::PackageManagerBase


Moved into DummyPkgHelpers

bundler/lib/dependabot/bundler/package_manager.rb

landongrindheim · 2024-09-17T19:31:02Z

bundler/spec/dependabot/bundler/package_manager_spec.rb


-      it "returns false" do
+      it "returns false, as unsupported takes precedence" do


🤔 I can see room for confusion if #deprecated? goes from true to false when support is dropped, but I think this is correct.

Yes, currently Bundler v1 is supported but deprecated. With the feature flag off, we show deprecation warnings. Once we switch the unsupported v1 feature flag on, Bundler v1 will no longer be supported, and instead of showing deprecation warnings, an unsupported error will be raised, stopping the process.

landongrindheim · 2024-09-17T19:40:02Z

common/lib/dependabot/package_manager.rb

@@ -64,12 +67,23 @@ def deprecated?
    #   package_manager.unsupported? #=> false
    sig { returns(T::Boolean) }
    def unsupported?
-      return true if unsupported_versions.include?(version)
+      false


⚠️ I'm concerned that this might have unintended consequences. Have you audited all package managers to ensure they'll still be supported when this merges?

Outside of the scope of this work, but #supported?, with a default of true would be a little easier for me to reason about 😅

Bundler was the first package manager to implement this feature, and I initially added the unsupported? method, though it wasn’t used at the time since it was part of a future task. For other package managers, we assume the version is supported by default, unless the unsupported? condition is explicitly implemented in the related eco-system.

Note: Currently, only Bundler is implementing this package manager, but we plan to have all ecosystems implement it in the future.

landongrindheim · 2024-09-17T19:43:07Z

updater/lib/dependabot/updater/group_update_creation.rb

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


🔎 Is the &. needed here for changes which happen across ecosystems?

Yes, because the other ecosystems don't yet have a package_manager implementation. We always need the package_manager object from the ecosystem to perform these operations.

Note: Currently, only Bundler has implemented this package manager, but we plan to extend it to all ecosystems in the future.

updater/spec/support/dummy_pkg_helpers.rb

landongrindheim · 2024-09-17T19:49:53Z

bundler/lib/dependabot/bundler/package_manager.rb

-      Version.new("2")
-    ].freeze, T::Array[Dependabot::Version])
+    # Keep versions in ascending order
+    SUPPORTED_BUNDLER_VERSIONS = T.let([Version.new("2")].freeze, T::Array[Dependabot::Version])


Assuming that we'd only support versions greater than X, we could remove the requirement of having developers manually sorting by checking that a version is greater than all versions in this collection.

That would fall apart if we supported, say PackageManager v1 and v3, but not v2. However, that would also fall apart here.

kbukum1 · 2024-09-17T20:43:06Z

@landongrindheim

Thank you for the review! I’ve addressed all the comments and provided responses.

Just a quick note: Currently, only Bundler has implemented this package manager (also only deprecation is active), but we plan to extend it to all ecosystems in the future.

…king properly.

… that we are throwing error if there is update and before running the package manager to update dependencies.

…of with the first one. Co-authored-by: Landon Grindheim <[email protected]>

… ff for bundler Co-authored-by: Landon Grindheim <[email protected]>

github-actions bot added the L: ruby:bundler RubyGems via bundler label Sep 13, 2024

kbukum1 commented Sep 13, 2024

View reviewed changes

kbukum1 force-pushed the kamil/bundler_v1_unsupported_error branch from 3eb843c to f818ef8 Compare September 16, 2024 03:00

kbukum1 commented Sep 16, 2024

View reviewed changes

kbukum1 marked this pull request as ready for review September 16, 2024 20:37

kbukum1 requested a review from a team as a code owner September 16, 2024 20:37

kbukum1 marked this pull request as draft September 16, 2024 21:15

kbukum1 marked this pull request as ready for review September 16, 2024 21:52

kbukum1 commented Sep 16, 2024

View reviewed changes

kbukum1 requested a review from abdulapopoola September 16, 2024 22:55

kbukum1 self-assigned this Sep 17, 2024

kbukum1 mentioned this pull request Sep 17, 2024

Change default to Bundler v2 when unsupported feature flag is on and Gemfile.lock exists #10617

Merged

5 tasks

landongrindheim approved these changes Sep 17, 2024

View reviewed changes

kbukum1 force-pushed the kamil/bundler_v1_unsupported_error branch from 1a7f064 to f1a4759 Compare September 17, 2024 20:43

kbukum1 added 4 commits September 17, 2024 14:00

add unsupported error

db2465d

removed version "1" from supported versions.

85abdaf

update support version message

db8e26a

add/update specs for unsupported error.

204645b

kbukum1 and others added 15 commits September 17, 2024 14:00

removed specs related to removed unused unsupported notice related code.

fa5fe2b

add unsupported error to pr creation operations.

3fc8f3f

update specs to set unsupported flag as disabled

1a0e2e4

update package manager spec for unsupported check.

db05a19

fix spec problems

09b10f8

update package_manager spec to ensure deprecation and unsupported wor…

b17ed14

…king properly.

fix lint issue

ce62f0a

updated specs for package managers for unstupported check.

9b0506b

removed unnecessary changes

c281ebf

update spec operations for package manager unsupported error

85f714e

moved raising support error for groups

7f11679

moved unsupporting error at the end of the update checking to be sure…

c49c3c4

… that we are throwing error if there is update and before running the package manager to update dependencies.

fix lint issue

54003aa

Replace unsupported version check with all supported version instead …

3a01e72

…of with the first one. Co-authored-by: Landon Grindheim <[email protected]>

added condition to check if package manager is bundler then check the…

9c344de

… ff for bundler Co-authored-by: Landon Grindheim <[email protected]>

kbukum1 force-pushed the kamil/bundler_v1_unsupported_error branch from f1a4759 to 9c344de Compare September 17, 2024 21:00

kbukum1 merged commit 93e1827 into main Sep 17, 2024
123 checks passed

kbukum1 deleted the kamil/bundler_v1_unsupported_error branch September 17, 2024 21:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Raise Error for Unsupported Bundler Version #10601

Raise Error for Unsupported Bundler Version #10601

kbukum1 commented Sep 13, 2024 •

edited

Loading

kbukum1 Sep 13, 2024

kbukum1 Sep 13, 2024

kbukum1 Sep 13, 2024 •

edited

Loading

kbukum1 Sep 13, 2024

kbukum1 Sep 13, 2024 •

edited

Loading

kbukum1 Sep 13, 2024 •

edited

Loading

kbukum1 Sep 16, 2024 •

edited

Loading

kbukum1 Sep 16, 2024 •

edited

Loading

kbukum1 Sep 16, 2024

kbukum1 Sep 16, 2024

landongrindheim Sep 17, 2024

kbukum1 Sep 17, 2024

kbukum1 Sep 16, 2024

kbukum1 commented Sep 16, 2024 •

edited

Loading

kbukum1 Sep 16, 2024

kbukum1 Sep 16, 2024

kbukum1 Sep 16, 2024

landongrindheim Sep 17, 2024

kbukum1 Sep 17, 2024

landongrindheim Sep 17, 2024

kbukum1 Sep 17, 2024 •

edited

Loading

landongrindheim Sep 17, 2024

kbukum1 Sep 17, 2024 •

edited

Loading

landongrindheim Sep 17, 2024

kbukum1 commented Sep 17, 2024


		it "returns false" do
		it "returns false, as unsupported takes precedence" do

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

Raise Error for Unsupported Bundler Version #10601

Raise Error for Unsupported Bundler Version #10601

Conversation

kbukum1 commented Sep 13, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

kbukum1 Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

kbukum1 Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

kbukum1 Sep 16, 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

kbukum1 commented Sep 16, 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

kbukum1 Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbukum1 commented Sep 17, 2024

kbukum1 commented Sep 13, 2024 •

edited

Loading

kbukum1 Sep 13, 2024 •

edited

Loading

kbukum1 Sep 13, 2024 •

edited

Loading

kbukum1 Sep 13, 2024 •

edited

Loading

kbukum1 Sep 16, 2024 •

edited

Loading

kbukum1 Sep 16, 2024 •

edited

Loading

kbukum1 commented Sep 16, 2024 •

edited

Loading

kbukum1 Sep 17, 2024 •

edited

Loading

kbukum1 Sep 17, 2024 •

edited

Loading