Format all Nix files using the officially approved formatter,
making the CI check introduced in the previous commit succeed:
nix-build ci -A fmt.check
This is the next step of the of the [implementation](https://github.com/NixOS/nixfmt/issues/153)
of the accepted [RFC 166](https://github.com/NixOS/rfcs/pull/166).
This commit will lead to merge conflicts for a number of PRs,
up to an estimated ~1100 (~33%) among the PRs with activity in the past 2
months, but that should be lower than what it would be without the previous
[partial treewide format](https://github.com/NixOS/nixpkgs/pull/322537).
Merge conflicts caused by this commit can now automatically be resolved while rebasing using the
[auto-rebase script](8616af08d9/maintainers/scripts/auto-rebase).
If you run into any problems regarding any of this, please reach out to the
[formatting team](https://nixos.org/community/teams/formatting/) by
pinging @NixOS/nix-formatting.
Due to Nix bug <https://github.com/NixOS/nix/issues/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):
* <https://github.com/NixOS/nixpkgs/pull/340046>
* <https://github.com/NixOS/nixpkgs/pull/352491>
* <https://github.com/NixOS/nixpkgs/pull/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.
The attrNamesOnly feature is used by
pkgs/top-level/release-attrpaths-superset.nix to return a superset of
all attributes that might be built by Hydra.
Before this change it would include all attribute paths to derivations that could
be found recursively, ignoring the recurseForDerivations and
recurseForRelease attributes that control Hydra's recursion.
This had the effect that it would end up with ~266000 attributes, most of which definitely won't be built by Hydra. We can remove those while staying true to the superset notion to end up with just ~97000, a reduction of ~63.6%! This also comes with an eval time reduction from 31.7s to 18.7s (-41.0%)!
As an example, all derivations in pypy310Packages don't get included
anymore, because it doesn't have a `.recurseForDerivations` set.
As a nice side effect, with `--arg enableWarnings false`, no more
warnings are printed, because things like
`checkpointBuildTools.mkCheckpointedBuild` (which is deprecated) isn't
being recursed to anymore.
This was added in <https://github.com/NixOS/nixpkgs/pull/19990> to
ensure that packages that previously weren’t building at all would
get at least one platform’s worth of coverage. However, since most
`runCommand` calls don’t explicitly set any `meta` information,
this meant that things like `tests.makeWrapper` were only being
run for `x86_64-linux`. Since most trivial builder calls should
work on all platforms, it doesn’t make much sense for them to be
restricted in this way, and we shouldn’t unnecessarily penalize
other platforms by treating an empty `meta.platforms` inconsistently
with `check-meta`’s maximally‐permissive interpretation. Actual
packages that only work on a subset of platforms should, of course,
set `meta.platforms` explicitly.
Mesa is a package like any other. There's no reason for it to be a
special case with its platforms listed in lib, because if other
packages want to refer to mesa's platforms, they can access the
platforms from the package meta like they would for any other package.
Having multiple attributes point to the same derivation makes debugging
harder because you often cannot just grep the canonical attribute name.
It is even annoying when multiple aliases are chained.
Having Hydra build aliases is recognized as redundant
2d0a7c4eee
so let’s do the same with their evaluation.
This commit will make Hydra ignore aliases when evaluating.
A nice benefit of this is that it will allow us to warn users when
an attribute is renamed, to assist them with early migration. Since
tracing messages during evaluation are not allowed because of Hydra,
we can currently only choose between having a silent alias and throwing.
This was last attempted in 2018 but ended up being reverted
because of widespread use of aliases that was not caught by CI:
8c025c67d5
CI has since been improved:
cda5aa2ac7
packagePlatforms previously ignored badPlatforms, probably because it is
a fairly [recent] addition. While check-meta.nix doesn't implement it
using subtractLists, it is basically equivalent to the following logic:
platformSet - badPlatformSet (= effectivePlatformSet)
which we implement using subtractLists (which has somewhat confusing
currying-optimized argument order).
This flaw was discovered when testing #177901 which heavily uses
badPlatforms.
[recent]: b0482248fe
- Make tests/lxd.nix use NixOS's lxdMeta & lxdImage to avoid relying on
3rd party containers such as Alpine Linux for testing purposes.
- Merge tests/lxd-image.nix into tests/lxd.nix, since now both have a
similar structure.
- Extract duplicated inline LXD configuration into a separate file,
- Add passthru.lxd-nftables & passthru.lxd-image-server.
This use of tryEval causes hydra to fully ignore evaluation failures of
packages that occur while trying to evaluate the hydra platforms it should be
built on. This includes failures that occur during evaluation of:
- The `.type` attribute value
- The `.meta.hydraPlatforms` or `.meta.platforms` attribute value
- The `.version` attribute, since this can determine whether
`.meta.position` is set
- For non-derivations, `.recurseForDerivations` or `.recurseForRelease`
Here's a minimal `release.nix` file, showcasing how a `.version` failure
is ignored:
let
packages = pkgs: {
success = pkgs.stdenv.mkDerivation {
name = "success";
};
ignoredFailure = pkgs.stdenv.mkDerivation {
pname = "ignored-failure";
version = throw "version error";
};
caughtFailure = pkgs.stdenv.mkDerivation {
name = "caught-failure";
src = throw "src error";
};
};
releaseLib = import <nixpkgs/pkgs/top-level/release-lib.nix> {
packageSet = args: packages (import <nixpkgs> args);
supportedSystems = [ "x86_64-linux" ];
};
in
releaseLib.mapTestOn (releaseLib.packagePlatforms releaseLib.pkgs)
Evaluating this with `hydra-eval-jobs` before this change yields:
$ hydra-eval-jobs release.nix -I nixpkgs=/path/to/nixpkgs
warning: `--gc-roots-dir' not specified
error: "error: --- ThrownError --- hydra-eval-jobs\nsrc error"
{
"caughtFailure.x86_64-linux": {
"error": "error: --- ThrownError --- hydra-eval-jobs\nsrc error"
},
"success.x86_64-linux": {
"description": "",
"drvPath": "/nix/store/q1sw933xd9bxfx6rcp0kqksbprj1wmwj-success.drv",
"homepage": "",
"isChannel": false,
"license": "",
"maintainers": "",
"maxSilent": 7200,
"nixName": "success",
"outputs": {
"out": "/nix/store/7awrz6hss4jjxvgbwi4wlyikncmslb7a-success"
},
"schedulingPriority": 100,
"system": "x86_64-linux",
"timeout": 36000
}
}
Where you can see that there is no job for the `ignoredFailure`
derivation. Compare this to after this change:
$ hydra-eval-jobs release.nix -I nixpkgs=/path/to/nixpkgs
warning: `--gc-roots-dir' not specified
error: "error: --- ThrownError --- hydra-eval-jobs\nsrc error"
error: "error: --- ThrownError --- hydra-eval-jobs\nversion error"
{
"caughtFailure.x86_64-linux": {
"error": "error: --- ThrownError --- hydra-eval-jobs\nsrc error"
},
"ignoredFailure": {
"error": "error: --- ThrownError --- hydra-eval-jobs\nversion error"
},
"success.x86_64-linux": {
"description": "",
"drvPath": "/nix/store/q1sw933xd9bxfx6rcp0kqksbprj1wmwj-success.drv",
"homepage": "",
"isChannel": false,
"license": "",
"maintainers": "",
"maxSilent": 7200,
"nixName": "success",
"outputs": {
"out": "/nix/store/7awrz6hss4jjxvgbwi4wlyikncmslb7a-success"
},
"schedulingPriority": 100,
"system": "x86_64-linux",
"timeout": 36000
}
}
Notice how `ignoredFailure` is now part of the result.
We were using `supportedMatches` both when getting the list of
platforms, and before `genAttrs` the derivation for each. Now we just
filter the second time,
Instead of intersecting system strings, we filter with the sort of
patterns used in `meta.platforms`.
Indicating this change `forTheseSystems` has been renamed to
`forMatchingSystems`, since the given list is now patterns to match, and
not the systems themselves. [Just as with `meta.platforms`, systems
strings are also supported for backwards compatibility.]
This is more flexible, and makes the `forMatchingSystems` and
packagePlatforms` cases more analogous.
Negative reasoning like `allBut` is a bad idea with an open world of
platforms. Concretely, if we add a new, quite different sort of
platform, existing packages with `allBut` will claim they work on it
even though they probably won't.
Currently the logic of generating nixpkgs Hydra jobs is to walk through
the pkgs evaluated for system = "x86_64-linux", collect any derivations
and their meta.platforms values. However, that doesn't work for
packages whose meta.platforms doesn't include x86_64-linux, as just
evaluating their meta attribute raises an error so they get skipped
completely.
As a less-intrusive fix (i.e. anything than rewriting the current package
enumeration logic), allow passing `config.allowUnsupportedSystem = true`
to permit evaluating packages regardless of their platform and use that
in the package listing phase.
Fixes#25200
The old hard-coded lists are now used to test system parsing.
In the process, make an `assertTrue` in release lib for eval tests; also
use it in release-cross
This allows customizing the nixpkgs arguments by the caller. My use case
is creating a personal nixpkgs channel containing some unfree packages.
The default is still to not build unfree packages, so for nixpkgs this
is no functional change.
[N.B., this package also applies to the commits that follow it in the same
PR.]
In most cases, buildPackages = pkgs so things work just as before. For
cross compiling, however, buildPackages is resolved as the previous
bootstrapping stage. This allows us to avoid the mkDerivation hacks cross
compiling currently uses today.
To avoid a massive refactor, callPackage will splice together both package
sets. Again to avoid churn, it uses the old `nativeDrv` vs `crossDrv` to do
so. So now, whether cross compiling or not, packages with get a `nativeDrv`
and `crossDrv`---in the non-cross-compiling case they are simply the same
derivation. This is good because it reduces the divergence between the
cross and non-cross dataflow. See `pkgs/top-level/splice.nix` for a comment
along the lines of the preceding paragraph, and the code that does this
splicing.
Also, `forceNativeDrv` is replaced with `forceNativePackages`. The latter
resolves `pkgs` unless the host platform is different from the build
platform, in which case it resolves to `buildPackages`. Note that the
target platform is not important here---it will not prevent
`forcedNativePackages` from resolving to `pkgs`.
--------
Temporarily, we make preserve some dubious decisions in the name of preserving
hashes:
Most importantly, we don't distinguish between "host" and "target" in the
autoconf sense. This leads to the proliferation of *Cross derivations
currently used. What we ought to is resolve native deps of the cross "build
packages" (build = host != target) package set against the "vanilla
packages" (build = host = target) package set. Instead, "build packages"
uses itself, with (informally) target != build in all cases.
This is wrong because it violates the "sliding window" principle of
bootstrapping stages that shifting the platform triple of one stage to the
left coincides with the next stage's platform triple. Only because we don't
explicitly distinguish between "host" and "target" does it appear that the
"sliding window" principle is preserved--indeed it is over the reductionary
"platform double" of just "build" and "host/target".
Additionally, we build libc, libgcc, etc in the same stage as the compilers
themselves, which is wrong because they are used at runtime, not build
time. Fixing this is somewhat subtle, and the solution and problem will be
better explained in the commit that does fix it.
Commits after this will solve both these issues, at the expense of breaking
cross hashes. Native hashes won't be broken, thankfully.
--------
Did the temporary ugliness pan out? Of the packages that currently build in
`release-cross.nix`, the only ones that have their hash changed are
`*.gcc.crossDrv` and `bootstrapTools.*.coreutilsMinimal`. In both cases I
think it doesn't matter.
1. GCC when doing a `build = host = target = foreign` build (maximally
cross), still defines environment variables like `CPATH`[1] with
packages. This seems assuredly wrong because whether gcc dynamically
links those, or the programs built by gcc dynamically link those---I
have no idea which case is reality---they should be foreign. Therefore,
in all likelihood, I just made the gcc less broken.
2. Coreutils (ab)used the old cross-compiling infrastructure to depend on
a native version of itself. When coreutils was overwritten to be built
with fewer features, the native version it used would also be
overwritten because the binding was tight. Now it uses the much looser
`BuildPackages.coreutils` which is just fine as a richer build dep
doesn't cause any problems and avoids a rebuild.
So, in conclusion I'd say the conservatism payed off. Onward to actually
raking the muck in the next PR!
[1]: https://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html
