jovandeginste / payme

QR code generator (ASCII & PNG) for SEPA payments
MIT License
85 stars 5 forks source link

payme added to nixpkgs #4

Closed cimm closed 5 months ago

cimm commented 5 months ago

Not really an issue as such, but I wanted to let you know I added payme to the Nix package repository and will make sure any updates get published in the foreseeable future. It's already available in the Nix unstable repository and will be available in the main channel with the next NixOS release.

This "issue" can be closed of course.

Thanks for building payme Jo!

jovandeginste commented 5 months ago

Do I need to add a flake file for easier maintenance?

AndersonTorres commented 5 months ago

Do I need to add a flake file for easier maintenance?

In which sense?

If you are talking about helping Nixpkgs, this is not needed. Nixpkgs repo uses bots like r-ryantm to track upstream updates and generate pull requests.

If you are talking about creating a flake for the sake of payme developers, this is a nice idea! With Nix and Flakes machinery you can provide development shells and other nice things for the contributors.

cimm commented 5 months ago

As @AndersonTorres already explained, no, there is no need for a flake. Personally, I use flakes for my development environments because it is easy to get up and running with all the dependencies. You could do the same for payme if that interests you, but it's not needed for users of payme to run the application; they can simply install it from the Nix repository.

jovandeginste commented 5 months ago

I use Nix on a very simple level, so if it's not needed, I won't bother. If anyone wants it, feel free to submit a PR!

jovandeginste commented 5 months ago

Oh, and thanks!

jovandeginste commented 5 months ago

I added the (unstable) nix pkg now; should it be the locally built one? Or should it be the released one? Because:

$ payme --version
payme version local (local), built at 2024-03-14T11:42:21+01:00
cimm commented 5 months ago

Good point; I didn't notice that. What is the difference between the "local" and the "release" versions? After a bit of searching, it looks like Golang does not have a debug or release difference, but I am not a Go developer, so I might be missing something.

jovandeginste commented 5 months ago

I believe the local file is recompiled on the user's system, which means they need the golang compiler; the release is already compiled and can be used as is (as long as their system is supported).

AndersonTorres commented 5 months ago

@cimm As far as I can remember, it's a typical occurrence in Nixpkgs to use a line like:

ldflags = [ "-s" "-w" "-X github.com/zrepl/zrepl/version.zreplVersion=${version}" ];

Maybe it fixes that version string...

jovandeginste commented 5 months ago

Yes, check the makefile or the GitHub actions for all relevant flags

cimm commented 5 months ago

Indeed, that's it. Adding the following ldflags when building the package shows the version information.

  ldflags = [
    "-X main.gitCommit=COMMIT"
    "-X main.gitRefName=REFNAME"
  ];
$ payme -v
payme version REFNAME (COMMIT), built at 2024-03-15T13:00:00+01:00

I am unsure about what information should be shown here.

The gitCommit uses github.sha, which, according to the github documentation is the commit SHA that triggered the workflow. For version 1.2.0 that would be 416d53e3f518898a0411889a5af08e8d9858e70e?

The gitRefName is the github.ref_name, which is the short ref name of the branch or tag that triggered the workflow. Would that be the master branch refs/remotes/origin/master?

So the version string would be the following, which feels wrong, as I expect the "v1.2.0" to be in there somewhere. What should be shown in the version string @jovandeginste? I tried checking the released version from github but it won't run on my machine.

payme version refs/remotes/origin/master (416d53e3f518898a0411889a5af08e8d9858e70e), built at 2024-03-15T13:00:00+01:00

To be clear, this doesn't change any functionality; it simply shows the correct version information instead of "local" which is nicer, of course.

jovandeginste commented 5 months ago

I expect you clone the git tag and not master; am I wrong? In that case, the ref name should contain the tag name, ie. the version, eg. "v1.2.0". The Sha should be the short name.

cimm commented 5 months ago

Sorry for the delay, I fixed the version string and opened a pull request on nixpkgs with the requested changes. Now waiting to be reviewed.

Thanks for the help!

cimm commented 5 months ago

Quick update: the pull request has been merged, the binary with the corrected version string will soon be in the Nix unstable repository. :+1:

jovandeginste commented 5 months ago

@cimm the version is good now, the build date is not set yet:

payme version v1.2.0 (416d53e3f518898a0411889a5af08e8d9858e70e), built at 2024-03-24T16:08:58+01:00

This "built at" always shows the current time (which is some default I added if no date is set...) Is it possible to either set it to the actual build time or the tag time?

AndersonTorres commented 5 months ago

Indeed it should be set to UNIX EPOCH (1970-01-01T00:00Z), since in Nixpkgs we don't use such timestamps. Timestamps are a source of impurity: two otherwise identical inputs should not generate different outputs.

jovandeginste commented 5 months ago

I will put forward that using the tag time should be constant too

cimm commented 5 months ago

Sure, we can add a date as well. I checked the 1.2.0 Darwin and Windows releases (the Linux amd64 doesn't work for me for some reason, but that's another story), and the Darwin and Windows binaries gives me the following version string:

$ payme --version
payme version v1.2.0 (416d53e3f518898a0411889a5af08e8d9858e70e), built at 2024-03-26T10:20:22+01:00
> payme.exe --version
payme version v1.2.0 (416d53e3f518898a0411889a5af08e8d9858e70e), built at 2024-03-26T11:07:28+01:00

Which is... today's date? Correct me if I am wrong, but the Darwin and Windows binary both show the current date and not the date it has been compiled, so what date do you want the nix version to show?

We could use the date of the last commit in the tag, which would be Feb 23? Or the epoch one as proposed above? Both dates would be inconsistent with the current Darwin and Windows releases, hence my question.

$ git log -1 --pretty='format:%cd' --date='format:%Y-%m-%dT%H:%M:%S'
2024-02-23T10:13:33
jovandeginste commented 5 months ago

O, wow - I'll look into that!

jovandeginste commented 5 months ago

Found the cause and fixed it in the latest release (v1.2.1)

cimm commented 5 months ago

I tested the 1.2.1 version, and it does show the build date now.

The problem now is that the date shown is the date of the GitHub Action. Since the GitHub Action is GitHub-specific (obviously, since it's using env.BUILD_TIME), it's not part of the git repository, so Nix cannot read the same date. The current payme v1.2.1 version string from the GitHub release:

payme version v1.2.1 (3807082e2d0bc7c5f33a2ee2c243335b7752b574), built 2024-03-26 11:08:42+00:00

There are a few options here.

  1. You change payme to use a reproducible date, like the last git commit date, for example.
  2. You drop the last part of the version string if the date is not set. The Nix version would than simply not show the build date. The GitHub Actions release would stay unchanged.
payme version v1.2.1 (3807082e2d0bc7c5f33a2ee2c243335b7752b574)
  1. We completely ignore the date and leave it as is:
payme version v1.2.1 (3807082e2d0bc7c5f33a2ee2c243335b7752b574), built manually
  1. We use the date of the last commit in the tag:
payme version v1.2.1 (3807082e2d0bc7c5f33a2ee2c243335b7752b574), built 2024-03-26 11:03:28+00:00

Personally, I am not a fan of the last option; since the time between the release version and the nix version would be a few minutes off (see the examples), I would go for the first, second, or third option.

What do you prefer?

jovandeginste commented 5 months ago

I was looking into using the git tag/commit date; this would probably mean rephrasing (no longer "build time" but rather "release time")

cimm commented 4 months ago

@jovandeginste I am unsure as to what to do with the version string in the payme nix package at the moment. What do you prefer from the options above?

jovandeginste commented 3 months ago

My apologies for taking so long to respond; is it possible to use the last commit date in UTC? I changed the github action to use it like this: TZ=UTC0 git show --quiet --date=iso-local --format="%cd"

jovandeginste commented 3 months ago

If that's too much work, "manually" is just as good

cimm commented 3 months ago

The last commit time is perfect, thank you. I opened a pull request to upgrade payme from 1.2.0 to 1.2.2 with the correct build time for nixpkgs: https://github.com/NixOS/nixpkgs/pull/308574.