buildDotnetModule: allow fetching nuget dependencies at compile time#314990
buildDotnetModule: allow fetching nuget dependencies at compile time#314990purepani wants to merge 5 commits intoNixOS:masterfrom
Conversation
github-actions
bot
added
the
8.has: documentation
| , ... } @ args: | ||
|
|
||
| assert projectFile == null -> throw "Defining the `projectFile` attribute is required. This is usually an `.csproj`, or `.sln` file."; | ||
| assert (nugetDeps != null && nugetSha256 != null) -> throw "The attributes `nugetDeps` and 'nugetSha256' are mutually exclusive!"; |
There was a problem hiding this comment.
Triggers with buildDotnetGlobalTools since it passes through the same attr.
| dotnetValidateLockfileHook = makeSetupHook | ||
| { | ||
| name = "dotnet-validate-lockfile-hook"; | ||
| deps = [ jq ]; |
There was a problem hiding this comment.
Depreciated; replace with propagagedBuildInputs
|
Result of 61 packages built:
|
|
Have you any package this new hash can be tested with? Maybe we can also remove the lock file for a package in nixpkgs for testing? |
|
I just tried it with jellyfin with no success; looks like some more work needs to be done. |
|
|
||
| # This is the last phase that runs before we error out about the hash being wrong | ||
| postFixup = lib.optionalString (nugetSha256 == null) '' | ||
| echo "Please set nugetSha256 to the hash below!" |
There was a problem hiding this comment.
New things should ideally only support hash
This allows user to specify a hash for the dependencies using the "nugetSha256" attribute, instead of having to manually generate a lockfile. This is done by generating a lockfile with `dotnet restore`, and then parsing the requested version ranges to see if anything is floating. Afterwards we generate a nix-based lockfile containing hashes and stable downloads for each dependency, which we can IFD. The checksum of this lockfile is specified with "nugetSha256". We want to use the checksum of the nix-based lockfile instead of hashing the entire nuget source so that we can independantly fetch all dependencies, and re-use them across derivations. Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
purepani
force-pushed
the
dotnet-module-fetch-impure-deps-new
branch
from
e2c84cf to
c84d0a1
Compare
Motivated to get this working again due to the recent ascii regression. It is unclean, and probably needs to be looked at a lot closer to pull any more recent updates that have been created from the original pr, but it now works, successfully building jellyfin! |
purepani
force-pushed
the
dotnet-module-fetch-impure-deps-new
branch
from
c84d0a1 to
5987390
Compare
purepani
left a comment
purepani
left a comment
There was a problem hiding this comment.
Still some updates needed, particularly documentation
| find -name paket.lock -exec sed -i 's+remote:.*+remote: @nugetSource@/lib+g' {} \; | ||
|
|
||
| dotnet tool restore --add-source "@nugetSource@/lib" | ||
| #dotnet tool restore --add-source "@nugetSource@/lib" |
There was a problem hiding this comment.
Commented out due to making a network call; I believe that telemetry will just need to be disabled with export DOTNET_CLI_TELEMETRY_OPTOUT=1 now that I think about it.
|
|
||
| passthru = { | ||
| inherit icu; | ||
| packages_list = packages; |
There was a problem hiding this comment.
Wasn't sure how to get the list of projects so I just readded this since the previous pr did that. If there's a better way do get the packages already retrieved from dotnet, then we can change this.
|
Oh so I guess I didn't notice that the original used IFD(Was it not banned back then?). Next step is to remove that. |
|
Should we wait for NuGet/Home#13288 to be considered before implementing something like this? |
wegank
added
the
2.status: merge conflict
wegank
added
the
2.status: stale
5 participants
A (quick) attempt to rebase #162548
I have tested building jellyfin so far, but nothing else to see if it broke anything. I haven't had a chance to update the warning or docs, but i wanted to at least get nixpkgs-review running on the PR. I also haven't had a chance to see if the hash specification still works, but i wanted to at least get nixpkgs-review running on it to make sure nothing else broke.
Pinging people from the last PR:
@jonringer @SuperSandro2000 @winterqt @IvarWithoutBones @Mic92
Hope these are reasonable pings; sorry if not!
Also I'm not quite so sure why almost everyone's name was removed from authoring the pr? I just noticed that, so I'll see if I can fix that tomorrow. There's probably a bunch documentation stuff that needs to be updated as well.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.