dotnet: infrastructure updates #361563

corngood · 2024-12-03T21:27:42Z

Things done

Add a 👍 reaction to pull requests you find important.

corngood · 2024-12-03T21:36:52Z

@GGG-KILLER This felt a bit off topic for your PR, so I created a draft PR.

I usually prefer separate PRs grouped by logical change, so if something goes wrong it's easier to pinpoint the change that broke something.

I do agree, but my intention was to reduce the number of changes going into master that require rebuilding everything dotnet. Maybe it isn't worth the effort. I got the feeling we were getting up into 'mass rebuild' territory anyway, and if the source-built runtime/sdk get more widely used, it's going to be pretty painful. I still try to write commits in such a way that it's all bisectable, even if that's hard to prove with nixpkgs-review, etc.

It would be good to discuss the pros and cons of this. Cc: @NixOS/dotnet

I also am not fond of the last 2 commits which just use nixfmt to format the deps files.

I don't mind dropping commits out of here if there are objections to them.

GGG-KILLER · 2024-12-03T21:40:06Z

I do agree, but my intention was to reduce the number of changes going into master that require rebuilding everything dotnet. Maybe it isn't worth the effort. I got the feeling we were getting up into 'mass rebuild' territory anyway, and if the source-built runtime/sdk get more widely used, it's going to be pretty painful. I still try to write commits in such a way that it's all bisectable, even if that's hard to prove with nixpkgs-review, etc.

Fair enough, even more so considering the queue wait times for ofborg and the time for things to arrive on unstable it might be better to have one PR as a whole.

I also am not fond of the last 2 commits which just use nixfmt to format the deps files.

I don't mind dropping commits out of here if there are objections to them.

I have the same objections mentioned on #358025, I think the better idea long term is to just move forward with JSON like the other builders do for overall consistency and correctness.

corngood · 2024-12-03T21:51:11Z

Fair enough, even more so considering the queue wait times for ofborg and the time for things to arrive on unstable it might be better to have one PR as a whole.

We could create a long-lived branch on nixpkgs for this purpose. There's some precedent for this in e.g. python-updates and haskell-updates, but it doesn't seem to be widespread. Some of them are built on hydra.

Haskell and python both do mass updates from external repositories, so it's a bit of a different problem. Usually when we're talking about mass rebuilds on dotnet, it's the sort of thing that would just go through staging. I think this is overkill for dotnet, but it's interesting: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/haskell-modules/HACKING.md

The thing with dotnet is that it's 'just about' feasible for someone to nixpkgs-review a change that touches all of it, which makes it very tempting to avoid staging.

corngood · 2024-12-03T22:43:40Z

I split the lockfile changes to #361579, since it shouldn't cause rebuilds.

GGG-KILLER · 2024-12-04T19:47:37Z

pkgs/by-name/fs/fsautocomplete/package.nix

  dotnet-sdk =
    with dotnetCorePackages;
-    combinePackages [
-      sdk_6_0
-      sdk_7_0
-      sdk_8_0
-    ];
+    sdk_8_0
+    // {
+      inherit (sdk_6_0) packages targetPackages;
+    };


I think this is more confusing than the earlier combinePackages. I have no idea what it does nor why it is needed.

Should probably create a helper function that does this and documents its behavior and when to use if we do decide to move forward with this pattern.

The idea is that we don't actually need more than one SDK, but we do need the targeting packages. This breaks the dependencies on the insecure packages.

It does feel messy. I started working on a bigger overhaul of how nuget packages are referenced, but it's not close to being ready.

The issue is that the packages themselves being used are also insecure, this feels like just swiping it under the rug as opposed to actually fixing the problem.

This one is gonna be a hard no from me, considering that many .NET CVEs were on SDK/runtime packages and not on the compiler/runtime itself.

For net 6, it definitely is just sweeping it under the rug, but I still think it's better for things to only depend on the SDKs they use, instead of using combinePackages solely to pull in nuget packages.

I'd be happy to mark all net6/7 nuget packages as insecure, but that's a separate problem IMO.

Another question, shouldn't we keep the 8.0 packages here (and elsewhere where this is used)?

And couldn't the instances where this uses combinePackages be replaced with the lib methods conmbinePackages uses instead of creating a full derivation?
It feels wasteful to create a whole combined SDK when we only need the packages and targeting packages from it.

