treewide: post-stc-ng test fixes #340046

K900 · 2024-09-06T10:41:13Z

Description of changes

This is insane. I have no idea.

Things done

Add a 👍 reaction to pull requests you find important.

…ileset shenanigans Something is wrong with source filtering and it makes installer tests fail. Move it to a subdirectory to not have to do that, at least for now. The Nix exorcism squad has been called.

…ue to stc-ng

…ng due to stc-ng

emilazy

I kind of hate that we don’t understand this, but I’m glad it fixes things.

Did you check if Lix or whatever has the same bug?

emilazy · 2024-09-06T10:43:48Z

pkgs/by-name/sw/switch-to-configuration-ng/package.nix

-      ./src
-    ];
-  };
+  src = ./src;


Non‐blocking nit: ./switch-to-configuration would match the cargo new naming convention.

emilazy · 2024-09-06T10:47:39Z

We should probably report this bug but I guess it seems hard to find a minimal reproducer (maybe --store + channels + filesets is just totally broken in general?).

Yarny0 · 2024-10-31T16:47:26Z

Might this be caused by NixOS/nix#11503 ? There is a minimal reproducer given (albeit it is so minimal it does not use filesets), and a less minimal reproducer (based on nixpkgs) is also stated in #334098 .

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

Due to Nix bug <NixOS/nix#11503>, `builtins.filterSource` and chroot stores interact in a confusing and broken way that breaks `lib.fileset`. This means that uses of the API inside Nixpkgs keep breaking the NixOS installer, blocking the channel. The resulting error messages are inscrutable (they look like “the installer test is trying to download curl…?” and eventually bottom out in a derivation that has the wrong `outPath` because of the chroot store causing an incorrect `lib.fileset` result). Whenever this happens, someone (well, in practice K900 or I) has to bisect the change that introduced it and remove the use of `lib.fileset`. This has happened at least three times in the past four months (I believe I might actually be missing one here, but these are the ones I remember and could easily dig up): * <htxps://github.com/NixOS/pull/340046> * <NixOS#352491> * <NixOS#369459> The options I see here are: 1. Forbid use of `lib.fileset` within Nixpkgs. This is the approach taken here. External users of Nixpkgs can continue to use the API as normal, but using it from within something that affects any release jobset `outPath`s will cause an evaluation failure with a hopefully‐ helpful error message. 2. Forbid `lib.fileset` and also all of the other library APIs that use `builtins.filterSource`. I’m happy to do this, but so far none of those have broken the installer, so I decided to start small and worry about the others if they end up causing a problem in practice. 3. Forbid `builtins.filterSource` directly. This is hard and would require more invasive `builtins.scopedImport` crimes to do at evaluation time. I think this would realistically have to be done in something like nixpkgs-vet instead and I didn’t have much luck shoehorning a check like this into that codebase when I tried. 4. Fix the Nix bug. This would be great! But also it doesn’t seem to be happening any time soon, it seems difficult to fix in a way that doesn’t subtly break compatibility with the previous semantics, and arguably the fix would need backporting all the way back to 2.3 given our minimum version policy. 5. Do nothing; have people continue to innocuously use `lib.fileset` throughout Nixpkgs, breaking the installer whenever one of them sneaks in to that closure, causing the channel to be blocked and requiring expensive bisections to narrow down the inscrutable test failure to the package using `lib.fileset`, which then needs moving back off it. This sucks for the people who keep having to track it down, holds back important channel bumps, and the criteria for when it’s okay to use `lib.fileset` are not realistically possible to teach to all contributors. I'd be happy to work on (2) as an alternative; (3) would be difficult and seems like overkill, (4) is not really something I trust myself to do and wouldn’t address the immediate problem, and (5) isn’t sustainable. I think that the current approach here is the best trade‐off for now, as `lib.fileset` seems to be the only prominent user of the `builtins.filterSource` API that works with full store paths, exposing it to the Nix bug. It’s unfortunate to lose the nice API, but since we can’t rely on it to produce correct results and the channels keep getting blocked as a result, I don’t think we really have an alternative right now.

Due to Nix bug <NixOS/nix#11503>, `builtins.filterSource` and chroot stores interact in a confusing and broken way that breaks `lib.fileset`. This means that uses of the API inside Nixpkgs keep breaking the NixOS installer, blocking the channel. The resulting error messages are inscrutable (they look like “the installer test is trying to download curl…?” and eventually bottom out in a derivation that has the wrong `outPath` because of the chroot store causing an incorrect `lib.fileset` result). Whenever this happens, someone (well, in practice K900 or I) has to bisect the change that introduced it and remove the use of `lib.fileset`. This has happened at least three times in the past four months (I believe I might actually be missing one here, but these are the ones I remember and could easily dig up): * <NixOS#340046> * <NixOS#352491> * <NixOS#369459> The options I see here are: 1. Forbid use of `lib.fileset` within Nixpkgs. This is the approach taken here. External users of Nixpkgs can continue to use the API as normal, but using it from within something that affects any release jobset `outPath`s will cause an evaluation failure with a hopefully‐ helpful error message. 2. Forbid `lib.fileset` and also all of the other library APIs that use `builtins.filterSource`. I’m happy to do this, but so far none of those have broken the installer, so I decided to start small and worry about the others if they end up causing a problem in practice. 3. Forbid `builtins.filterSource` directly. This is hard and would require more invasive `builtins.scopedImport` crimes to do at evaluation time. I think this would realistically have to be done in something like nixpkgs-vet instead and I didn’t have much luck shoehorning a check like this into that codebase when I tried. 4. Fix the Nix bug. This would be great! But also it doesn’t seem to be happening any time soon, it seems difficult to fix in a way that doesn’t subtly break compatibility with the previous semantics, and arguably the fix would need backporting all the way back to 2.3 given our minimum version policy. 5. Do nothing; have people continue to innocuously use `lib.fileset` throughout Nixpkgs, breaking the installer whenever one of them sneaks in to that closure, causing the channel to be blocked and requiring expensive bisections to narrow down the inscrutable test failure to the package using `lib.fileset`, which then needs moving back off it. This sucks for the people who keep having to track it down, holds back important channel bumps, and the criteria for when it’s okay to use `lib.fileset` are not realistically possible to teach to all contributors. I'd be happy to work on (2) as an alternative; (3) would be difficult and seems like overkill, (4) is not really something I trust myself to do and wouldn’t address the immediate problem, and (5) isn’t sustainable. I think that the current approach here is the best trade‐off for now, as `lib.fileset` seems to be the only prominent user of the `builtins.filterSource` API that works with full store paths, exposing it to the Nix bug. It’s unfortunate to lose the nice API, but since we can’t rely on it to produce correct results and the channels keep getting blocked as a result, I don’t think we really have an alternative right now.

Due to Nix bug <NixOS/nix#11503>, `builtins.filterSource` and chroot stores interact in a confusing and broken way that breaks `lib.fileset`. This means that uses of the API inside Nixpkgs keep breaking the NixOS installer, blocking the channel. The resulting error messages are inscrutable (they look like “the installer test is trying to download `curl`…?” and eventually bottom out in a derivation that has the wrong `outPath` because of the chroot store causing an incorrect `lib.fileset` result). Whenever this happens, someone (well, in practice K900 or I) has to bisect the change that introduced it and remove the use of `lib.fileset`. This has happened at least three times in the past four months (I believe I might actually be missing one here, but these are the ones I remember and could easily dig up): * <NixOS#340046> * <NixOS#352491> * <NixOS#369459> The options I see here are: 1. Forbid use of `lib.fileset` within Nixpkgs until the Nix bug is fixed. This is the approach taken here. External users of Nixpkgs can continue to use the API as normal, but using it from within something that affects any release jobset `outPath`s will cause an evaluation failure with a hopefully‐helpful error message. 2. Forbid `lib.fileset` and also all of the other library APIs that use `builtins.filterSource`. I’m happy to do this, but so far none of those have broken the installer, so I decided to start small and worry about the others if they end up causing a problem in practice. 3. Forbid `builtins.filterSource` directly. This is hard and would require more invasive `builtins.scopedImport` crimes to do at evaluation time. I think this would realistically have to be done in something like nixpkgs-vet instead and I didn’t have much luck shoehorning a check like this into that codebase when I tried. 4. Fix the Nix bug. This would be great! But also it doesn’t seem to be happening any time soon, it seems difficult to fix in a way that doesn’t subtly break compatibility with the previous semantics, and arguably the fix would need backporting all the way back to 2.3 given our minimum version policy. 5. Do nothing; have people continue to innocuously use `lib.fileset` throughout Nixpkgs, breaking the installer whenever one of them sneaks in to that closure, causing the channel to be blocked and requiring expensive bisections to narrow down the inscrutable test failure to the package using `lib.fileset`, which then needs moving back off it. This sucks for the people who keep having to track it down, holds back important channel bumps, and the criteria for when it’s okay to use `lib.fileset` are not realistically possible to teach to all contributors. I'd be happy to work on (2) as an alternative; (3) would be difficult and seems like overkill, (4) is not really something I trust myself to do and wouldn’t address the immediate problem, and (5) isn’t sustainable. I think that the current approach here is the best trade‐off for now, as `lib.fileset` seems to be the only prominent user of the `builtins.filterSource` API that works with full store paths, exposing it to the Nix bug. It’s unfortunate to lose the nice API, but since we can’t rely on it to produce correct results and the channels keep getting blocked as a result, I don’t think we really have an alternative right now.

Due to Nix bug <NixOS/nix#11503>, `builtins.filterSource` and chroot stores interact in a confusing and broken way that breaks `lib.fileset`. This means that uses of the API inside Nixpkgs keep breaking the NixOS installer, blocking the channel. The resulting error messages are inscrutable (they look like “the installer test is trying to download `curl`…?” and eventually bottom out in a derivation that has the wrong `outPath` because of the chroot store causing an incorrect `lib.fileset` result). Whenever this happens, someone (well, in practice K900 or I) has to bisect the change that introduced it and remove the use of `lib.fileset`. This has happened at least three times in the past four months (I believe I might actually be missing one here, but these are the ones I remember and could easily dig up): * <#340046> * <#352491> * <#369459> The options I see here are: 1. Forbid use of `lib.fileset` within Nixpkgs until the Nix bug is fixed. This is the approach taken here. External users of Nixpkgs can continue to use the API as normal, but using it from within something that affects any release jobset `outPath`s will cause an evaluation failure with a hopefully‐helpful error message. 2. Forbid `lib.fileset` and also all of the other library APIs that use `builtins.filterSource`. I’m happy to do this, but so far none of those have broken the installer, so I decided to start small and worry about the others if they end up causing a problem in practice. 3. Forbid `builtins.filterSource` directly. This is hard and would require more invasive `builtins.scopedImport` crimes to do at evaluation time. I think this would realistically have to be done in something like nixpkgs-vet instead and I didn’t have much luck shoehorning a check like this into that codebase when I tried. 4. Fix the Nix bug. This would be great! But also it doesn’t seem to be happening any time soon, it seems difficult to fix in a way that doesn’t subtly break compatibility with the previous semantics, and arguably the fix would need backporting all the way back to 2.3 given our minimum version policy. 5. Do nothing; have people continue to innocuously use `lib.fileset` throughout Nixpkgs, breaking the installer whenever one of them sneaks in to that closure, causing the channel to be blocked and requiring expensive bisections to narrow down the inscrutable test failure to the package using `lib.fileset`, which then needs moving back off it. This sucks for the people who keep having to track it down, holds back important channel bumps, and the criteria for when it’s okay to use `lib.fileset` are not realistically possible to teach to all contributors. I'd be happy to work on (2) as an alternative; (3) would be difficult and seems like overkill, (4) is not really something I trust myself to do and wouldn’t address the immediate problem, and (5) isn’t sustainable. I think that the current approach here is the best trade‐off for now, as `lib.fileset` seems to be the only prominent user of the `builtins.filterSource` API that works with full store paths, exposing it to the Nix bug. It’s unfortunate to lose the nice API, but since we can’t rely on it to produce correct results and the channels keep getting blocked as a result, I don’t think we really have an alternative right now. (cherry picked from commit 8725e46)

K900 added 3 commits September 6, 2024 13:35

switch-to-configuration-ng: move sources to a subdirectory to avoid f…

e4b8a0a

…ileset shenanigans Something is wrong with source filtering and it makes installer tests fail. Move it to a subdirectory to not have to do that, at least for now. The Nix exorcism squad has been called.

nixos/tests/installer: add some Perl libraries that are now missing d…

7d34add

…ue to stc-ng

nixos/tests/containers-imperative: add Perl library that is now missi…

91b7e41

…ng due to stc-ng

K900 requested review from jmbaur and emilazy September 6, 2024 10:41

github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Sep 6, 2024

K900 merged commit b39277a into NixOS:master Sep 6, 2024
12 of 13 checks passed

emilazy approved these changes Sep 6, 2024

View reviewed changes

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Sep 6, 2024

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

f820064

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

6e6d729

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

63a18b2

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

6dc11e6

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

bf11afd

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

ea4a651

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

625f3b3

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

cc077a0

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

1adbfe8

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

8da0cb1

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 30, 2024

wip: filesets

76fde43

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

e90a256

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

68a135a

* <NixOS#340046> * <NixOS#352491> * <NixOS#369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

8935d0e

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

8bd8f20

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

dbecdd4

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

963b620

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

a03a901

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

f10d68c

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

e55f7c6

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

7b4a1ca

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Dec 31, 2024

wip: filesets

5d2ae2e

* <hxxps://github.com/NixOS/pull/340046> * <hxxps://github.com/NixOS/pull/352491> * <hxxps://github.com/NixOS/pull/369459>

emilazy mentioned this pull request Dec 31, 2024

release: forbid use of lib.fileset in Nixpkgs #369694

Merged

13 tasks

ElvishJerricco mentioned this pull request Dec 31, 2024

builtins.path and builtins.filterSource use wrong filter paths with --store argument NixOS/nix#11503

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

treewide: post-stc-ng test fixes #340046

treewide: post-stc-ng test fixes #340046

K900 commented Sep 6, 2024 •

edited

Loading

emilazy left a comment

emilazy Sep 6, 2024

emilazy commented Sep 6, 2024

Yarny0 commented Oct 31, 2024

treewide: post-stc-ng test fixes #340046

treewide: post-stc-ng test fixes #340046

Conversation

K900 commented Sep 6, 2024 • edited Loading

Description of changes

Things done

emilazy left a comment

Choose a reason for hiding this comment

emilazy Sep 6, 2024

Choose a reason for hiding this comment

emilazy commented Sep 6, 2024

Yarny0 commented Oct 31, 2024

K900 commented Sep 6, 2024 •

edited

Loading