woeusb: add p7zip to runtime deps enable extra feature #47982
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WoeUSB depends on presence of '7z` binary in the path to execute an extra step. As Windows 7's installation media doesn't place the required EFI bootloaders in the right location, WoeUSB extracts them from the system image manually using '7z' binary which it checks with 'command -v 7z'. See related code at: https://github.com/slacka/WoeUSB/blob/aea4f91783d60fe092605cf26b996d1ba593f976/src/woeusb#L1530
timokau
reviewed
Oct 6, 2018
| @@ -36,7 +36,7 @@ stdenv.mkDerivation rec { | |||
| # should be patched with a less useless default PATH, but for now | |||
| # we add everything we need manually. | |||
| wrapProgram "$out/bin/woeusb" \ | |||
| --set PATH '${stdenv.lib.makeBinPath [ coreutils dosfstools findutils gawk gnugrep grub2_light ncurses ntfs3g parted utillinux wget ]}' | |||
| --set PATH '${stdenv.lib.makeBinPath [ coreutils dosfstools findutils gawk gnugrep grub2_light ncurses ntfs3g parted utillinux wget p7zip ]}' | |||
There was a problem hiding this comment.
I would prefer it if you make it clear in the expression that p7zip is an optional dependency. I don't mind it being enabled by default.
That would mean an withWin7EfiSupport ? true or something like that argument and using ++ lib.optionals withWin7EfiSupport [ p7zip ] together with a comment explaining what you did in the PR message (what 7z is needed for) here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WoeUSB depends on presence of '7z` binary in the path to execute an extra step.
As Windows 7's installation media doesn't place the required EFI bootloaders
in the right location, WoeUSB extracts them from the system image manually
using '7z' binary which it checks with 'command -v 7z'.
See related code at:
https://github.com/slacka/WoeUSB/blob/aea4f91783d60fe092605cf26b996d1ba593f976/src/woeusb#L1530
Motivation for this change
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)