jq: build from Git tag (instead of dist tarball) #71375

bb010g · 2019-10-19T01:38:02Z

This involves vendoring the Bundler specification for the docs's Rake invocation for now, but next release will mean a switch to Python (patching ~the same files). (I've already done this on Python for docs building, and it's kind of easier. Less vendoring for sure.)

We also now check with Valgrind by default, as per upstream. This takes longer. (We can turn this off by default if necessary to restore old installCheck times.)

Motivation for this change

I wanted to try out an unstable version of jq, and needed to make most of these changes anyways.

As per #31236, building from source (here, Git) should be encouraged over building from release or distribution tarballs. This makes it easier to patch the derivation to use alternative VCS branches, among other benefits.

Valgrind is on by default in the Makefile, but we haven't ran the tests with it because they automatically turned it off when dependency discovery failed.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
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 nix-review --run "nix-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)

out bin lib man doc dev

before 96 29_528_224 29_492_112 32_064 14_568 29_541_768

after 96 (+0) 29_528_224 (±0) 29_492_112 (±0) 32_784 (+720) 14_568 (±0) 29_541_768 (±0)

(Building with { enableValgrindChecks = false; } does not change the closure size.)
Ensured that relevant documentation is up to date
Fits CONTRIBUTING.md.

Notify maintainers

cc @7c6f434c @globin

This change is

bb010g · 2019-10-19T02:07:55Z

This file contains what the Python transition will likely look like, the meat being:

https://github.com/bb010g/nur-packages/blob/d212b8f1bf018754b515efd979dca9e5df6e8e16/pkgs/development/tools/jq/default.nix#L19-L25

let
  # from ./docs/Pipfile{,.lock}
  docsPythonPackages = ps: [
    ps.jinja2
    ps.lxml
    ps.markdown
    ps.pyyaml_3 or ps.pyyaml
  ];
in

