payme: fix version number string #297480
payme: fix version number string #297480
Conversation
cimm
changed the title
pkgs/by-name/pa/payme/package.nix
Outdated
| "-s" | ||
| "-w" | ||
| "-X main.gitRefName=v${version}" | ||
| "-X main.gitCommit=416d53e3f518898a0411889a5af08e8d9858e70e" |
There was a problem hiding this comment.
I'm not sure if hard coding the commit here is a great idea.
This is probably pretty easy to overlook and probably (i'm not sure on that?) can't get updated by r-ryantm
There was a problem hiding this comment.
Good catch, this won't be updated. We should just hardcode this to a dummy value
There was a problem hiding this comment.
IIRC r-ryantm runs the updateScript. Then we (?) can write a script that updates these values.
There was a problem hiding this comment.
Thanks for the feedback, I moved the commit to a variable in f4f5a30, is there a better option?
There was a problem hiding this comment.
Is this value useful somehow?
If it isn't, then we can do as @SuperSandro2000 suggested and fill it with zeros or with deadfish five times - deadfishdeadfishdeadfishdeadfishdeadfish.
Else, I need to re-read the manuals on how to write updater scripts...
There was a problem hiding this comment.
I searched the nixpkgs repository for possible solutions. There seem to be two solutions to this. The first is to hard code the commit like I did in commit f4f5a30 (see 1, 2 or 3 for previous work). Then there is the second option, which stores the commit in a temporary file to use later (see 1, 2 or 3 for previous work).
The second option seems the most automated one. I tried using this second option, with a temporary COMMIT file, but could not get it to work. Even though it works if I try to build one of the other packages linked above.
src = fetchFromGitHub {
owner = "jovandeginste";
repo = "payme";
rev = "v${version}";
hash = "sha256-WE/sAs0VSeb5UKkUy1iyjyXtgDmlQhdZkw8HMMSbQiE=";
leaveDotGit = true;
postFetch = ''
git -C $out rev-parse HEAD > $out/COMMIT
find "$out" -name .git -print0 | xargs -0 rm -rf
'';
};
[...]
preBuild = ''
ldflags+=" -X main.gitCommit=$(cat COMMIT)"
'';The build fails as it can't find the COMMIT file. I don't know if it's never created (as in the postFetch is not executed) or if I am pointing to the wrong location in the preBuild step?
cat: COMMIT: No such file or directory
Any idea what I am missing here?
There was a problem hiding this comment.
@AndersonTorres As for your question, whether this is useful? It depends on your point of view, I suppose. The only thing I am trying to achieve here is adding the commit to the version string; it doesn't "change" the application, but adding the commit to the application's version output would make the nix version consistent with the binary compiled by the application developer.
There was a problem hiding this comment.
Any idea what I am missing here?
$out from $src is not the same as from outer derivation.
About the value of commit hash, this is precisely the work for an updater script.
I suggest looking at how unstableGitUpdater grabs Git hashes and steal copy their ideas.
There was a problem hiding this comment.
I got it; the changes in 39b733e find the commit dynamically; it's no longer hard-coded in the file. I learned something new, thanks for the help!
ofborg
bot
added
11.by: package-maintainer
| @@ -8,11 +8,27 @@ buildGoModule rec { | |||
| owner = "jovandeginste"; | |||
| repo = "payme"; | |||
| rev = "v${version}"; | |||
| hash = "sha256-WE/sAs0VSeb5UKkUy1iyjyXtgDmlQhdZkw8HMMSbQiE="; | |||
| hash = "sha256-2gZgmYgLaJQRQ+3VOUDnMm5QBjfKyxyutVf9NrbGO3g="; | |||
There was a problem hiding this comment.
This is somewhat dangerous, since it modified the hash.
However, given that I can't provide a better solution right now, I will not pursue this.
There was a problem hiding this comment.
The easiest solution would be to just fill that has with zeros and don't worry about it but if you want to go the complicated route, sure.
Description of changes
The current payme version in nixpkgs unstable does not show the correct version string for the application, notice the "local" in the version number:
This pull request corrects to version string as requested by the application developer:
Background discussion can be found in the payme repo issue.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.