And if possible, I'd like to have the packages be marked as insecure before we start actually using them so this doesn't end up unmarking packages as insecure and people mistakenly think these are safe to use.

I'm just having a go at this, by applying knownVulnerabilities to the packages as well:

$ nix eval -f. fsautocomplete «error: Package ‘Microsoft.AspNetCore.App.Ref-6.0.36’ in /home/david/src/nixpkgs/pkgs/build-support/dotnet/fetch-nupkg/default.nix:23 is marked as insecure, refusing to evaluate. Known issues: - Dotnet SDK 6.0.428 is EOL, please use 8.0 (LTS) or 9.0 (Current) You can install it anyway by allowing this package, using the following methods: a) To temporarily allow all insecure packages, you can use an environment variable for a single invocation of the nix tools: $ export NIXPKGS_ALLOW_INSECURE=1 Note: When using `nix shell`, `nix build`, `nix develop`, etc with a flake, then pass `--impure` in order to allow use of environment variables. b) for `nixos-rebuild` you can add ‘Microsoft.AspNetCore.App.Ref-6.0.36’ to `nixpkgs.config.permittedInsecurePackages` in the configuration.nix, like so: { nixpkgs.config.permittedInsecurePackages = [ "Microsoft.AspNetCore.App.Ref-6.0.36" ]; } c) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add ‘Microsoft.AspNetCore.App.Ref-6.0.36’ to `permittedInsecurePackages` in ~/.config/nixpkgs/config.nix, like so: { permittedInsecurePackages = [ "Microsoft.AspNetCore.App.Ref-6.0.36" ]; }

It's going to be really painful for anyone who wants to use permittedInsecurePackages.

Well, there's no way around it unfortunately. I think making it easy for people to use insecure packages isn't an objective of nixpkgs, but worst case they can do something like

nixpkgs.config.permittedInsecurePackages = lib.map (pkg: pkg.name) pkgs.dotnetCorePackages.sdk_6_0.packages;

I guess you could make a pseudo-package referenced by the sdk, runtime, nuget packages, and mark that as insecure?

I don't know if it's worth the trouble though.

nixpkgs.config.permittedInsecurePackages = lib.map (pkg: pkg.name) pkgs.dotnetCorePackages.sdk_6_0.packages;

You'd also need to bring in targetPackages which is an attrset of lists. It's still doable, but cumbersome.

I guess you could make a pseudo-package referenced by the sdk, runtime, nuget packages, and mark that as insecure?

Sounds like too much work for something we plan to remove in the long term, but it would make the deprecation process easier perhaps. If it's easy enough we could do it to make it easier on us to deprecate them as well.

You'd also need to bring in targetPackages which is an attrset of lists. It's still doable, but cumbersome.

Yeah, but I'd say allowing usage of insecure things is a non-objective. If people really want to use them, it should be doable but I wouldn't go out of my way to make it easy.

Sounds like too much work for something we plan to remove in the long term, but it would make the deprecation process easier perhaps. If it's easy enough we could do it to make it easier on us to deprecate them as well.

I actually don't think it would work anyway. I know you need to add the wrapper packages to permittedInsecurePackages, so I assume you'd still have to add everything that's transiently insecure.

GGG-KILLER · 2024-12-04T19:49:15Z

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

@@ -17,6 +17,7 @@ let
    finalAttrs:
    {
      enableParallelBuilding ? true,
+      strictDeps ? true,


Sorry for my lack of knowledge, I tried looking at the source for mkDerivation but I couldn't understand what this flag does, and #353937 hasn't been done to document what it does.

Also, won't this break out-of-tree packages using buildDotnetModule? We should probably document this breaking change.

This came from:

#358824

You're right that it needs documentation.

Oh okay, makes sense. Saw another PR that adds a little description to it so now I have an idea how it works.

pkgs/development/compilers/dotnet/update.sh

pkgs/games/space-station-14-launcher/space-station-14-launcher.nix

gepbird · 2024-12-05T20:20:01Z

pkgs/by-name/ga/garnet/package.nix

-  dotnet-sdk =
-    with dotnetCorePackages;
-    combinePackages [
-      sdk_6_0
-      sdk_8_0
-    ];
+  dotnet-sdk = dotnetCorePackages.sdk_8_0;


Same thing with this package too.

Also the same change is being done in #361658

gepbird · 2024-12-05T20:20:27Z

pkgs/by-name/be/beatsabermodmanager/package.nix

-  dotnet-sdk = with dotnetCorePackages; combinePackages [
-    sdk_7_0
-    sdk_6_0
-  ];
+  dotnet-sdk = dotnetCorePackages.sdk_7_0;


Same, and #339370 will cause a merge conflict

This simplifies the code a bit and makes it possible to use dotnetCorePackages.combinePackages as dotnet-runtime again. Partial revert of ddd08e4.

GGG-KILLER · 2024-12-05T20:44:24Z

@gepbird the bump to the 8.0 SDK wasn't mistakenly done for the SS14 launcher, maybe I worded myself wrong, it is done because the package CAN be built with a newer SDK, although it still uses packages from the old runtime 6.0, which can be vulnerable and are part of .NET 6.

However it seems @corngood has already removed the 6.0 reference in a newer commit so I guess it wasn't actually needed anymore 😅. Sorry for the confusion, you can now ignore my comment in the other thread.

corngood · 2024-12-06T13:44:57Z

I'm going to close this and try to break it down into separate PRs. I think this was a bad idea.

github-actions bot added 6.topic: python 6.topic: games 6.topic: dotnet Language: .NET labels Dec 3, 2024

corngood mentioned this pull request Dec 3, 2024

nuget-to-nix: Generated file not formatted #358025

Closed

corngood force-pushed the dotnet-next branch from 81931b8 to 20d82da Compare December 3, 2024 22:41

github-actions bot removed the 6.topic: python label Dec 3, 2024

github-actions bot added 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500 labels Dec 3, 2024

GGG-KILLER suggested changes Dec 4, 2024

View reviewed changes

gepbird mentioned this pull request Dec 4, 2024

.NET 6/7 are EOL #326335

Open

17 tasks

gepbird reviewed Dec 5, 2024

View reviewed changes

corngood and others added 10 commits December 5, 2024 16:40

dafny: fix build with strictDeps

7b508c6

github-runner: fix build with strictDeps

7f4c05f

tagger: fix build with strictDeps

535cfda

dotnet: add WindowsDesktop.App to targetPackages

786747c

buildDotnetModule: enable strictDeps by default

3fdc09c

dotnet: enable strictDeps

70bf01c

dotnet: make wrappers usable as DOTNET_ROOT

81dd644

This simplifies the code a bit and makes it possible to use dotnetCorePackages.combinePackages as dotnet-runtime again. Partial revert of ddd08e4.

dotnet-outdated: format with nixfmt

95d7a5a

dotnetCorePackages.combinePackages: use wrapper

0dcba5f

treewide: remove uneccessary uses of dotnetCorePacakges.combinePacakges

abd7695

corngood force-pushed the dotnet-next branch from 20d82da to abd7695 Compare December 5, 2024 20:42

corngood closed this Dec 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dotnet: infrastructure updates #361563

dotnet: infrastructure updates #361563

corngood commented Dec 3, 2024

corngood commented Dec 3, 2024

GGG-KILLER commented Dec 3, 2024

corngood commented Dec 3, 2024

corngood commented Dec 3, 2024

GGG-KILLER Dec 4, 2024

corngood Dec 4, 2024

GGG-KILLER Dec 4, 2024 •

edited

Loading

corngood Dec 5, 2024

GGG-KILLER Dec 5, 2024

corngood Dec 5, 2024

GGG-KILLER Dec 5, 2024

corngood Dec 5, 2024

GGG-KILLER Dec 5, 2024

corngood Dec 6, 2024

GGG-KILLER Dec 4, 2024

corngood Dec 4, 2024

GGG-KILLER Dec 4, 2024

gepbird Dec 5, 2024

gepbird Dec 5, 2024

GGG-KILLER commented Dec 5, 2024

corngood commented Dec 6, 2024

dotnet: infrastructure updates #361563

dotnet: infrastructure updates #361563

Conversation

corngood commented Dec 3, 2024

Things done

corngood commented Dec 3, 2024

GGG-KILLER commented Dec 3, 2024

corngood commented Dec 3, 2024

corngood commented Dec 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GGG-KILLER Dec 4, 2024 • edited Loading

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

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

GGG-KILLER commented Dec 5, 2024

corngood commented Dec 6, 2024

GGG-KILLER Dec 4, 2024 •

edited

Loading