Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

dotnet: migrate nuget lockfiles to JSON #362278

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

dotnet: migrate nuget lockfiles to JSON #362278

wants to merge 8 commits into from
+67,096 −49,582

Conversation

GGG-KILLER
Copy link
Contributor

@GGG-KILLER GGG-KILLER commented Dec 6, 2024 •
edited
Loading

This changes the buildDotnetModule lockfiles to be JSON instead of Nix.

Changes were made in a backwards-compatible fashion, however a warning was added to guide users to migrate to the new format.

Existing in-tree packages were migrated to the new format using a helper script and manual changes for places that didn't have the file directly specified.

cc: @NixOS/dotnet @MattSturgeon @UlyssesZh @TomaSajt @SuperSandro2000 @Atemu

Closes #362182, closes #325053.
Fixes #358025.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: python 8.has: documentation This PR adds or changes documentation 6.topic: games 6.topic: dotnet Language: .NET labels Dec 6, 2024
@nix-owners nix-owners bot requested review from mmahut and RaghavSood December 6, 2024 02:00
@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from 466c5b9 to f7e34db Compare December 6, 2024 02:01
@GGG-KILLER GGG-KILLER changed the title Migrate NuGet lockfiles to JSON dotnet: migrate nuget lockfiles to JSON Dec 6, 2024
@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from f7e34db to a91a587 Compare December 6, 2024 02:06
@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from a91a587 to 4996549 Compare December 6, 2024 02:14
@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from 4996549 to a29b4ca Compare December 6, 2024 02:44
@GGG-KILLER

This comment was marked as outdated.

Sign in to view
MattSturgeon
MattSturgeon reviewed Dec 6, 2024
View reviewed changes
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial minor questions/suggestions. Thanks for working on this, it's looking good from my perspective!

pkgs/build-support/dotnet/make-nuget-deps/default.nix Outdated Show resolved Hide resolved
pkgs/build-support/dotnet/make-nuget-deps/default.nix Outdated Show resolved Hide resolved
pkgs/by-name/nu/nuget-to-json/package.nix Outdated Show resolved Hide resolved
pkgs/build-support/dotnet/add-nuget-deps/default.nix Show resolved Hide resolved
pkgs/by-name/nu/nuget-to-json/nuget-to-json.sh Outdated Show resolved Hide resolved
pkgs/by-name/nu/nuget-to-json/nuget-to-json.sh Outdated Show resolved Hide resolved
pkgs/build-support/dotnet/make-nuget-deps/default.nix Outdated Show resolved Hide resolved
Atemu
Atemu reviewed Dec 6, 2024
View reviewed changes
Copy link
Member

@Atemu Atemu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small notes

pkgs/build-support/dotnet/add-nuget-deps/default.nix Outdated Show resolved Hide resolved
pkgs/build-support/dotnet/add-nuget-deps/default.nix Outdated Show resolved Hide resolved
pkgs/build-support/dotnet/add-nuget-deps/default.nix Outdated Show resolved Hide resolved
pkgs/build-support/dotnet/add-nuget-deps/default.nix Outdated Show resolved Hide resolved
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 6, 2024
@UlyssesZh UlyssesZh mentioned this pull request Dec 7, 2024
Open
13 tasks
@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from 0081275 to 4b66dfa Compare December 12, 2024 19:04
@GGG-KILLER
Copy link
Contributor Author

Removed the lib.warn on nix-based lockfiles since the SDK still uses them and I don't plan to migrate it away from it right now as it seems rather complicated.

@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from 4b66dfa to 2f2c7bd Compare December 12, 2024 19:07
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 13, 2024
@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from 2f2c7bd to efab88d Compare December 14, 2024 02:16
@GGG-KILLER
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 362278

x86_64-linux

⏩ 1 package blacklisted:
  • nixos-install-tools
✅ 3 packages built:
  • nixpkgs-manual
  • nuget-to-json
  • nuget-to-nix

@GGG-KILLER GGG-KILLER removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 14, 2024
@GGG-KILLER
Copy link
Contributor Author

I think this PR should be good to merge now since all feedback has been addressed.

@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from efab88d to d090ef7 Compare December 14, 2024 02:56
gepbird
gepbird suggested changes Dec 14, 2024
View reviewed changes
doc/languages-frameworks/dotnet.section.md Outdated Show resolved Hide resolved
doc/languages-frameworks/dotnet.section.md Outdated Show resolved Hide resolved
@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from d090ef7 to 8f87cfb Compare December 14, 2024 09:01
@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from 8f87cfb to 50e4c7b Compare December 14, 2024 09:06
gepbird
gepbird approved these changes Dec 14, 2024
View reviewed changes
Copy link
Contributor

@gepbird gepbird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested one package: both json and nix fetch deps scripts work and builds with the generated deps file.

This PR will probably cause merge conflicts for 10 other (non-treewide) PRs, I think that's fine
  • issue-365007.diff|3 col 1| --- a/pkgs/by-name/vr/vrcadvert/deps.nix
  • issue-364325.diff|261 col 1| --- a/pkgs/by-name/to/tone/nuget-deps.nix
  • issue-364200.diff|32 col 1| --- a/pkgs/servers/web-apps/kavita/nuget-deps.nix
  • issue-363469.diff|211 col 1| --- a/pkgs/by-name/pa/parabolic/deps.nix
  • issue-361232.diff|17 col 1| --- a/pkgs/by-name/wa/wasabibackend/deps.nix
  • issue-358401.diff|45 col 1| --- a/pkgs/servers/web-apps/kavita/nuget-deps.nix
  • issue-345209.diff|104 col 1| --- a/pkgs/tools/games/scarab/deps.nix
  • issue-339220.diff|3 col 1| --- a/pkgs/by-name/so/sonarr/deps.nix
  • issue-339370.diff|3 col 1| --- a/pkgs/by-name/be/beatsabermodmanager/deps.nix
  • issue-310237.diff|22 col 1| --- a/pkgs/development/tools/azure-functions-core-tools/deps.nix

doc/languages-frameworks/dotnet.section.md Show resolved Hide resolved
@ofborg ofborg bot added 8.has: clean-up 8.has: package (new) This PR adds a new package labels Dec 14, 2024
@GGG-KILLER GGG-KILLER force-pushed the nuget-lockfile-to-json branch from 50e4c7b to c05508f Compare December 16, 2024 15:42
@GGG-KILLER GGG-KILLER requested a review from corngood December 16, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gepbird gepbird gepbird approved these changes

@mmahut mmahut Awaiting requested review from mmahut

@RaghavSood RaghavSood Awaiting requested review from RaghavSood

@Atemu Atemu Awaiting requested review from Atemu

@MattSturgeon MattSturgeon Awaiting requested review from MattSturgeon

@corngood corngood Awaiting requested review from corngood

Assignees
No one assigned
Labels
6.topic: dotnet Language: .NET 6.topic: games 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 8.has: changelog 8.has: clean-up 8.has: documentation This PR adds or changes documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

nuget-to-nix: Generated file not formatted
6 participants
@GGG-KILLER @Atemu @corngood @MattSturgeon @gepbird @wegank