pkgs/top-level: Remove __allowFileset option

[MrQubo]

This options was created as a workaround for NixOS/nix#11503. It is no longer needed.

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.
• Follows the automation/AI policy.

[MrQubo]

I don't think it's worth a release note.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/remove-allowfileset/78735/11

[emilazy]

I’d like this hack to go away, but I think it’ll take a bit of additional work to make that happen.

We do still nominally support upgrading systems from Nix 2.18, which does not have the fix, and we’re unlikely to ever bump the version requirement given that Lix forked from 2.18 and has frozen the language version – hence moving to a feature detection model in #433057. So if we wanted to enforce the fix – which was a breaking evaluation semantics change, albeit to more sensible semantics – we’d need to do some kind of feature detection in minfeatures.nix. In particular, Nix and Lix both did independent fixes that I believe differed in implementation details, and I don’t think anyone has validated that the two fixes actually agree on the tricky cases (especially since it’s hard to determine what the semantics should be in the first place when using chroot stores).

It was very painful to debug the issues caused by the chroot store bug – see #369694 for more detail about that – and lib.fileset is mostly useful for third‐party packages used for development rather than within Nixpkgs (because there’s little reason to filter source trees in Nixpkgs to avoid rebuilds; a src directory usually works just as well).

So I would like us to be careful about having appropriate safeguards before reverting this: say, ensuring that we run the lib.fileset tests inside a chroot store as part of our lib test builds, that those successfully detect the issue we kept running into previously. and that any divergences between Nix and Lix would be caught by the existing CI stuff that does outpath comparison between them on upgrades.

That said – a silver lining is that NixOS upgrades likely aren’t using chroot stores, so although those are important for the installer we might be able to get away with them not quite working right across upgrades. We could perhaps do a very simple lib.fileset call in minfeatures.nix to detect wrong result in unfixed versions when using chroot stores – that wouldn’t impact anyone upgrading from versions without the fix as long as they’re not using a chroot store, and would ensure that things fail early in the installer tests if it ever regressed again rather than causing really opaque failures. I would feel more confident with thorough lib test coverage for the chroot store case, but I could live with just a minfeatures.nix check if it’s feasible.

Less critical: it would be nice if this targeted staging-next, as it will otherwise cause conflicts in pkgs/top-level/default.nix due to the x86_64-darwin drop, and it would be nice to have more explanation in the commit message and the standard revert footer for the commit it’s undoing.

[MrQubo]

@emilazy
As you clearly explained that version check is meaningless, I agree on the fact that a check to minfeatures.nix should be added to detect that builtins.filterSource works as expected, at very least that it does not behave as before fix for NixOS/nix#11503 was merged.

The nix issue has its own regression test in nix: https://github.com/NixOS/nix/pull/12512/changes#diff-6da8d04dba077db70d2324f9dc907fb2c94e5f1cf303a72a40641d1e044122b0R5. And a few of other tests in that file are present. lix seems to have its own regression tests for that bug as well: https://git.lix.systems/lix-project/lix/commit/62ee2aea297528568098bae103e9a3af7658b4b5.

I'm wondering that maybe instead of adding tests for chroot store to nixpkgs, we should add more tests for chroot store to nix. It would be nice to ensure that nixpkgs works correctly with chroot stores, but checking every package would be infeasible. If we assume good tests coverage in upstream, then I'm not sure if tests in nixpkgs would be very helpful?

How much helpful the tests in nixpkgs would be boils down to how many differences between non-chroot and chroot stores are present in nix language runtime API. I couldn't find any, but I'm not competent enough to feel confident about that. But if the nix goal is to completely hide away from runtime that chroot store is used, then in my opinion there's no need for tests covering chroot store in nixpkgs.

Detecting divergences between nix and lix in my opinion doesn't belong to nixpkgs, but either lix or a separate project. I think it would be helpful if both nix and lix had a common set of tests, if that's already not the case.