Skip to content
This repository was archived by the owner on Nov 1, 2022. It is now read-only.

snap: fix sorting of git tags #2772

Merged
merged 1 commit into from
Jan 21, 2020
Merged

snap: fix sorting of git tags #2772

merged 1 commit into from
Jan 21, 2020

Conversation

dholbach
Copy link
Collaborator

Make sorting of git tags during git tags more reliable.

@hiddeco
Copy link
Member

hiddeco commented Jan 21, 2020

What is the tag you are aiming for, so that I can wrap my head around the (changed) logic?

@stefanprodan
Copy link
Member

You may want to exclude the releases that start with v or include only those. Due to go mod we duplicate every release e.g. v1.17.0 and 1.17.0

@dholbach
Copy link
Collaborator Author

I'm having a similar discussion with Martina at eksctl-io/eksctl#1742 (comment)

Our approach worked up until now, but broke in the case of eksctl because of e.g. 0.13.0-rc.0. Letting git handle the order of tags is more reliable.

The idea is to get the last available tag.

@hiddeco
Copy link
Member

hiddeco commented Jan 21, 2020

@dholbach the git tag --sort=taggerdate output does not look very reliable to me (or at least, not what the sort name suggests it to be), as on my local system the output looks like this:

hidde@baldr:~/Projects/flux$ git tag --sort=taggerdate
0.1.0
0.2.0
0.3.0
1.0.0
1.0.0-beta
1.0.1
1.0.2
1.1.0
1.10.0
1.10.1
1.11.0
1.11.1
1.12.0
1.12.1
1.12.2
1.12.3
1.13.0
1.13.1
1.13.2
1.13.3
1.14.0
1.14.1
1.14.2
1.15.0
1.16.0
1.17.0
1.17.1
1.2.0
1.2.1
1.2.2
1.2.3
1.2.4
1.2.5
1.3.0
1.3.1
1.4.0
1.4.1
1.4.2
1.5.0
1.6.0
1.7.0
1.7.1
1.8.0
1.8.1
1.8.2
1.9.0
chart-0.10.0
chart-0.10.1
chart-0.10.2
chart-0.11.0
chart-0.12.0
chart-0.13.0
chart-0.14.0
chart-0.14.1
chart-0.15.0
chart-0.16.0
chart-0.2.2
chart-0.3.0
chart-0.3.2
chart-0.3.3
chart-0.3.4
chart-0.4.0
chart-0.4.1
chart-0.5.0
chart-0.5.1
chart-0.5.2
chart-0.6.0
chart-0.6.1
chart-0.6.2
chart-0.6.3
chart-0.7.0
chart-0.8.0
chart-0.9.0
chart-0.9.1
chart-0.9.2
chart-0.9.3
chart-0.9.4
chart-0.9.5
chart-1.0.0
chart-1.1.0
helm-0.1.0-alpha
helm-0.1.1-alpha
helm-0.10.0
helm-0.10.1
helm-0.2.0
helm-0.2.1
helm-0.3.0
helm-0.4.0
helm-0.5.0
helm-0.5.1
helm-0.5.2
helm-0.5.3
helm-0.6.0
helm-0.7.0
helm-0.7.1
helm-0.8.0
helm-0.9.0
helm-0.9.1
helm-0.9.2
v1.15.0
v1.16.0
v1.17.0
v1.17.1
pre-split
master-6cc08e4
master-ccb9a99
master-0d109dd
chart-0.2.0
chart-0.2.1

What about using the available GitHub API for releases, and do something like: curl -s https://api.github.com/repos/fluxcd/flux/releases/latest | jq -r .tag_name? This produces the following:

hidde@baldr:~/Projects/flux$ curl -s https://api.github.com/repos/fluxcd/flux/releases/latest
| jq -r .tag_name
1.17.1

@dholbach
Copy link
Collaborator Author

curl -s https://api.github.com/repos/fluxcd/flux/releases/latest | jq -r .tag_name

Sure this would work, or we could go with this instead:

git tag --sort=committerdate |  egrep -v '^(v|chart-|helm-|master-|pre-split)'

@dholbach
Copy link
Collaborator Author

This is really weird. For weaveworks/eksctl only taggerdate will work. Maybe I'll change to your suggestion in the end.

If would've been nice to just deduce this from the git tree, but... 🤷‍♂️ ...

@stefanprodan
Copy link
Member

@dholbach can you please rebase with master instead of merging it. Isn't curl needed in the build-packages list?

@dholbach
Copy link
Collaborator Author

Sorry. Forgot to rebase. It appears curl is already present by default. For me the build passed just fine in a clean vm.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dholbach dholbach merged commit 3903cf8 into fluxcd:master Jan 21, 2020
@dholbach dholbach deleted the snap-fix-git-tag-sort branch January 21, 2020 14:07
@2opremio 2opremio added this to the 1.18.0 milestone Jan 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants