Rewriting the NuGet updater #10865
Labels
L: dotnet:nuget
NuGet packages via nuget or dotnet
Comments
|
Would this new version also be able to correctly apply package source mappings from the nuget.config file and use authenticated feeds? Currently I cannot seem to utilize a nuget.config file in my repository that looks like this one: https://learn.microsoft.com/en-us/nuget/consume-packages/package-source-mapping#enable-by-manually-editing-nugetconfig Reading the dependabot logs it looks like it is completely ignoring the nuget.config file (making api requests looking for the package on both sources) and then it fails with an auth error after it has repeatedly made valid authenticated requests for packages that are not in the private feed. I'm wondering if this is one of those wait for this ticket to be completed and then try it things or if a separate ticket should be entered to track this asap. I'm assuming that in rewriting this a dependency on nuget will be taken and an innate understanding of the nuget.config file (and other oddball features this dotnet tooling works with) will be brought in. |
|
@bbarry Authenticated feeds already work and Do you have a public repo with the |
|
I do not have a public repo for this but I'll see what I can do about reproducing it with one. My repo that fails when I add a nuget feed with package source mapping is working without it. detailsThe dependabot.yaml file for it is: and we have a nuget.config file and a .sln file in the root of the repo: If I change the nuget.config file to this then dependabot fails with an auth error to the private feed after making a number of successful requests: and the log has authenticated requests to the private feed as well as requests from the public feed that I wouldn't expect (this might be a defect in nuget or dotnet tooling based on the fact that The failed log with a bunch of seemingly clearly irrelevant lines removed: It successfully recognizes that the private feed needs to be in the authenticated list and makes a request to it and gets a 200 response (after failing once) but then once it starts requesting actual packages they fail with 401s repeatedly. I would expect (but do not see) a line repeated in the proxy logs for each of the requests to the private feed: which does appear after I change the nuget.config file back to the first version |
|
I'd need to see the whole log to really see what's happening regarding the private feed. If you can't post it here and are willing to share it via email, you can send it directly to me where only I'll see it and I'll delete it as soon as I'm done. Regarding the unexpected requests to the feed, I'd be able to tell from the full log, but my guess is that it's doing the correct thing. The behavior of the NuGet protocol is that it will query every feed for |
|
I can share it here, there is really nothing else in it that I didn't share already as far as I know (I expanded all the collapsable sections from the action log and added newlines around the timings to separate the sections: logThe repo is currently in a state where dependabot is working, but I have the nuget.config file without the |
|
@bbarry Thank you for the logs, I think I know what's going on. Your |
|
@brettfo Could you please consider adding .nfproj file as a replacement of .csproj? There is a project under .NET Foundation that uses different extension due to different build system (.net for MCU) Pinging @josesimoes |
|
@torbacz We're not yet in a position to enable more than just the three we currently support, |
|
Hi @brettfo , |
|
@AndrewNikolin our workaround is a workflow that run on PRs labels with ".NET" to run Workflow
name: Update Lockfiles
on:
pull_request:
types: [ labeled ]
workflow_dispatch:
jobs:
lockfiles:
if: ${{ github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && github.event.label.name == '.NET' ) }}
runs-on: ubuntu-latest
permissions:
contents: write
packages: read
steps:
- uses: actions/checkout@v4
with:
token: ${{ secrets.LOCKFILES_UPDATE_PAT }}
- name: Update Lockfiles
run: dotnet restore --force-evaluate
- name: Commit
uses: stefanzweifel/git-auto-commit-action@v5
with:
commit_message: "chore: update lockfiles"
|
|
@a-jackson thank you! I've seen that approach, but I'm just a little concerned that when run by default it'll stop dependabot rebasing as you mentioned. I've noticed that it does that quite often, sometimes even after a single dependency PR has been merged it starts updating the others. Will likely need some integration to run only when requested |
|
@AndrewNikolin updating lock files in other projects would be a separate work item that could be addressed with the current updater and isn't dependent on the rewrite. The main issue is one of prioritization and timing; in practice lock files aren't commonly used so that feature would get bumped in favor of other wider reaching fixes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The NuGet updater for dependabot is getting rewritten from the ground up, all the way from
git clone ...tocreate_pull_request. This is meant to be a sort of tracking issue as well as hopefully explaining why it needs to be done.There is no current ETA, but it will take a while. More details below.
First a quick FAQ:
dependabot-corebroken/busted/wrong/etc.?dependabot-corecode that will have to be re-implemented in C#. A non-exhaustive list:Why does the NuGet updater need to be rewritten?
First some details. N.b., I'm going to lie a little bit here; nothing major, just to simplify the discussion.
/client.SomePackage/1.0.0andSomePackage/2.0.0in the same directory/file.For an ecosystem like NPM this works out well. If a file location is given, it will be
package.json. Otherwise if the file location is omitted, it is assumed to bepackage-lock.json.For NuGet, this simply doesn't work.
.csproj..csprojis still needed to know where an update might need to be performed..slnfile referencing a.csproj.csprojfile referencing another.csprojvia<ProjectReference ... />.projfile referencing another.projor.csproj.Because an update job started in a single directory can navigate to any other directory in the repo and because there is no "default" file for transitive dependencies, the dependencies discovered can be complicated.
Consider this example:
/tests.UnitTests.sln..slnreferences the following projects (full repo paths given):/tests/Client/ClientUnitTests.csproj/tests/Server/ServerUnitTests.csproj/srcthat I'm omitting for this example)/tests.Now on to dependencies.
ServerUnitTests.csprojwas created first and has a single top-level dependency:xunit.extensions/2.0.0. This means it also has a transitive dependency ofxunit/2.0.0.ClientUnitTests.csprojwas added and there is no relationship between the client and server test projects. WhenClientUnitTests.csprojwas added, an explicit top-level dependency ofxunit/2.4.1was added.Already we're in trouble. For an update job started in the
/testsdirectory, we need to report the following:xunit.extensions2.0.0true/tests/Server/ServerUnitTests.csprojxunit2.0.0false/tests/Server/ServerUnitTests.csprojBUT WE CAN'T REPORT THIS TO DEPENDABOT.xunit2.4.1true/tests/Client/ClientUnitTests.csprojAs it stands, there is no way to report those dependencies up the chain; we have to keep file location information with all of them which by default marks them as top-level (in the way that the common code considers things.)
It gets worse.
Once the dependencies are passed back to the common
dependabot-corecode, the dependencies are all collapsed based on the name (because you can't have multiple versions of the same dependency (except you can in NuGet)) and only the first version is kept. The common code then uses those versions to determine if an update job needs to be performed.If the purpose of an update job was to move
xunit/2.0.0toxunit/2.4.1and if the dependency from the server project was reported first, an update will be performed because2.0.0 < 2.4.1. If, however,xunit/2.4.1from the client project was reported first, the common code will think that everything is up to date, because2.4.1 == 2.4.1, and no update is necessary. There are more complications, but I'll leave it here for now.Conclusion
Rewriting the NuGet updater entirely in C# because the concept of a dependency in NuGet (top-level vs. transitive, different versions of the same dependency, etc.) is a really weird beast.
The text was updated successfully, but these errors were encountered: