Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chartpress only considers tags in the current branch - it's too narrow, right? #143

Closed
consideRatio opened this issue Feb 21, 2022 · 3 comments

Comments

@consideRatio
Copy link
Member

When chartpress considers git tags to get a version for the images/helm chart, I think the logic it use is to take the latest tag on the current branch. I think we need to assume that we should use the latest tag on any branch instead.

chartpress/chartpress.py

Lines 150 to 157 in 9eb6b0a

def _get_latest_tag(**kwargs):
"""Get the latest tag on a commit in branch or return None."""
try:
# If the git command output is my-tag-14-g0aed65e,
# then the return value will become my-tag.
return (
_check_output(
["git", "describe", "--tags", "--long"],

@minrk
Copy link
Member

minrk commented Feb 21, 2022

I think we need to assume that we should use the latest tag on any branch instead.

I'm not so sure. Let's say we have a 1.x backport branch, which would be created from 1.0 or later (whichever was the last commit in master before forking to start the backport). For example:

Screen Shot 2022-02-21 at 12 38 16

where there's a 1.x and main branch. The 1.x branch has 1.1.1 and 1.2.1 tags. Both branches share a 1.0.0 tag, and the main branch only has 2.0.0.

The 'latest' tag in time is 1.1.2. This tag is only on the 1.x branch. The latest version tag is 2.0.0. This is only on main. Logically, the 1.x branch would get 1.1.2 as the latest tag, while main would get 2.0.0. This is what would happen with the current implementation using git describe. If we used either the latest-in-time (1.1.2) or latest-in-version (2.0.0) on both branches, one would always be wrong.

The case where we could arguably get the wrong answer is before the first tag on main is published, but after a backport release, so only 1.1.1 is published, no 2.0.0:

Screen Shot 2022-02-21 at 12 46 22

Here, 1.x will get everything right, while main would still get 1.0.0. Arguably, we could expect 1.1.1 on main, as that's the most recent release. But that doesn't make perfect sense, because main is not a descendent of 1.1.1 on the graph. We will get the accurate information that this is 5 commits after 1.0.0, but not any information about how it relates to 1.1.1, and, perhaps most importantly, the prerelease versions won't sort correctly until there's been a new tag on main (I think this is the main problem we need to solve for chartpress repos with backport branches).

For publish-from-dev repos like our charts, I think the best solution might be to publish dev tags when we start the backport fork (or when we would start the backport fork, if we decide to), which would also get the right info from git describe (jupyterhub/team-compass#471).

In short: I don't think this is a problem we should solve in chartpress.

@minrk
Copy link
Member

minrk commented Feb 21, 2022

Update: in jupyterhub/team-compass#471 I mentioned that chartpress may need an update to produce valid helm versions from dev tags in git. But I think that's probably all we should deal with in this repo to support the tags-on-multiple-branches situation.

@consideRatio
Copy link
Member Author

Given discussion provided by @minrk above, I agree fully using all branches latest version will be messy enough to not want to go for it - I'm inclined to think either we do something that is easy to understand and reliable, or not at all - and that considering all branches isn't easy to understand or reliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants