(reverted) Add useChartVersion and change appended version suffix (now like 1.2.3-0.dev.git.3.h123)

[minrk]

Instead of 1.2.3-n003.habc produce 1.2.3-0.dev.git.3.habc

Problem

Our current dev versions don't sort correctly for various reasons:

• n999 and n1000 sort alphabetically, resulting in incorrect ordering
• prereleases are relative to the most recent tag, which results in incorrect ordering after every release (1.2.3 > 1.2.3-n003.habc, when it should be the other way around) unless repos publish a prerelease tag immediately after every stable release

The only way to get prereleases to sort correctly is to produce them relative to the next release, not the previous release, which we get from git describe.

Plus, the version information is misleading - there's no indication of whether a dev version is preparing for a minor release or a major one - only that it's after some version in the past.

Proposal

• Use numerical field instead of n012 zero-padded text fields for commit count. New tag scheme appends .git.{n}.h{sha} to prereleases (e.g. 1.2.3-alpha.1 -> 1.2.3-alpha.1.git.5.habc), so commit counts are compared numerically instead of alphabetically.
• Uses leading 0.dev. for base prerelease versions, if building a dev version from a non-prerelease version (e.g. 1.2.3 -> 1.2.3-0.dev.git.5.habc) so that it sorts before alpha
• new config (opt-in) useChartVersion to use the version in Chart.yaml as the base version, instead of the most recent git tag.
• When using the new mode, chartpress --reset resets the version by stripping the .git.{n}.h{sha} suffix, leaving the base version alone, rather than clobbering with invalid set-by-chartpress tag.

Notably, this does not solve the prerelease < stable release ordering by default, it only allows repos to solve it by opting in to the useChartVersion config, and specifying the dev version in Chart.yaml, as is standard practice in software packages (1. tag a release, 2. set version to $next-dev). I think this is the right approach, becuse the next version is not guessable, so it has to come from the maintaners somehow, either:

1. in Chart.yaml (this implementation)
2. chartpress.yaml (also works, but Chart.yaml seems simpler to me)
3. in a git tag (forces unnatural release process)

EDIT: original description

When we added the n00 prefix, I misunderstood semver restrictions about leading 0s. We can have leading 0s on fields that aren't just digits (e.g. 0git is okay, but 05 is not), so the prefix was only strictly necessary on the hash, not the count. And we can have fields that are exactly one 0 (.0. is okay, .00 is not). So I think the n00 prefix was not the best solution - n.5 is better than n005. It's correctly sortable (n.1000 > n.999, unlike n1000 and n999), and separates the numerical field from the 'channel' field.

Further, once we have two fields, I think we should sort before 'alpha', so that a pre-alpha dev release (1.2.3-$tag) will sort before the following alpha (1.2.3-alpha.$tag). I chose 0git for this, but I'm not sure that's best. Coud use 0.dev.git

To this end, I propose changing our tag scheme from n{n:03}.h{hash} to 0git.{n}.h{hash}:

• allows fully numerical fields to sort properly
• still avoids leading 0s on numerical fields
• prefix with 0git. to sort before 'alpha.'

Other options:

1. No 0git. prefix, just use numbers. This might be confusing for a version that already has a number, like 1.0.0-alpha.1.5.habc or --long tags with no commits like 1.0.0-0.habc.
2. use just 0: 1.0.0-alpha.1.0.5.habc
3. use just git or n - I like these better aesthetically, but they have the problem that they sort after alpha and beta, so 1.2.3-git.5.habc > 1.2.3-alpha.1

closes #174

[manics]

Another related idea- include something along the lines of bump2version or tbump in chartpress to reduce the cognitive load in figuring out the version and ensuring you don't accidentally create a tag that doesn't appear in the correct. The above discussion about rc/alpha/beta etc is quite complicated, so being able to tell chartpress to bump a version based on a limited choice (e.g. major/minor/patch/dev/rc) could be helpful instead of making it completely free text as in #152?

[minrk]

Yeah, integrating with tbump might be nice for setting the version.

[minrk]

A proposal describing the logic on how: a) chartpress manipulates some version to set a dev version

Given a base version (last tag from git describe, or 'current' version from chart):

1. if already a prerelease, append .git.{n}.h{sha} to the prerelease fields.
2. if not a prerelease, append -0.dev.git.{n}.h{sha} to ensure it sorts before '-alpha' (we could leave out '.dev')

Given 5 commits from the last tag and sha='abc', this would produce tags:

