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 -> 2022-05-26 #217345

Closed
wants to merge 2 commits into from
Closed

jq: 1.6 -> 2022-05-26 #217345

wants to merge 2 commits into from

Conversation

edef1c
Copy link
Member

@edef1c edef1c commented Feb 20, 2023

Description of changes

jq has not seen a release in four years, despite a bug that silently mangles large numbers being fixed several years ago. Let's just use git HEAD instead.

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.05 Release Notes (or backporting 22.11 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.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 20, 2023
@lf-
Copy link
Member

lf- commented Feb 20, 2023

merge conflict fyi

jq has not seen a release in four years, despite a bug that silently
mangles large numbers being fixed several years ago. Let's just use
git HEAD instead.
@edef1c edef1c removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 20, 2023
Copy link
Member

@lf- lf- left a comment

Choose a reason for hiding this comment

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

Nice. fetchTarball is a good solution to avoiding depending on GitHub archive hashes

src = fetchurl {
url = "https://github.com/stedolan/jq/releases/download/jq-${version}/jq-${version}.tar.gz";
sha256 = "sha256-XejI4pqqP7nMa0e7JymfJxNU67clFOOsytx9OLW7qnI=";
src = builtins.fetchTarball {
Copy link
Member

Choose a reason for hiding this comment

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

We can't use builtins.fetchTarball because fetching then happens at eval time. This should work fine if it switches back to fetchurl.

Copy link
Member

Choose a reason for hiding this comment

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

The trouble is that the hash is then of the archive itself which is not guaranteed stable. that said, bazel seems to have Hyrum's law'd it onto github's priority list to keep them stable even though it's not guaranteed.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, sorry -- I meant fetchzip, which will unpack the resulting tarball and gives me the same hash:

$ nix repl '<nixpkgs>'
# [...snip...]
nix-repl> :b pkgs.fetchzip { url = "https://github.com/stedolan/jq/archive/cff5336ec71b6fee396a95bb0e4bea365e0cd1e8.tar.gz"; sha256 = ""; }
warning: found empty hash, assuming 'sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA='
error: hash mismatch in fixed-output derivation '/nix/store/hhilyllp37xpf5shzmnjf1bdkr3fis3d-source.drv':
         specified: sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
            got:    sha256-gsngye7EAmdSqH7ZH1g8/sQFj1nJWEGHpt48T8qXjZ0=

nix-repl> builtins.fetchTarball { url = "https://github.com/stedolan/jq/archive/cff5336ec71b6fee396a95bb0e4bea365e0cd1e8.tar.gz"; sha256 = ""; }
warning: found empty hash, assuming 'sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA='
error:
       … while calling the 'fetchTarball' builtin

         at «string»:1:1:

            1| builtins.fetchTarball { url = "https://github.com/stedolan/jq/archive/cff5336ec71b6fee396a95bb0e4bea365e0cd1e8.tar.gz"; sha256 = ""; }
             | ^

       error: hash mismatch in file downloaded from 'https://github.com/stedolan/jq/archive/cff5336ec71b6fee396a95bb0e4bea365e0cd1e8.tar.gz':
         specified: sha256:0000000000000000000000000000000000000000000000000000
         got:       sha256:17cdjz54yg6yls3l2n69b67hbi7y7ic1znbym196f0n4xv4y1jc2

nix-repl> 
$ nix hash to-sri 'sha256:17cdjz54yg6yls3l2n69b67hbi7y7ic1znbym196f0n4xv4y1jc2'
sha256-gsngye7EAmdSqH7ZH1g8/sQFj1nJWEGHpt48T8qXjZ0=

builtins.fetchTarball is not allowed in Nixpkgs (and is why ofborg fails): it moves the source fetching from build time to eval time, which can easily balloon said eval time.

Copy link
Member

Choose a reason for hiding this comment

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

Right. fetchFromGitHub is a thin wrapper around fetchzip which i imagine isn't going to be acceptable per the comment above, however i may be wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, indeed, you are right. I missed that comment.

Choose a reason for hiding this comment

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

How about manually doing what fetchzip does? It avoids depending on unzip but should avoid the issue um unstable archive hashes.

Suggested change
src = builtins.fetchTarball {
src = fetchurl {
url = "https://github.com/stedolan/jq/archive/cff5336ec71b6fee396a95bb0e4bea365e0cd1e8.tar.gz";
sha256 = "sha256-gsngye7EAmdSqH7ZH1g8/sQFj1nJWEGHpt48T8qXjZ0=";
recursiveHash = true;
downloadToTemp = true;
postFetch = ''
unpackDir="$TMPDIR/unpack"
mkdir "$unpackDir"
cd "$unpackDir"
renamed="$TMPDIR/jq.tar.gz"
mv "$downloadedFile" "$renamed"
tar -xf "$renamed"
chmod -R +w "$unpackDir"
fn=$(cd "$unpackDir" && ls -A)
mv "$unpackDir/$fn" "$out"
chmod 755 "$out"
'';
};

Copy link
Member

Choose a reason for hiding this comment

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

I'm not quite following, what's the issue with just using fetchFromGitHub?

Copy link
Member

Choose a reason for hiding this comment

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

If you change it to fetchFromGitHub (same with fetchzip), you get this when trying to access jq from stdenv.__bootPackages:

$ nix-instantiate -A stdenv.__bootPackages.jq
error:
       … while calling the 'derivationStrict' builtin

         at <nix/derivation-internal.nix>:9:12:

            8|
            9|   strict = derivationStrict drvAttrs;
             |            ^
           10|

       … while evaluating derivation 'jq-1.6'
         whose name attribute is located at /home/vin/workspace/vcs/nixpkgs/master/pkgs/stdenv/generic/make-derivation.nix:293:7

       … while evaluating attribute 'src' of derivation 'jq-1.6'

         at /home/vin/workspace/vcs/nixpkgs/master/pkgs/development/tools/jq/default.nix:15:3:

           14|   # Note: do not use fetchpatch or fetchFromGitHub to keep this package available in __bootPackages
           15|   src = fetchFromGitHub {
             |   ^
           16|     # url = "https://github.com/stedolan/jq/releases/download/jq-${version}/jq-${version}.tar.gz";

       (stack trace truncated; use '--show-trace' to show the full trace)

       error: function 'anonymous lambda' called with unexpected argument 'nativeBuildInputs'

       at /home/vin/workspace/vcs/nixpkgs/master/pkgs/build-support/fetchurl/boot.nix:5:1:

            4|
            5| { url ? builtins.head urls
             | ^
            6| , urls ? []

Copy link
Member

Choose a reason for hiding this comment

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

Ah, so jq is used during bootstrapping? That's the piece of information I was missing.

@@ -8,12 +8,12 @@

stdenv.mkDerivation rec {
pname = "jq";
version = "1.6";
version = "2022-05-26";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
version = "2022-05-26";
version = "unstable-2022-05-26";

Since this isn't a release, we should prefix with unstable- to prevent version sorting from thinking the date is an actual version.

reckenrode added a commit to reckenrode/nixpkgs that referenced this pull request Jun 30, 2023
The configure script for jq 1.6 does not define certain symbols required
on Darwin to make `drem`, `significand`, and `lgamma_r` available. This
works fine on clang 11 but fails with clang 16.

This is fixed upstream. It can be removed once NixOS#217345 is merged or jq
is released and updated to 1.7.
@lheckemann
Copy link
Member

Looking at jqlang/jq#2305, https://github.com/jqlang/jq might be sensible to use as the "new upstream". It seems to be run by the (most recently) active maintainers of stedolan/jq.

@Artturin
Copy link
Member

Artturin commented Aug 3, 2023

#246515

@alyssais alyssais closed this Sep 22, 2023
@edef1c edef1c deleted the jq branch June 27, 2024 02:31
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.

8 participants