(I'm not sure how to do this like with Ruby and respect the Pipenv.lock.)

pkgs/development/tools/jq/default.nix

Mic92 · 2019-10-19T06:26:20Z

This file contains what the Python transition will likely look like, the meat being:

bb010g/nur-packages:pkgs/development/tools/jq/default.nix@d212b8f#L19-L25
let
  # from ./docs/Pipfile{,.lock}
  docsPythonPackages = ps: [
    ps.jinja2
    ps.lxml
    ps.markdown
    ps.pyyaml_3 or ps.pyyaml
  ];
in
(I'm not sure how to do this like with Ruby and respect the Pipenv.lock.)

That looks simple enough. Let's not mess with pipenv. I remember having headaches from the last time I used it.

pkgs/development/tools/jq/nix-version.patch

risicle · 2019-10-20T16:10:48Z

jq fails to build for me on macos 10.13:

FAIL: tests/optionaltest
FAIL: tests/mantest
FAIL: tests/jqtest
FAIL: tests/onigtest
FAIL: tests/shtest
PASS: tests/utf8test
FAIL: tests/base64test
============================================================================
Testsuite summary for jq 1.6
============================================================================
# TOTAL: 7
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  6
# XPASS: 0
# ERROR: 0
============================================================================

bb010g · 2019-10-20T23:48:04Z

@risicle Could you build with {-K | --keep-failed} and paste the relevant test log output? (If it's long, GitHub lets you surround it in a <details> block (with empty one-line buffers between the HTML opening & closing tags and the contained Markdown fenced code blocks for the parser).) I'm guessing Valgrind isn't happy, and we never ran into this before?

veprbl · 2019-12-08T19:27:15Z

On darwin I get all seven tests fail with

valgrind: mmap-FIXED(0x7fff5f400000, 8388608) failed in UME (load_unixthread1) with error 22 (Invalid argument).

stale · 2020-06-05T20:19:20Z

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

Search for maintainers and people that previously touched the related code and @ mention them in a comment.
Ask on the NixOS Discourse.
Ask on the #nixos channel on irc.freenode.net.

SuperSandro2000 · 2021-01-18T11:55:28Z

@bb010g please resolve the merge conflict.

bb010g · 2021-01-18T21:13:21Z

Rebased. Valgrind checks are no longer enabled by default, which should fix macOS builds.

pkgs/development/tools/jq/default.nix

SuperSandro2000 · 2021-01-19T10:56:22Z

Result of nixpkgs-review pr 71375 run on x86_64-linux 1

4 packages marked as broken and skipped:

linuxPackages_4_4.sysdig
linuxPackages_4_9.sysdig
linuxPackages_hardkernel_4_14.sysdig
linuxPackages_hardkernel_latest.sysdig

2 packages blacklisted:

tests.nixos-functions.nixos-test
tests.nixos-functions.nixosTest-test

4 packages failed to build:

linuxPackages-libre.sysdig
oq
plex-media-player
vscode-extensions.vadimcn.vscode-lldb

104 packages built:

anki
ankisyncd
bash-my-aws
cargo-download
carnix
celluloid
chrome-gnome-shell
code-server
common-updater-scripts
corectrl
curaLulzbot
curseradio
disnix
disnixos
dune-release
dydisnix
element-desktop
glslang
google-cloud-sdk (google-cloud-sdk-gce)
hydra-unstable
i3-layout-manager
imgur-screenshot
jellyfin-mpv-shim
jftui
jq
kodi-cli
libplacebo
linuxPackages.sysdig (linuxPackages_5_4.sysdig)
linuxPackages_4_14.sysdig
linuxPackages_4_19.sysdig
linuxPackages_5_10.sysdig (linuxPackages_latest.sysdig)
linuxPackages_5_9.sysdig
linuxPackages_hardened.sysdig
linuxPackages_latest-libre.sysdig
linuxPackages_latest_hardened.sysdig
linuxPackages_latest_xen_dom0.sysdig
linuxPackages_latest_xen_dom0_hardened.sysdig
linuxPackages_lqx.sysdig
linuxPackages_testing_bcachefs.sysdig
linuxPackages_xen_dom0.sysdig
linuxPackages_xen_dom0_hardened.sysdig
linuxPackages_zen.sysdig
lorri
lutris
lutris-free
lutris-unwrapped
memtest86-efi
minitube
mnemosyne
monado
mpc-qt
mpv
mpv-unwrapped
mpvScripts.mpris
nix-prefetch
nix-prefetch-docker
nix-update
nixFlakes (nixUnstable)
nixos-generators
nixos-rebuild
nixpkgs-review
ocamlformat (ocamlformat_0_15_0)
ocamlformat_0_11_0
ocamlformat_0_12
ocamlformat_0_13_0
ocamlformat_0_14_0
ocamlformat_0_14_1
ocamlformat_0_14_2
ocamlformat_0_14_3
pb_cli
playonlinux
plex-mpv-shim
python37Packages.jq
python37Packages.mpv
python37Packages.yq
python38Packages.jq
python38Packages.mpv
yq (python38Packages.yq)
python39Packages.jq
python39Packages.mpv
python39Packages.yq
qimgv
qmplay2
r2mod_cli
rmount
rofi-systemd
slack-cli
somafm-cli
sublime-music
sway-contrib.grimshot
swiftshader
sysdig
systemfd
todoman
toml2nix
tuijam
twa
vscode-extensions.matklad.rust-analyzer
vscode-extensions.ms-vscode.cpptools
vulkan-extension-layer
vulkan-tools
vulkan-tools-lunarg
vulkan-validation-layers
xrgears

7c6f434c · 2021-01-19T20:57:55Z

pkgs/development/tools/jq/default.nix

+}:
+
+assert enableValgrindChecks -> valgrind != null;
+assert enableValgrindChecks -> which != null;


Maybe just have less/valgrind/which in the arguments without ? null and drop this assertion? They won't be used anyway except conditionally (and evaluation is lazy anyway).

Isn't this pattern meant for callPackage on the file's expression from scopes that don't necessarily have optional package arguments present, though? Destructuring attrset arguments eagerly check for their named parameters. Admittedly, it's a niche case, but without an overall style decision in Nixpkgs I'd prefer to keep with the usual style (that has minor technical merits).

The assertions are there for clearer error messages, and are a pattern (to some extent) throughout Nixpkgs too AFAIK.

Thanks for reminding me to take a look at less, not sure actually why that's there at the moment.

Think I included less for $src/tests/mantest, but it runs fine without it. Removed.

I'd prefer to keep with the usual style

Usually style is to add an enable flag and use lib.optionals.

As per NixOS#31236, building from source (here, Git) should be encouraged over building from release or distribution tarballs. This makes it easier to patch the derivation to use alternative VCS branches, among other benefits. This involves vendoring the Bundler specification for the docs's Rake invocation for now, but next release will mean a switch to Python (patching approximately the same files).

bb010g · 2021-01-19T21:32:37Z

Result of nixpkgs-review pr 71375 run on x86_64-linux 1
4 packages failed to build:
* linuxPackages-libre.sysdig

* oq

* plex-media-player

* vscode-extensions.vadimcn.vscode-lldb

It doesn't look like any of these failures are due to jq.

SuperSandro2000 · 2021-02-12T01:33:47Z

pkgs/development/tools/jq/default.nix

+  meta = {
+    description = ''A lightweight and flexible command-line JSON processor'';
+    downloadPage = "https://stedolan.github.io/jq/download/";
+    homepage = "https://stedolan.github.io/jq/";
+    license = lib.licenses.mit;
+    maintainers = with lib.maintainers; [ raskin globin bb010g ];
+    platforms = with lib.platforms; linux ++ darwin;


Suggested change

meta = {

description = ''A lightweight and flexible command-line JSON processor'';

downloadPage = "https://stedolan.github.io/jq/download/";

homepage = "https://stedolan.github.io/jq/";

license = lib.licenses.mit;

maintainers = with lib.maintainers; [ raskin globin bb010g ];

platforms = with lib.platforms; linux ++ darwin;

meta = with lib; {

description = ''A lightweight and flexible command-line JSON processor'';

downloadPage = "https://stedolan.github.io/jq/download/";

homepage = "https://stedolan.github.io/jq/";

license = licenses.mit;

maintainers = with maintainers; [ raskin globin bb010g ];

platforms = platforms.unix;

I don't think that last line would work.

SuperSandro2000 · 2021-02-12T01:34:33Z

pkgs/development/tools/jq/default.nix

+assert enableValgrindChecks -> valgrind != null;
+assert enableValgrindChecks -> which != null;


Suggested change

assert enableValgrindChecks -> valgrind != null;

assert enableValgrindChecks -> which != null;

This is not required and breaks overriding in certain cases.

SuperSandro2000 · 2021-02-12T01:34:42Z

pkgs/development/tools/jq/default.nix

+, bundlerApp ? null
+# features
+# (enabling Valgrind checks for testing severly increases build time)
+, enableValgrindChecks ? false
+, valgrind ? null, which ? null


Suggested change

, bundlerApp ? null

# features

# (enabling Valgrind checks for testing severly increases build time)

, enableValgrindChecks ? false

, valgrind ? null, which ? null

, bundlerApp

# features

# (enabling Valgrind checks for testing severly increases build time)

, enableValgrindChecks ? false

, valgrind, which

stale · 2021-08-13T22:55:50Z

I marked this as stale due to inactivity. → More info

sarcasticadmin · 2021-08-18T00:35:39Z

Leveraging the git tag seems to have been fixed in #117895

I think this can be closed

ofborg bot added the 8.has: documentation label Oct 19, 2019

ofborg bot requested review from 7c6f434c and globin October 19, 2019 01:50

ofborg bot added 11.by: package-maintainer 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Oct 19, 2019

bb010g force-pushed the jq-github branch from fa142c6 to 32e4ab3 Compare October 19, 2019 02:01

Mic92 reviewed Oct 19, 2019

View reviewed changes

pkgs/development/tools/jq/default.nix Outdated Show resolved Hide resolved

Mic92 reviewed Oct 19, 2019

View reviewed changes

pkgs/development/tools/jq/nix-version.patch Show resolved Hide resolved

bb010g force-pushed the jq-github branch from 32e4ab3 to 21bcd67 Compare October 20, 2019 23:51

stale bot added the 2.status: stale label Jun 5, 2020

stale bot removed the 2.status: stale label Jan 18, 2021

SuperSandro2000 added 2.status: merge conflict 2.status: stale labels Jan 18, 2021

stale bot removed the 2.status: stale label Jan 18, 2021

bb010g force-pushed the jq-github branch from 21bcd67 to b36f6dd Compare January 18, 2021 21:12

ofborg bot added 10.rebuild-linux: 101-500 and removed 2.status: merge conflict 10.rebuild-linux: 11-100 labels Jan 18, 2021

SuperSandro2000 reviewed Jan 18, 2021

View reviewed changes

pkgs/development/tools/jq/default.nix Outdated Show resolved Hide resolved

7c6f434c reviewed Jan 19, 2021

View reviewed changes

bb010g force-pushed the jq-github branch from b36f6dd to 2fa23b4 Compare January 19, 2021 21:15

ofborg bot requested a review from 7c6f434c January 19, 2021 21:24

SuperSandro2000 added the 2.status: merge conflict label Feb 12, 2021

SuperSandro2000 reviewed Feb 12, 2021

View reviewed changes

stale bot added the 2.status: stale label Aug 13, 2021

stale bot removed the 2.status: stale label Aug 18, 2021

Atemu closed this Aug 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jq: build from Git tag (instead of dist tarball) #71375

jq: build from Git tag (instead of dist tarball) #71375

bb010g commented Oct 19, 2019 •

edited

Loading

bb010g commented Oct 19, 2019 •

edited

Loading

Mic92 commented Oct 19, 2019

risicle commented Oct 20, 2019

bb010g commented Oct 20, 2019

veprbl commented Dec 8, 2019

stale bot commented Jun 5, 2020

SuperSandro2000 commented Jan 18, 2021

bb010g commented Jan 18, 2021

SuperSandro2000 commented Jan 19, 2021

7c6f434c Jan 19, 2021

bb010g Jan 19, 2021

bb010g Jan 19, 2021

SuperSandro2000 Jan 19, 2021

bb010g commented Jan 19, 2021

SuperSandro2000 Feb 12, 2021 •

edited

Loading

Atemu Feb 12, 2021

SuperSandro2000 Feb 12, 2021

SuperSandro2000 Feb 12, 2021

stale bot commented Aug 13, 2021

sarcasticadmin commented Aug 18, 2021

	out	bin	lib	man	doc	dev
before	96	29_528_224	29_492_112	32_064	14_568	29_541_768
after	96 (+0)	29_528_224 (±0)	29_492_112 (±0)	32_784 (+720)	14_568 (±0)	29_541_768 (±0)

		assert enableValgrindChecks -> valgrind != null;
		assert enableValgrindChecks -> which != null;

jq: build from Git tag (instead of dist tarball) #71375

jq: build from Git tag (instead of dist tarball) #71375

Conversation

bb010g commented Oct 19, 2019 • edited Loading

Motivation for this change

Things done

Notify maintainers

bb010g commented Oct 19, 2019 • edited Loading

Mic92 commented Oct 19, 2019

risicle commented Oct 20, 2019

bb010g commented Oct 20, 2019

veprbl commented Dec 8, 2019

stale bot commented Jun 5, 2020

SuperSandro2000 commented Jan 18, 2021

bb010g commented Jan 18, 2021

SuperSandro2000 commented Jan 19, 2021

7c6f434c Jan 19, 2021

Choose a reason for hiding this comment

bb010g Jan 19, 2021

Choose a reason for hiding this comment

bb010g Jan 19, 2021

Choose a reason for hiding this comment

SuperSandro2000 Jan 19, 2021

Choose a reason for hiding this comment

bb010g commented Jan 19, 2021

SuperSandro2000 Feb 12, 2021 • edited Loading

Choose a reason for hiding this comment

Atemu Feb 12, 2021

Choose a reason for hiding this comment

SuperSandro2000 Feb 12, 2021

Choose a reason for hiding this comment

SuperSandro2000 Feb 12, 2021

Choose a reason for hiding this comment

stale bot commented Aug 13, 2021

sarcasticadmin commented Aug 18, 2021

bb010g commented Oct 19, 2019 •

edited

Loading

bb010g commented Oct 19, 2019 •

edited

Loading

SuperSandro2000 Feb 12, 2021 •

edited

Loading