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 all 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
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 ];
};
})
1 change: 0 additions & 1 deletion pkgs/top-level/aliases.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,6 @@ mapAliases {
emacsMacport = emacs-macport; # Added 2023-08-10
emacsNativeComp = emacs; # Added 2022-06-08
emacsWithPackages = throw "'emacsWithPackages' has been renamed to/replaced by 'emacs.pkgs.withPackages'"; # Converted to throw 2024-10-17
emacsPackages = emacs.pkgs; # Added 2025-03-02

EmptyEpsilon = empty-epsilon; # Added 2024-07-14
enyo-doom = enyo-launcher; # Added 2022-09-09
Expand Down
24 changes: 13 additions & 11 deletions pkgs/top-level/all-packages.nix
View file
Open in desktop
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
Copy link
Contributor

@wolfgangwalther wolfgangwalther Aug 17, 2025

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.

Copy link
Member Author

@jopejoe1 jopejoe1 Aug 17, 2025

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.

Copy link
Contributor

@wolfgangwalther wolfgangwalther Aug 17, 2025

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?

Copy link
Member

@sternenseemann sternenseemann Aug 17, 2025

Choose a reason for hiding this comment

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

See also #126934

Copy link
Member

@sternenseemann sternenseemann Aug 17, 2025 •
edited
Loading

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.

Copy link
Contributor

@jbedo jbedo Aug 17, 2025

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

Copy link
Member Author

@jopejoe1 jopejoe1 Aug 19, 2025 •
edited
Loading

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.

Copy link
Contributor

@wolfgangwalther wolfgangwalther Aug 26, 2025

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.

Copy link
Contributor

@jian-lin jian-lin Aug 27, 2025

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 Expand Up @@ -11539,6 +11539,8 @@ with pkgs;
emacs-nox = emacs30-nox;
emacs-pgtk = emacs30-pgtk;

emacsPackages = recurseIntoAttrs emacs.pkgs;

emacsPackagesFor =
emacs:
import ./emacs-packages.nix {
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
Loading
Loading