versionCheckHook: Default `versionCheckProgram` to `mainProgram` by bengsparks · Pull Request #412768 · NixOS/nixpkgs

bengsparks · 2025-05-31T21:36:37Z

lib.getExe prefers to use meta.mainProgram over pname. Not setting meta.mainProgram leads to lib.getExe emitting a warning, stating that this "behavior is deprecated, because it leads to surprising errors when the assumption does not hold."

nixpkgs/pkgs/by-name/ve/versionCheckHook/hook.sh

Line 29 in e78d238

if [[ -z "${pname-}" ]]; then

The above snippet shows that when versionCheckProgram is not set, versionCheckHook will simply select the package's pname and ignore the meta.mainProgram entry, which is counter-intuitive, and exhibits precisely the behaviour that lib.getExe describes as "deprecated".

To remedy this, this PR introduces a new environment variable called NIX_MAIN_PROGRAM, which derives its value from the meta.mainProgram.
With the newly introduced NIX_MAIN_PROGRAM, versionCheckHook prefers meta.mainProgram over pname, but fallbacks to the latter when the former is omitted.

This PR was tested by adding a temporary opt-in flag in make-derivation.nix (credits go to @emilazy for this cool trick!), opting in to this behaviour in the hello package, and changing its pname value to something nonsensical, but the package still builds. Attempting this on the master branch causes the package to fail, despite meta.mainProgram being set to hello.

(click to open) @LordGrimmauld contributed this script to detect potential package breakage

#! /usr/bin/env nix-shell
#! nix-shell -i bash -p bash -p jq -p lixPackageSets.latest.nix-eval-jobs

flake_path="$HOME/coding/nixpkgs"

check_drv() {
  read -r drv_nej

  # no main program set, or not depending on version check hook
  if jq -e '.meta.mainProgram == null or .meta.mainProgram == .attr or .inputDrvs == null or (.inputDrvs | keys_unsorted | map(test("version-check-hook")) | any | not)' <<< "$drv_nej" > /dev/null ; then
    return 0
  fi

  drv_path=$(jq -r '.drvPath' <<< "$drv_nej" )
  main_program=$(jq -r '.meta.mainProgram' <<< "$drv_nej" )
  drv_json=$(nix derivation show "${drv_path}^*")

  # echo "checking hook usage: $drv_path"

  # version check hook does not apply or explicit version check program is set
  if jq -e --arg main_program "$main_program" 'to_entries[] | .value.env | (.doInstallCheck != "1" or .versionCheckProgram != null or .pname == $main_program)' <<< "$drv_json" > /dev/null ; then
    return 0
  fi

  echo "found problematic hook usage: $drv_path"
}

nix-eval-jobs "$flake_path" --meta --workers 4 2> /dev/null | while read -r drv_nej; do
  check_drv <<< "$drv_nej"
done

When run against d313d94, it outputs

found problematic hook usage: /nix/store/r2abajkwmfcikjhdnmdgg38zff821za2-clash-rs-0.7.7.drv
found problematic hook usage: /nix/store/05pjqa09y3ci1wz4pc18yn2mdfmq95dm-clouddrive2-0.8.19.drv
found problematic hook usage: /nix/store/dcd2bz42k630vpnn83lwaq5j88w1hp7p-libcava-0.10.3.drv
found problematic hook usage: /nix/store/0mha517kyylapsvb8pmjc2wlv54ig0sq-picom-pijulius-8.2-unstable-2025-03-01.drv
found problematic hook usage: /nix/store/dfdhb7i0cc82zxhhk4q5db5cmm3q75j2-python3.12-mypy-boto3-builder-8.11.0.drv
found problematic hook usage: /nix/store/p0k9sijxa3n5bivl75iznbl1his48vn5-python3.13-mypy-boto3-builder-8.11.0.drv

Add a 👍 reaction to pull requests you find important.

philiptaron

I'm amenable to this change.

I see you've marked the PR as draft.

What do you think the path to making it ready for review is?
What do we need to do before merging it?
Does anyone think we shouldn't do this?
If we don't do this, what are some other options?

pkgs/stdenv/generic/make-derivation.nix

bengsparks · 2025-06-03T19:56:11Z

@philiptaron thanks for taking the time to review!

