search

hercules-ci / flake-parts

❄️ Simplify Nix Flakes with the module system
https://flake.parts
MIT License
699 stars 38 forks source link

apps: use lib.getExe to find executable #180

Closed arcuru closed 6 months ago

arcuru commented 1 year ago

Use nixpkgs version of getExe to determine the executable name.

This brings it inline with the behavior of the rest of the ecosystem, which allows a fallback to the pname/name.

I tested this with a local repo to confirm.

roberth commented 1 year ago

It's convenient but can lead to bad error messages; at least as currently implemented. I've started a discussion at https://github.com/NixOS/nixpkgs/pull/167247/files#r1279561455 and at least the author is positive about it.

roberth commented 11 months ago

Made some progress upstream to make its getExe safer too; it now warns when meta.mainProgram is unset.

This brings it inline with the behavior of the rest of the ecosystem, which allows a fallback to the pname/name.

There's a subtle difference in context. I'll refer to my reply on the discourse thread.

I now realize that I didn't inform you about the discussion upstream. That was not intentional. I hope you're ok with the changes, even though they don't have your intended effect in the short term.

Thank you for bringing this up!

arcuru commented 11 months ago

No worries, I did notice the upstream discussion. I was under the impression that meta.mainProgram wasn't set for most of nixpkgs, so I'm happy to see that it was fairly easy to add it to the few missing places.

I'm not sure what your plan is here, but I think the best option would probably be to match your new version in nixpkgs, so we'd warn but still allow the fallback at least for now. Whether that's done by using this current change or by rewriting that new impl here for backwards compat with older nixpkgs would be up to you.

Feel free to take over this and do whatever. I don't have major objections to any of the options (including just closing this PR) now that it's clear meta.mainProgram is the agreed upon standard/required way of declaring the name.

roberth commented 6 months ago

Thanks! I think Nixpkgs getExe has proven itself by now, so flake-parts now follows it, with the old implementation as a polyfill just in case.