Merge branch 'main' into harry/add-support-callable-workflows

Author: thavaahariharangit

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/FileWriters/FileWriterWorkerTests.cs

@@ -812,10 +812,12 @@ await TestAsync(
             discoveryWorker: new TestDiscoveryWorker(args =>
             {
                 discoveryRequestCount++;
-                var result = discoveryRequestCount switch
+                if (discoveryRequestCount <= 3)
                 {
-                    // initial request, report 1.0.0
-                    1 => new WorkspaceDiscoveryResult()
+                    // 1 - initial request
+                    // 2 - pre-edit request
+                    // 3 - post-edit request - no change made, indicates failure
+                    return Task.FromResult(new WorkspaceDiscoveryResult()
                     {
                         Path = "/",
                         Projects = [
@@ -829,26 +831,10 @@ await TestAsync(
                                 ReferencedProjectPaths = []
                             }
                         ]
-                    },
-                    // post-edit request, report 1.0.0 again, indicating the file edits didn't produce the desired result
-                    2 => new WorkspaceDiscoveryResult()
-                    {
-                        Path = "/",
-                        Projects = [
-                            new ProjectDiscoveryResult()
-                            {
-                                FilePath = "project.csproj",
-                                Dependencies = [new Dependency("Some.Dependency", "1.0.0", DependencyType.PackageReference)],
-                                TargetFrameworks = ["net9.0"],
-                                AdditionalFiles = [],
-                                ImportedFiles = [],
-                                ReferencedProjectPaths = []
-                            }
-                        ]
-                    },
-                    _ => throw new NotSupportedException($"Didn't expect {discoveryRequestCount} discovery requests"),
-                };
-                return Task.FromResult(result);
+                    });
+                }
+
+                throw new NotSupportedException($"Didn't expect {discoveryRequestCount} discovery requests");
             }),
             dependencySolver: null, // use real worker
             fileWriter: null, // use real worker
@@ -867,6 +853,80 @@ await TestAsync(
         );
     }
 
+    [Fact]
+    public async Task EndToEnd_PriorFileEditResolvedDependencyInSubsequentFile()
+    {
+        // via a ProjectReference, two projects have the same dependency and updating the root causes the other dependency to also be updated and not result in unnecessarily pinning anything
+        await TestAsync(
+            dependencyName: "Some.Dependency",
+            oldDependencyVersion: "1.0.0",
+            newDependencyVersion: "2.0.0",
+            files: [
+                ("src/a/a.csproj", """
+                    <Project Sdk="Microsoft.NET.Sdk">
+                      <PropertyGroup>
+                        <TargetFramework>net9.0</TargetFramework>
+                      </PropertyGroup>
+                      <ItemGroup>
+                        <ProjectReference Include="..\b\b.csproj" />
+                      </ItemGroup>
+                      <ItemGroup>
+                        <PackageReference Include="Unrelated.Dependency" Version="3.0.0" />
+                      </ItemGroup>
+                    </Project>
+                    """),
+                ("src/b/b.csproj", """
+                    <Project Sdk="Microsoft.NET.Sdk">
+                      <PropertyGroup>
+                        <TargetFramework>net9.0</TargetFramework>
+                      </PropertyGroup>
+                      <ItemGroup>
+                        <PackageReference Include="Some.Dependency" Version="1.0.0" />
+                      </ItemGroup>
+                    </Project>
+                    """),
+                ("Directory.Build.props", "<Project />"),
+                ("Directory.Build.targets", "<Project />"),
+            ],
+            packages: [
+                MockNuGetPackage.CreateSimplePackage("Some.Dependency", "1.0.0", "net9.0"),
+                MockNuGetPackage.CreateSimplePackage("Some.Dependency", "2.0.0", "net9.0"),
+                MockNuGetPackage.CreateSimplePackage("Unrelated.Dependency", "3.0.0", "net9.0"),
+            ],
+            discoveryWorker: null, // use real worker
+            dependencySolver: null, // use real worker
+            fileWriter: null, // use real worker
+            expectedFiles: [
+                ("src/a/a.csproj", """
+                    <Project Sdk="Microsoft.NET.Sdk">
+                      <PropertyGroup>
+                        <TargetFramework>net9.0</TargetFramework>
+                      </PropertyGroup>
+                      <ItemGroup>
+                        <ProjectReference Include="..\b\b.csproj" />
+                      </ItemGroup>
+                      <ItemGroup>
+                        <PackageReference Include="Unrelated.Dependency" Version="3.0.0" />
+                      </ItemGroup>
+                    </Project>
+                    """),
+                ("src/b/b.csproj", """
+                    <Project Sdk="Microsoft.NET.Sdk">
+                      <PropertyGroup>
+                        <TargetFramework>net9.0</TargetFramework>
+                      </PropertyGroup>
+                      <ItemGroup>
+                        <PackageReference Include="Some.Dependency" Version="2.0.0" />
+                      </ItemGroup>
+                    </Project>
+                    """),
+            ],
+            expectedOperations: [
+                new PinnedUpdate() { DependencyName = "Some.Dependency", NewVersion = NuGetVersion.Parse("2.0.0"), UpdatedFiles = ["/src/b/b.csproj"] }
+            ]
+        );
+    }
+
     private static async Task TestAsync(
         string dependencyName,
         string oldDependencyVersion,

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/FileWriters/FileWriterWorker.cs

@@ -209,7 +209,62 @@ NuGetVersion newDependencyVersion
             foreach (var projectDiscovery in orderedProjectDiscovery)
             {
                 var projectFullPath = Path.Join(repoContentsPath.FullName, initialDiscoveryResult.Path, projectDiscovery.FilePath).FullyNormalizedRootedPath();
-                var updatedFiles = await TryPerformFileWritesAsync(_fileWriter, repoContentsPath, initialProjectDirectory, projectDiscovery, resolvedDependencies.Value);
+                var projectDirectory = new DirectoryInfo(Path.GetDirectoryName(projectFullPath)!);
+                var projectRelativePath = Path.GetRelativePath(repoContentsPath.FullName, projectFullPath).FullyNormalizedRootedPath();
+                var projectRelativeDirectory = Path.GetDirectoryName(projectRelativePath)!.NormalizePathToUnix();
+                _logger.Info($"Attempting to update {dependencyName} for {projectRelativePath}");
+
+                // rerun discovery because a previous file update may have already fixed this
+                var rerunWorkspaceDiscovery = await _discoveryWorker.RunAsync(repoContentsPath.FullName, projectRelativeDirectory);
+                var rerunProjectDiscovery = rerunWorkspaceDiscovery.GetProjectDiscoveryFromFullPath(repoContentsPath, new FileInfo(projectFullPath));
+                if (rerunProjectDiscovery is null)
+                {
+                    _logger.Warn($"  Unable to re-run project discovery for project {projectRelativePath}.");
+                    continue;
+                }
+
+                var candidateDependencyToUpdate = rerunProjectDiscovery.Dependencies.FirstOrDefault(d => d.Name.Equals(dependencyName, StringComparison.OrdinalIgnoreCase));
+                if (candidateDependencyToUpdate?.Version is null)
+                {
+                    _logger.Warn($"  Unable to find dependency after discovery rerun.");
+                    continue;
+                }
+
+                if (!NuGetVersion.TryParse(candidateDependencyToUpdate.Version, out var candidateDependencyCurrentVersion))
+                {
+                    _logger.Warn($"  Unable to parse discovered version number from string: {candidateDependencyToUpdate.Version}");
+                    continue;
+                }
+
+                if (candidateDependencyCurrentVersion >= newDependencyVersion)
+                {
+                    _logger.Info($"  Dependency is already up to date at version {candidateDependencyCurrentVersion}, possibly from a previous operation.");
+                    continue;
+                }
+
+                var rerunTopLevelDependencies = rerunProjectDiscovery.Dependencies
+                    .Where(d => !d.IsTransitive)
+                    .ToImmutableArray();
+                var rerunDesiredDependencies = rerunTopLevelDependencies.Any(d => d.Name.Equals(dependencyName, StringComparison.OrdinalIgnoreCase))
+                    ? rerunTopLevelDependencies.Select(d => d.Name.Equals(dependencyName, StringComparison.OrdinalIgnoreCase) ? newDependency : d).ToImmutableArray()
+                    : rerunTopLevelDependencies.Concat([newDependency]).ToImmutableArray();
+                var resolvedDependenciesInThisproject = await _dependencySolver.SolveAsync(rerunTopLevelDependencies, rerunDesiredDependencies, targetFramework);
+                if (resolvedDependenciesInThisproject is null)
+                {
+                    _logger.Warn($"  Unable to solve dependency conflicts for {projectRelativePath}/{targetFramework}.");
+                    continue;
+                }
+
+                var updatedFiles = await TryPerformFileWritesAsync(_fileWriter, repoContentsPath, projectDirectory, rerunProjectDiscovery!, resolvedDependenciesInThisproject.Value);
+                if (updatedFiles.Length == 0)
+                {
+                    _logger.Info("  Files were unable to be updated.");
+                }
+                else
+                {
+                    _logger.Info($"  Successfully updated the following files: {string.Join(", ", updatedFiles)}");
+                }
+
                 allUpdatedFiles.AddRange(updatedFiles);
             }
 

python/helpers/lib/parser.py

@@ -32,7 +32,7 @@ def version_from_req(specifier_set):
                 next(iter(specifier_set)).operator in {"==", "==="}):
             return next(iter(specifier_set)).version
 
-    def parse_requirement(entry, pyproject_path):
+    def parse_requirement(entry, pyproject_path, requirement_type=None):
         try:
             req = Requirement(entry)
         except InvalidRequirement as e:
@@ -46,14 +46,19 @@ def parse_requirement(entry, pyproject_path):
                 "file": pyproject_path,
                 "requirement": str(req.specifier),
                 "extras": sorted(list(req.extras)),
+                "requirement_type": requirement_type,
             }
             return data
 
-    def parse_toml_section_pep621_dependencies(pyproject_path, dependencies):
+    def parse_toml_section_pep621_dependencies(
+        pyproject_path, dependencies, requirement_type=None
+    ):
         requirement_packages = []
 
         for dependency in dependencies:
-            parsed_dependency = parse_requirement(dependency, pyproject_path)
+            parsed_dependency = parse_requirement(
+                dependency, pyproject_path, requirement_type
+            )
             requirement_packages.append(parsed_dependency)
 
         return requirement_packages
@@ -75,7 +80,9 @@ def parse_toml_section_pep735_dependencies(
         for entry in dependencies:
             # Handle direct requirement
             if isinstance(entry, str):
-                parsed_dependency = parse_requirement(entry, pyproject_path)
+                parsed_dependency = parse_requirement(
+                    entry, pyproject_path, group_name
+                )
                 requirement_packages.append(parsed_dependency)
             # Handle include-group directive
             elif isinstance(entry, dict) and "include-group" in entry:
@@ -100,7 +107,8 @@ def parse_toml_section_pep735_dependencies(
             dependencies_toml = project_section['dependencies']
             runtime_dependencies = parse_toml_section_pep621_dependencies(
                 pyproject_path,
-                dependencies_toml
+                dependencies_toml,
+                "dependencies"
             )
             dependencies.extend(runtime_dependencies)
 
@@ -111,7 +119,8 @@ def parse_toml_section_pep735_dependencies(
             for group in optional_dependencies_toml:
                 group_dependencies = parse_toml_section_pep621_dependencies(
                     pyproject_path,
-                    optional_dependencies_toml[group]
+                    optional_dependencies_toml[group],
+                    group
                 )
                 dependencies.extend(group_dependencies)
 
@@ -128,7 +137,8 @@ def parse_toml_section_pep735_dependencies(
         if 'requires' in build_system_section:
             build_system_dependencies = parse_toml_section_pep621_dependencies(
                 pyproject_path,
-                build_system_section['requires']
+                build_system_section['requires'],
+                "build-system.requires"
             )
             dependencies.extend(build_system_dependencies)
 

python/lib/dependabot/python/file_parser/pyproject_files_parser.rb

@@ -43,11 +43,16 @@ def dependency_set
 
         sig { returns(Dependabot::FileParsers::Base::DependencySet) }
         def pyproject_dependencies
-          if using_poetry?
-            poetry_dependencies
-          else
-            pep621_pep735_dependencies
-          end
+          dependencies = Dependabot::FileParsers::Base::DependencySet.new
+
+          # Parse Poetry dependencies if [tool.poetry] section exists
+          dependencies += poetry_dependencies if using_poetry?
+
+          # Parse PEP 621/735 dependencies if those sections exist
+          # This handles hybrid projects that have both Poetry and PEP 621 sections
+          dependencies += pep621_pep735_dependencies if using_pep621? || using_pep735?
+
+          dependencies
         end
 
         sig { returns(Dependabot::FileParsers::Base::DependencySet) }
@@ -84,11 +89,15 @@ def pep621_pep735_dependencies
           parse_pep621_pep735_dependencies.each do |dep|
             # If a requirement has a `<` or `<=` marker then updating it is
             # probably blocked. Ignore it.
-            next if dep["markers"].include?("<")
+            next if dep["markers"]&.include?("<")
 
             # If no requirement, don't add it
             next if dep["requirement"].empty?
 
+            # Skip build-system.requires dependencies when using Poetry
+            # Poetry manages its own build system dependencies
+            next if using_poetry? && dep["requirement_type"] == "build-system.requires"
+
             dependencies <<
               Dependency.new(
                 name: normalised_name(dep["name"], dep["extras"]),

python/lib/dependabot/python/update_checker.rb

@@ -181,7 +181,10 @@ def subdependency_resolver
 
       sig { returns(Symbol) }
       def pyproject_resolver
-        return :poetry if poetry_based?
+        # For hybrid projects with both [tool.poetry] and [project] sections but no lockfile,
+        # use the requirements resolver to handle PEP 621 dependencies
+        # For pure Poetry projects, use Poetry resolver even without lockfile
+        return :poetry if poetry_based? && (poetry_lock || !standard_details)
 
         :requirements
       end

python/spec/dependabot/python/file_parser/pyproject_files_parser_spec.rb

@@ -276,7 +276,7 @@
           [{
             requirement: "==0.3.0",
             file: "pyproject.toml",
-            groups: [],
+            groups: ["dependencies"],
             source: nil
           }]
         )
@@ -394,6 +394,26 @@
 
         its(:length) { is_expected.to be > 0 }
       end
+
+      context "with PEP 621 and Poetry configuration" do
+        subject(:dependencies) { parser.dependency_set.dependencies }
+
+        let(:pyproject_fixture_name) { "pep621_with_poetry.toml" }
+
+        its(:length) { is_expected.to eq(2) }
+
+        it "has the correct dependencies with requirement types" do
+          expect(dependencies.map(&:name)).to contain_exactly("fastapi", "pydantic")
+
+          fastapi = dependencies.find { |d| d.name == "fastapi" }
+          expect(fastapi.version).to eq("0.115.5")
+          expect(fastapi.requirements.first[:groups]).to eq(["dependencies"])
+
+          pydantic = dependencies.find { |d| d.name == "pydantic" }
+          expect(pydantic.version).to eq("2.8.2")
+          expect(pydantic.requirements.first[:groups]).to eq(["dependencies"])
+        end
+      end
     end
 
     describe "with pep 735" do

python/spec/dependabot/python/update_checker_spec.rb

@@ -447,6 +447,47 @@
             .to eq(Gem::Version.new("2.5.0"))
         end
       end
+
+      context "when including hybrid Poetry + PEP621 dependencies without lockfile" do
+        let(:pyproject_fixture_name) { "pep621_with_poetry.toml" }
+        let(:dependency_files) { [pyproject] }
+
+        it "delegates to PipVersionResolver for PEP 621 dependencies" do
+          dummy_resolver =
+            instance_double(described_class::PipVersionResolver)
+          allow(described_class::PipVersionResolver).to receive(:new)
+            .and_return(dummy_resolver)
+          expect(dummy_resolver)
+            .to receive(:latest_resolvable_version)
+            .and_return(Gem::Version.new("2.5.0"))
+          expect(checker.latest_resolvable_version)
+            .to eq(Gem::Version.new("2.5.0"))
+        end
+      end
+
+      context "when including hybrid Poetry + PEP621 dependencies with lockfile" do
+        let(:pyproject_fixture_name) { "poetry_exact_requirement.toml" }
+        let(:poetry_lock) do
+          Dependabot::DependencyFile.new(
+            name: "poetry.lock",
+            content: fixture("poetry_locks", "exact_version.lock")
+          )
+        end
+        let(:dependency_files) { [pyproject, poetry_lock] }
+
+        it "delegates to PoetryVersionResolver when lockfile exists" do
+          dummy_resolver =
+            instance_double(described_class::PoetryVersionResolver)
+          allow(described_class::PoetryVersionResolver).to receive(:new)
+            .and_return(dummy_resolver)
+          expect(dummy_resolver)
+            .to receive(:latest_resolvable_version)
+            .with(requirement: ">=2.0.0,<=2.6.0")
+            .and_return(Gem::Version.new("2.5.0"))
+          expect(checker.latest_resolvable_version)
+            .to eq(Gem::Version.new("2.5.0"))
+        end
+      end
     end
   end