tweag / nickel

Better configuration for less
https://nickel-lang.org/
MIT License
2.23k stars 85 forks source link

Wrong version is reported by `nickel --version` for release artifacts #1956

Closed olorin37 closed 2 weeks ago

olorin37 commented 2 weeks ago

Describe the bug New binary and also docker image (as mentioned in discussion #1950)

To Reproduce Just run (for newly downloaded binary from github releases page):

❯ ./nickel-x86_64-linux --version
nickel-lang-cli nickel 1.6.0 (rev 95967eb)

Then you will get wrong version number as binary was build before version bump up.

Expected behavior

❯ ./nickel-x86_64-linux --version
nickel-lang-cli nickel 1.7.0 (rev 03cf743)

I had provided commit has where version bump up has been actually done.

Environment

Additional context I was asking before about release strategy (on discord or matrix) e.g. why each delivery do not bump up the version, or pre-release versions could be used to accumulate more changes (commits). Probably improvement in this versioning strategy could be beneficial too. Of course, at least order of actions should be changed. "release.sh" with version bump up first then generation of the release artifacts.

yannham commented 2 weeks ago

For others passing by, this is a little bug in the release script. However, the artifacts attached to the 1.7.0 release, besides this version mismatch, target just one commit before the version bump and should otherwise be identical to the actual 1.7.0 release. This will be fixed, but this is to say, you can use the provided artifacts as the 1.7 release for all intent of purpose in the meantime.

olorin37 commented 2 weeks ago

[T]he artifacts attached to the 1.7.0 release [...] target just one commit before the version bump and should otherwise be identical to the actual 1.7.0 release

Yep, of course, fortunately there is also revision in version information which can be verify in case of uncertainty.

vkleen commented 2 weeks ago

I suspected it might be https://github.com/tweag/nickel/blob/d8cdd772aa4b9efb5a4711bcd3f042b214b605ce/.github/workflows/release-artifacts.yaml#L94 not setting the ref parameter properly for releases. But I just tested that separately, and it does in fact pick up the release tag to check out. So this leaves me as confused as before...

vkleen commented 2 weeks ago

Looking at the commit history, the release script tagged 95967eb400a7656063b8159b0124e1bf7799d030 as v1.7.0, but the version number bump is committed afterwards. That's most likely the problem.

olorin37 commented 2 weeks ago

Exactly, this is the case.

yannham commented 2 weeks ago

Argh, the workflow is fine in the end - I just probably mis-selected the release branch and used master instead :sweat:. Sorry for the confusion. Releasing a new patch version just for that might prove annoying. I think I'll just delete the current release and replace it.

yannham commented 2 weeks ago

This should be fixed by the overridden 1.7 release.