NixOS / hydra

Hydra, the Nix-based continuous build system
http://nixos.org/hydra
GNU General Public License v3.0
1.17k stars 300 forks source link

`renderInputDiff`: Increase git hash length 6 -> 8 #1258

Closed nh2 closed 1 year ago

nh2 commented 1 year ago

Nixpkgs has so many commits that length 6 is often ambiguous, making the use of the shown values with git difficult.

nh2 commented 1 year ago

Example: https://hydra.nixos.org/eval/1784527?filter=openssl&compare=1784507&full=#tabs-inputs

image

In nixpkgs git:

% git range-diff 1b4722...b3a8f7
error: short object ID 1b4722 is ambiguous
hint: The candidates are:
hint:   1b4722674c3 commit 2022-10-31 - Merge pull request #198780 from NixOS/backport-198737-to-release-22.05
hint:   1b4722ac821 commit 2015-03-19 - prosody: add dependency on zlib (close #6894)
SuperSandro2000 commented 1 year ago

In the long run it would probably be a good idea to use git to determine how long those hashes should be.

ajs124 commented 1 year ago

Yeah, iirc the database has a shortrev field for this, but the input plugins don't populate it.

edolstra commented 1 year ago

How about 7 characters? That's what Nix's shortRev flake attribute uses.

dasJ commented 1 year ago

I think the best way of doing it would be to use whatever git uses which scales dynamically with repo size (but is never less than 7). The scaling algorithm is already relevant for nixpkgs which uses 11 characters in my local checkout.

nh2 commented 1 year ago

There are currently

$ git rev-list --all | wc -l
426593

many commits in the repo and

$ echo $(( $(git rev-list --all | wc -l) - $(git rev-list --all | cut -c-6 | sort -u | wc -l) ))       
5250

commits that will be unclickable on Hydra. Respectively:

cut -c-6: 5250
cut -c-7:  327
cut -c-8:   24
cut -c-9:    2
cut -c-10:   0

I think the best way of doing it would be to use whatever git uses which scales dynamically with repo size

Wouldn't that require the Hydra render page to perform git operations, which might be slow?


Why is it important that the "changes" link use as-short-as-possible commits anyway?

Might as well show 12 chars and be done with it for a while: 1b4722674c31 to b3a8f7ed267e

vcunat commented 1 year ago

I think the numbers are worse, because (most?) git operations won't proceed smoothly even in case of collision with non-commit objects (certainly trees). While the number of revisions is under 0.5M, I have 3.8M objects (git count-objects -v). EDIT: even just 4-fold increase would require one additional hex char to get the same collision rate (collision probability of a single hash against all of them).

Oh and I think we should merge (and deploy) this or something simple soon, and we can optionally think of some more ideal solution later. I see no reason for this to remain open for months.

nh2 commented 2 months ago

cut -c-8: 24

Might as well show 12 chars and be done with it for a while

PR to bump the length to 12: https://github.com/NixOS/hydra/pull/1396

We shouldn't waste time on such papercuts.