Skip to content
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.

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

addAttrsToDerivation seems broken #306953

Open
allsey87 opened this issue Apr 26, 2024 · 6 comments
Open

addAttrsToDerivation seems broken #306953

allsey87 opened this issue Apr 26, 2024 · 6 comments
Labels
0.kind: bug Something is broken

Comments

@allsey87
Copy link

allsey87 commented Apr 26, 2024

Describe the bug

I am trying to use addAttrsToDerivation to add some default compilation flags but I think this function is broken. The inline comment suggests the usage should be:

stdenvNoOptimise =
 addAttrsToDerivation
   { env.NIX_CFLAGS_COMPILE = "-O0"; }
   stdenv;

Although a quick search through Github, shows most people using it without the env before NIX_CFLAGS_COMPILE. In either case the following errors are reported:

With env:

error: attribute 'libc_bin' missing

Without env

error: The ‘env’ attribute set cannot contain any attributes passed to derivation. The following attributes are overlapping: NIX_CFLAGS_COMPILE

Steps To Reproduce

Use nix-build to try and compile GNU hello with an additional compilation flag via an overlay:

{
  system ? builtins.currentSystem,
  sources ? import ./nix/sources.nix,
}:

let
  myOverlay = self: super: {
    stdenv = super.addAttrsToDerivation { NIX_CFLAGS_COMPILE = "-O0"; } super.stdenv;
  };
  # 23.11
  pkgs = import sources.nixpkgs {
    overlays = [
      myOverlay
    ];
  };

in pkgs.hello

Expected behavior

GNU hello builds with the compilation flag -O0.

Additional context

I think the changes in PR #225787 around how env is handled broke both withCFlags and addAttrsToDerivation. The issue with withCFlags was raised in #225740 and fixed in #225929, however, addAttrsToDerivation seems to have been overlooked and seems to have the same problem. In fact, both that issue and this issue mention the error message:

attribute 'libc_bin' missing

Notify maintainers

@Artturin

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
this path will be fetched (0.00 MiB download, 0.00 MiB unpacked):
  /nix/store/mh3dkpww5py0fnrihk45pw7hvkj5fxxc-nix-info
copying path '/nix/store/mh3dkpww5py0fnrihk45pw7hvkj5fxxc-nix-info' from 'https://cache.nixos.org'...
 - system: `"x86_64-linux"`
 - host os: `Linux 6.8.7-zen1-1-zen, Debian GNU/Linux, 12 (bookworm), nobuild`
 - multi-user?: `no`
 - sandbox: `no`
 - version: `nix-env (Nix) 2.16.1`
 - nixpkgs: `/home/developer/.nix-defexpr/channels/nixpkgs`
@allsey87 allsey87 added the 0.kind: bug Something is broken label Apr 26, 2024
@allsey87
Copy link
Author

Using an older version of nixpkgs (20.09), my MRE works fine without the env prefix. I also checked on the master branch and it seems that the issue is present both there and in 23.11.

@allsey87
Copy link
Author

allsey87 commented Apr 26, 2024

Interesting enough, this does seem to work (on master and 23.11):

pkgs = import sources.nixpkgs {
  config.replaceStdenv = {pkgs}: pkgs.addAttrsToDerivation {
    env.NIX_CFLAGS_COMPILE = "-O0";
  } pkgs.stdenv;
};

@apt-install-coffee
Copy link

apt-install-coffee commented Jun 28, 2024

I'm also running into the same issue when either applying stdenvAdapters to overlays or crossOverlays:

crossOverlays = [
        (final: prev: { stdenv = prev.stdenvAdapters.useMoldLinker prev.stdenv; })
        (final: prev: { stdenv = (prev.stdenvAdapters.withCFlags [ "-Os" "-flto" "-ffat-lto-objects" ] prev.stdenv); })
 ];

using them with replaceCrossStdenv:

config.replaceCrossStdenv = { buildPackages, baseStdenv }:
        (buildPackages.useMoldLinker
          (buildPackages.withCFlags [ "-Os" "-flto" "-ffat-lto-objects" ]
            baseStdenv)
  );

or changing env directly:

config.replaceCrossStdenv = { buildPackages, baseStdenv }:
        buildPackages.addAttrsToDerivation
          {
            env.NIX_CFLAGS_LINK = "-fuse-ld=mold";
            env.NIX_CFLAGS_COMPILE = "-Os -flto -ffat-lto-objects";
          }
          baseStdenv;

I was able to replicate @allsey87 's success with

config.replaceStdenv = { pkgs }:
        pkgs.addAttrsToDerivation
          {
            env.NIX_CFLAGS_LINK = "-fuse-ld=mold";
            env.NIX_CFLAGS_COMPILE = "-Os -flto -ffat-lto-objects";
          }
          pkgs.stdenv;

so perhaps the issue is in pkgs/stdenv/cross, since it strips config.replaceStdenv, but not overlays?
@allsey87 are you also cross compiling?

@allsey87
Copy link
Author

allsey87 commented Jun 28, 2024

I was cross compiling for Emscripten but I have since given up on Nix and started using Bazel

@ConnorBaker
Copy link
Contributor

Usage with env.NIX_CFLAGS_COMPILE is correct; the error is because it looks like the function doesn't provide any sort of recursive merging of the attribute set.

In your opening post, the error you saw when using it with env was caused by the env attribute set you passed (the singleton containing NIX_CFLAGS_COMPILE) replacing the existing env attribute set (which contained libc_bin).

Unfortunately, I haven't had time to look your findings, @apt-install-coffee :(

@SomeoneSerge
Copy link
Contributor

@allsey87 if we expose extendMkDerivationArgs the solution would be something like

extendMkDerivationArgs (args: { env = args.env or { } // { NIX_COMPILE_CFLAGS = args.env.NIX_COMPILE_CFLAGS or "" + "..."; }; }) stdenv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken
Projects
None yet
Development

No branches or pull requests

4 participants