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

jq: 1.6 -> 1.7 #246515

Merged
merged 4 commits into from
Sep 16, 2023
Merged

jq: 1.6 -> 1.7 #246515

merged 4 commits into from
Sep 16, 2023

Conversation

ncfavier
Copy link
Member

@ncfavier ncfavier commented Aug 1, 2023

After five years, jq is getting a new release! https://github.com/jqlang/jq/releases/tag/jq-1.7

Not sure if this deserves a release note? The breaking changes seem relatively limited, but jq is kind of a core package.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ncfavier
Copy link
Member Author

jq 1.7rc2 introduces a cycle between outputs and I'm not sure how to resolve it. dev depends on bin because of the default propagatedBuildOutputs value, and bin now depends on dev because of jqlang/jq@583e4a2. We could:

  • Break the first link by not including bin in the propagated build outputs (I think this could break builds?)
  • Break the second link using removeReferencesTo
  • Merge bin and dev (how?)

@Artturin
Copy link
Member

jq 1.7rc2 introduces a cycle between outputs and I'm not sure how to resolve it. dev depends on bin because of the default propagatedBuildOutputs value, and bin now depends on dev because of jqlang/jq@583e4a2. We could:

* Break the first link by not including `bin` in the propagated build outputs (I think this could break builds?)

Would break many builds because dev is the default output chosen.

* Break the second link using `removeReferencesTo`

This would be good.

* Merge `bin` and `dev` (how?)

By removing dev from outputs.

@ncfavier
Copy link
Member Author

This would be good.

Done that.

@ncfavier ncfavier changed the title jq: 1.6 -> 1.7rc1 jq: 1.6 -> 1.7rc2 Aug 30, 2023
@ncfavier ncfavier marked this pull request as ready for review September 7, 2023 07:48
@ncfavier ncfavier changed the title jq: 1.6 -> 1.7rc2 jq: 1.6 -> 1.7 Sep 7, 2023
@miallo
Copy link
Contributor

miallo commented Sep 8, 2023

Nice that you are already onto this! 🎉

Unfortunately when I apply this patch I am running into a test failure. For anyone else also running into this, this seems to be tracked here: jqlang/jq#2885

@ncfavier
Copy link
Member Author

ncfavier commented Sep 8, 2023

Tests are passing for me (and for ofborg) on x86_64-linux, aarch64-linux and aarch64-darwin. Are you on x86_64-darwin?

What revision did you apply this patch to?

@ncfavier
Copy link
Member Author

ncfavier commented Sep 8, 2023

As far as I can tell this is a locale problem, so Nix builds should not be affected. Are you building with the sandbox enabled?

@miallo
Copy link
Contributor

miallo commented Sep 8, 2023

Tests are passing for me (and for ofborg) on x86_64-linux, aarch64-linux and aarch64-darwin. Are you on x86_64-darwin?

Interesting... No x86_64-linux (i.e. NixOS)

What revision did you apply this patch to?

Current nixos-unstable (0bffda1). Only other two commits in my fork are unrelated (they add two options to nixos-rebuild).

As far as I can tell this is a locale problem, so Nix builds should not be affected. Are you building with the sandbox enabled?

Nope, just a normal nixos-rebuild switch... 🤔

@ncfavier
Copy link
Member Author

ncfavier commented Sep 8, 2023

Nope, just a normal nixos-rebuild switch... 🤔

Well then the sandbox should be enabled by default. Please ensure that your /etc/nix/nix.conf contains sandbox = true.

@ncfavier
Copy link
Member Author

ncfavier commented Sep 8, 2023

Can you paste the .drv file that fails to build somewhere? (name and contents)

@miallo
Copy link
Contributor

miallo commented Sep 8, 2023

Please ensure that your /etc/nix/nix.conf contains sandbox = true.

It does

Can you paste the .drv file that fails to build somewhere? (name and contents)

Oh... Damn... I only saw the error with jq, but in fact it is from python3.10-yq:

/nix/store/y0nkkmhxhc3fkhc3q0wwqjm0ks8lqh08-python3.10-yq-3.2.2.drv Derive([("dist","/nix/store/4g45xi9866myvha3vxq8yahm29gxpsga-python3.10-yq-3.2.2-dist","",""),("out","/nix/store/64lyjyh5q39akgwcd71yj53yaw4i15k4-python3.10-yq-3.2.2","","")],[("/nix/store/02kq0w1zn1v9qzvanz1gvsds6vf1kkh6-python3.10-argcomplete-3.1.1.drv",["out"]),("/nix/store/0sawbbskl41aq4ii9kl5w77r4m92das8-bash-5.2-p15.drv",["out"]),("/nix/store/1ybxfm6v8ijnr72wm1fx4d3ry59256sr-pypa-install-hook.drv",["out"]),("/nix/store/4lky73gnpd3k90hlhlybik83hv5b12a4-python-remove-tests-dir-hook.drv",["out"]),("/nix/store/599xrhrkyx743jz8lsqjmhhjynswr87f-python3.10-xmltodict-0.13.0.drv",["out"]),("/nix/store/7miy4j2xy6p97f34savhl0jsgy5syf0b-python-catch-conflicts-hook.drv",["out"]),("/nix/store/9663sml6h17kv61wg8am9dmpvfccwrq3-setuptools-check-hook.drv",["out"]),("/nix/store/9fwlz0myh3lwwmj3rcic4ssv7ygrig6x-stdenv-linux.drv",["out"]),("/nix/store/9r57db02g9pvj2fl914m5y9xjma26zr5-pytest-check-hook.drv",["out"]),("/nix/store/9zjcxviqkl4mr6wsvs9554w5ywnr3fas-python3.10-setuptools-scm-7.1.0.drv",["out"]),("/nix/store/bcchpqhqph735wvgkvnf3bc6mzfwjj8g-python3.10-tomlkit-0.12.1.drv",["out"]),("/nix/store/hw5ygglyq53qzx75gdv01wh8qnmd66hq-wrap-python-hook.drv",["out"]),("/nix/store/l0kin7jf2qm401vzqrlrd2cb7zz1x8ah-jq-path.patch.drv",["out"]),("/nix/store/mpaa4mcp8zvhxc5mg10cwmnmhqkzvlak-python-imports-check-hook.sh.drv",["out"]),("/nix/store/mv79m4vxlqrjsasnwf2j9gaa256vyi6h-python3-3.10.12.drv",["out"]),("/nix/store/mx50p8hbrwg9xiimaanmyg7yyq6szdpf-python3.10-pyyaml-6.0.1.drv",["out"]),("/nix/store/n66c5lb52caw80dcyacp2c61bz88vk46-python-remove-bin-bytecode-hook.drv",["out"]),("/nix/store/pqmn7mhg2fz0dz58a7h1sxbd4vvgznv1-ensure-newer-sources-hook.drv",["out"]),("/nix/store/s894x9568fhi765589235am0yjnw2m00-yq-3.2.2.tar.gz.drv",["out"]),("/nix/store/vbhxxiq5ylm82qdpds92ybjpv7hbmc9c-python-namespaces-hook.sh.drv",["out"]),("/nix/store/yav55g1lzz5smcm6lfvjkiczaacx80gz-setuptools-setup-hook.drv",["out"]),("/nix/store/ysl7znxpxa7ibnxpiflgn8mbvls8zx0d-python-output-dist-hook.drv",["out"])],["/nix/store/6xg259477c90a229xwmb53pdfkn6ig3g-default-builder.sh"],"x86_64-linux","/nix/store/m36d29gn5gm9bk0g7fcln1v8171hvn95-bash-5.2-p15/bin/bash",["-e","/nix/store/6xg259477c90a229xwmb53pdfkn6ig3g-default-builder.sh"],[("LANG","C.UTF-8"),("__structuredAttrs",""),("buildInputs",""),("builder","/nix/store/m36d29gn5gm9bk0g7fcln1v8171hvn95-bash-5.2-p15/bin/bash"),("cmakeFlags",""),("configureFlags",""),("depsBuildBuild",""),("depsBuildBuildPropagated",""),("depsBuildTarget",""),("depsBuildTargetPropagated",""),("depsHostHost",""),("depsHostHostPropagated",""),("depsTargetTarget",""),("depsTargetTargetPropagated",""),("disallowedReferences",""),("dist","/nix/store/4g45xi9866myvha3vxq8yahm29gxpsga-python3.10-yq-3.2.2-dist"),("doCheck",""),("doInstallCheck","1"),("mesonFlags",""),("name","python3.10-yq-3.2.2"),("nativeBuildInputs","/nix/store/vagb0sjv83ybi435i6iiv10hjrdghph9-python3-3.10.12 /nix/store/cwcdrvv94c4rys2cgxhs2byn6a4rczgr-wrap-python-hook /nix/store/2ay7q7ligvy0zkhb22z9hb6ksk68rgcf-ensure-newer-sources-hook /nix/store/4bm7si6b8hjfr3vncl1b3phckawdbhah-python-remove-tests-dir-hook /nix/store/3l5vkplnmwxdmccajy7knsx6hrzj9jf7-python-catch-conflicts-hook /nix/store/ihc2jyzhx3crnlws34qhhv4d73zp6zy6-python-remove-bin-bytecode-hook /nix/store/b9284xij30zf4zfmqvvamqag3gzfqrmh-setuptools-setup-hook /nix/store/414d6mi9bw0d3p48sy5z3468n445kzz8-pypa-install-hook /nix/store/l2lrx1kx0phn947rmylddrday6l6z1ig-python-imports-check-hook.sh /nix/store/4x04hyzyz8pfh2dqdvx7j5svp315j66k-python-namespaces-hook.sh /nix/store/9gcs29995dq04c8r8sx46nxczz083vw1-python-output-dist-hook /nix/store/6bx9kdapg02a19mkrhdyhdl4s0rnajyv-python3.10-setuptools-scm-7.1.0 /nix/store/47y9smr6b6g7542p1iahj1qzv4ncdr7a-setuptools-check-hook /nix/store/rkapa2ajfn7bphiy98bjyrgnfgvb2isa-pytest-check-hook"),("out","/nix/store/64lyjyh5q39akgwcd71yj53yaw4i15k4-python3.10-yq-3.2.2"),("outputs","out dist"),("patches","/nix/store/cvwrvbb09jqqz5mv2f4jl5rzzj58ycfv-jq-path.patch"),("pname","yq"),("postFixup","wrapPythonPrograms\n"),("propagatedBuildInputs","/nix/store/pngxdvmmc32m3hsy8psnxbk2rfrkdqyx-python3.10-argcomplete-3.1.1 /nix/store/31zpzgn2hr04mmg7dqp8gkf5n99i29ab-python3.10-pyyaml-6.0.1 /nix/store/r6ii1v9v4wrr6m29dksl5raqjn29q95n-python3.10-tomlkit-0.12.1 /nix/store/mdbiflqwg46s7yn46cww05zcqp52709z-python3.10-xmltodict-0.13.0 /nix/store/vagb0sjv83ybi435i6iiv10hjrdghph9-python3-3.10.12"),("propagatedNativeBuildInputs",""),("pytestFlagsArray","test/test.py"),("pythonImportsCheck","yq"),("src","/nix/store/7a84byjhj33rrc5yjd4mkz1yqiwkc6pn-yq-3.2.2.tar.gz"),("stdenv","/nix/store/7cni7ndy2pm18ysl5znq6znb30sxp156-stdenv-linux"),("strictDeps","1"),("system","x86_64-linux"),("version","3.2.2")])

@ncfavier
Copy link
Member Author

ncfavier commented Sep 8, 2023

Is jq failing to build or not? If so, please paste that .drv file. This PR is not about yq.

@miallo
Copy link
Contributor

miallo commented Sep 8, 2023

Sorry for the confusion - the jq–build is indeed working, but pkgs.yq depends on jq and with this PR yq fails the tests.

AFAICT the issue is indeed because of a change of behaviour of jq of something in the yq test and I raised an issue there kislyuk/yq#172

@miallo
Copy link
Contributor

miallo commented Sep 8, 2023

The reason why I directly jumped to the conclusion that it had to do with the mentioned jq issue is that that log contained the lines:

==57649== Command: /usr/ports/textproc/jq/work/jq-jq-1.7/jq -b -n --jsonargs null invalid
==57649== 
/usr/ports/textproc/jq/work/jq-jq-1.7/jq: invalid JSON text passed to --jsonargs
Use /usr/ports/textproc/jq/work/jq-jq-1.7/jq --help for help with command-line options,
or see the jq manpage, or online docs  at https://jqlang.github.io/jq

which was pretty much what I saw in the error message so I felt confirmed. I did not question my beliefs. Sorry for that!

What is the procedure for such a dependency failure? This PR in itself is great, but I don't know how to deal with it breaking other packages...

@ncfavier
Copy link
Member Author

ncfavier commented Sep 8, 2023

We can wait for known problems to be resolved upstream and bump the reverse dependencies simultaneously.

python3Packages.jq also seems to have failing tests.

@miallo
Copy link
Contributor

miallo commented Sep 9, 2023

python3Packages.jq also seems to have failing tests.

Good catch! I created an issue over there as well: mwilliamson/jq.py#93

@ncfavier
Copy link
Member Author

Successfully built nix.

@ofborg ofborg bot requested review from benley and SuperSandro2000 September 10, 2023 11:22
@miallo miallo mentioned this pull request Sep 10, 2023
12 tasks
@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 10, 2023
@delroth delroth added 12.approvals: 2 This PR was reviewed and approved by two reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package and removed 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Sep 11, 2023
@Artturin
Copy link
Member

Artturin commented Sep 11, 2023

Added a release note

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Sep 11, 2023
@delroth delroth removed the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Sep 11, 2023
@ofborg ofborg bot requested a review from benley September 11, 2023 18:48
@delroth delroth added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Sep 11, 2023
@delroth delroth added 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people and removed 12.approvals: 2 This PR was reviewed and approved by two reputable people labels Sep 12, 2023
@benley
Copy link
Member

benley commented Sep 16, 2023

Aside from the pending comment about the release notes, are there any remaining objections to merging this change?

@ncfavier ncfavier merged commit 57b9e41 into NixOS:staging Sep 16, 2023
@ncfavier ncfavier deleted the jq branch September 16, 2023 17:15
@Artturin Artturin mentioned this pull request Sep 16, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 8.has: changelog 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 501+ 10.rebuild-linux: 2501-5000 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants