bat-extras: init at 20200408 #85982

lilyball · 2020-04-25T03:30:18Z

Motivation for this change

Some wrappers tools for bat. Not by the same dev but they are linked on https://github.com/sharkdp/bat.

This is a rewrite of #85836 that fixes all the issues I identified. I kind of got nerd-sniped here. This version exposes each script as a separate attribute nested under bat-extras. This way you can install e.g. just batgrep without pulling in prettybat and all its dependencies.

Things done

bbigras · 2020-04-25T17:57:16Z

I can't build. I tested with nix-review and manually.

❯ nix-build -A bat-extras
these derivations will be built:
  /nix/store/1gxi6iqpf7y743grqr8gg4z6y7i3dw97-bat-extras-20200408.drv
  /nix/store/0yhja69mi61xw49xm2gw313z4dpn54c6-bat-extras-batgrep-20200408.drv
  /nix/store/cknfmj8xfqgf7g6i4sscpwl3aqd2njgr-bat-extras-batgrep-20200408.drv
  /nix/store/f5kiyrbmmjvxnpgbkp8dnncmm5i4cbfm-bat-extras-batgrep-20200408.drv
  /nix/store/mwbza033kvnxclgvr04w71pfqfqrkc1r-bat-extras-batgrep-20200408.drv
building '/nix/store/1gxi6iqpf7y743grqr8gg4z6y7i3dw97-bat-extras-20200408.drv'...
unpacking sources
unpacking source archive /nix/store/mp8aadl3sfy1lx8w1i0dk7gipr1wv1qr-source
source root is source
patching sources
patching script interpreter paths in test.sh .test-framework/bin/best.sh
test.sh: interpreter directive changed from "/usr/bin/env bash" to "/nix/store/z1l2n01xdfcm9bkkir83c32mkpvv51zq-bash-4.4-p23/bin/bash"
.test-framework/bin/best.sh: interpreter directive changed from "/usr/bin/env bash" to "/nix/store/z1l2n01xdfcm9bkkir83c32mkpvv51zq-bash-4.4-p23/bin/bash"
building
This will not install the script.
Use --install for a global install.

Preparing scripts...
Building scripts...
    [1/4] /build/source/src/batgrep.sh
          Reading...
          Preprocessing...
          Minifying...
          Compressing [skipped]
          Building...
          Installing [skipped]
    [2/4] /build/source/src/batman.sh
          Reading...
          Preprocessing...
          Minifying...
          Compressing [skipped]
          Building...
          Installing [skipped]
    [3/4] /build/source/src/batwatch.sh
          Reading...
          Preprocessing...
          Minifying...
          Compressing [skipped]
          Building...
          Installing [skipped]
    [4/4] /build/source/src/prettybat.sh
          Reading...
          Preprocessing...
          Minifying...
          Compressing [skipped]
          Building...
          Installing [skipped]
running tests
--------------------------------------------------------------------------------
| Test Suite: lib_pager                                                        |
--------------------------------------------------------------------------------
[FAIL] env_bat_pager        :: Exited with code 126.
[FAIL] less_args            :: Exited with code 126.
[FAIL] less_detection       :: Expectation failed: [ less_but_renamed = less ]
[FAIL] less_version         :: Expectation failed: [ 0 = 473 ]
 ....                       :: Expectation failed: [ 0 = 551 ]

Totals:
    PASS: 0 / 4
    FAIL: 4 / 4
--------------------------------------------------------------------------------
| Test Suite: lib_opt                                                          |
--------------------------------------------------------------------------------
[PASS] long                 :: 3 ms
[PASS] long_value_explicit  :: 4 ms
[PASS] long_value_implicit  :: 3 ms
[PASS] short_value_explicit :: 4 ms
[PASS] short_value_implicit :: 4 ms

Totals:
    PASS: 5 / 5
    FAIL: 0 / 5

ALL TESTS PASSED.
--------------------------------------------------------------------------------
| Test Suite: lib_version                                                      |
--------------------------------------------------------------------------------
[PASS] compare_eq           :: 5 ms
[PASS] compare_ge           :: 5 ms
[PASS] compare_gt           :: 5 ms
[PASS] compare_le           :: 6 ms
[PASS] compare_lt           :: 4 ms
[PASS] compare_ne           :: 4 ms

Totals:
    PASS: 6 / 6
    FAIL: 0 / 6

ALL TESTS PASSED.
builder for '/nix/store/1gxi6iqpf7y743grqr8gg4z6y7i3dw97-bat-extras-20200408.drv' failed with exit code 2
cannot build derivation '/nix/store/0yhja69mi61xw49xm2gw313z4dpn54c6-bat-extras-batgrep-20200408.drv': 1 dependencies couldn't be built
error: build of '/nix/store/0yhja69mi61xw49xm2gw313z4dpn54c6-bat-extras-batgrep-20200408.drv', '/nix/store/cknfmj8xfqgf7g6i4sscpwl3aqd2njgr-bat-extras-batgrep-20200408.drv', '/nix/store/f5kiyrbmmjvxnpgbkp8dnncmm5i4cbfm-bat-extras-batgrep-20200408.drv', '/nix/store/mwbza033kvnxclgvr04w71pfqfqrkc1r-bat-extras-batgrep-20200408.drv' failed

lilyball · 2020-04-25T23:23:48Z

Interesting. Now I’m glad I took the time to make sure the library tests ran. What platform are you testing on?

bbigras · 2020-04-26T00:16:47Z

x86_64

lilyball · 2020-04-26T00:18:41Z

NixOS, Linux, or macOS?

bbigras · 2020-04-26T00:25:31Z

Oh sorry, NixOS.

lilyball · 2020-04-26T06:29:46Z

I've fixed the library tests. batman is still failing, which I'll investigate tomorrow.

lilyball · 2020-04-26T06:35:34Z

pkgs/tools/misc/bat-extras/default.nix

+
+      # Patch shebangs now because our tests rely on them
+      postPatch = ''
+        patchShebangs --host bin/${name}


Oops this is actually wrong, I should be passing --build and then allowing the normal fixup patch after all, I think. I’m not actually sure how check phases are supposed to work in cross-compile situations, but if we are cross-compiling we need to test with the build environment and then switch to the host.

That is, assuming patchShebangs won’t just ignore any shebangs already pointing to the nix store. Guess I’ll find out tomorrow.

Looks like patchShebangs won't patch anything already pointing into the nix store. I guess I'm not sure the right solution here. I'll just leave it as it is for now since this means the installed output will be patched for the host. Alternatively we could install, then run check using the uninstalled bits, which will allow us to patch the install separately, but I think that's getting a bit complicated when I don't even know if check phases run when cross-compiling.

lilyball · 2020-04-26T22:19:09Z

batman and prettybat tests bypassed in NixOS, they seem to be broken upstream there (eth-p/bat-extras#19).

bbigras · 2020-04-26T23:28:48Z

Result of nixpkgs-review pr 85982 1

4 packages built:

- bat-extras.batgrep
- bat-extras.batman
- bat-extras.batwatch
- bat-extras.prettybat

bbigras · 2020-04-29T13:55:30Z

I tested running the commands. Sorry for the delay.

batman works but it prints the whole man page (it's like it's not using less).

prettybat seems fine using all the formatter.

❯ ./batgrep -t toml reqwest ~/dev/
/nix/store/g9skvj9cvyyn837k04gghhgrvwi8z84f-bat-extras-batgrep-20200408/bin/.batgrep-wrapped: line 137: less -R: command not found

❯ ./batwatch ~/tmp/bleh.sh 
/nix/store/klpfbc1m2s2xmvr79k0x0l644yfdrakq-bat-extras-batwatch-20200408/bin/.batwatch-wrapped: line 359: less -R: command not found

lilyball · 2020-04-29T19:17:40Z

PAGER='less -R' failing is an issue with bat-extras. It's treating the env var as a command name, not as a shell string.

batman doesn't invoke the pager directly, but it does use it to construct the BAT_PAGER env var for bat to use. Unfortunately since the script is interpreting PAGER it still isn't handling less -R correctly so it's effectively doing BAT_PAGER="'less -R'", and bat handles a garbage pager by disabling paging.

Which is to say, if you set PAGER=less then things should page correctly. If you clear PAGER entirely then paging should mostly work except batgrep doesn't page at all if PAGER or BAT_PAGER isn't set.

All of this is upstream functionality, not a consequence of this package. The behavior differs from #85836 in that that PR blindly overwrite PAGER to be less for batgrep and batwatch (but would be expected to have the same issue with batman).

lilyball · 2020-04-29T19:22:43Z

I filed the upstream issue as eth-p/bat-extras#24

nixos-discourse · 2020-04-30T19:25:58Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/140

eth-p · 2020-05-01T04:21:25Z

Before you merge it, would there be an issue if I changed the version format to YYYY.MM.DD instead of YYYYMMDD as part of the next release (tomorrow)? I had planned on doing it since the last release, but I don't want to cause any issues with package managers.

A couple of other changes to note in that upcoming release:

The tput command is no longer required for anything after building. I replaced it with stty size
I added a batdiff script that optionally uses delta for printing diffs.

lilyball · 2020-05-02T00:27:28Z

AFAIK the actual format of the version string is fairly irrelevant as far as Nix is concerned. When nix-env checks for upgrades I think it just checks if the version is different, not if it's considered newer, because the assumption is whatever's in the nixpkgs channel is the latest known version.

lilyball mentioned this pull request Apr 25, 2020

bat-extras: init at 20200408 #85836

Closed

10 tasks

ofborg bot added the 8.has: package (new) This PR adds a new package label Apr 25, 2020

ofborg bot requested a review from bbigras April 25, 2020 03:37

ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Apr 25, 2020

lilyball force-pushed the bat-extras branch from 5352a56 to 1e28b2f Compare April 26, 2020 06:29

lilyball commented Apr 26, 2020

View reviewed changes

bat-extras: init at 20200408

c27bd9a

lilyball force-pushed the bat-extras branch from 1e28b2f to c27bd9a Compare April 26, 2020 22:18

eth-p mentioned this pull request May 1, 2020

Pager functionality breaks if PAGER contains arguments eth-p/bat-extras#24

Closed

DamienCassou merged commit 9c60228 into NixOS:master May 2, 2020

lilyball deleted the bat-extras branch May 2, 2020 21:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bat-extras: init at 20200408 #85982

bat-extras: init at 20200408 #85982

lilyball commented Apr 25, 2020

bbigras commented Apr 25, 2020

lilyball commented Apr 25, 2020 via email

bbigras commented Apr 26, 2020

lilyball commented Apr 26, 2020 via email

bbigras commented Apr 26, 2020

lilyball commented Apr 26, 2020

lilyball Apr 26, 2020

lilyball Apr 26, 2020

lilyball Apr 26, 2020

lilyball commented Apr 26, 2020

bbigras commented Apr 26, 2020

bbigras commented Apr 29, 2020

lilyball commented Apr 29, 2020

lilyball commented Apr 29, 2020

nixos-discourse commented Apr 30, 2020

eth-p commented May 1, 2020 •

edited

Loading

lilyball commented May 2, 2020

bat-extras: init at 20200408 #85982

bat-extras: init at 20200408 #85982

Conversation

lilyball commented Apr 25, 2020

Motivation for this change

Things done

bbigras commented Apr 25, 2020

lilyball commented Apr 25, 2020 via email

bbigras commented Apr 26, 2020

lilyball commented Apr 26, 2020 via email

bbigras commented Apr 26, 2020

lilyball commented Apr 26, 2020

lilyball Apr 26, 2020

Choose a reason for hiding this comment

lilyball Apr 26, 2020

Choose a reason for hiding this comment

lilyball Apr 26, 2020

Choose a reason for hiding this comment

lilyball commented Apr 26, 2020

bbigras commented Apr 26, 2020

bbigras commented Apr 29, 2020

lilyball commented Apr 29, 2020

lilyball commented Apr 29, 2020

nixos-discourse commented Apr 30, 2020

eth-p commented May 1, 2020 • edited Loading

lilyball commented May 2, 2020

eth-p commented May 1, 2020 •

edited

Loading