Skip to content

packages-config.nix: don't index stuff thats not getting evaled #434501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jopejoe1 wants to merge 6 commits into from
Closed

packages-config.nix: don't index stuff thats not getting evaled #434501

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fbe17ff
windows.mingw_w64_pthreads: put deprecation behind alias
jopejoe1 Aug 17, 2025
1079516
windows.sdk: move throw behind meta.license check
jopejoe1 Aug 17, 2025
1cdfcf6
packages-config.nix: remove redundant package sets
jopejoe1 Aug 17, 2025
1f4ee10
packages-config.nix: eval stuff that's appearing on search.nixos.org
jopejoe1 Aug 17, 2025
288d13a
packages-config.nix: move emcasPackages back to all-packages.nix and …
jopejoe1 Aug 17, 2025
04948a4
packages-config.nix: only use to remove packages from search
jopejoe1 Aug 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions pkgs/os-specific/windows/default.nix
View file
Open in desktop
Comment thread
jopejoe1 marked this conversation as resolved.
Outdated
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
lib,
config,
stdenv,
buildPackages,
pkgs,
Expand All @@ -9,7 +10,9 @@
}:

lib.makeScope newScope (
self: with self; {
self:
with self;
{
dlfcn = callPackage ./dlfcn { };

mingw_w64 = callPackage ./mingw-w64 {
Expand All @@ -31,8 +34,6 @@ lib.makeScope newScope (

mingw_w64_headers = callPackage ./mingw-w64/headers.nix { };

mingw_w64_pthreads = lib.warn "windows.mingw_w64_pthreads is deprecated, windows.pthreads should be preferred" self.pthreads;

mcfgthreads = callPackage ./mcfgthreads { stdenv = crossThreadsStdenv; };

npiperelay = callPackage ./npiperelay { };
Expand All @@ -43,4 +44,7 @@ lib.makeScope newScope (

sdk = callPackage ./msvcSdk { };
}
// lib.optionalAttrs config.allowAliases {
mingw_w64_pthreads = lib.warn "windows.mingw_w64_pthreads is deprecated, windows.pthreads should be preferred" self.pthreads;
}
)
194 changes: 98 additions & 96 deletions pkgs/os-specific/windows/msvcSdk/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,102 +24,104 @@ let
else
throw "Unsupported system";
in
if !config.microsoftVisualStudioLicenseAccepted then
throw ''
Microsoft Software License Terms are not accepted with config.microsoftVisualStudioLicenseAccepted.
Please read https://visualstudio.microsoft.com/license-terms/mt644918/ and if you agree, change your
config to indicate so.
''
else
stdenvNoCC.mkDerivation (finalAttrs: {
inherit version;
pname = "msvc-sdk";
dontUnpack = true;

strictDeps = true;
nativeBuildInputs = [ xwin ];

outputHashAlgo = "sha256";
outputHashMode = "recursive";
outputHash = hashes.${arch};

__structuredAttrs = true;
xwinArgs = [
"--accept-license"
"--cache-dir=xwin-out"
"--manifest=${./manifest.json}"
"--arch=${arch}"
"splat"
"--preserve-ms-arch-notation"
];

buildPhase = ''
runHook preBuild

xwin "''${xwinArgs[@]}"
mkdir "$out"
mv xwin-out/splat/* "$out"

runHook postBuild
'';

dontFixup = true;
dontInstall = true;

passthru = {
updateScript = ./update.nu;
tests = {
hello-world = testers.runCommand {
name = "hello-msvc";

nativeBuildInputs = [
llvmPackages.clang-unwrapped
llvmPackages.bintools-unwrapped
];

script = ''
set -euo pipefail

cat > hello.c <<- EOF
#include <stdio.h>

int main(int argc, char* argv[]) {
printf("Hello world!\n");
return 0;
}
EOF

clang-cl --target=x86_64-pc-windows-msvc -fuse-ld=lld \
/vctoolsdir ${finalAttrs.finalPackage}/crt \
/winsdkdir ${finalAttrs.finalPackage}/sdk \
./hello.c -v

if test ! -f hello.exe; then
echo "hello.exe not found!"
exit 1
else
touch $out
fi
'';
};
stdenvNoCC.mkDerivation (finalAttrs: {
inherit version;
pname = "msvc-sdk";
dontUnpack = true;

strictDeps = true;
nativeBuildInputs = [ xwin ];

outputHashAlgo = "sha256";
outputHashMode = "recursive";
outputHash =
if !config.microsoftVisualStudioLicenseAccepted then
throw ''
Microsoft Software License Terms are not accepted with config.microsoftVisualStudioLicenseAccepted.
Please read https://visualstudio.microsoft.com/license-terms/mt644918/ and if you agree, change your
config to indicate so.
''
else
hashes.${arch};

__structuredAttrs = true;
xwinArgs = [
"--accept-license"
"--cache-dir=xwin-out"
"--manifest=${./manifest.json}"
"--arch=${arch}"
"splat"
"--preserve-ms-arch-notation"
];

buildPhase = ''
runHook preBuild

xwin "''${xwinArgs[@]}"
mkdir "$out"
mv xwin-out/splat/* "$out"

runHook postBuild
'';

dontFixup = true;
dontInstall = true;

passthru = {
updateScript = ./update.nu;
tests = {
hello-world = testers.runCommand {
name = "hello-msvc";

nativeBuildInputs = [
llvmPackages.clang-unwrapped
llvmPackages.bintools-unwrapped
];

script = ''
set -euo pipefail

cat > hello.c <<- EOF
#include <stdio.h>

int main(int argc, char* argv[]) {
printf("Hello world!\n");
return 0;
}
EOF

clang-cl --target=x86_64-pc-windows-msvc -fuse-ld=lld \
/vctoolsdir ${finalAttrs.finalPackage}/crt \
/winsdkdir ${finalAttrs.finalPackage}/sdk \
./hello.c -v

if test ! -f hello.exe; then
echo "hello.exe not found!"
exit 1
else
touch $out
fi
'';
};
};

meta = {
description = "MSVC SDK and Windows CRT for cross compiling";
homepage = "https://developer.microsoft.com/en-us/windows/downloads/windows-sdk/";
maintainers = [ lib.maintainers.RossSmyth ];
license = {
deprecated = false;
fullName = "Microsoft Software License Terms";
shortName = "msvc";
spdxId = "unknown";
url = "https://www.visualstudio.com/license-terms/mt644918/";
};
platforms = lib.platforms.all;
# The arm32 manifest is missing critical pieces.
broken = stdenvNoCC.hostPlatform.isAarch32;
sourceProvenance = [ lib.sourceTypes.binaryNativeCode ];
teams = [ lib.teams.windows ];
};

meta = {
description = "MSVC SDK and Windows CRT for cross compiling";
homepage = "https://developer.microsoft.com/en-us/windows/downloads/windows-sdk/";
maintainers = [ lib.maintainers.RossSmyth ];
license = {
deprecated = false;
fullName = "Microsoft Software License Terms";
shortName = "msvc";
spdxId = "unknown";
free = false;
url = "https://www.visualstudio.com/license-terms/mt644918/";
};
})
platforms = lib.platforms.all;
# The arm32 manifest is missing critical pieces.
broken = stdenvNoCC.hostPlatform.isAarch32;
sourceProvenance = [ lib.sourceTypes.binaryNativeCode ];
teams = [ lib.teams.windows ];
};
})
22 changes: 11 additions & 11 deletions pkgs/top-level/all-packages.nix
View file
Open in desktop
Comment thread
jopejoe1 marked this conversation as resolved.
Outdated
Original file line number Diff line number Diff line change
Expand Up @@ -2330,7 +2330,7 @@ with pkgs;

roundcube = callPackage ../servers/roundcube { };

roundcubePlugins = dontRecurseIntoAttrs (callPackage ../servers/roundcube/plugins { });
roundcubePlugins = recurseIntoAttrs (callPackage ../servers/roundcube/plugins { });

rsyslog = callPackage ../tools/system/rsyslog {
withHadoop = false; # Currently Broken
Expand Down Expand Up @@ -3461,9 +3461,9 @@ with pkgs;

importNpmLock = callPackages ../build-support/node/import-npm-lock { };

nodePackages_latest = dontRecurseIntoAttrs nodejs_latest.pkgs;
nodePackages_latest = recurseIntoAttrs nodejs_latest.pkgs;

nodePackages = dontRecurseIntoAttrs nodejs.pkgs;
Comment on lines 3464 to 3466

@wolfgangwalther wolfgangwalther Aug 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting here, but that's for the whole commit: This also causes all these packages to additionally be built by hydra, right? I think that's a massive number of packages added.

I don't think we should or even can do this for all of these.

I see that you want to include all of these in Eval, and that makes sense. At the same time, we'd have to ensure that they're not built in Hydra, though.

@jopejoe1 jopejoe1 Aug 17, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like most of these come from rPackages so it's probably fine to just leave that one out for now and see what can be done about that.

@wolfgangwalther wolfgangwalther Aug 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rPackages was disabled in ccd1029, like the others, too. The discussion for this was in #12203. It was also noted:

I suppose we could disable the builds for R since those packages tend to compile very quickly

So maybe, build-wise, adding R packages wouldn't be that much of a problem. Not sure. Eval times would certainly go up, though. For GHA, this is an increase of 15 seconds, from 2:30 to 2:45 right now. That should work.

I can't estimate what the consequences for Hydra would be, however.

@vcunat you were involved in that issue above back then as well. WDYT? Are we ready to enable rPackages again or not?

@sternenseemann sternenseemann Aug 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also #126934

@sternenseemann sternenseemann Aug 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rPackages was disabled in ccd1029, like the others, too. The discussion for this was in #12203. It was also noted:

Note that some packages that have recurseIntoAttrs (e.g. ocamlPackages) were disabled this way, too. So it is also worth checking (though it may be hard with the time that has passed) why dontRecurseIntoAttrs was added for these package sets. I suspect that eval time on a simple nix-env (which is the stable way to list packages) was a factor, too.

@jbedo jbedo Aug 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rPackages was disabled in ccd1029, like the others, too. The discussion for this was in #12203. It was also noted:

I suppose we could disable the builds for R since those packages tend to compile very quickly

So maybe, build-wise, adding R packages wouldn't be that much of a problem. Not sure. Eval times would certainly go up, though. For GHA, this is an increase of 15 seconds, from 2:30 to 2:45 right now. That should work.

I can't estimate what the consequences for Hydra would be, however.

@vcunat you were involved in that issue above back then as well. WDYT? Are we ready to enable rPackages again or not?

I would very much like to see rPackages re-enabled if there's sufficient capacity. With it disabled, we have very little visibility of the state of the tree as the r-updates jobset has also been disabled in hydra since last year. Currently I'm running a hydra instance on my desktop to help with bumping rPackges while maintining a good overall state.

I've attached a plot using data I took from my hydra instance (buildoutputs table in postgres) showing the build times for packages. It shows that the vast majority of builds in the tree are quite small, with 99% of jobs finishing in under 60s. Evaluation seems to take ~ 108s.

hydra-rPackages-eCDF

@jopejoe1 jopejoe1 Aug 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also #126934

I opened a similar PR for ocamlPackages a few months ago, #406555, but that one also included some cleanup so that it still passes current CI standards, and most of the other ones were done in #405039.

@wolfgangwalther wolfgangwalther Aug 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's split the commit that adds all the recurseIntoAttrs into separate commits in separate PRs - one for each package set that is touched. This will allow us to see rebuild numbers for each of these changes. Then we can judge these case-by-case.

@jian-lin jian-lin Aug 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could disable the builds for R since those packages tend to compile very quickly

So maybe, build-wise, adding R packages wouldn't be that much of a problem.

For hydra.nixos.org, it seems build time of each packages affects much less the throughput than the number of packages to build. In other words, many small packages are much harder than a few large packages for hydra.nixos.org.

This is our experience for emacsPackages. emacsPackages are 6k (for one platform) small packages. Building them takes 30 minutes on my desktop. However, due to its large number, we were kindly asked by the infra team to target staging. (I totally support infra team's request and I am sorry we added some extra work for them. I mention these only to help the discussion here: estimating the potential influence of this PR.)

nodePackages = recurseIntoAttrs nodejs.pkgs;

node2nix = nodePackages.node2nix;

Expand Down Expand Up @@ -3970,7 +3970,7 @@ with pkgs;
inherit (darwin) DarwinTools;
};

platformioPackages = dontRecurseIntoAttrs (callPackage ../development/embedded/platformio { });
platformioPackages = recurseIntoAttrs (callPackage ../development/embedded/platformio { });
platformio =
if stdenv.hostPlatform.isLinux then
platformioPackages.platformio-chrootenv
Expand Down Expand Up @@ -5164,10 +5164,10 @@ with pkgs;

# Haskell and GHC

haskell = callPackage ./haskell-packages.nix { };
haskell = recurseIntoAttrs (callPackage ./haskell-packages.nix { });

haskellPackages =
dontRecurseIntoAttrs
recurseIntoAttrs
# Prefer native-bignum to avoid linking issues with gmp
# GHC 9.6 rts can't be built statically with hadrian, so we need to use 9.4
# until 9.8 is ready
Expand Down Expand Up @@ -7857,7 +7857,7 @@ with pkgs;

#GMP ex-satellite, so better keep it near gmp
# A GMP fork
gns3Packages = dontRecurseIntoAttrs (callPackage ../applications/networking/gns3 { });
gns3Packages = recurseIntoAttrs (callPackage ../applications/networking/gns3 { });
gns3-gui = gns3Packages.guiStable;
gns3-server = gns3Packages.serverStable;

Expand Down Expand Up @@ -9654,7 +9654,7 @@ with pkgs;

rstudioServerWrapper = rstudioWrapper.override { rstudio = rstudio-server; };

rPackages = dontRecurseIntoAttrs (
rPackages = recurseIntoAttrs (
callPackage ../development/r-modules {
overrides = (config.rPackageOverrides or (_: { })) pkgs;
}
Expand Down Expand Up @@ -10502,7 +10502,7 @@ with pkgs;
inherit (linuxPackages) kernel;
};

fusePackages = dontRecurseIntoAttrs (
fusePackages = recurseIntoAttrs (
callPackage ../os-specific/linux/fuse {
util-linux = util-linuxMinimal;
}
Expand Down Expand Up @@ -10938,7 +10938,7 @@ with pkgs;

v4l-utils = callPackage ../os-specific/linux/v4l-utils { };

windows = callPackages ../os-specific/windows { };
windows = recurseIntoAttrs (callPackages ../os-specific/windows { });

wpa_supplicant = callPackage ../os-specific/linux/wpa_supplicant { };

Expand Down Expand Up @@ -11184,7 +11184,7 @@ with pkgs;
pretendard-std
;

sourceHanPackages = dontRecurseIntoAttrs (callPackage ../data/fonts/source-han { });
sourceHanPackages = recurseIntoAttrs (callPackage ../data/fonts/source-han { });
source-han-sans = sourceHanPackages.sans;
source-han-serif = sourceHanPackages.serif;
source-han-mono = sourceHanPackages.mono;
Expand Down
5 changes: 3 additions & 2 deletions pkgs/top-level/haskell-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ in
# `name: pkgs: pkgs.haskell.packages.${name}.ghc == pkgs.buildPackages.haskell.compiler.${name}.ghc`.
# This isn't problematic since pkgsBuildBuild.buildPackages is also build->build,
# just something to keep in mind.
compiler =
compiler = pkgs.lib.recurseIntoAttrs (
let
bb = pkgsBuildBuild.haskell;
in
Expand Down Expand Up @@ -496,7 +496,8 @@ in
name: compiler.${name}.override { enableNativeBignum = true; }
)
);
};
}
);

# Default overrides that are applied to all package sets.
packageOverrides = self: super: { };
Expand Down
24 changes: 0 additions & 24 deletions pkgs/top-level/packages-config.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,6 @@
super:
with super;
lib.mapAttrs (_: set: recurseIntoAttrs set) {
inherit (super)
agdaPackages
apacheHttpdPackages
fusePackages
gns3Packages
haskellPackages
idrisPackages
nodePackages
nodePackages_latest
platformioPackages
rPackages
roundcubePlugins
sourceHanPackages
zabbix60
windows
;

# Make sure haskell.compiler is included, so alternative GHC versions show up,
# but don't add haskell.packages.* since they contain the same packages (at
# least by name) as haskellPackages.
haskell = super.haskell // {
compiler = recurseIntoAttrs super.haskell.compiler;
};

# emacsPackages is an alias for emacs.pkgs
# Re-introduce emacsPackages here so that emacs.pkgs can be searched.
emacsPackages = emacs.pkgs;
Comment thread
wolfgangwalther marked this conversation as resolved.
Outdated
Expand Down