diff-closures: print sizes with dynamic unit

[MarcelCoding]

Motivation

Provide human-readable units for data sizes.

12345.0 MiB now will be rendered 120.5 GiB.

Things done

• I've tried to use renderSize in all other places, but it could be that I've missed some. Feel free to point me to missing places and I'll try my best to also update these.
• I've also implemented support for negative numbers as an argument to renderSize. This is needed e.g. for the diff-closures command. (See the updated unit tests for an example.)
• I've also replaced all usaged of showBytes with renderSize.

Things not done

• Because of the nature of renderActivity in the progress bar and my non-existent c/cpp experience, I wasn't able to integrate renderSize there.
• I've also did not touch the implementation in the perl code, because I can't easily access the cpp function?

Examples

Example output of a diff-closures execution:

ocamllex-grammar: 0.0.0+rev=c5cf996 → ∅, -34.5 KiB
ocl-icd: -543.2 KiB
odin-grammar: 0.0.0+rev=d2ca8ef → ∅, -2.2 MiB
onedrive: 2.5.7 → ∅, -4.0 MiB
onlyoffice-desktopeditors: 9.0.0, 9.0.0-fhsenv → ∅, -1.0 GiB
open-sans: 1.11 → ∅, -2.7 MiB

Example output of nix-collect-garbage:

deleting '/nix/store/239y8khqvp04qy5jffk7hlfqsaxg3sh5-nix-manual-2.31.2-man'
deleting '/nix/store/wkhn5l4s556njmxfxph5d4pl6mb9m48m-winapi-i686-pc-windows-gnu-0.4.0'
deleting unused links...
note: currently hard linking saves 10.5 GiB
27309 store paths deleted, 32.7 GiB freed

---

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[SuperSandro2000]

Just applied this on 2.30 and works like a charm. Would be nice to back port this, too but no blocker if not. 🎉

[xokdvium]

Thanks. This supersedes #14358 it looks like

[MarcelCoding]

Yeah, I actually did not see the other PR, most likely because removing showBytes was just a side quest while discovering renderSize after I've implemented by own function ;D Should I add the author of #14358 as co-authored?

[xokdvium]

Should I add the author of #14358 as co-authored?

Feel free. Don't think @lovesegfault would object

[lovesegfault]

Nice, I'll close my PR :)

[internal-nix-ci]

Successfully created backport PR for 2.30-maintenance:

• [Backport 2.30-maintenance] diff-closures: print sizes with dynamic unit #14380

[internal-nix-ci]

Successfully created backport PR for 2.31-maintenance:

• [Backport 2.31-maintenance] diff-closures: print sizes with dynamic unit #14381

[internal-nix-ci]

Successfully created backport PR for 2.32-maintenance:

• [Backport 2.32-maintenance] diff-closures: print sizes with dynamic unit #14382

[xokdvium]

@MarcelCoding it has occurred to me that nix-output-monitor might rely on this and sure enough: https://github.com/maralorn/nix-output-monitor/blob/07169b3894ab7cb1ee01d766145ab03bf2dc7a69/lib/NOM/Parser.hs#L76-L86. Though it seems like an incredibly bad idea to parse logs meant for human consumption that's what we have at the moment.

[Mic92]

Could we have the json format printing raw data and than it's formatted late in the process? It would still break current nom but maybe more maintainable in the future.

[xokdvium]

the json format printing raw data

internal-json already prints raw byte count, so that's good. But in this case it's not an Activity so it won't appear in structured logs.

[xokdvium]

Anyway, I will revert the backports on release branches since it will surely break things.

EDIT: Though I'm open to other ideas. In general it doesn't sound like a good place to be that inherently human readable output is treated as a de-facto stable interface.

[maralorn]

In general it doesn't sound like a good place to be that inherently human readable output is treated as a de-facto stable interface.

FTR: I totally agree.

I generally regard the human-readable parsing part of nom deprecated. However this message is one of the few bits left that we can’t do without. Because this message is used verbatim in the internal-json format and there is no structured way to receive this data.

That being said it wouldn’t be hard to adapt the nom parser to this change in a backwards-compatible way.

[maralorn]

Uh, I take this back. I actually think users of nom build will not be affected by this change. nom ignores this line when using the internal-json parser.

Only users of nix-build |& nom will be affected by this. They will not see a correct number of planned downloads.

[xokdvium]

@maralorn I wonder if #14455 is the best way forward for now.

[maralorn]

@maralorn I wonder if #14455 is the best way forward for now.

idk. I really didn’t expect nix to care for noms terrible hacks. 😆 So I wouldn’t really mind if this breaks some usages. As written above it will be mostly fine, this only affects a deprecated usage pattern.

But I can extend the parser in nom and do a new release and then we are fine. We just maybe don’t want to backport the nix change to stable without backporting the nom change as well.