search

dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.74k stars 1.03k forks source link

Dependency scanning fails for an MSBuild SDK NuGet package referenced via an `<Sdk>` tag #8615

Open Zastai opened 11 months ago

Zastai commented 11 months ago

Is there an existing issue for this?

Package ecosystem

NuGet

Package manager version

.NET SDK 8.0.100

Language version

No response

Manifest location and content before the Dependabot update

the project file currently has:

<Sdk Name="MetaBrainz.Build.Sdk" Version="3.1.0" />

to pull in the build SDK from a NuGet package (see also #2839).

dependabot.yml content

dependabot.yml

Updated dependency

MetaBrainz.Build.Sdk, going from 3.1.0 to 3.1.1.

What you expected to see, versus what you actually saw

A PR is created that modifies the project file to have

<Sdk Name="MetaBrainz.Build.Sdk" Version="3.1.1" />

Native package manager behavior

As far as I know, no native tooling handles these kinds of references. However, Dependabot should (I had a PR merged that handled the various ways an SDK reference like this can be written).

Images of the diff or a link to the PR, issue, or logs

Logs of failed Dependabot run.

It looks like Dependabot finds the package reference just fine, and correctly determines that a new version is available:

...
updater | 2023/12/14 20:01:59 INFO <job_762449425> Checking if MetaBrainz.Build.Sdk 3.1.0 needs updating
...
updater | 2023/12/14 20:01:59 INFO <job_762449425> Latest version is 3.1.1
...
updater | Finding updated dependencies for MetaBrainz.Build.Sdk.
...
updater | 2023/12/14 20:01:59 INFO <job_762449425> Updating MetaBrainz.Build.Sdk from 3.1.0 to 3.1.1

However, it then runs "NuGetUpdater.Cli" to modify the reference, and this apparently doesn't do SDK packages:

updater | /opt/nuget/NuGetUpdater/NuGetUpdater.Cli update --repo-root /home/dependabot/dependabot-updater/repo --solution-or-project /home/dependabot/dependabot-updater/repo/MetaBrainz.Common/MetaBrainz.Common.csproj --dependency MetaBrainz.Build.Sdk --new-version 3.1.1 --previous-version 3.1.0  --verbose
...
updater |   No global.json files found.
updater |   No dotnet-tools.json files found.
updater | Running for project [/home/dependabot/dependabot-updater/repo/MetaBrainz.Common/MetaBrainz.Common.csproj]
updater |   Running for SDK-style project
updater |     Package [MetaBrainz.Build.Sdk] Does not exist as a dependency in [/home/dependabot/dependabot-updater/repo/MetaBrainz.Common/MetaBrainz.Common.csproj].
updater | Update complete.

Smallest manifest that reproduces the issue

A project containing only

<Project>
  <Sdk Name="MetaBrainz.Build.Sdk" Version="3.1.0" />
</Project>

will likely reproduce the issue.

In order for a build to succeed, the following files should be present alongside the project file: build-package.ps1, LICENSE.md and README.md.

Zastai commented 11 months ago

Did some initial digging.

MSBuildHelper.GetTopLevelPackageDependenyInfos() does not return the SDK reference to begin with. Given how the global.json updater simply checks for a version and immediately updates it, I'm not convinced that it needs to either. I'm just not sure whether there needs to be an SdkReferenceUpdater, or just extra functionality in SdkPackageUpdater.

In any case, there are 3 syntaxes for MSBuild SDK references:

  1. A single Sdk attribute on the Project element. This is a semicolon-separated list of SDKs. An SDK can be "Foo/1.2.3" to reference a NuGet package (if it's specified as just "Foo" it can still be a package but then the version is managed in global.json which has its own updater). A special consideration here seems to be that Foo/min=1.2.3 is allowed, which makes 1.2.3 a minimum version rather than an exact one.
  2. One or more Sdk elements under a Project, with Name and Version (and/or MinimumVersion) attributes.
  3. Sdk and Version (and/or MinimumVersion) attributes on an Import element (Version can be omitted, but then the version is managed by global.json again; I assume MinimumVersion is allowed here too).

For syntax 1, ProjectRootElement.Sdk contains the attribute contents, and with a split on ';' it's easy to extract info using SdkReference.TryParse().

For syntax 2, ProjectRootElement provides no convenience property (will be logging an MSBuild issue for that), Children.OfType<ProjectSdkElement> should give you those elements.

For syntax 3, ProjectRootElement.Imports gets you all Imports with properties for each of the attributes.

My assumption of the required processing would be: