dependabot / dependabot-core

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

`PackageReference` and `PackageVersion` element does not take `Condition` into consideration #9299

Open samtrion opened 4 months ago

samtrion commented 4 months ago

Is there an existing issue for this?

Package ecosystem

nuget

Package manager version

latest

Language version

Multiple TargetFrameworks: net6.0;net7.0;net8.0

Manifest location and content before the Dependabot update

No response

dependabot.yml content

# To get started with Dependabot version updates, you'll need to specify which
# package ecosystems to update and where the package manifests are located.
# Please see the documentation for all configuration options:
# https://docs.github.com/github/administering-a-repository/configuration-options-for-dependency-updates

version: 2
updates:
  - package-ecosystem: "github-actions"
    directory: "/"
    schedule:
      interval: "daily"
    commit-message:
      prefix: "build(ci)"
    labels:
      - "dependency-actions"
    open-pull-requests-limit: 50

  - package-ecosystem: "nuget"
    directory: "/"
    schedule:
      interval: "daily"
    commit-message:
      prefix: "build(deps)"
    labels:
      - "dependency-nuget"
    open-pull-requests-limit: 50
    # groups:
    #   coverlet:
    #     patterns:
    #       - "coverlet*"
    #   testcontainers:
    #     patterns:
    #       - "testcontainers*"
    #   verify:
    #     patterns:
    #       - "verify*"
    #   xunit:
    #     patterns:
    #       - "xunit"
    #       - "xunit*"

  - package-ecosystem: "gitsubmodule"
    directory: "/"
    schedule:
      interval: "daily"
    commit-message:
      prefix: "build(mods)"
    labels:
      - "dependency-gitmodule"
    open-pull-requests-limit: 50
    groups:
      submodules:
        patterns:
          - "*"

Updated dependency

Microsoft.AspNetCore.TestHost

What you expected to see, versus what you actually saw

I expect, that the line 23 (see screenshot) will be updated, and the lines 24 and 25 will be skipped because of the conditional update.

Native package manager behavior

No response

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

image

Smallest manifest that reproduces the issue

No response

jzabroski commented 1 month ago

If you use Choose instead of Condition, does it work?


<Choose>
    <When Condition="'$(TargetFramework)'=='net6.0'">
        <ItemGroup>
           <PackageVersion Update="Microsoft.AspNet.TestHost" Version="6.0.26" />
        </ItemGroup>
    </When>
    <When Condition="'$(TargetFramework)'=='net7.0'">
        <ItemGroup>
           <PackageVersion Update="Microsoft.AspNet.TestHost" Version="7.0.14" />
        </ItemGroup>
    </When>
    <Otherwise>
        <Message Importance="High" Text="Microsoft.AspNet.TestHost has unsupported target framework '$(TargetFramework)' based upon build configuration. Please update." />
    </Otherwise>
</Choose>
samtrion commented 1 month ago

@jzabroski It was worth a try, but Choose is not evaluated within the Directory.Packages.props. So it could not assign a package version at all.

Lot's of NU1202 & other errors

samtrion commented 1 month ago

However, ItemGroups with conditions work as a workaround.

<ItemGroup Condition=" '$(TargetFramework)' == 'net8.0'">
  <PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="8.0.3" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'net7.0'">
  <PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="7.0.17" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'net6.0'">
  <PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="6.0.26" />
</ItemGroup>
jzabroski commented 1 month ago

Then, in a Directory.Packages.props file, does Visual Studio 2022 support PackageVersion with a Condition attribute? I actually don't know the answer.

samtrion commented 1 month ago

Then, in a Directory.Packages.props file, does Visual Studio 2022 support PackageVersion with a Condition attribute? I actually don't know the answer.

Yes, without problems so far.

AraHaan commented 2 days ago

This is odd because Dependabot for some reason is not updating my PackageVersion nodes in my https://github.com/Elskom/runtime repository for my Directory.Packages.props files (and yes I got multiple ones, one repo root, one in src folder, and 1 in ref folder in the repo, and possibly some other ones as well as some projects might also override it as well to reference other projects in the sln file too).

AraHaan commented 2 days ago

https://github.com/Elskom/runtime/pull/294 this should have been automatically created for me by dependabot but RIP.

jzabroski commented 2 days ago

This is odd because Dependabot for some reason is not updating

-- @AraHaan

Did you look at your Dependabot logs? The logs are somewhat nested under https://github.com/<organization>/<repository>/network/dependencies where <organization> for you would be Elskom and <repository> would be runtime

AraHaan commented 2 days ago

This is odd because Dependabot for some reason is not updating -- @AraHaan

Did you look at your Dependabot logs? The logs are somewhat nested under https://github.com/<organization>/<repository>/network/dependencies where <organization> for you would be Elskom and <repository> would be runtime

https://github.com/Elskom/runtime/actions/runs/9942475305/job/27463995927

/usr/local/dotnet/current/sdk/8.0.300/NuGet.targets(169,5): error : '[*-*]' is not a valid version string. [/tmp/package-dependency-resolution_MvzTrd/Project.csproj]

It seems something caused this to happen on it which is odd considering it works locally on my nuget.config file.