dotnet: infrastructure improvements #336824

corngood · 2024-08-23T17:54:32Z

Description of changes

The most important parts of this change are:

dotnet-sdk-setup-hook: configure nuget in sourceRoot
- Previously we created a nuget.config in $NIX_BUILD_TOP (or wherever genericBuild was executed from. Instead, we now attempt to make the changes to the root nuget.config in sourceRoot, creating a default one if it doesn't exist.
- This should fix things like clps2c-compiler: init at 1.0.1 #339716.
- It uses quite a bit of hairy xmlstarlet logic that I would like to eventually move to something more robust (C# maybe, if bootstrapping can be done in a sane way?).
dotnetCorePackages: split fetch-deps logic into addNuGetDeps
- This allows fetch-deps to be added to any stdenv package, not just via buildDotnetModule.
- msbuild: use addNuGetDeps is a good example of how this is used
avalonia: init at 11.0.10
- Add source-built avalonia package.
- This is currently only used in nexusmods-app. I've tested all other Avalonia 11 apps with it, but I want to get a better idea of how upgrading is going to work before forcing it on all maintainers.
- avalonia: 11.0.10 -> 11.0.11
  - This demonstrates how an upgrade would work.
add mapNuGetDependencies
- This is an opt-in setting that adds all packages found in nix dependencies to packageSourceMapping.
- It's currently only used in nexusmods-app in order to force it to use avalonia 11.0.11.
- I see this potentially being the default at some point.

Supporting work:

nuget-to-nix: use dotnet from PATH
nuget-package-hook: move package unpacking from buildDotnetModule
- This means that any package outputting $out/share/nuget/source/*.nupkgs will have them repacked and fixed-up as with buildDotnetModule.
dotnet-sdk-setup-hook: run pre/postConfigureNuGet hooks
- These are handy for packages that need to do hacks on nuget.config, dependency versions, etc.
These things make nuget-to-nix significantly faster:
- nuget-to-nix: use xmlstarlet instead of xq (yq)
- add-nuget-deps: use NUGET_HTTP_CACHE_PATH from user's HOME

Unrelated fixes to problems I found along the way:

disable parallel builds for some packages with intermittent failures after buildDotnetModule: inherit enableParallelBuilding #339335
misc cleanup to dotnet infrastructure code
QOL changes for update-dotnet-lockfiles
- update-dotnet-lockfiles: skip packages that don't evaluate
- update-dotnet-lockfiles: add startWith flag

Things done

Add a 👍 reaction to pull requests you find important.

GGG-KILLER

A few initial comments

pkgs/build-support/dotnet/build-dotnet-module/default.nix

pkgs/build-support/dotnet/build-dotnet-module/hooks/dotnet-install-hook.sh

GGG-KILLER · 2024-08-23T18:50:40Z

pkgs/build-support/dotnet/nuget-to-nix/default.nix

@@ -25,7 +25,6 @@ runCommandLocal "nuget-to-nix" {
      curl
      gnugrep
      gawk
-      dotnet-sdk


Wouldn't this be a breaking change since it now retrieves the dotnet from path (which isn't guaranteed to be there)?

Yeah, I'm still not sure this is a good idea, but having to override it all over the place seems wrong.

GGG-KILLER · 2024-08-23T18:56:08Z

pkgs/by-name/av/avalonia/package.nix

+          with dotnetCorePackages;
+          combinePackages [
+            sdk_6_0
+            sdk_7_0_1xx


Should we really add a new dependency on .NEY 7 when it's EOL?

I specifically chose the version of avalonia used by nexusmods-app, and it's pinned to 7.0.1xx. Newer versions are on 8 I believe.

I'll do some experimenting with newer versions, but I don't think it's a worse situation than pulling the binary blob, which was also built with the EOL sdk.

Is this a situation where it'd be useful to have multiple versions of avalonia packaged? Or perhaps that's jumping the gun, if it's currently only used by nexusmods-app...

I hope that we won't have to do any more than package multiple major versions of packages.

For example, right now I see things in nixpkgs using avalonia:

0.10.12

0.10.13

0.10.15

0.10.18

11.0.4

11.0.5

11.0.9

11.0.10

11.0.11

So maybe we'd get away with just avalonia_11, until 12 exists? 0.10 could probably stay as binary. Some work will probably still have to be done to relax version constraints and ensure we're prioritising packages from nixpkgs.

Another good example is newtonsoft.json. To support all major versions currently used in nixpkgs, we'd need newtonsoft-json_9, _10, _11, _12, and _13.

MattSturgeon

Most of this goes a little over my head, but I like the high level direction!

Speaking from the nexusmods perspective, this seems useful.

MattSturgeon · 2024-08-23T19:18:52Z

pkgs/by-name/ne/nexusmods-app/deps.nix

Glad to see 14 deps dropped here.

Slightly confused why there's still a handful of Avalonia deps in the file.

Also confused why running fetch-deps has given us new hashes for each dep even though their versions are unchanged?

Slightly confused why there's still a handful of Avalonia deps in the file.

Some are from separate repos I believe (buildsystem?), but some are just older versions, which is something I need to investigate.

Also confused why running fetch-deps has given us new hashes for each dep even though their versions are unchanged?

I believe that's just changing to SRI hashes, and the actual hash values aren't actually changed.

pkgs/by-name/av/avalonia/package.nix

pkgs/by-name/ne/nexusmods-app/package.nix

pkgs/build-support/dotnet/make-nuget-deps/default.nix

js6pak · 2024-09-05T22:10:48Z

add avalonia as a source-built package and patch it so it can find libX11 etc from nixpkgs

It would also be fairly easy to make a patchelf-like tool that replaces the shared library names in DllImportAttributes and/or inject an DllImportResolver inside built IL assembly files.
I could make one if you think it's a good way to go about getting rid of runtimeDeps.

corngood · 2024-09-06T03:01:31Z

I've split some of this stuff into: #339953.

@js6pak patching sounds like an interesting idea. The PR I referenced has the package overriding mechanism in it, and you can see how it adds the runtime deps specifically for Avalonia.X11. If that was replaced with assembly patching, it would be more robust.

corngood · 2024-09-08T15:59:53Z

I've been doing some more work on the source-built avalonia, and I'm trying to figure out the best way to consume it in other packages.

Say we add the package avalonia, and try to keep it up to date. That means we'd have avalonia 11.1.3 in nixpkgs. We can add it as a dependency, and if the dependent package is looking for that exact version, it will use it. If it's looking for e.g. 11.0.10, it won't be used. This is because of:

https://learn.microsoft.com/en-us/nuget/concepts/dependency-resolution#lowest-applicable-version

I can think of a couple of ways of approaching this.

use packageSourceMapping to map these packages to the _nix source, which will make them the only available versions in the build.
patch the dependent projects to use the correct version, or possibly floating versions?

I've implemented (1), and it's promising, but you can't map by version (NuGet/Home#13351), so you can't really have a mix of versions from nuget.org and nixpkgs for a specific package. It also complicates things because turning on packageSourceMapping requires all sources in the project to use it (so we have to do a bunch more nuget.config patching).

(2) is more work per-package, and if you use floating versions, your source-built dependency can be overridden in fetch-deps when a newer version is released on nuget.org.

Anyone have any thoughts/ideas? @GGG-KILLER @js6pak @MattSturgeon maybe?

MattSturgeon · 2024-09-08T20:17:28Z

My first thought is that if we want fuzzy versions, this should be up to the package to declare. Replacing nuget x.y.z with source-built x.y.z should usually be fine, but doing fuzzy matching like x.y.* could be more likely to introduce unexpected issues?

Alternatively, instead of doing version matching, we could simply have a insteadOfNuget inputs set? An attrset mapping nuget names to derivations to be used instead of what is listed in deps.nix.

This has the pro and con of being more explicit about which inputs are handled by nuget and which come from nixpkgs.

corngood · 2024-09-08T21:56:04Z

My first thought is that if we want fuzzy versions, this should be up to the package to declare. Replacing nuget x.y.z with source-built x.y.z should usually be fine, but doing fuzzy matching like x.y.* could be more likely to introduce unexpected issues?

Well a dependency like this from nexusmods-app:

<PackageVersion Include="Avalonia" Version="11.0.10" />

Really means >= 11.0.10, so if only 11.0.11 (or 12.0.0 for that matter) exists, it will use it without complaining.

NuGet calls normal semver dependencies 'floating versions'. If those were more widely used (for example nexusmods-app could probably use 11.0.* or 11.*), then this would be more intuitive IMO.

GGG-KILLER · 2024-09-09T11:36:24Z

Well a dependency like this from nexusmods-app:
<PackageVersion Include="Avalonia" Version="11.0.10" />
Really means >= 11.0.10, so if only 11.0.11 (or 12.0.0 for that matter) exists, it will use it without complaining.

Given this, couldn't we have the option of just packaging the latest version for each major without needing to change anything?

corngood · 2024-09-09T13:02:21Z

Given this, couldn't we have the option of just packaging the latest version for each major without needing to change anything?

Yeah, sort of, but only if you use the package source mapping to force it to pick packages from nixpkgs instead of nuget.org.

If nixpkgs has 11.0.11, then nugetmods-app will use it if it's the only option. If it can find both 11.0.11 (from nixpkgs) and 11.0.10 (from nuget.org), it'll use the later due to 'lowest applicable version'.

I personally think we should force the nixpkgs dependencies, as long as they meet the project constraints. The default behaviour (i.e. 'lowest applicable version') seems like a nightmare for rolling out security fixes. We'd also be encouraging projects to declare dependencies more carefully than just >= 11.0.10.

GGG-KILLER · 2024-09-09T13:05:08Z

Yeah, sort of, but only if you use the package source mapping to force it to pick packages from nixpkgs instead of nuget.org.

If nixpkgs has 11.0.11, then nugetmods-app will use it if it's the only option. If it can find both 11.0.11 (from nixpkgs) and 11.0.10 (from nuget.org), it'll use the later due to 'lowest applicable version'.

But given that we use a directory as a source on the dotnet restore stage, how would it get in touch with nuget.org? Or do you mean on the fetch-deps stage?

xq has such a long startup time that this significantly improves performance.

linkNugetPackages and linkNuGetPackagesAndSources can now be disabled by setting them to false. linkNugetPackages will use _linkPackages, which is much faster.

This allows the script to run on platforms that can't fetch all of the packages, or without allowing unfree.

This allows you to resume after a failure by passing the name of the first package you want to be fetched.

corngood · 2024-09-17T04:21:07Z

But given that we use a directory as a source on the dotnet restore stage, how would it get in touch with nuget.org? Or do you mean on the fetch-deps stage?

Sorry @GGG-KILLER, I missed this question.

Yeah, I was thinking of fetch-deps. If it's able to find a "better" (older, and closer to the minimum version) package on nuget.org it'll download it and add it to the lockfile.

huantianad · 2024-09-17T17:09:40Z

Hi, sorry just saw this and the changes for Scarab, should I be making a PR upstream to include this change?

corngood · 2024-09-17T17:22:35Z

@huantianad possibly. I found a few others that did the same thing. It's not really wrong exactly, it's just depending on the user's global nuget configuration. It's pretty easy to hack around in nixpkgs, and might even be worthy of a shared hook/option.

A lot of projects are explicit about package sources like this: https://github.com/AvaloniaUI/Avalonia/blob/master/NuGet.Config. Other projects don't even specify a nuget configuration. In those cases we create a default config which includes nuget.org.

Maybe we should be adding nuget.org when there isn't a <clear/> in the root config?

SuperSandro2000 · 2024-09-23T14:12:21Z

Does dotnet-runtime need to retain the dependency on the setup hook at runtime?

eg right now ArchiSteamFarm uses this https://github.com/NixOS/nixpkgs/blob/d40eb2f76888d5936a1f91ebce4c01f34ae43f10/pkgs/applications/misc/ArchiSteamFarm/default.nix#L23C3-L23C17 and through that has a dependency on xmlstarlet which I didn't have on my system before.

corngood · 2024-09-23T14:17:22Z

@SuperSandro2000 the runtime should use only dotnet-setup-hook, and not dotnet-sdk-setup-hook. I also wouldn't expect it to be a runtime dependency of a package built with the sdk. I'll have to take a look at what's going on.

Edit: oh, I see, it's nuget-package-hook

corngood · 2024-09-23T14:42:24Z

Ah yeah, I think this is the way to go:

commit 7e5015015a600a3e35e2835c939bebb3acc6b57c
Author: David McFarland <[email protected]>
Date:   Mon Sep 23 11:40:03 2024 -0300

    dotnet: remove nuget-package-hook from runtime packages

diff --git a/pkgs/development/compilers/dotnet/common.nix b/pkgs/development/compilers/dotnet/common.nix
index dea6c8c405a7..0d0dc66514fb 100644
--- a/pkgs/development/compilers/dotnet/common.nix
+++ b/pkgs/development/compilers/dotnet/common.nix
@@ -45,7 +45,9 @@ stdenv.mkDerivation (
         inherit lndir xmlstarlet;
       });
 
-    propagatedBuildInputs = (args.propagatedBuildInputs or [ ]) ++ [ nugetPackageHook ];
+    propagatedBuildInputs =
+      (args.propagatedBuildInputs or [ ])
+      ++ lib.optional (type == "sdk") nugetPackageHook;
 
     nativeBuildInputs = (args.nativeBuildInputs or [ ]) ++ [ installShellFiles ];

This removes a bunch of stuff from the ArchiSteamFarm closure.

github-actions bot added the 6.topic: dotnet Language: .NET label Aug 23, 2024

corngood force-pushed the dotnet-avalonia branch 2 times, most recently from 401a09c to ebbaff2 Compare August 23, 2024 18:04

ofborg bot added the 8.has: package (new) This PR adds a new package label Aug 23, 2024

ofborg bot requested review from l0b0 and MattSturgeon August 23, 2024 18:45

ofborg bot added 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500 labels Aug 23, 2024

GGG-KILLER reviewed Aug 23, 2024

View reviewed changes

corngood force-pushed the dotnet-avalonia branch from ebbaff2 to 88761e1 Compare August 23, 2024 19:27

MattSturgeon reviewed Aug 23, 2024

View reviewed changes

corngood force-pushed the dotnet-avalonia branch 2 times, most recently from 48b508d to 574ebcb Compare August 23, 2024 19:52

ofborg bot requested a review from MattSturgeon August 23, 2024 21:01

MattSturgeon reviewed Aug 23, 2024

View reviewed changes

pkgs/build-support/dotnet/make-nuget-deps/default.nix Outdated Show resolved Hide resolved

corngood force-pushed the dotnet-avalonia branch from 574ebcb to fe51471 Compare August 24, 2024 00:32

ofborg bot requested a review from MattSturgeon August 24, 2024 01:05

corngood mentioned this pull request Aug 28, 2024

nexusmods-app: 0.4.1 -> 0.6.1 #331150

Merged

20 tasks

corngood mentioned this pull request Sep 6, 2024

dotnet: add override mechanism for nuget packages #339953

Merged

13 tasks

wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 10, 2024

corngood force-pushed the dotnet-avalonia branch from 3cfb72d to 974e546 Compare September 13, 2024 21:42

github-actions bot added the 8.has: documentation label Sep 13, 2024

corngood added 15 commits September 17, 2024 01:12

nuget-to-nix: use xmlstarlet instead of xq (yq)

e6bca73

xq has such a long startup time that this significantly improves performance.

add-nuget-deps: use cacert via nativeBuildInputs

4bb1510

add-nuget-deps: expose fetch derivation as passthru.fetch-drv

2066931

add-nuget-deps: set HOME outside of nix-shell

a418943

add-nuget-deps: use NUGET_HTTP_CACHE_PATH from user's HOME

0e5b1ff

avalonia: init at 11.0.10

3aad599

nexusmods-app: use source-built avalonia

57f1e66

avalonia: 11.0.10 -> 11.0.11

e33bdf8

dotnet-sdk-setup-hook.sh: add linkNuGetPackagesAndSources

e6e2f93

linkNugetPackages and linkNuGetPackagesAndSources can now be disabled by setting them to false. linkNugetPackages will use _linkPackages, which is much faster.

addNuGetDeps: print name of package being fetched

f5e9dd4

fetchNupkg: conservatively set sourceProvenance

be577a2

update-dotnet-lockfiles: skip packages that don't evaluate

3a7d2b8

This allows the script to run on platforms that can't fetch all of the packages, or without allowing unfree.

update-dotnet-lockfiles: add startWith flag

988b57f

This allows you to resume after a failure by passing the name of the first package you want to be fetched.

addNuGetDeps: fix fetch-deps output path handling

424bbc4

doc/languages-frameworks/dotnet: improve documenation of fetch-deps

d8bd63a

corngood force-pushed the dotnet-avalonia branch from dae56bc to d8bd63a Compare September 17, 2024 04:14

corngood marked this pull request as ready for review September 17, 2024 04:15

ofborg bot requested review from MattSturgeon, taktoa and l0b0 September 17, 2024 05:08

corngood merged commit 1b7f8c9 into NixOS:master Sep 17, 2024
25 of 27 checks passed

corngood deleted the dotnet-avalonia branch September 17, 2024 17:17

huantianad mentioned this pull request Sep 17, 2024

fix(NuGet.Config): be more precise about nuget package sources fifty-six/Scarab#229

Merged

corngood mentioned this pull request Sep 23, 2024

dotnet: remove nuget-package-hook from runtime packages #343978

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dotnet: infrastructure improvements #336824

dotnet: infrastructure improvements #336824

corngood commented Aug 23, 2024 •

edited

Loading

GGG-KILLER left a comment

GGG-KILLER Aug 23, 2024

corngood Aug 23, 2024

GGG-KILLER Aug 23, 2024

corngood Aug 23, 2024

MattSturgeon Aug 23, 2024

corngood Aug 24, 2024

MattSturgeon left a comment

MattSturgeon Aug 23, 2024

corngood Aug 23, 2024

js6pak commented Sep 5, 2024

corngood commented Sep 6, 2024

corngood commented Sep 8, 2024

MattSturgeon commented Sep 8, 2024 •

edited

Loading

corngood commented Sep 8, 2024

GGG-KILLER commented Sep 9, 2024 •

edited

Loading

corngood commented Sep 9, 2024 •

edited

Loading

GGG-KILLER commented Sep 9, 2024

corngood commented Sep 17, 2024

huantianad commented Sep 17, 2024

corngood commented Sep 17, 2024

SuperSandro2000 commented Sep 23, 2024

corngood commented Sep 23, 2024 •

edited

Loading

corngood commented Sep 23, 2024

dotnet: infrastructure improvements #336824

dotnet: infrastructure improvements #336824

Conversation

corngood commented Aug 23, 2024 • edited Loading

Description of changes

Things done

GGG-KILLER left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MattSturgeon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

js6pak commented Sep 5, 2024

corngood commented Sep 6, 2024

corngood commented Sep 8, 2024

MattSturgeon commented Sep 8, 2024 • edited Loading

corngood commented Sep 8, 2024

GGG-KILLER commented Sep 9, 2024 • edited Loading

corngood commented Sep 9, 2024 • edited Loading

GGG-KILLER commented Sep 9, 2024

corngood commented Sep 17, 2024

huantianad commented Sep 17, 2024

corngood commented Sep 17, 2024

SuperSandro2000 commented Sep 23, 2024

corngood commented Sep 23, 2024 • edited Loading

corngood commented Sep 23, 2024

corngood commented Aug 23, 2024 •

edited

Loading

MattSturgeon commented Sep 8, 2024 •

edited

Loading

GGG-KILLER commented Sep 9, 2024 •

edited

Loading

corngood commented Sep 9, 2024 •

edited

Loading

corngood commented Sep 23, 2024 •

edited

Loading