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

Fix the problem of forgotten runHook {pre,post}* calls #175605

Open
roberth opened this issue May 31, 2022 · 6 comments
Open

Fix the problem of forgotten runHook {pre,post}* calls #175605

roberth opened this issue May 31, 2022 · 6 comments

Comments

@roberth
Copy link
Member

roberth commented May 31, 2022

Project description

We advise to do

fooPhase = ''
  runHook preFoo

  put your commands here

  runHook postFoo

why not:

fooCommands = ''
  put your commands here
''

and have stdenv do the right thing?

This will save a lot of nagging and presumably a bunch of frustration.

These identifiers have not yet been used in Nixpkgs.

The only similarity is with buildCommand, singular, which is the variable backing the runCommand function. Its purpose is to avoid the phases and simply do a thing.

Metadata

  • homepage URL:
  • source URL:
  • license: mit, bsd, gpl2+ , ...
  • platforms: unix, linux, darwin, ...
@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/low-effort-and-high-impact-tasks/21095/7

@ncfavier
Copy link
Member

ncfavier commented Jan 12, 2023

Instead of introducing new attributes, could we make runHook X idempotent and always run pre/post hooks before and after phases, even if they're overriden?

I fear that having buildCommands will just lead to a different type of nagging ("remember to use buildCommands" instead of "remember to use runHooks").

@roberth
Copy link
Member Author

roberth commented Jan 25, 2023

So I guess the question is whether we want to break people's code or do a treewide? I'd favor the latter. We could deprecate the setting of *Phase in the long run.

@Artturin
Copy link
Member

i stumbled upon a rfc related to this NixOS/rfcs#32

@roberth
Copy link
Member Author

roberth commented Jan 25, 2023

It doesn't seem to have been implemented; asking.

@ncfavier
Copy link
Member

break people's code or do a treewide

I don't think my suggestion requires either? Unless you mean that running hooks that were previously not run could break things, but hooks are empty by default so this seems dubious

@roberth roberth mentioned this issue Jul 6, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants