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

[Backport release-24.11] dotnet: migrate nuget lockfiles to JSON #366269

Merged
merged 10 commits into from
Dec 19, 2024
Merged

[Backport release-24.11] dotnet: migrate nuget lockfiles to JSON #366269

merged 10 commits into from
Dec 19, 2024

Conversation

corngood
Copy link
Contributor

@corngood corngood commented Dec 18, 2024 •
edited
Loading

Backport of #362278

I've removed all conflicting packages from the treewide commit.

Cc: @NixOS/dotnet

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.

@corngood corngood requested a review from GGG-KILLER December 18, 2024 19:28
@github-actions github-actions bot added 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog 6.topic: games 6.topic: dotnet Language: .NET labels Dec 18, 2024
@corngood
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 366269

x86_64-linux

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

@MattSturgeon
Copy link
Contributor

I've removed all conflicting packages from the treewide commit.

Would it be reasonable to re-generate json lockfiles for those packages too, and amend the treewide commit?

✅ 3 packages built:

Had me worried there was an eval difference, but of course it is the infrastructure packages that are actually supposed to be different.

@corngood
Copy link
Contributor Author

Would it be reasonable to re-generate json lockfiles for those packages too, and amend the treewide commit?

I sort of don't think it's worth the effort, because it's mostly due to packages that have already made changes that weren't backported. If someone wants to do it though, I have no problem with that.

@MattSturgeon
Copy link
Contributor

MattSturgeon commented Dec 18, 2024 •
edited
Loading

Would it be reasonable to re-generate json lockfiles for those packages too, and amend the treewide commit?

I sort of don't think it's worth the effort, because it's mostly due to packages that have already made changes that weren't backported.

That's fair.

If someone wants to do it though, I have no problem with that.

If so, probably best to use @GGG-KILLER's script: migrate-nuget.sh

@ofborg ofborg bot added 8.has: clean-up 8.has: package (new) This PR adds a new package labels Dec 19, 2024
@GGG-KILLER
Copy link
Contributor

If possible I think it'd be better to migrate all lockfiles to JSON to make it easier on people who want to backport changes after our treewide on master.

If you'd like I could do that on another PR since I don't have write perms to your branch.

@corngood
Copy link
Contributor Author

@GGG-KILLER I can always pull your change here if you share it somewhere

GGG-KILLER and others added 8 commits December 19, 2024 13:57
@GGG-KILLER
nuget-to-json: init
8f626ea 
(cherry picked from commit 1726304)
@GGG-KILLER
nuget-to-nix: refactor on top of nuget-to-json and deprecate
9c05a0d 
(cherry picked from commit 3cc4563)
@MattSturgeon @GGG-KILLER
addNuGetDeps: support loading JSON lockfiles
cf30bb1 
In addition to loading nix lockfiles

(cherry picked from commit bccae9e)
@GGG-KILLER
buildDotnetModule.fetch-deps: use json lockfiles by default
7707d12 
(cherry picked from commit c94c508)
@GGG-KILLER
dotnetCorePackages.mkNugetDeps: support loading JSON lockfiles
de32a2c 
(cherry picked from commit 8ea7650)
@GGG-KILLER
update-dotnet-lockfiles: update comment to mention nuget-to-json
f39ee3d 
(cherry picked from commit 318b37a)
@GGG-KILLER
treewide: migrate nix-based dotnet lockfiles to JSON
9cf669b 
(cherry picked from commit 1cd9516)
@GGG-KILLER
docs: add buildDotnetModule changes to release notes.
61f266e 
(cherry picked from commit 2b4190a)
@GGG-KILLER
Copy link
Contributor

GGG-KILLER commented Dec 19, 2024 •
edited
Loading

@corngood I've pushed my changes to https://github.com/GGG-KILLER/nixpkgs/tree/backport-362278-to-release-24.11

I haven't added any changes from #366556 nor #366574 because they're still under review.

@GGG-KILLER @corngood
addNugetDeps.fetch-deps: remove set -u
23b5162 
It was causing issues with other hooks and things used in the build for packages so I opted to remove it as it wasn't there to start with.
@GGG-KILLER @corngood
nuget-to-json: fix tools not being added to the lockfile
a0485c9
@corngood corngood force-pushed the backport-362278-to-release-24.11 branch from e78ec6d to a0485c9 Compare December 19, 2024 19:44
@corngood
Copy link
Contributor Author

@GGG-KILLER I grabbed your branch, plus those two fixes. Let me know what you think. Thanks for doing the rest of the conversions.

@MattSturgeon
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 366269

x86_64-linux

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

GGG-KILLER
GGG-KILLER approved these changes Dec 19, 2024
View reviewed changes
Copy link
Contributor

@GGG-KILLER GGG-KILLER left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@corngood corngood merged commit c59b3cb into NixOS:release-24.11 Dec 19, 2024
21 of 23 checks passed
@corngood corngood deleted the backport-362278-to-release-24.11 branch December 19, 2024 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GGG-KILLER GGG-KILLER GGG-KILLER approved these changes

@MattSturgeon MattSturgeon MattSturgeon approved these changes

@Atemu Atemu Awaiting requested review from Atemu

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.
3 participants
@corngood @MattSturgeon @GGG-KILLER