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.
@ttuegel ttuegel requested a review from mweinelt Nov 13, 2020
@ttuegel ttuegel requested review from FRidh and jonringer as code owners Nov 13, 2020
@ofborg ofborg bot added the 6.topic: python label Nov 13, 2020
@ofborg ofborg bot requested a review from womfoo Nov 13, 2020
@@ -27,6 +27,7 @@ buildPythonPackage rec {
pyyaml
xmltodict
argcomplete
jq

This comment has been minimized.

@jonringer

jonringer Nov 14, 2020
Contributor

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.

This comment has been minimized.

@ttuegel

ttuegel Nov 16, 2020
Author Member

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

@ttuegel ttuegel force-pushed the ttuegel:yq-needs-jq branch from bc21de9 to 3181baa Nov 16, 2020
@ttuegel ttuegel changed the title yq: add jq to propagatedBuildInputs yq: fix path to jq Nov 16, 2020
@ttuegel ttuegel force-pushed the ttuegel:yq-needs-jq branch from 3181baa to c580a82 Nov 16, 2020
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}"
'';
Comment on lines 25 to 35

This comment has been minimized.

@kalbasit

kalbasit Nov 16, 2020
Member

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 ttuegel:yq-needs-jq branch 4 times, most recently from f0826c4 to d26a776 Dec 25, 2020
yq requires jq at runtime, so that the package is broken unless the path to jq
is patched in.
@ttuegel ttuegel force-pushed the ttuegel:yq-needs-jq branch from d26a776 to 4074d4a Dec 26, 2020
@ttuegel ttuegel merged commit b5bf9da into NixOS:master Dec 27, 2020
19 checks passed
19 checks passed
tests
Details
action
Details
Evaluation Performance Report Evaluator Performance Report
Details
Wait for ofborg
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="4074d4a"; rev="4074d4ad98fb41093c492e5e57492ef65caf1d13"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="4074d4a"; rev="4074d4ad98fb41093c492e5e57492ef65caf1d13"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="4074d4a"; rev="4074d4ad98fb41093c492e5e57492ef65caf1d13"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="4074d4a"; rev="4074d4ad98fb41093c492e5e57492ef65caf1d13"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="4074d4a"; rev="4074d4ad98fb41093c492e5e57492ef65caf1d13"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="4074d4a"; rev="4074d4ad98fb41093c492e5e57492ef65caf1d13"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="4074d4a"; rev="4074d4ad98fb41093c492e5e57492ef65caf1d13"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
yq, yq.passthru.tests on aarch64-linux Success
Details
yq, yq.passthru.tests on x86_64-linux Success
Details
@ttuegel ttuegel deleted the ttuegel:yq-needs-jq branch Dec 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.