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

yq: fix path to jq #103737

Merged
merged 1 commit into from Dec 27, 2020
Merged

yq: fix path to jq #103737

merged 1 commit into from Dec 27, 2020

Conversation

ttuegel
Copy link
Member

@ttuegel ttuegel commented Nov 13, 2020

yq requires jq at runtime, so that the package is broken without patching in the path to jq.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@@ -27,6 +27,7 @@ buildPythonPackage rec {
pyyaml
xmltodict
argcomplete
jq
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the package is "shelling" out to it, this should be patched in the source to do so. Otherwise there's no guarantee that it will work when imported as a python module.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out! I patched the package; could you please take another look?

@ttuegel ttuegel changed the title yq: add jq to propagatedBuildInputs yq: fix path to jq Nov 16, 2020
Comment on lines 25 to 35
patches = [ ./jq-path.patch ];

postPatch = ''
substituteInPlace test/test.py --replace "expect_exit_codes={0} if sys.stdin.isatty() else {2}" "expect_exit_codes={0}"
substituteInPlace yq/__init__.py --subst-var-by JQ "${lib.getBin pkgs.jq}/bin/jq"
substituteInPlace test/test.py \
--subst-var-by JQ "${lib.getBin pkgs.jq}/bin/jq" \
--replace "expect_exit_codes={0} if sys.stdin.isatty() else {2}" "expect_exit_codes={0}"
'';
Copy link
Member

@kalbasit kalbasit Nov 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can actually replace the variables you have with substitute within the patches declaration so you don't really need the postPatch for the JQ substitution.

Example:

patches = [
(substituteAll {
src = ./mpv.patch;
inherit mpv;
})
];

@ttuegel ttuegel force-pushed the yq-needs-jq branch 4 times, most recently from f0826c4 to d26a776 Compare December 26, 2020 11:35
yq requires jq at runtime, so that the package is broken unless the path to jq
is patched in.
@ttuegel ttuegel merged commit b5bf9da into NixOS:master Dec 27, 2020
@ttuegel ttuegel deleted the yq-needs-jq branch December 27, 2020 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants