makefile: add command to generate git release tags #720

ffranr · 2023-12-05T12:35:04Z

Closes #729

This commit adds a check to this project's GitHub release workflow to ensure that the release version string is SemVer compliant.

ffranr · 2023-12-05T13:42:27Z

@guggero had the idea of adding a make release-tag command. The command would derive the correct release tag from the version specified in version.go. Then, it could check that the version string is SemVer compliant. And then finally create the git tag release tag.

In the github release workflow we could then compare the output of make release-tag to the git tag as a final consistency check.

We could then copy this command and release workflow check to LND and other projects.

I'll update this PR with that command.

ffranr · 2023-12-05T15:25:29Z

According to this website, the right way to tag rc2 would be 0.3.2-alpha.2:
https://jubianchi.github.io/semver-check/#/version/0.3.2-alpha.2

Where the trailing 2 indicates prerelease 2.

ffranr · 2023-12-21T13:55:39Z

@guggero @jharveyb I think these are the changes we discussed on our call (I should have taken more notes...):

Standardise version.go across projects.
Parse version.go directly when creating a git release tag.
Add a new make command to generate a git release tag.
Check version when building release binaries as part of release workflow.

Let me know if I'm missing anything.

guggero

Nice, looks pretty good! Just a couple of suggestions to fix the actual make target.

version.go

Makefile

scripts/get-release-tag-str.sh

Makefile

guggero

Cool, looking forward to the standardized tagging procedure in all our projects 💯

jharveyb

Great PR to improve our tooling and release flow!

Some nits and clarifying comments but looks close.

scripts/get-git-tag-name.sh

jharveyb · 2024-01-10T18:53:57Z

Makefile

+		echo "Script encountered an error with exit status $$exit_status."; \
+	fi; \
+	echo "Adding git tag: $$tag"; \
+	git tag -s -a "$$tag" -m "Release $$tag";


Should this have the word Release at all? Not sure where this message shows up tbh.

I think that the git tag message is a git artefact only and isn't used in a github release.

Here we create the git tag message as part of the release-tag make command which is release specific. The message is just a bit of extra documentation. The $$tag is probably redundant. We could possibly expand the message to say that the tag was created using the release-tag command? I think I'll make that change. Let me know what you think.

Ah, the convention for -m would be: -m "<project_name> <git_tag>". So maybe we could pass in the project name from the command line as well? And hard code it in the Makefile as we do with the name of the version.go file.

Why is that the convention? It doesn't seem like a good idea to me.

The git_tag is already included in the tag artefact outside of the message field. Why repeat that in the message field? And I think the project name is obvious, so I don't see why it should be included either.

Why don't we describe in the message how the tag was actually created given that it will be produced by executing a command?

jharveyb · 2024-01-10T18:55:03Z

scripts/get-git-tag-name.sh

+# compliant version string.
+check_git_tag_name() {
+  local tag_name="$1"
+  semver_regex="^v([0-9]+)\.([0-9]+)\.([0-9]+)(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?(\+[0-9A-Za-z-]+)?$"


Would be good to have a citation for the source of this regex, my cursory searches for a bash-compatible semver regex were all much longer (maybe those are stricter than we want)?

The regex that you're referring to wasn't specific enough. It allowed leading zeros for example.

I've generated a better regex with the help of AI. And also better documentation.

The new semver regex is more general than what we need. So I might do a second round of matching on a LL specific regex.

I'm wondering also if there is a golang tagging one we could find somewhere?

I've been thinking a bit more about this. I think that we don't need a regex which checks for semver compliance in all its possible forms. We just need a check for a Lightning Labs version compliance as a subset of semver compliance. We can just manually check that our simpler regex is a subset of semver.

So I think our regex here can be much simpler and we don't need a golang tagging regex check either.

Do you agree?

version.go

jharveyb · 2024-01-10T19:01:00Z

version.go

-		version = fmt.Sprintf("%s-%s", version, preRelease)
+
+	// Append any status and pre-release strings to the version string.
+	switch {


Bit confused on this switch, do we ever want appStatus without preRelease? From the tapd tags it looks like we only do both or neither, ex. v0.3.2, v0.3.0-alpha.rc3, but no v0.3.0-alpha nor v0.3.0-rc3.

Or is this to also match a different pattern used for a different project?

Or is this to also match a different pattern used for a different project?

I think we should make sure that possibility is available to us. I don't think we have a strict org wide rule around the use of status without pre-release. For instance, v0.3.0-alpha seems like a reasonable tag to me.

K, I thought leaving out the rc1 suffix when using alpha or beta is what caused issues previously? I don't recall exactly which tag caused those though.

What caused the issue previously was leaving out -alpha while just using an rc tag (e.g. 0.3.2-rc2), because then the version check of the release script didn't work as it just did a substring comparison with what the app version string reported (v0.3.2-alpha.rc2).
So I think we should also update the check_tag_correct function in scripts/release.sh to use the new get-git-tag-name.sh script and compare it to the checked out tag.

And to answer your other question, just having the status is what we want for non-RC releases in all other projects than this one (e.g. lnd).

guggero

Sorry, found a couple of more things while testing 😅

guggero · 2024-01-12T09:19:58Z

version.go

-		version = fmt.Sprintf("%s-%s", version, preRelease)
+
+	// Append any status and pre-release strings to the version string.
+	switch {


What caused the issue previously was leaving out -alpha while just using an rc tag (e.g. 0.3.2-rc2), because then the version check of the release script didn't work as it just did a substring comparison with what the app version string reported (v0.3.2-alpha.rc2).
So I think we should also update the check_tag_correct function in scripts/release.sh to use the new get-git-tag-name.sh script and compare it to the checked out tag.

And to answer your other question, just having the status is what we want for non-RC releases in all other projects than this one (e.g. lnd).

version.go

Makefile

scripts/get-git-tag-name.sh

.github/workflows/release.yaml

lightninglabs-deploy · 2024-01-19T10:35:48Z

@ffranr, remember to re-request review from reviewers when ready

The modifications in this commit streamline version.go for easier parsing with bash, setting the stage for enhancements in a subsequent commit.

This commit removes the characters `.`, `-`, and ` ` from the semantic alphabet.

ffranr · 2024-01-24T17:09:34Z

Addressed (responded to) all review comments. New code pushed. Re-requested reviews.

guggero · 2024-01-29T13:08:07Z

One last issue with the tag name (#720 (comment)), then we're good to go 🎉

This commit introduces a makefile command that generates a git release tag by parsing the version string from version.go via the newly created script get-git-tag-name.sh.

This commit adds a check to this project's GitHub release workflow to ensure that the release version string is SemVer compliant. It also indirectly ensures that the version string matches the tag that would have been created with the `make release-tag` command.

This commit narrows the field of acceptable characters to 0-9 and a-z. The alphabet is compliant with SemVer but also stricter.

This commit modifies the release build script such that it checks the validity of the checked-out git tag by cross-referencing with the output of the `get-git-tag-name.sh` script.

guggero

tACK, LGTM 🎉

jharveyb

LGTM, great to see the alphabet changes + Makefile updates! 💯

ffranr requested a review from guggero December 5, 2023 12:35

ffranr self-assigned this Dec 5, 2023

ffranr force-pushed the validate-release-version branch 2 times, most recently from 20697bc to 9667434 Compare December 5, 2023 12:43

ffranr force-pushed the validate-release-version branch from 9667434 to 671d34e Compare December 5, 2023 15:23

dstadulis mentioned this pull request Dec 5, 2023

[doc]: Document version-tagging specification #715

Closed

dstadulis added the build label Dec 5, 2023

ffranr linked an issue Dec 7, 2023 that may be closed by this pull request

[feature]: generate git release tags via a new make command #729

Closed

ffranr changed the title ~~.github/workflows: ensure release tag version string is valid SemVer~~ makefile: add command to generate git release tags Dec 7, 2023

guggero removed their request for review December 13, 2023 09:01

ffranr force-pushed the validate-release-version branch 4 times, most recently from ee48da3 to 6187c40 Compare December 21, 2023 13:52

ffranr requested review from guggero and jharveyb December 21, 2023 13:55

guggero reviewed Jan 9, 2024

View reviewed changes

version.go Outdated Show resolved Hide resolved

version.go Show resolved Hide resolved

version.go Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

scripts/get-release-tag-str.sh Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

ffranr force-pushed the validate-release-version branch from 6187c40 to 781d160 Compare January 9, 2024 11:28

ffranr requested a review from guggero January 9, 2024 11:28

guggero approved these changes Jan 9, 2024

View reviewed changes

jharveyb reviewed Jan 10, 2024

View reviewed changes

ffranr force-pushed the validate-release-version branch 2 times, most recently from 3c588c4 to 693ec07 Compare January 11, 2024 15:39

guggero self-requested a review January 11, 2024 16:35

guggero requested changes Jan 12, 2024

View reviewed changes

version: separate status from pre-release

89a2377

The modifications in this commit streamline version.go for easier parsing with bash, setting the stage for enhancements in a subsequent commit.

version: simplify version semantic alphabet

3da789e

This commit removes the characters `.`, `-`, and ` ` from the semantic alphabet.

ffranr force-pushed the validate-release-version branch from 693ec07 to 1ca78d5 Compare January 24, 2024 17:08

ffranr requested review from guggero and jharveyb January 24, 2024 17:09

ffranr force-pushed the validate-release-version branch from aaa02b4 to b4b391e Compare January 29, 2024 15:10

ffranr added 6 commits January 29, 2024 15:23

makefile: add command release-tag for generating a release git tag

183b11a

This commit introduces a makefile command that generates a git release tag by parsing the version string from version.go via the newly created script get-git-tag-name.sh.

.github/workflows: enforce version check when building release binaries

6114235

version: narrow alphabet of acceptable version string field characters

c82d079

This commit narrows the field of acceptable characters to 0-9 and a-z. The alphabet is compliant with SemVer but also stricter.

scripts: make release checks git tag using script get-git-tag-name.sh

ee0b36a

This commit modifies the release build script such that it checks the validity of the checked-out git tag by cross-referencing with the output of the `get-git-tag-name.sh` script.

make: cluster version flags together for clarity

4d6aa3a

ffranr force-pushed the validate-release-version branch from b4b391e to 4d6aa3a Compare January 29, 2024 15:23

guggero approved these changes Jan 30, 2024

View reviewed changes

jharveyb approved these changes Feb 1, 2024

View reviewed changes

ffranr added this pull request to the merge queue Feb 1, 2024

Merged via the queue into main with commit 1ca17f6 Feb 1, 2024
14 checks passed

guggero deleted the validate-release-version branch February 1, 2024 16:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

makefile: add command to generate git release tags #720

makefile: add command to generate git release tags #720

ffranr commented Dec 5, 2023 •

edited

Loading

ffranr commented Dec 5, 2023 •

edited

Loading

ffranr commented Dec 5, 2023 •

edited

Loading

ffranr commented Dec 21, 2023 •

edited

Loading

guggero left a comment

guggero left a comment

jharveyb left a comment

jharveyb Jan 10, 2024

ffranr Jan 11, 2024

guggero Jan 12, 2024

ffranr Jan 23, 2024

jharveyb Jan 10, 2024

ffranr Jan 11, 2024 •

edited

Loading

ffranr Jan 11, 2024

jharveyb Jan 11, 2024

ffranr Jan 23, 2024

jharveyb Jan 10, 2024

ffranr Jan 11, 2024 •

edited

Loading

jharveyb Jan 11, 2024

guggero Jan 12, 2024

guggero left a comment

guggero Jan 12, 2024

lightninglabs-deploy commented Jan 19, 2024

ffranr commented Jan 24, 2024

guggero commented Jan 29, 2024

guggero left a comment

jharveyb left a comment

makefile: add command to generate git release tags #720

makefile: add command to generate git release tags #720

Conversation

ffranr commented Dec 5, 2023 • edited Loading

ffranr commented Dec 5, 2023 • edited Loading

ffranr commented Dec 5, 2023 • edited Loading

ffranr commented Dec 21, 2023 • edited Loading

guggero left a comment

Choose a reason for hiding this comment

guggero left a comment

Choose a reason for hiding this comment

jharveyb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ffranr Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ffranr Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guggero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lightninglabs-deploy commented Jan 19, 2024

ffranr commented Jan 24, 2024

guggero commented Jan 29, 2024

guggero left a comment

Choose a reason for hiding this comment

jharveyb left a comment

Choose a reason for hiding this comment

ffranr commented Dec 5, 2023 •

edited

Loading

ffranr commented Dec 5, 2023 •

edited

Loading

ffranr commented Dec 5, 2023 •

edited

Loading

ffranr commented Dec 21, 2023 •

edited

Loading

ffranr Jan 11, 2024 •

edited

Loading

ffranr Jan 11, 2024 •

edited

Loading