After incorporating the suggested changes I'll need to tweak the docs to remove references to env.mainProgram, and make adjustments for pname.
I'll be slightly changing the hook.sh to flatten the if statements (3 levels of ifs are too deep for my liking), but otherwise all is good I think.
buildGraalvmNativeImage: switch to lib.extendMkDerivation #413663 fixes the eval errors stemming from buildGraalvmNativeImage; its defaulting of mainProgram = executable where executable ? args.pname breaks this PR's eval (I wonder if this is in fact an occurrence of Tracking issue: superfluous use of pname #277994?) (and if it is, it might just the kind of occurrence that this PR can remedy!) There is also the glibc eval error that can be found here, but your comment would fix this.
We'll also need to rerun @LordGrimmauld's script and see if anything noteworthy breaks, but I am cautiously optimistic.
As documented in the PR, meta.mainProgram now influences rebuilds. This is a departure from the status quo, where meta does not cause an effect on rebuilds. I don't think the ramifications of this are massive (why would one name a mainProgram that does not point to a valid program?) lib.getExe already influences rebuilds this way.
In fact, versionCheckHook considering meta.mainProgram provides a better interface than versionCheckProgram, as this requires slightly awkward usage of ${placeholder "out"} to pick the path to the binary. Considering that meta.mainProgram already influences lib.getExe, I don't think it's an issue for this is happen. I don't think mainProgram changes very often anyhow 😄
I'm out of ideas on this one 😄. I'm not aware of any alternate sources of mainProgram, and pname should not be considered due to the aforementioned issue (and the spirit of this PR!)

philiptaron · 2025-06-10T22:47:03Z

Merged #413663.

pkgs/by-name/ve/versionCheckHook/hook.sh

doc/hooks/versionCheckHook.section.md

adisbladis · 2025-08-03T23:40:22Z

I just spotted this for the first time in a build and find the behaviour of this really confusing.
When I saw NIX_MAIN_PROGRAM in my build environment I immediately thought it must be a bug.

I'm not a fan of introducing a new environment variable with questionable semantics: overriding a meta variable can have the unintended consequence of a rebuild.
Afaik this is not the case for anything else in nixpkgs, and is really not obvious or intuitive.
Nixpkgs is still missing a lot of meta.mainProgram entries and adding them via overrides should not mean that you're not getting binary cache hits.

I'm tempted to revert before this can make it out into a release.
At the very least I think we should revert until we can have a proper discussion around the expected semantics of meta and overrides.

emilazy · 2025-08-04T01:38:43Z

FWIW, though I think it is correct that changing a package’s meta previously couldn’t result in rebuilds of that package – unless you count turning a throw into a derivation or vice versa as a rebuild, in which case license and so on already could (this isn’t totally academic, since it means changing a package’s licence can result in new Hydra jobs with the same costs as everything else the rebuild labels track) – in the case of meta.mainProgram specifically, the stronger property of “changing meta does not result in rebuilds of any package” already didn’t hold, because of lib.getExe; i.e. it already wasn’t safe to change the meta.mainProgram of a package with a large reverse closure.

So I think the weakening of the property here is more benign than it may appear. This might be more an argument for lifting mainProgram out of meta, though. It’s a bit tricky since it also makes sense to consume from the kinds of things that look at meta dumps.

(There are also clearly metadata‐ish things like pname that do inevitably cause rebuilds… and of course versionCheckHook defaulting to pname is a horrible hack that does need addressing, one way or the other. Using the same source of truth as lib.getExe makes sense to me for that.)

adisbladis · 2025-08-04T01:55:04Z

...lib.getExe...

The fact that lib.getExe works as it does is not a problem. It's well documented and understood by it's consumers.

the weakening of the property

I think your point only makes sense in isolation.
This changes meta to be a derivation busting self reference. A huge semantic change that I'd like to avoid.
It sets a precedence not just for meta.mainProgram but weakens the expected properties of all of meta.

emilazy · 2025-08-04T02:02:40Z

The fact that lib.getExe works as it does is not a problem. It's well documented and understood by it's consumers.

I personally think it’s surprising that we’d not be okay with meta causing a rebuild in the package it’s attached to, but okay with it causing rebuilds in potentially thousands of downstream packages. That seems like it eliminates the conceptual advantage of “metadata is just metadata, it doesn’t affect the ‘data plane’”, which I agree is a sensible property. So, insofar as we want to strongly enforce that property, I think that lib.getExe looking in meta is a problem.

At the same time, clearly a mainProgram declaration is both the correct thing to use both for lib.getExe and for things like versionCheckHook, and also useful for things that want to consume package metadata en masse. So I’m not sure how best to square this particular circle. The best idea I can come up with is to lift mainProgram to be a main derivation argument, make lib.getExe prefer pkg.mainProgram and versionCheckHook use $mainProgram, and then inherit it back into meta for compatibility and external consumers. What do you think of that plan?

adisbladis · 2025-08-04T02:24:29Z

I personally think it’s surprising that we’d not be okay with meta causing a rebuild in the package it’s attached to, but okay with it causing rebuilds in potentially thousands of downstream packages

This is also not great, but at least it does what it says on the tin.
I was coming at this from the perspective of a nixpkgs user whose previous no-rebuild overrides suddenly started being heavy after a channel bump, and from that perspective I don't think getExe is problematic.
When I put on my nixpkgs developer hat it is of course an obvious downside in how getExe works.

The best idea I can come up with is to lift mainProgram to be a main derivation argument, make lib.getExe prefer pkg.mainProgram and versionCheckHook use $mainProgram, and then inherit it back into meta for compatibility and external consumers. What do you think of that plan?

Great plan that moves the option to where it actually makes semantic sense.
I'm all for it!

RossSmyth · 2025-08-04T19:54:56Z

Also meta.mainProgram already can cause rebuilds of the package it is set on because of a peculiar pattern that this PR directly fixes.
rg "versionCheckProgram.*meta"

vcunat · 2025-08-05T11:43:16Z

Yeah, but without this PR, getting rid of the getExe warning (by explicitly defining the same mainProgram) would not cause any rebuilds. Now it does.

We'd have to set `versionCheckProgram` manually. But even then, running `bw` requires `$HOME` to be writable but we cannot pass environment variables to the hook. Not-cherry-picked-because: #412768 and #411609 were not backported

nixpkgs-ci · 2025-10-21T15:00:12Z

Successfully created backport PR for staging-25.05:

[Backport staging-25.05] versionCheckHook: Default versionCheckProgram to mainProgram #454228

JohnRTitor · 2025-10-21T15:00:44Z

Without this, versionCheckHook is not respecting meta.mainProgram and thus leading to build failure in #454223 (comment)

Requesting a backport.

katexochen · 2025-11-25T11:38:43Z

I was coming at this from the perspective of a nixpkgs user whose previous no-rebuild overrides suddenly started being heavy after a channel bump, and from that perspective I don't think getExe is problematic.

Exactly this. This is causing tons of rebuilds in my downstream project. I really think it is a step in the wrong direction and it breaks downstream users in a terrible way.

SomeoneSerge · 2025-12-01T14:55:17Z

The best idea I can come up with is to lift mainProgram to be a main derivation argument, make lib.getExe prefer pkg.mainProgram and versionCheckHook use $mainProgram, and then inherit it back into meta for compatibility and external consumers. What do you think of that plan?

Great plan that moves the option to where it actually makes semantic sense.
I'm all for it!

Should probably have been the first choice (if I understand you: start deprecating drv.meta.mainProgram, move to drv). A rather questionable change, and the more questionable to make it into the release? Despite the relatable motivation

emilazy · 2025-12-01T17:33:44Z

#430810 is there for people to pick up if they would like to move to mainProgram being outside of meta, FWIW.

azuwis · 2025-12-17T11:34:23Z

This change introduces redundant derivations for certain packages, specifically those differing only in their meta.mainProgram attribute. Hydra is unnecessarily rebuilding these packages, as seen in the following files:

nixpkgs/pkgs/top-level/all-packages.nix

Lines 3322 to 3327 in 83e544a

    
           netcat = libressl.nc.overrideAttrs (old: { 
        
             meta = old.meta // { 
        
               description = "Utility which reads and writes data across network connections — LibreSSL implementation"; 
        
               mainProgram = "nc"; 
        
             }; 
        
           });

nixpkgs/pkgs/top-level/all-packages.nix

Line 8950 in 83e544a

dig = lib.addMetaAttrs { mainProgram = "dig"; } bind.dnsutils;

nixpkgs/pkgs/by-name/sb/sbom4files/package.nix

Lines 5 to 7 in 83e544a

    
           (python3Packages.toPythonApplication python3Packages.sbom4files).overrideAttrs (previousAttrs: { 
        
             meta.mainProgram = "sbom4files"; 
        
           })

The problem with patterns like sbom4files can be fixed with pull request #471631. However, for multi-output derivations like libressl and bind, there is currently no mechanism to set meta attributes specific to each output. How can we balance the need to define meta.mainProgram for specific outputs with the goal of avoiding unnecessary rebuilds?

SomeoneSerge · 2025-12-18T16:09:23Z

@azuwis I think your example shows that versionCheckProgram should have a NIX_MAIN_PROGRAM at all, but instead expect something like a bash array expectPrograms; mkDerivation could then pre-initialize, conditional on elem "bin" outputs, expectPrograms = [ pname ], and meta.mainProgram = head expectPrograms; sbom4files would set expectPrograms = [ "expect4files" "expect4dot" ]. Derivations that wish to opt-out set expectPrograms = [ ]

EDIT: Ah, I was mistaken, there's no "sbom4files" executable, and the python package's mainProgram is unset. The argument remains though

It failed to build caused by NixOS/nixpkgs#412768

github-actions bot added 6.topic: stdenv Standard environment 8.has: documentation This PR adds or changes documentation labels May 31, 2025

github-project-automation bot added this to Stdenv May 31, 2025

nix-owners bot requested review from Artturin, Ericson2314, RossComputerGuy, emilazy, philiptaron and reckenrode May 31, 2025 21:38

bengsparks marked this pull request as draft May 31, 2025 21:46

bengsparks changed the base branch from master to staging May 31, 2025 22:07

nixpkgs-ci bot closed this May 31, 2025

nixpkgs-ci bot reopened this May 31, 2025

github-project-automation bot moved this to Done in Stdenv May 31, 2025

ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 31, 2025

bengsparks force-pushed the versionCheckHookTrick branch from efa5bab to 258f27f Compare May 31, 2025 22:40

bengsparks mentioned this pull request Jun 3, 2025

buildGraalvmNativeImage: switch to lib.extendMkDerivation #413663

Merged

13 tasks

philiptaron reviewed Jun 3, 2025

View reviewed changes

pkgs/stdenv/generic/make-derivation.nix Outdated Show resolved Hide resolved

bengsparks force-pushed the versionCheckHookTrick branch from 258f27f to fd442d3 Compare June 3, 2025 23:57

ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 4, 2025

bengsparks force-pushed the versionCheckHookTrick branch from fd442d3 to e53fdd3 Compare June 11, 2025 09:14

bengsparks marked this pull request as ready for review June 11, 2025 09:24

bengsparks commented Jun 11, 2025

View reviewed changes

pkgs/by-name/ve/versionCheckHook/hook.sh Outdated Show resolved Hide resolved

bengsparks commented Jun 11, 2025

View reviewed changes

doc/hooks/versionCheckHook.section.md Outdated Show resolved Hide resolved

niklaskorz mentioned this pull request Jul 14, 2025

protonplus: 0.5.8 -> 0.5.9 #425112

Merged

13 tasks

Rhys-T mentioned this pull request Jul 16, 2025

update.nix suddenly failing to evaluate Rhys-T/nur-packages#140

Closed

langsjo mentioned this pull request Jul 17, 2025

stdenv: assert that env is an attrset #426211

Merged

13 tasks

emilazy mentioned this pull request Aug 4, 2025

stdenv.mkDerivation: support mainProgram as a top‐level attribute #430810

Draft

13 tasks

doronbehar mentioned this pull request Aug 21, 2025

versionCheckHook: add versionCheckScript parameter #434354

Open

13 tasks

Defelo mentioned this pull request Aug 22, 2025

[Backport release-25.05] radicle-node: use canonical release reference, add updateScript, modernize, adopt #435999

Merged

1 task

github-actions bot mentioned this pull request Oct 16, 2025

[Backport release-25.05] bitwarden-cli: 2025.5.0 -> 2025.9.0 #452659

Merged

1 task

JohnRTitor added the backport staging-25.05 label Oct 21, 2025

nixpkgs-ci bot mentioned this pull request Oct 21, 2025

[Backport staging-25.05] versionCheckHook: Default versionCheckProgram to mainProgram #454228

Closed

1 task

github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label Oct 21, 2025

emilazy mentioned this pull request Oct 28, 2025

stdenv: allow preserving meta fields in derivation #420575

Open

zackad added a commit to zackad/dotfiles that referenced this pull request Dec 28, 2025

hm: temporarily disable cbconvert

66fbdd5

It failed to build caused by NixOS/nixpkgs#412768

Uh oh!

Conversation

bengsparks commented May 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philiptaron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bengsparks commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philiptaron commented Jun 10, 2025

Uh oh!

Uh oh!

Uh oh!

adisbladis commented Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emilazy commented Aug 4, 2025

Uh oh!

adisbladis commented Aug 4, 2025

Uh oh!

emilazy commented Aug 4, 2025

Uh oh!

adisbladis commented Aug 4, 2025

Uh oh!

RossSmyth commented Aug 4, 2025

Uh oh!

vcunat commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nixpkgs-ci bot commented Oct 21, 2025

Uh oh!

JohnRTitor commented Oct 21, 2025

Uh oh!

katexochen commented Nov 25, 2025

Uh oh!

SomeoneSerge commented Dec 1, 2025

Uh oh!

emilazy commented Dec 1, 2025

Uh oh!

azuwis commented Dec 17, 2025

Uh oh!

SomeoneSerge commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

bengsparks commented May 31, 2025 •

edited

Loading

bengsparks commented Jun 3, 2025 •

edited

Loading

adisbladis commented Aug 3, 2025 •

edited

Loading

vcunat commented Aug 5, 2025 •

edited

Loading

SomeoneSerge commented Dec 18, 2025 •

edited

Loading