Skip to content

explicitly set meta.mainProgram#9162

Merged
edolstra merged 1 commit intoNixOS:masterfrom
eclairevoyant:add-mainprogram
Oct 17, 2023
Merged

explicitly set meta.mainProgram#9162
edolstra merged 1 commit intoNixOS:masterfrom
eclairevoyant:add-mainprogram

Conversation

@eclairevoyant
Copy link
Contributor

@eclairevoyant eclairevoyant commented Oct 15, 2023

Motivation

Setting meta.mainProgram properly allows using nix run correctly.

Context

NixOS/nixpkgs#246386 and nix run uses the same field.

Priorities

Add 👍 to pull requests you find important.

@edolstra
Copy link
Member

This already works for me:

$ nix run nix -- --version
nix (Nix) 2.19.0pre20231013_d070d8b

@eclairevoyant
Copy link
Contributor Author

eclairevoyant commented Oct 16, 2023

  • It's better to be explicit (as the linked PR mentions) rather than make assumptions
  • This ensures the correct program is run even if pname/name is overridden for whatever reason
  • There is no downside to adding this, only upside?

@NobbZ
Copy link
Contributor

NobbZ commented Oct 16, 2023

This already works for me:

$ nix run nix -- --version
nix (Nix) 2.19.0pre20231013_d070d8b

This change is more important for using lib.getExe from nixpkgs than it is for nix run.

@edolstra
Copy link
Member

It's better to be explicit (as the linked PR mentions) rather than make assumptions

Not sure I agree with that. Convention over configuration and all that. Requiring thousands of packages to set mainProgram seems un-ergonomic.

@eclairevoyant
Copy link
Contributor Author

Convention over configuration

I have never heard such a phrase, where does it come from?
Anyway, we are not talking about thousands of programs in this PR, only 1.

I do not see a reason to not-add it, and I see many reasons to add it, so I do not understand the strong and unnecessary pushback here.

@eclairevoyant
Copy link
Contributor Author

eclairevoyant commented Oct 16, 2023

If you want to know about the original nixpkgs PR (from 3 months ago) and why the warning was added, perhaps @roberth would be a better representative to explain the full context behind the PR than myself.

@Gerg-L
Copy link
Contributor

Gerg-L commented Oct 16, 2023

If you want to know about the PR, perhaps @roberth would be a better representative to explain the full context behind the PR than myself.

The PR:
NixOS/nixpkgs#246386

@edolstra
Copy link
Member

I have never heard such a phrase, where does it come from?

https://en.wikipedia.org/wiki/Convention_over_configuration

Anyway, I'll merge this since it is indeed just a one-liner.

@edolstra edolstra merged commit a9b8595 into NixOS:master Oct 17, 2023
@eclairevoyant eclairevoyant deleted the add-mainprogram branch October 17, 2023 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants