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

nix: pass makeWrapperArgs to wrapProgram #3003

Merged
merged 1 commit into from
Jul 9, 2022

Conversation

nrdxp
Copy link
Contributor

@nrdxp nrdxp commented Jul 7, 2022

Even though in the default package this list is empty, it is still useful to apply extra wrapper args downstream.

For example, one may wish to wrap Helix with the PATH set to include various language servers by default if not installed with the --suffix arg.

Full list of possible args:
https://github.com/NixOS/nixpkgs/blob/1c70b694fe0fece5ae74beb747a40f1b7237d251/pkgs/build-support/setup-hooks/make-wrapper.sh#L14-L35

One can see a real world example in this commit:
https://github.com/nrdxp/nrdos/commit/d8bf68e639343566d33545f189616bba57e0ed3f

Even though in the default package this list is empty, it is still
useful to apply extra wrapper args downstream. For example, one may
wish to wrap Helix with the `PATH` set to include various language
servers by default if not installed with the `--suffix` arg.

Full list of possible args:
https://github.com/NixOS/nixpkgs/blob/1c70b694fe0fece5ae74beb747a40f1b7237d251/pkgs/build-support/setup-hooks/make-wrapper.sh#L14-L35
@nrdxp
Copy link
Contributor Author

nrdxp commented Jul 7, 2022

looks like I can't request reviews so cc @yusdacra in case there is a better way to do this

@yusdacra
Copy link
Contributor

yusdacra commented Jul 7, 2022

Looks fine to me. One can just use overrideAttrs to override the package downstream so this will work fine if you pass makeWrapperArgs.

@nrdxp
Copy link
Contributor Author

nrdxp commented Jul 7, 2022

Originally I wanted to add an option to override so one could use helix.override { makeWrapperArgs = []; } but I realized there is no override function on Helix in the first place, so I had to go with the more verbose overrideAttrs route.

@yusdacra
Copy link
Contributor

yusdacra commented Jul 7, 2022

Originally I wanted to add an option to override so one could use helix.override { makeWrapperArgs = []; } but I realized there is no override function on Helix in the first place, so I had to go with the more verbose overrideAttrs route.

Yeah, the Rust builders on dream2nix do not expose an override, which is my fault I believe 😅 I didn't really expect that anyone would want to use override since there is really nothing special being passed to the derivation for any "configuration" of sorts. Mostly I expected people would just write their own wrappers around the packages since modifying the package in this way will trigger rebuilds, which is not something that would be wanted I think...

@the-mikedavis the-mikedavis merged commit 718c3ba into helix-editor:master Jul 9, 2022
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.

3 participants