check consistency of tags that we push

[marten-seemann]

It would be nice to run gorelease in CI, every time when a new v* tag is pushed. That would allow us to verify that the release obeys semver rules and we don't break downstream users, as we've done before (multiple times).

Unfortunately, gorelease doesn't support verifying of existing tags (see golang/go#47532), it can only suggest an appropriate version number before the version is released.

Alternative Suggestions

Imagine that we have one text file ./version per repository, which contains a single line with the current version number, e.g. "v0.5.2". Whenever we want to cut a new release, we'd send a PR to bump that version number, e.g. to "v0.5.3". That would give us two nice properties:

1. Version bumps could be reviewed.
2. We could have a GitHub Actions workflow running gorelease. If the version number from the PR is smaller than what gorelease suggests, that workflow would fail.

Once the PR is merged, we'd have another GitHub Actions workflow that would create and push a new release using that version number.

The main problems with this suggestion are:

1. There's no way to forbid manual pushing of tags. This is a deficiency of GitHub, that the company has failed to address for a long time.
2. We'd have to teach everyone in the company to use this new process.

Thoughts, @Stebalien and @mvdan?

---

Some recent (non exhaustive) examples where this has come up:

• 2021-09-25:
  • libp2p/go-libp2p-peerstore@v0.2.8...v0.2.9
  • https://protocollabs.slack.com/archives/CDF4W878S/p1632571272080600
• 2021-10-04
  • Feat/datamodel selector retrieval filecoin-project/lotus#6393 (comment)
  • https://protocollabs.slack.com/archives/C01QL4RUA79/p1633387776401700

[mvdan]

My main worry is that, while this would be able to spot mistakes, it would often do so when it's too late - once the tag is already pushed. Trying to change the workflow (e.g. changing a "version" file first) seems invasive enough that most developers probably won't be convinced.

It seems like GitHub will be further enhancing repo protections and permission roles though, so hopefully at some point we can disable straight tag pushes, much like how one can disable straight master pushes. For example: github/roadmap#166

[marten-seemann]

My main worry is that, while this would be able to spot mistakes, it would often do so when it's too late - once the tag is already pushed.

I fully agree that this is a hack. But finding the problem a little bit too late is much better than finding it a lot too late. The fix is easy: just create a new (higher) patch release that doesn't contain breaking changes. If you manage to do that in a short enough time frame, most downstream users won't even notice that you messed up.

so hopefully at some point we can disable straight tag pushes, much like how one can disable straight master pushes

There's a feature request to protect tag pushes from Feb 19: https://git.luolix.topmunity/t/feature-request-protected-tags/1742. It doesn't like there's any interest from GitHub's side to implement this, although people have been begging for this feature for years. Maybe I'm misreading the roadmap, but they seem more concerned with enforcing rules on commits, but it doesn't even mention the word "tag" once.

[mvdan]

I think your point about "a little too late" is good - the follow-up fix release can also retract the bad tag, meaning that the bad version should be invisible to people who are navigating or searching versions.

You're right that there's no ETA or guarantee that tag protection will happen. I was only mentioning that they seem to be improving the settings in its vicinity, so I remain hopeful they'll do something at some point. We shouldn't sit around and wait, though. And even if we do get tag protection, we'd still need to come up with some alternative way for developers to publish tags safely.

Here's another thought: big enough projects will likely be releasing betas or RCs before the final release. If this tag workflow can sound the alarm when one of those tags breaks backwards compatibility, that can be fixed before the final stable release.

[BigLep]

@marten-seemann : thanks for writing this up. This is awesome.

1. "We'd have to teach everyone in the company to use this new process." Do we really have to teach everyone? Isn't most of the PL go package releasing happening within the w3dt Stewards team? I believe that's a doable set to have to educate and get onboard. Also, even getting some coverage here is better than none right? For example, I assume if only key package like go-ipfs, go-libp2p, etc. followed this, it would still help a lot.
2. Your proposal seems sane and makes sense. Has anyone already built/documented this that we can just copy or use it?

[iand]

I'm in favour of having version information in (or alongside) the source code, keeping it independent of the source control and build system. Many other OSS projects take this approach (protobuf-go / go-ethereum / consul / drone / gin ).

I've been looking for an elegant way to do this for a long time and now, with Go's new file embedding, I think it can be very simple:

1. Add a version file as @marten-seemann suggests, containing only a semantic version
2. For Go repos, embed this file using //go:embed so that it is available at runtime (for --version or similar output)
3. The CI system uses a tool like gorelease to verify that the version is in a compatible sequence with the previous version in the main branch.
4. After a merge to main branch, auto tag the repo with the new version.

That keeps everything consistent. The build system can still inject a git hash if desired but the base version of the software remains the same no matter how the user builds it.

[Stebalien]

We usually use https://pkg.go.dev/runtime/debug#ReadBuildInfo to get the version number of libraries so embedding is really only useful for top-level repos.

But yeah, doing this through PRs would be really nice. Although I'm slightly concerned that we have too many repos:

1. More difficult to cut a release (maybe not terrible).
2. More difficult to tag a patch release (requires a branch). Again, maybe not the end of the world.

I'd try it on an active repo or two and see how it goes. Don't embed yet, that's too hard to revert.

[marten-seemann]

@aschmahmann pointed out that there are few changes that are truly backwards-compatible in Go, as described in this excellent article: https://blog.merovius.de/2015/07/29/backwards-compatibility-in-go.html.
It turns out that gorelease applies a very strict definition: For example, just adding a field to an exported struct, would be treated as a backwards-incomptabile change:

❯ gorelease
# github.com/libp2p/go-libp2p-tls
## compatible changes
Identity.NewField: added

# summary
Inferred base version: v0.2.0
Suggested version: v0.3.0

This is more strict than what the Go team uses for its own releases: they regularly add fields to the crypto/tls.Config. I'm not sure if their approach to backwards compatibility is outlined anywhere, maybe @mvdan knows more?

I'm leaning towards taking a more relaxed approach to backwards compatibility that would be more in line with (what I've observed from) the Go team. Unfortunately, that would mean that we can't rely as heavily on gorelease any more, and most certainly not as a yes / no decision in CI. What we could do instead is just have web3-bot comment the output of gorelease on the PR, and maybe approve the PR if it finds the new tag acceptable (but not block / dis-approve if it doesn't).

[mvdan]

Technically speaking, adding a field is indeed a breaking change. Going from type T struct{A int} to type T struct{A int; B string} breaks T{3} for example, unless the user follows vet and uses T{A: 3}.

There are other kinds of "soft breaking changes", which are technically backwards incompatible, but in practice are very unlikely to break any users unless they do funky stuff. For example, moving a type from one package to another, leaving a type alias in the original; if one inspects the type via reflection, it changes from orig/pkg.Type to new/pkg.Type. Another example is changing func TakesBool(b bool) to type NamedBool bool; func TakesBool(b NamedBool), which won't break TakesBool(false), but can break other users.

I thought gorelease followed the same rules that are applied to the standard library, though. I think something's wrong if that's not the case - either the docs for crypto/tls are misleading (they should mention that fields will be added to Config), or gorelease should have a mode that's compatible with this more relaxed/practical take of backwards compatibility.

Seems worth asking upstream. Using gorelease as a suggestion, rather than enforced check, seems sane to me - the tool is experimental for now, anyway.

[galargh]

This has already been implemented as https://github.com/protocol/.github/blob/master/VERSIONING.md 🎉