New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bat: Add update script #104072
bat: Add update script #104072
Conversation
} | ||
|
||
oldVersion="$(nix-instantiate --eval -E "with import ./. {}; lib.getVersion ${pname}" | tr -d '"')" | ||
latestTag="$(git -c 'versionsort.suffix=-' ls-remote --exit-code --refs --sort='version:refname' --tags git@github.com:sharkdp/bat.git '*' | tail --lines=1 | cut --delimiter='/' --fields=3 | sed 's|^v||g')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be easy to reuse something from fetchFromGitHub. Also ssh links only work when the people have working ssh to github. Https works for everyone.
oldVersion="$(nix-instantiate --eval -E "with import ./. {}; lib.getVersion ${pname}" | tr -d '"')" | ||
latestTag="$(git -c 'versionsort.suffix=-' ls-remote --exit-code --refs --sort='version:refname' --tags git@github.com:sharkdp/bat.git '*' | tail --lines=1 | cut --delimiter='/' --fields=3 | sed 's|^v||g')" | ||
|
||
if [ ! "$oldVersion" = "$latestTag" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if [ ! "$oldVersion" = "$latestTag" ]; then | |
if [ "$oldVersion" != "$latestTag" ]; then |
update-source-version ${pname} "$latestTag" --version-key=version --print-changes | ||
nixpkgs="$(git rev-parse --show-toplevel)" | ||
default_nix="$nixpkgs/pkgs/tools/misc/bat/default.nix" | ||
dummy_cargo="19vhhxfyx3nrngcs6dvwldnk9h4lvs7xjkb31aj1y0pyawz882h9" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use only 000 to make sure that we don't use this one by accident?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be
dummy_cargo="19vhhxfyx3nrngcs6dvwldnk9h4lvs7xjkb31aj1y0pyawz882h9" | |
dummy_cargo="${stdenv.lib.fakeSha256}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, the "dummy cargo" hash right now looks to be the current hash, meaning as written this won't work if the machine doing the updating has already built this derivation.
old_cargo=$(grep '^\s*cargoSha256 = ' pkgs/tools/misc/bat/default.nix | awk -F= '{print $2}' | tr -d '" ;') | ||
sed -i "s|$old_cargo|$dummy_cargo|g" "$default_nix" | ||
new_cargo=$(nix-build -A bat 2>&1 | grep sha256 | grep got | awk -F: '{print $3}') | ||
sed -i "s|$dummy_cargo|$new_cargo|g" "$default_nix" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nix-update can also handle this with less code. It knows about github releases and cargoSha256
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ryantm's bot @r-ryantm seems to know how to update cargo packages already using nixpkgs-update, as evidenced by #93033. Is there some reason we can't let it handle bat
as well? I'm not sure what sort of delay there is when letting it deal with packages.
Or alternatively, is there any way to hook into its logic for the updateScript
? I don't know if nixpkgs-update is available in any way from nixpkgs. Maybe once nix flakes is stable?
updateScript = writeScript "update.sh" '' | ||
#!${stdenv.shell} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use writeShellScript
instead you can avoid the shebang line and Nix will syntax-check it for you.
updateScript = writeScript "update.sh" '' | |
#!${stdenv.shell} | |
updateScript = writeShellScript "update.sh" '' |
update-source-version ${pname} "$latestTag" --version-key=version --print-changes | ||
nixpkgs="$(git rev-parse --show-toplevel)" | ||
default_nix="$nixpkgs/pkgs/tools/misc/bat/default.nix" | ||
dummy_cargo="19vhhxfyx3nrngcs6dvwldnk9h4lvs7xjkb31aj1y0pyawz882h9" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be
dummy_cargo="19vhhxfyx3nrngcs6dvwldnk9h4lvs7xjkb31aj1y0pyawz882h9" | |
dummy_cargo="${stdenv.lib.fakeSha256}" |
old_cargo=$(grep '^\s*cargoSha256 = ' pkgs/tools/misc/bat/default.nix | awk -F= '{print $2}' | tr -d '" ;') | ||
sed -i "s|$old_cargo|$dummy_cargo|g" "$default_nix" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surely we can do this with one sed command instead of trying to read the old cargo value out and then replacing it? The hash only shows up once in the file so it's not like we need to replace it multiple times.
update-source-version ${pname} "$latestTag" --version-key=version --print-changes | ||
nixpkgs="$(git rev-parse --show-toplevel)" | ||
default_nix="$nixpkgs/pkgs/tools/misc/bat/default.nix" | ||
dummy_cargo="19vhhxfyx3nrngcs6dvwldnk9h4lvs7xjkb31aj1y0pyawz882h9" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, the "dummy cargo" hash right now looks to be the current hash, meaning as written this won't work if the machine doing the updating has already built this derivation.
I don't see anything about the way bat is written that would stop it from getting updated by r-ryantm. The last time it tried to update it was in May and it stopped early because there was already an open PR for the update. This update script looks more complicated than necessary. Can we look for other Rust packages that already have updatescripts? https://github.com/Mic92/nix-update/ supports rust updates by the way. |
Motivation for this change
cargoSha256
, so the update script is a little complex.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)