Merge pull request #5102 from sigurdm/bump_when_publish_to_none

Git dependency support for pub

Dockerfile

@@ -251,7 +251,7 @@ RUN DART_ARCH=${TARGETARCH} \
 # We pull the dependency_services from the dart-lang/pub repo as it is not
 # exposed from the Dart SDK (yet...).
 RUN git clone https://github.com/dart-lang/pub.git /opt/dart/pub \
-  && git -C /opt/dart/pub checkout 62bb244059415cf0c78b24151472efd46ad7569a \
+  && git -C /opt/dart/pub checkout 1e3c17ea871e6a80c720aa998f37cbd3913bc287 \
   && dart pub global activate --source path /opt/dart/pub \
   && chmod -R o+r "/opt/dart/pub" \
   && chown -R dependabot:dependabot "$PUB_CACHE" \

common/lib/dependabot/metadata_finders/base.rb

@@ -9,7 +9,7 @@ class Base
       require "dependabot/metadata_finders/base/release_finder"
       require "dependabot/metadata_finders/base/commits_finder"
 
-      PACKAGE_MANAGERS_WITH_RELIABLE_DIRECTORIES = %w(npm_and_yarn).freeze
+      PACKAGE_MANAGERS_WITH_RELIABLE_DIRECTORIES = %w(npm_and_yarn pub).freeze
 
       attr_reader :dependency, :credentials
 

pub/README.md

@@ -4,14 +4,17 @@ Dart (pub) support for [`dependabot-core`][core-repo].
 
 ### Limitations
 
- - No support for updating git-dependencies
-   * `dart pub` in general doesn't read versions from git, so upgrade logic is limited.
-   * Some variant of support for updating git-dependencies could be added in the future.
+ - Limited updating of git-dependencies
+   * `dart pub` in general doesn't read versions numbers from git, so upgrade logic is limited to upgrading to what the 'ref' is pointing to.
+   * If you pin to a specific revision in `pubspec.yaml` dependabot will not find upgrades.
+   * If you give a branch in `pubspec.yaml` dependabot will upgrade to the
+     latest revision that branch is pointing to, and update pubspec.lock
+     accordingly.
  - No support for security advisory integration.
  - If the version found is ignored (by dependabot config) no update will happen (even if, an earlier version could be used)
- - Very limited metadata support (just retrieves the github link)
+ - Limited metadata support (just retrieves the repository link).
+ - No support for auhtentication of private package repositories (mostly a configuration issue).
  - Only stable versions of Dart and Flutter supported.
- - No support for private package repositories (mostly a configuration issue).
  - `updated_dependencies_after_full_unlock` only allows updating to a later version, if the latest version that is mutually compatible with other dependencies is the latest version of the said package. This is a dependabot limitation.
 
 ### Running locally
@@ -45,6 +48,7 @@ allows checking for available updates.
       "version": "<version>",
 
       "kind": "direct" || "dev" || "transitive",
+      "source": <source-info>
 
       // Version constraint, as written in `pubspec.yaml`, null for
       // transitive dependencies.
@@ -68,6 +72,7 @@ allows checking for available updates.
       "name":        "<package-name>",       // name of current dependency
       "version":     "<version>",            // current version
       "kind":        "direct" || "dev" || "transitive",
+      "source": <source-info>
       "constraint":  "<version-constraint>" || null, // null for transitive deps
 
       // Latest desirable version of the current dependency,
@@ -110,6 +115,8 @@ allows checking for available updates.
            "name":                     "<package-name>",
            "version":                  "<new-version>" || null, // null, if removed
            "kind":                     "direct" || "dev" || "transitive",
+           "source": <source-info>
+           "previousSource": <source-info>
            "constraintBumped":         "<version-constraint>" || null, // null, if transitive
            "constraintBumpedIfNeeded": "<version-constraint>" || null, // null, if transitive
            "constraintWidened":        "<version-constraint>" || null, // null, if transitive
@@ -139,6 +146,8 @@ allows checking for available updates.
            "name":                     "<package-name>",
            "version":                  "<new-version>" || null, // null, if removed
            "kind":                     "direct" || "dev" || "transitive",
+           "source": <source-info>
+           "previousSource": <source-info>
            "constraintBumped":         "<version-constraint>" || null, // null, if transitive
            "constraintBumpedIfNeeded": "<version-constraint>" || null, // null, if transitive
            "constraintWidened":        "<version-constraint>" || null, // null, if transitive
@@ -168,6 +177,8 @@ allows checking for available updates.
            "name":                     "<package-name>",
            "version":                  "<new-version>" || null, // null, if removed
            "kind":                     "direct" || "dev" || "transitive",
+           "source": <source-info>
+           "previousSource": <source-info>
            "constraintBumped":         "<version-constraint>" || null, // null, if transitive
            "constraintBumpedIfNeeded": "<version-constraint>" || null, // null, if transitive
            "constraintWidened":        "<version-constraint>" || null, // null, if transitive
