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

lib/meta: add getExe to get the main program of a drv #167247

Merged
merged 1 commit into from Apr 26, 2022

Conversation

Artturin
Copy link
Member

@Artturin Artturin commented Apr 4, 2022

Description of changes

add a way to get the main program of a derivation

Closes #137032
Alternative to #158461
based on a comment by @edolstra

It seems better to me to have a function that returns the main program for a package, rather than pollute the attribute namespace of every package with a magic attribute. I.e. I'd prefer getMainProgram pkgs.hello to pkgs.hello.exe.

hopefully this at least unstalls #158461

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@roberth
Copy link
Member

roberth commented Apr 5, 2022

Duplicate of #138418

@roberth roberth marked this as a duplicate of #138418 Apr 5, 2022
@roberth roberth mentioned this pull request Apr 5, 2022
6 tasks
@Artturin Artturin changed the title lib/meta: add getMainProgram to get the main program of a drv lib/meta: add exe to get the main program of a drv Apr 23, 2022
@Artturin
Copy link
Member Author

Artturin commented Apr 23, 2022

alright there's 3 choices now
pick your favourite :)

A fourth option would be getExe

based on the comment by @roberth #138418 (comment)

@Artturin Artturin marked this as not a duplicate of #138418 Apr 23, 2022
@Artturin Artturin changed the title lib/meta: add exe to get the main program of a drv lib/meta: add getExe to get the main program of a drv Apr 24, 2022
@Artturin Artturin merged commit c95f5e1 into NixOS:master Apr 26, 2022
@Artturin Artturin deleted the addgetmainprog branch April 26, 2022 18:42
@Artturin Artturin mentioned this pull request Aug 2, 2022
10 tasks
=> "/nix/store/am9ml4f4ywvivxnkiaqwr0hyxka1xjsf-mustache-go-1.3.0/bin/mustache"
*/
getExe = x:
"${lib.getBin x}/bin/${x.meta.mainProgram or (lib.getName x)}";
Copy link
Member

Choose a reason for hiding this comment

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

Having second thoughts about the lib.getName x part.

  • A package should make its main program explicit. Explicit is good.
  • When /bin/${name} does not exist, the error is delayed until runtime, and most likely the caller won't know why /bin/foo, which doesn't exist, was ever accessed. Especially if they didn't make the call to getExe themselves; for example when a NixOS option does it for them.

Should we make meta.mainProgram mandatory for getExe? I think we should!

Copy link
Member

Choose a reason for hiding this comment

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

Or at least we could warn about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It'll have to be a warning for at least a release as this is already used widely outside of nixpkgs

Copy link
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a way to get the main program of a derivation.
2 participants