nixpkgs icon indicating copy to clipboard operation
nixpkgs copied to clipboard

treewide: remove `refs/tags/` from github release `meta.changelog` urls

Open pbsds opened this issue 1 year ago • 5 comments

diff of jq <packages.json 'to_entries[]|"\(.key) \(.value.meta.changelog)"' -r:

https://gist.github.com/pbsds/49b2e2ae5c9b967a0972bbc3c2597c12

Description of changes

the majority of this pr was made using:

#!/usr/bin/env nix-shell
#!nix-shell --pure -i bash -p ripgrep sd jq git lix

export NIXPKGS_ALLOW_UNFREE=1
export NIXPKGS_ALLOW_INSECURE=1
export NIXPKGS_ALLOW_BROKEN=1

git restore .

test -s packages.json || ( set -x;
  time nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f ./. -qaP --json --meta --show-trace --no-allow-import-from-derivation > packages.json
)

jq <packages.json 'to_entries[]
        | select(.value.meta.position==null|not)
        | select(.value.meta.changelog==null|not)
        | "\(.key)\t\(.value.meta.changelog)\t\(.value.meta.position)"' -r |
    sed -e "s#\t$(realpath .)/#\t#" |
    sed -e 's#:\([0-9]*\)$#\t\1#' |
    grep -E '/releases/tag/refs/tags/' |
    while read attrpath changelog fname col; do
        echo "$attrpath"
        (set -x
            sd -F '/releases/tag/${src.rev}' '/releases/tag/${lib.removePrefix "refs/tags/" src.rev}' "$fname"
            sd -F '/releases/tag/${finalAttrs.src.rev}' '/releases/tag/${lib.removePrefix "refs/tags/" finalAttrs.src.rev}' "$fname"
            sd -F '/releases/tag/${self.src.rev}' '/releases/tag/${lib.removePrefix "refs/tags/" self.src.rev}' "$fname"
        )
    done

( set -x;
  time nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f ./. -qaP --json --meta --show-trace --no-allow-import-from-derivation > packages2.json
)

diff -u \
  <(jq <packages.json  'to_entries[]|"\(.key) \(.value.meta.changelog)"' -r | sort) \
  <(jq <packages2.json 'to_entries[]|"\(.key) \(.value.meta.changelog)"' -r | sort) | tee diff.patch

Things done

  • Built on platform(s)
    • [ ] x86_64-linux
    • [ ] aarch64-linux
    • [ ] x86_64-darwin
    • [ ] aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • [ ] sandbox = relaxed
    • [ ] sandbox = true
  • [ ] 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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
  • [x] Fits CONTRIBUTING.md.

Add a :+1: reaction to pull requests you find important.

pbsds avatar Aug 30 '24 02:08 pbsds

I check them quite often, it should be encouraged rather than discouraged IMO

pbsds avatar Aug 30 '24 03:08 pbsds

Thank you for making your script public in this PR and others, Peder (@pbsds). I learn so much from these "worked demonstrations". It's really appreciated.

philiptaron avatar Aug 30 '24 03:08 philiptaron

Since they are computed values, I wonder if they couldn't be automated away in 90% of the cases. Not a blocker for this PR, but more as a follow-up conversation (see #338366).

zimbatm avatar Aug 30 '24 09:08 zimbatm

Hmmm. I wonder if we could abstract this with a tag argument to fetchFromGitHub which understands refs/tags. That would eliminate both the refs/tags in the src and also the refs/tags removal in the changelog entries.

It would be mutually exclusive with rev, since it's another way of specifying it.

philiptaron avatar Aug 30 '24 14:08 philiptaron

I strongly support a tag argument regardless of anything to do with changelogs.

emilazy avatar Aug 30 '24 15:08 emilazy

What's the point of this change? Is making the URLs a little nicer worth it? EDIT: Sorry, I should have read the actual script. It doesn't change uses of blob/${src.rev} so this is an actual improvement.

dotlambda avatar Oct 04 '24 21:10 dotlambda