• 1.0.0 -> 1.0.0-0.dev.git.5.habc
• 1.0.0-xyz -> 1.0.0-xyz.git.5.habc
• 1.0.0-alpha.1 -> 1.0.0-alpha.1.git.5.habc
• 1.0.0-0.dev -> 1.0.0-0.dev.git.5.habc

Importantly, this does not solve the "dev versions sort after releases" if you don't opt-in to the version-from-chart scheme or use dev tags. This is the situation we have today, and I think it's appropriate to say that you should get the version from the chart if you want prereleases to be relative to the releases they are actually 'pre.' I don't think we should be guessing what the next release might be.

b) chartpress decides the version to manipulate,

opt-in, use the version in the chart exactly as-is (optionally: strip a standard '-set.by.chartpress' suffix):

If `config["versionFromChart"]`:
    base_version = chart["version"]
else:
    base_version = last_tag_on_branch

and c) we should change release documentation in a helm chart repo using chartpress.

Yes, I believe it should be:

V=2.0.0
# equivalent of tbump $V
chartpress --reset --tag "$V"
git commit -a -m "bump version to $V"
git tag -a -m "release $V" "$V"

NEXT_V = "2.1.0-0.dev"
# equivalent of tbump --no-tag $NEXT_V
chartpress --reset --tag "$NEXT_V"
git commit -a -m "bump version to $NEXT_V"

git push --atomic
# next published chart version will be 2.1.0-0.dev.git.1.habc

Optionally, we could have chartpress execute the git commit/tag, like tbump does.

You would get the same versions if you skipped the versionFromChart scheme if you tagged the commit after release as 2.1.0-0.dev (n would be lower by 1, but otherwise identical).

The NEXT_V step is only necessary for releases, not prereleases (alpha, beta, etc.). If you skip/omit the additional NEXT_V commit, the only downside is that your prereleases will evaluate below your last release (as is already the case today).

[minrk]

I think post-release refers to + parts in a semver version, which is something helm won't handle well.

Yes, I should have been clearer. It's the right place semantically, but it's not available to us, which is why we have the issues we are having now - putting information relative to the last release in a field counting down from it.

Only the Helm chart's version needs to be updated, not the images.

True, and this is what chartpress --reset --tag x currently does (chart.resetVersion and chart.resetTag are set via different paths). This might be a bug! But we can make a new entrypoint, or add further clarification of what to do when, if that would be helpful.

EDIT by Erik: I opened #174 about this.

We have a pre-commit git hook in z2jh that makes us check that chartpress --reset doesn't lead to changes

I think chartpress --reset stripping the .git.n... suffix instead of totally resetting the whole version string ought to do the right thing here.

One idea i consider is to introduce new configuration in chartpress.yaml, that when set, will be used as a version to append build suffix to, whenever no explicit git tag is set.

I think we do need something in chartpress.yaml to make the new behavior opt-in. I'm not sure it should be the version itself, though. I think that should be in the Chart.yaml, since there's already a field for it and we should use it. I think it should be your option b): "if chartpress.yaml config says it should" as that's the simplest, most explicit choice. Nothing to infer from something else.

chartpress --reset maybe shouldn't update the version entry in Chart.yaml

I think the right thing to do here is to strip the .git... suffix rather than clobber the whole string regardless of its value. This would also fix the chartpress --reset pre-commit issue.

[consideRatio]

Not feeling confident about this still, but I made a review pass. By mistake I made comments associated with a specific commit instead of the entire PR =/

[minrk]

@consideRatio resolved conflicts with #173

[consideRatio]

Thanks @minrk! I'm looking at this now and trying to go through all the comments and understand the current state and summarize the situation and what we are going for.

I hope it is okay that i mark comments as outdated, this PR has lots of discussion that it becomes hard to overview. I figure hiding disucssion that led up to the current state makes it more managable - if not only for myself.

[consideRatio]

Thanks @minrk for your work on this!

I think this is acceptable pretty much as it is, I left some smaller comments and I didn't review the tests but I hope its okay to go for anyhow.

[consideRatio]

I found logic introuced that with the chartpress CLI is unreachable that I'm not sure how it makes sense to rework.

addressed with comments

[minrk]

@consideRatio I tried to make the reset logic clearer by doing more in build_chart so that the version to reset to is set in the same place, whether useChartVersion is true or false.

I also added more test coverage, including for #174 to show that it is indeed fixed.

[consideRatio]

Thank you @minrk!!!

[minrk]

Thanks for your thoughtful review!

[consideRatio]

For visibility, I've opened #176 and #177 as followups to this PR.