@@ -192,6 +203,7 @@ allows checking for available updates.
          "name":            "<package-name>",
          "version":         "<new-version>",
          "constraint":      "<version-constraint>" or null,
+         "source": <source-info>
       },
       ...
    ],
@@ -202,3 +214,18 @@ allows checking for available updates.
 }
 # Modifies pubspec.yaml and pubspec.lock on disk
 ```
+
+
+The <source-info> is either `null` (no information provided) or a map providing
+details about the package source in a manner specific to the
+package-environment.
+
+For a git dependency it will usually contain the git-url,
+the path inside the repo and the ref. For a repository package it would contain
+the url of the repository.
+```js
+{
+  "type": "git" || "hosted" || "path" || "sdk", // Name of the source.
+  ... // Other keys are free form json information about the dependency
+}
+```

pub/lib/dependabot/pub/helpers.rb

@@ -91,7 +91,7 @@ def parse_listed_dependency(json)
           params[:requirements] << {
             requirement: constraint,
             groups: [json["kind"]],
-            source: nil, # TODO: Expose some information about the source
+            source: json["source"],
             file: "pubspec.yaml"
           }
         end
@@ -159,9 +159,11 @@ def dependencies_to_json(dependencies)
           nil
         else
           deps = dependencies.map do |d|
+            source = d.requirements.empty? ? nil : d.requirements.first[:source]
             obj = {
               "name" => d.name,
-              "version" => d.version
+              "version" => d.version,
+              "source" => source
             }
 
             obj["constraint"] = d.requirements[0][:requirement].to_s unless d.requirements.nil? || d.requirements.empty?

pub/lib/dependabot/pub/metadata_finder.rb

@@ -11,25 +11,33 @@ class MetadataFinder < Dependabot::MetadataFinders::Base
       private
 
       def look_up_source
-        repo = pub_listing.dig("latest", "pubspec", "repository")
+        source = dependency.requirements&.first&.dig(:source)
+        if source&.dig("type") == "git"
+          result = Source.from_url(source.dig("description", "url"))
+          result.directory = source.dig("description", "path")
+          result.commit = source.dig("description", "resolved-ref")
+          return result
+        end
+        repository_url = source&.dig("description", "url") || "https://pub.dev"
+
+        listing = repository_listing(repository_url)
+        repo = listing.dig("latest", "pubspec", "repository")
         # The repository field did not always exist in pubspec.yaml, and some
         # packages specify a git repository in the "homepage" field.
-        repo ||= pub_listing.dig("latest", "pubspec", "homepage")
+        repo ||= listing.dig("latest", "pubspec", "homepage")
         return nil unless repo
 
         Source.from_url(repo)
       end
 
-      def pub_listing
-        return @pub_listing unless @pub_listing.nil?
-
+      def repository_listing(repository_url)
         response = Excon.get(
-          "https://pub.dev/api/packages/#{dependency.name}",
+          "#{repository_url}/api/packages/#{dependency.name}",
           idempotent: true,
           **SharedHelpers.excon_defaults
         )
 
-        @pub_listing = JSON.parse(response.body)
+        JSON.parse(response.body)
       end
     end
   end

pub/lib/dependabot/pub/update_checker.rb

@@ -10,13 +10,10 @@ class UpdateChecker < Dependabot::UpdateCheckers::Base
       include Dependabot::Pub::Helpers
 
       def latest_version
-        version = Dependabot::Pub::Version.new(current_report["latest"])
+        version = version_unless_ignored(current_report["latest"], current_version: dependency.version)
+        raise AllVersionsIgnored if version.nil? && @raise_on_ignored
 
-        return version if ignore_requirements.none? { |r| r.satisfied_by?(version) }
-        return version if version == version_class.new(dependency.version)
-        return nil unless @raise_on_ignored
-
-        raise AllVersionsIgnored
+        version
       end
 
       def latest_resolvable_version_with_no_unlock
@@ -25,13 +22,7 @@ def latest_resolvable_version_with_no_unlock
         entry = current_report["compatible"].find { |d| d["name"] == dependency.name }
         return nil unless entry
 
-        new_version = Dependabot::Pub::Version.new(entry["version"])
-        # We ignore this solution, if any of the requirements in
-        # ignored_versions satisfy the version we're proposing as an upgrade
-        # target.
-        return nil if ignore_requirements.any? { |r| r.satisfied_by?(new_version) }
-
-        new_version
+        version_unless_ignored(entry["version"])
       end
 
       def latest_resolvable_version
@@ -40,13 +31,7 @@ def latest_resolvable_version
         entry = current_report["singleBreaking"].find { |d| d["name"] == dependency.name }
         return nil unless entry
 
-        new_version = Dependabot::Pub::Version.new(entry["version"])
-        # We ignore this solution, if any of the requirements in
-        # ignored_versions satisfy the version we're proposing as an upgrade
-        # target.
-        return nil if ignore_requirements.any? { |r| r.satisfied_by?(new_version) }
-
-        new_version
+        version_unless_ignored(entry["version"])
       end
 
       def updated_requirements
@@ -61,11 +46,41 @@ def updated_requirements
 
       private
 
+      # Returns unparsed_version if it looks like a git-revision.
+      #
+      # Otherwise it will be parsed with Dependabot::Pub::Version.new and
+      # checked against the ignored_requirements:
+      #
+      # * If not ignored the parsed Version object will be returned.
+      # * If current_version is non-nil and the parsed version is the same it
+      #   will be returned.
+      # * Otherwise returns nil
+      def version_unless_ignored(unparsed_version, current_version: nil)
+        if git_revision?(unparsed_version)
+          unparsed_version
+        else
+          new_version = Dependabot::Pub::Version.new(unparsed_version)
+          if !current_version.nil? && !git_revision?(current_version) &&
+             Dependabot::Pub::Version.new(current_version) == new_version
+            return new_version
+          end
+          return nil if ignore_requirements.any? { |r| r.satisfied_by?(new_version) }
+
+          new_version
+        end
+      end
+
+      def git_revision?(version_string)
+        version_string.match?(/^[0-9a-f]{6,}$/)
+      end
+
       def latest_version_resolvable_with_full_unlock?
         entry = current_report["multiBreaking"].find { |d| d["name"] == dependency.name }
         # This a bit dumb, but full-unlock is only considered if we can get the
         # latest version!
-        entry && latest_version == Dependabot::Pub::Version.new(entry["version"])
+        entry && ((!git_revision?(entry["version"]) &&
+                  latest_version == Dependabot::Pub::Version.new(entry["version"])) ||
+                  latest_version == entry["version"])
       end
 
       def updated_dependencies_after_full_unlock

pub/spec/dependabot/pub/file_parser_spec.rb

@@ -36,7 +36,7 @@
           requirement: "2.0.0",
           groups: ["direct"],
           file: "pubspec.yaml",
-          source: nil
+          source: { "description" => { "name" => "retry", "url" => "https://pub.dartlang.org" }, "type" => "hosted" }
         }])
       end
     end
@@ -53,7 +53,7 @@
           requirement: "^2.0.0",
           groups: ["direct"],
           file: "pubspec.yaml",
-          source: nil
+          source: { "description" => { "name" => "retry", "url" => "https://pub.dartlang.org" }, "type" => "hosted" }
         }])
       end
 
@@ -64,7 +64,7 @@
           requirement: ">=1.17.10 <=1.17.12",
           groups: ["dev"],
           file: "pubspec.yaml",
-          source: nil
+          source: { "description" => { "name" => "test", "url" => "https://pub.dartlang.org" }, "type" => "hosted" }
         }])
       end
 

pub/spec/dependabot/pub/metadata_finder_spec.rb

@@ -14,6 +14,15 @@
       body: fixture("pub_dev_responses/simple/#{dependency.name}.json"),
       headers: {}
     )
+    fixture_name = "another_org_responses/simple/#{dependency.name}.json"
+    fixture_file = File.join("spec/fixtures", fixture_name)
+    if File.exist?(fixture_file)
+      stub_request(:get, "https://another.org/api/packages/#{dependency.name}").to_return(
+        status: 200,
+        body: fixture(fixture_name),
+        headers: {}
+      )
+    end
   end
 
   let(:dependency) do
@@ -67,4 +76,49 @@
       expect(finder.source_url).to eq "https://github.com/dart-lang/protobuf"
     end
   end
+
+  describe "#source_url" do
+    let(:dependency) do
+      Dependabot::Dependency.new(
+        name: "retry",
+        version: "1.3.0",
+        requirements: [{
+          file: "pubspec.yaml",
+          requirement: "~3.0.0",
+          groups: [],
+          source: { "description" => { "name" => "retry", "url" => "https://another.org" }, "type" => "hosted" }
+        }],
+        package_manager: "pub"
+      )
+    end
+    it "works for alternative hosts" do
+      expect(finder.source_url).to eq "https://github.com/another_org/dart-neats"
+    end
+  end
+
+  describe "#source_url" do
+    let(:dependency) do
+      Dependabot::Dependency.new(
+        name: "retry",
+        version: "1.3.0",
+        requirements: [{
+          file: "pubspec.yaml",
+          requirement: "~3.0.0",
+          groups: [],
+          source: {
+            "type" => "git",
+            "description" => {
+              "url" => "https://github.com/google/dart-neats",
+              "path" => "retry",
+              "ref" => "1adc00411d4e1184d248d0147de3348a287f2fea"
+            }
+          }
+        }],
+        package_manager: "pub"
+      )
+    end
+    it "works for git dependencies" do
+      expect(finder.source_url).to eq "https://github.com/google/dart-neats/tree/HEAD/retry"
+    end
+  end
 end