Skip to content

Conversation

@bengsparks
Copy link
Contributor

@bengsparks bengsparks commented Jun 3, 2025

#412768's eval breaks because executable ? args.pname fails due to buildGraalvmNativeImage defaulting meta.mainProgram to executable.
The error message reads:

>        error: attribute 'pname' missing
>        at /nix/store/9shgmyz0h0sn35l4bw782ij1a4kik0k1-source/pkgs/build-support/build-graalvm-native-image/default.nix:8:16:
>             7|   removeReferencesTo,
>             8|   executable ? args.pname,

This is surprising, as every package that uses buildGraalvmNativeImage supplies the pname field, as it is just a wrapper around stdenv.mkDerivation.

I suspect the combination of lib.intersectAttrs in callPackage(With) at

allArgs = intersectAttrs fargs autoArgs // args;
and the buildGraalvmNativeImage = (callPackage ../build-support/build-graalvm-native-image { }.override) is to blame, but I cannot be sure.

Due to the change in the buildGraalvmNativeImage function, I don't think it's possible to make this a 0-rebuild PR.

I tested this PR by

  1. observing the failure on versionCheckHook: Default versionCheckProgram to mainProgram #412768
  2. cherry-picking this commit onto its branch
  3. rebuilding the package that broke in said PR
  4. rebuilding all packages that this PR influences

I have also applied diffoscope to all of the packages that nixpkgs-review rev HEAD printed on my system, and all seem to have remained identical, but I'm not entirely confident, as it seems there are minimal changes in some .JAR files, but all files seem to be present.
If anyone has recommendations for options to tweak in diffoscope, let me know 😄

Here is the body of the script
#! /usr/bin/env nix-shell
#! nix-shell -i bash -p diffoscope

for package in \
  babashka \
  babashka-unwrapped \
  bbin \
  certificate-ripper \
  clj-kondo \
  cljfmt \
  cljstyle \
  clojure-lsp \
  cq \
  jet \
  keylight-cli \
  neil \
  scala-update \
  vscode-extensions.betterthantomorrow.calva \
  yamlscript \
  zprint
do
  echo $package
  diffoscope "$(nix-build -A $package)" $HOME/.cache/nixpkgs-review/rev-7211d31c9a6b6448191643af108b97b93ab0254c/results/$package-aarch64-darwin \
    --html diffs/$package.html \
    --exclude-directory-metadata yes
  # nix-build -A $package
done


Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jun 3, 2025
@bengsparks bengsparks marked this pull request as ready for review June 3, 2025 18:23
@bengsparks
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 413663
Commit: 9ae921508f2647f0ec3237e9e719bb0f75fe9202


aarch64-linux

✅ 20 packages built:
  • babashka
  • babashka-unwrapped
  • bbin
  • certificate-ripper
  • clj-kondo
  • cljfmt
  • cljstyle
  • clojure-lsp
  • cq
  • jet
  • keylight-cli
  • neil
  • scala-update
  • tests.writers.bin.babashka
  • tests.writers.simple.babashka
  • tests.writers.wrapping.babashka
  • tests.writers.wrapping.babashka-bin
  • vscode-extensions.betterthantomorrow.calva
  • yamlscript
  • zprint

aarch64-darwin

✅ 20 packages built:
  • babashka
  • babashka-unwrapped
  • bbin
  • certificate-ripper
  • clj-kondo
  • cljfmt
  • cljstyle
  • clojure-lsp
  • cq
  • jet
  • keylight-cli
  • neil
  • scala-update
  • tests.writers.bin.babashka
  • tests.writers.simple.babashka
  • tests.writers.wrapping.babashka
  • tests.writers.wrapping.babashka-bin
  • vscode-extensions.betterthantomorrow.calva
  • yamlscript
  • zprint

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

We want to use lib.extendMkDerivation. I'm going to make a PR against your branch...

openjdk_headless = jdk_headless;

graalvmPackages = recurseIntoAttrs (callPackage ../development/compilers/graalvm { });
buildGraalvmNativeImage =
Copy link
Contributor

Choose a reason for hiding this comment

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

wow, yikes

@philiptaron
Copy link
Contributor

Done: bengsparks#1

@bengsparks bengsparks changed the title buildGraalvmNativeImage: switch to lib.makeOverridable buildGraalvmNativeImage: switch to lib.extendMkDerivation Jun 10, 2025
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 10, 2025
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 10, 2025
@bengsparks
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 413663
Commit: f5eb7b94bc0e0d3369cfa4e86a6dff51a79919c8


aarch64-darwin

✅ 20 packages built:
  • babashka
  • babashka-unwrapped
  • bbin
  • certificate-ripper
  • clj-kondo
  • cljfmt
  • cljstyle
  • clojure-lsp
  • cq
  • jet
  • keylight-cli
  • neil
  • scala-update
  • tests.writers.bin.babashka
  • tests.writers.simple.babashka
  • tests.writers.wrapping.babashka
  • tests.writers.wrapping.babashka-bin
  • vscode-extensions.betterthantomorrow.calva
  • yamlscript
  • zprint

@philiptaron philiptaron merged commit 8bd125d into NixOS:master Jun 10, 2025
18 checks passed
@bengsparks bengsparks deleted the bGVM branch June 10, 2025 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants