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

treewide: add meta.mainProgram to (almost) all packages with a single binary #297084

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

stuebinm
Copy link
Contributor

@stuebinm stuebinm commented Mar 19, 2024

Description of changes

Set meta.mainProgram automatically for "trivial" cases of packages which produce only one binary (as determined by the nixpkgs-unstable channel's program.sqlite database).

Motivation:

#246386 has deprecated the assumption that a package's main program (if not set explicitly otherwise) has the same name as the package itself. Thus using lib.getExe on a vast number of existing packages has become essentially unsupported.

Unfortunately, it seems infeasable to set mainProgram for all existing packages manually. A rough estimate of the problem's scale was given in #219567 (comment), which estimates last year that there were around ~5500 packages affected.

I've previously opened individual PRs adding mainProgram to single packages when I came across one where it was missing, but honestly I don't look forward to dealing with this until ~2030 at least (rate estimated by running the above-linked script again now), so I thought it was worth looking at another approach.

Implementation

Builing on @emilylange's script (linked above), it's pretty easy to get a list of candidate packages which produce exactly one binary (which must hence be that package's mainProgram), but which do not actually set this in their meta block. I consider these "trivial" cases to be fixed. Currently (before this PR), there are 6921 of these.

Using meta.position, it is further (mostly) easy to get each candidate's definition site (exception: things produced e.g. via the builders in build-support/trivial-builders, where meta.position points to the builder's definition instead).

Candidates can then have a meta.mainProgram entry added to their source automatically, for which I wrote a little rust tool to edit package definitions (loosely based on nix-doc-munge which was written for the doc migration from AsciiDoc).

Consideration & special cases:

The automatic rewriting does two things:

  • if the package definition already has a meta block, it will add a mainProgram = ... in the next line after the description entry
  • if it does not yet have a meta block, it will add a meta.mainProgram = .. attribute to the end of the attribute set
  • however, some language-specific build systems (notably haskell) do not have a separate meta block and instead keep description, mainProgram, etc., as direct attributes of the package description. I have found no better way to reliably identify these than to see if the derivation has a top-level description (or similar) attribute; in this case, no meta block will be created.

Results

Of 5457 files defining the candidate packages, 5373 were edited; in 84 cases my script did nothing:

  • most of these are files that occur more than once in meta.position of different packages, which I simply chose to ignore
  • in 20 cases it found no suitable place for the meta attribute
  • in 15 cases it found an existing meta attrset but failed to edit it (these
    lack description attributes, and it only knows how to insert something after that)

Re-searching for candidates with the changes applied reduced their number to 381. Still not nothing, but a lot more manageable.

Unexpected results

I thought this would be a no-op as far as packages are concerned, but turns out it is not:

Result of nixpkgs-review run on x86_64-linux 1

4 packages built:
  • python311Packages.waitress-django
  • python311Packages.waitress-django.dist
  • python312Packages.waitress-django
  • python312Packages.waitress-django.dist

This is a single package which has src = ./. set, so since its own package definition is part of the source, it does get changed.

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (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.

The nixpkgs-unstable channel's programs.sqlite was used to identify
packages producing exactly one binary, and these automatically added
to their package definitions wherever possible.
@github-actions github-actions bot added 6.topic: xfce The Xfce Desktop Environment 6.topic: nodejs 6.topic: pantheon The Pantheon desktop environment 6.topic: cinnamon Desktop environment 6.topic: LXQt The Lightweight Qt Desktop Environment 6.topic: Lumina DE The Lumina Desktop Environment 6.topic: Enlightenment DE The Enlightenment Desktop Environment 6.topic: mate The MATE Desktop Environment 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 6.topic: php labels Mar 19, 2024
@mweinelt
Copy link
Member

I skimmed the changeset, and it looks plausible. Given that it doesn't break eval or check-meta, let's give it a shot before it rots away.

Thanks for your work on this!

@mweinelt mweinelt merged commit afa6550 into NixOS:master Mar 19, 2024
21 checks passed
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 21, 2024
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 21, 2024
@tpwrules
Copy link
Contributor

There are some packages like the Python library Amaranth which have found a random utility script as the meta.mainProgram rather than what the package is about.

I don't object to this PR but I think it's worth being aware of.

@stuebinm
Copy link
Contributor Author

@tpwrules yes, that occurred to me as well — in the end, I didn't do anything about it, because I don't think that this is, as such, wrong? It might still enable nice things to have it there (be it for nix run or lib.getExe) and even if not, it shouldn't hurt to have that line there.

(wondering out loud for a bit: perhaps it would be useful to have a meta.isLibrary-kind of flag, to explicitly say "even if this does have a main program, that's not really the point of the package? But then, this would again mean a couple thousand lines added to nixpkgs, for unclear benefits)

@stuebinm stuebinm deleted the trivial-meta-mainprograms branch April 2, 2024 14:14
@stuebinm stuebinm mentioned this pull request Apr 13, 2024
13 tasks
@stuebinm stuebinm mentioned this pull request Apr 25, 2024
13 tasks
@thrix thrix mentioned this pull request Apr 30, 2024
trueNAHO added a commit to trueNAHO/dotfiles that referenced this pull request May 20, 2024
Lock package versions using absolute paths to increase package purity by
wrapping scripts in 'pkgs.writeShellApplication' with runtime
dependencies, or by passing the following values in ascending order of
availability to "lib.getExe" or "lib.getExe'":

1. config.<MODULE>.finalPackage
2. config.<MODULE>.package
3. pkgs.<PACKAGE>

The 'lib.getExe' function causes the following errors for the
'rofi-pass-unstable-2024-02-13' and 'xplr-0.21.5' packages:

    trace: warning: getExe: Package "<PACKAGE>" does not have the
    meta.mainProgram attribute. We'll assume that the main program has
    the same name for now, but this behavior is deprecated, because it
    leads to surprising errors when the assumption does not hold. If the
    package has a main program, please set `meta.mainProgram` in its
    definition to make this warning go away. Otherwise, if the package
    does not have a main program, or if you don't control its
    definition, use getExe' to specify the name to the program, such as
    lib.getExe' foo "bar".

In the future, updating the '/flake.lock' file to
NixOS/nixpkgs#297084 should resolve this issue.
@KiaraGrouwstra
Copy link
Contributor

wrote a little rust tool to edit package definitions

hm, this link seems dead?
i'm seeing some seemingly trivial cases like keepassxc and visidata not have the attribute set yet.

@stuebinm
Copy link
Contributor Author

hm, this link seems dead?

ah, i renamed the repository once it turned into a slightly more general tool; I've now updated the link to again point at the version which was used for this PR (if you want to use it, be aware it's very much single-purpose & brittle)

i'm seeing some seemingly trivial cases like keepassxc and visidata not have the attribute set yet.

neither of these are "trivial" for the purposes of this PR; they both provide more than one binary (an easy way to check this is to use search.nixos.org, which uses the same programs.sqlite file to provide the "programs provided" listing).

I had deliberately excluded such packages which provide a binary named identically to their pname attribute, since that program is not always the actual "main program" in any intuitive sense (a good counterexample being the coreutils package providing a coreutils binary), so a wholly automated update would have likely led to unexpected (& unintended) consequences.

@NixOS NixOS locked and limited conversation to collaborators Oct 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
6.topic: cinnamon Desktop environment 6.topic: emacs Text editor 6.topic: Enlightenment DE The Enlightenment Desktop Environment 6.topic: erlang 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: julia 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 6.topic: kernel The Linux kernel 6.topic: Lumina DE The Lumina Desktop Environment 6.topic: LXQt The Lightweight Qt Desktop Environment 6.topic: mate The MATE Desktop Environment 6.topic: nodejs 6.topic: ocaml 6.topic: pantheon The Pantheon desktop environment 6.topic: php 6.topic: python 6.topic: qt/kde 6.topic: rust 6.topic: vim 6.topic: xfce The Xfce Desktop Environment 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants