Oddities in branching structure

[benknoble]

Ok, it's late, I'm tired, and I've spent a lot of time staring at git log --oneline --graph output, so forgive me if this is incoherent

I (re?)discovered that git describe <commit> is generally not very helpful in the Racket source repo, and it took me quite a while to connect the dots to Proposed stable branch discipline. In essence, our release branch structure looks something like this:

# git log --oneline --graph origin/master v8.18 ^v8.17
[…] origin/master
* 02afeae0aaf Post-release version for the v8.18 release
| * 657a006747c (tag: v8.18, origin/stable) Update version number for the v8.18 release
| * fd356c40969 Merge branch 'stable' into release
| * 2924d2a6934 update makefile for v8.18 release
| * fa9a291badf repair use of SO_KEEPALIVE
| * 211c85dfbdb HISTORY.txt: Copyediting on the 8.18 notes
| * 9025155e36d Racket HISTORY.txt notes for v8.18
| * 9a3f3c3440d configure: natipkg should use own zlibs
| * 9e63746bf51 fix spelling of "uninterruptible"
| * c5133edd3ca Alpha version number for the v8.18 release
|/
* 3cdfe1e0584 Fix sign of lcm of negative non-integers
* 8964d417e9b Chez Scheme: fix lcm signature
[…]

The (faux) merge of the branch stable is where we merge the old v8.17 into the new v8.18^[1]. Here's the same graph, but with v8.17 included and v8.16 excluded:

# git log --oneline --graph origin/master v8.18 v8.17 v8.16
[…]
* 02afeae0aaf Post-release version for the v8.18 release
| * 657a006747c (tag: v8.18, origin/stable) Update version number for the v8.18 release
| *   fd356c40969 Merge branch 'stable' into release
| |\  
| | * a7b6a6b6f5e (tag: v8.17) Update version number for the v8.17 release
| | * 61980389648 Merge branch 'stable' into release
| | * a9a11615dda update makefile for v8.17 release
| | * 07d65da50b6 json docs: fix history notes for `#:replace-malformed-surrogate?`
| | * 0c04ed5d0d0 Racket HISTORY.txt notes for v8.17
| | * 6c8a4197674 configure: try `-S` with `strip` by default
| | * ea849204d5f Alpha version number for the v8.17 release
| * | 2924d2a6934 update makefile for v8.18 release
| * | fa9a291badf repair use of SO_KEEPALIVE
| * | 211c85dfbdb HISTORY.txt: Copyediting on the 8.18 notes
| * | 9025155e36d Racket HISTORY.txt notes for v8.18
| * | 9a3f3c3440d configure: natipkg should use own zlibs
| * | 9e63746bf51 fix spelling of "uninterruptible"
| * | c5133edd3ca Alpha version number for the v8.18 release
|/ /  
* | 3cdfe1e0584 Fix sign of lcm of negative non-integers
* | 8964d417e9b Chez Scheme: fix lcm signature
* | c95ce54ad1d Chez Scheme: add real?, rational?, exact? and inexact? to cpprim
* | 4a4c31e3b27 ffi/unsafe/atomic: add `unsafe-{start,end}-uninterruptable`
* | 72256e40d0c doc: update Guide and Reference on futures
* | 1fce381d2eb extracter: rewrite, bugfix, and test mutated-vars
* | 19b20dadace cs io, thread: tweaks to appease a small extractor change
* | 95ef65a7b23 net,http-client: handle RST during body read
* | 1718693e247 Chez Scheme: Add vector-copy!
* | bb286e74b37 Chez Scheme: fix LaTeX typo in release notes
* | b676e176e38 Chez Scheme: Treats # as a delimited in R6RS mode when reading numbers
* | 55addd10221 Chez Scheme: Allow setting history file location from environment
* | 9dd9ebdabc7 Chez Scheme: Rewrite build-let in cptypes
* | 02439f4a99c docs: fix docs, `racket/mutability` not in `racket`
* | 1ba09184c80 document `configure` flags in "src/README.txt"
* | 834996cd7fa configure: add `--guibindir=<path>`
* | 333edbbcaec configure: infer `--enable-missingpkgs` default
* | 2a408328b66 ffi/unsafe/objc: use `(ssytem-type 'arch)` instead of `(system-library-subpath)`
* | 7316999bec2 configure: add `--enable-nonatipkg`
* | 087f57b86b2 configure: add `--enable-missingpkgs` and `--enable-pkgs=<pkgs>`
* | ea7065d3206 build: repair unixstyle-install Mach-O executable test
* | 05bb6374001 native-libs: patch for Cairo PDF-font-substitution issue
* | 974f1f24ce2 syntax/parse: fix fixup pass
* | 85ecfcb8730 build: make `DESTDIR` install work for Mac OS
* | 5049fbde5c4 build: include entitlements when signing on XONX
* | 3b00fc3a706 Chez Scheme: correct `powl` that was intended to be `pow`
* | 7dbcc9f68c8 BC: update to Unicode 16.0
* | c7588a9fc9a sync tests and BC for `expt` improvement
* | de67e337203 Chez Scheme: improve `expt` on a flonum to a large exact power
* | ec011165a46 Allow deprecated aliases of top-level bindings
* | 7357b806ad5 Require deprecated alias targets be bound
* | 0fe086b70b3 syntax/parse: handle zero clauses case
* | 0ad0f03e0ae net/imap: add `imap-move`
* | ac0488ac146 remove obsolete build files
* | a01b41bee62 Add deprecated aliases as `define-deprecated-alias`
* | f3e7e127351 doc: (guide) Fix grammar bugs in two files
* | f45b2248f44 doc: (inside) Fix grammar bug in values.scrbl
* | f9613a02b9f doc: (reference) Fix grammar bugs in paths.scrbl
* | 0bb2358cf3d Chez Scheme: add cflonum to cptypes
* | 2c03772c7ab Chez Scheme: Fixed some typos in BUILDING document.
* | 6a04819773a Chez Scheme: Unicode 16.0.0
* | 8c3fa61df80 Chez Scheme: Fix typo in macppc machine-type in release notes
* | c692153fba2 Chez Scheme Version 10.3.0-pre-release.1
* | a3a1f1ba49a Chez Scheme Version 10.2.0
* | ce09ec53ba7 Chez Scheme: Empty dir-path in path-build no longer causes invalid memory reference
* | 0be56d7d159 Chez Scheme: the --help option now uses stdout remove unneeded fflush
* | 41edd9cf010 Chez Scheme: Flush stdout when printing the version
* | 56e753e64d3 Chez Scheme: in the Section -> in Section
* | 92199e4e25a Fix the document of `racket/signature`
* | 5d65c77a6c8 xml: make structs serializable, but enforce contracts
* | 26f9654f101 xml/xexpr: pcdata structs are not X-expressions
* | bf08ef2e52e struct: add `#:properties prop-alist-expr` option
* | 3104f290b0f distro-build: pull `boot/pb` to include in source
* | c6ee64d11b9 build: propagate `RACKET=...` to nested targets
* | c48608de364 ustart: adjust readlink argument size
* | 420f110a61c Bump Bogdanp/setup-racket from 1.13 to 1.14
* | 16aff9a2bb4 distro-build: repair directory clean up again
* | b566e42f367 distro-build: repair directory clean up
* | 1d3fed17ab3 distro-build: use a Docker container to clean up created directories
* | 21435fa336b tell distro-build/pack-built which packages to catalog
* | b9b017261bf io: repair wrong-variable use in checking for special Windows paths
* | a874b1ecdf1 expander: fix variable distnctness in extraction simplifier
* | d8dd6d9a98f fix repair of cross-read of ".zo"
* | d6ed6721470 repair `zo-parse` after internal change to `read-linklet-bundle-hash`
* | 4b4e5679f1c BC: unbreak after change to ".zo" reading
* | f036f1e67f6 repair cross-read and cross-write of ".zo"
* | 06f13d69a24 io: fix fallback for (find-system-path 'host-addon-dir)
* | b90bbcd09b8 Bump Bogdanp/setup-racket from 1.12 to 1.13
* | 49b60212df5 Fix function name typo in pkg.scrbl
* | 6787b924f89 guide: show that `init-expr`s can't see `proc-id` in named `let`
* | 0a9ae9e1381 build,ac: use xcode-select to infer SDK paths
* | ee6bf5ff83c racket/class: add support interface method default implementations
* | 13916d2973d Mach-O: write section for embedded segment
* | 579de774492 Revert "CS configure: turn off `strip` by default"
* | 696d5af52ed docs: note the default imports provided by single-{unit,signature} modules
* | 714f6fc45a2 BC: repair JIT bug related to `case-lambda`
* | 6b4649ed489 Fix typo
* | 72f7bf7da82 repair docs for `compiled-expression-summarize-target-machine`
* | 43615483a6b docs: add `compiled-expression-summarize-target-machine-info`
* | a358cc5731e Link to `raco demod` in `raco exe`
* | fe63fef2a3e reference: clarify `datum->syntax` recursive conversion
* | 40faf7b6fa4 Make the deftech for "X-expression" point somewhere useful
* | d4ed2a1218a BC: add missing cast in new linklet function's implementation
* | 7da21be6c31 update Racket Build Guide with new tips on building distributions
* | 8bea2d7e8c5 makefile: add `clean-clients`
* | 65864a2bd69 add recompilation-cache option to `raco setup`
* | 640468361af distro-build support for no-installer mode
* | a96b5bbb08a fix expected results for contract-equivalent?
* | 757ac2cd9cc add `(find-system-path 'host-addon-dir)`
* | 19f0cf90d79 repair non-cross client distro-build
* | b3880684089 remove broken contract optimization
* | be8f33843f2 repair libffi compilation for AArch64
* | 8774f940c19 build: clear log directory when starting installers
* | aec1c4f38c0 "compiled/ephemeral": use for `compiler/demod`, auto-prune for pkg
* | 10a17d63307 restore use of `use-compiled-file-check` in load compiled handler
* | eccd36c22b6 ci: bump Ubuntu runners to 22.04
* | 7722ce74308 build guide: goal-oriented signposting and other clarifications
* | 9af6be2e4b8 zuofile: repair configuration-argument `pwd` normalization
* | d885eeaac3f CS: embed boot files properly for PPC Mac OS
* | bc154d3a678 Fix Mach-O handling for cross-compilation with different endianness
* | 7d60ed374b3 CS configure: turn off `strip` by default
* | 669af4c0b15 remove obsolete Mac OS resource step
* | b2c11bcad2f propagate rcodesign configuration for distro-build
* | b61f978c3c3 repair "system.rktd" and boot file embedding for Mac OS cross build
* | ba98a416ad1 json docs: fix history notes for `#:replace-malformed-surrogate?`
* | 2d788eb7deb Racket HISTORY.txt notes for v8.17
* | 75d1b5d9b8c typo: in -> and
* | fe5e3342f4f configure: try `-S` with `strip` by default
* | c67c320a508 Post-release version for the v8.17 release
|/  
* f9f912874e2 Use Bogdanp/setup-racket@v1.12 in `raco pkg new`
[…]

The net result of this structure is that git describe v8.18~10 (just to pick an arbitrary commit), which I might usually expect to be part of v8.17… is described as v5.0.1-28916-g8964d417e9b! That's because v8.17 (and most of the other "modern" versions) aren't actually reachable from main commits, so we track back all the way to v5.0.1. This suggests that commit was based on v5.0.1, even though we all know that's silly

Of course, the query I'm usually interested in ("which version has this commit?") with git describe --contains works just fine.

So it's not a problem per se, but it is unusual to me. For example, taking the Git source code, git describe v2.51.0~200 gives v2.49.0-725-gb4847a4477—right, that commit is roughly based on top of v2.49.0. With --contains, I see it was released in v2.50.0. Makes sense.

I don't know that I have a proposed solution at the moment—I'm pretty unfamiliar with the process at play here and this may not be the kind of thing that needs a solution. OTOH, the commit graph may also be confusing to contributors who go history spelunking to look for more context. I would normally think of changes in v8.18 as being descendants of v8.17, and that is not the case (even though v8.18 itself is such a descendant).

It seems the way Git makes this work is by not cutting release branches; non-maintenance releases are tagged directly off master. Commits might continue landing in next while master is frozen for release, and then these are eventually merged (separately) to master once unfrozen. That sounds like a big change for Racket, so I'm not proposing we go down that route just for one person's complaint about a somewhat-obscure corner of Git And at least ranges like v8.17..v8.18 still work just fine!

Perhaps an interim solution would be to tag the "release cuts" like v8.18-cutoff or something; when I do that locally, I get somewhat satisfying results:

# git tag v8.18-cutoff origin/master^{/Post-release}^
# git describe --tags
v8.18-cutoff-98-g5f7f2f5aabf

I'm curious what folks think? I'd also love to learn more about the current process and how it came to be, if such information exists.

---

1. But we don't really merge changes: v8.17 has a few of these "release" commits whose changes we discard with -s ours. ↩︎

[ryanc]

How we use git branches is described as part of the Racket Release Process, which is documented on the following wiki pages:

• Release Overview -- General information and schedule
• Release Process -- Describes the git branch discipline for releases

I don't know if there's a document describing how the stable branch is managed. The discussion you already linked might be the best resource on that.

[jbclements]

I agree with you that the merge graph of the released versions looks appalling.

I'm almost sure that I don't currently have even a small fraction of the possible disciplines or their consequences in my head right now, so I'm speaking from a place of partial ignorance, but it looks to me like our current process is built around the primacy of master, and the consistency of developer experience around "continue to commit to master as usual". Oh! I also think there was the desire to have the previous release be an ancestor of the current release, which would (uh, probably?) not be the case in the discipline I think you're proposing.

Honestly, I wonder what @LiberalArtist has to say here?

[LiberalArtist]

@benknoble

it took me quite a while to connect the dots to Proposed stable branch discipline

I don't have all of this paged in, but I've got some pointers to discussion that came before that thread. I believe the issue of the current stable branch structure was first raised here:

@LiberalArtist

With the caveat that I don't know how this would work for other aspects of the release process, here's an idea that would make sense to me from a Git perspective, partially inspired by the sort of "soft launch" we tried this release cycle. It also tries to address a much less significant quirk I noticed, which is that the head of the stable branch tends to point to a merge commit like Merge branch 'stable' into release · racket/racket@424d167 · GitHub rather than the commit pointed to by the release tag.

I took a first crack at putting that proposal into practice in Make `release` a descendant of `stable` by LiberalArtist · Pull Request #4460 · racket/racket · GitHub. We picked that back up with:

@LiberalArtist

@jbclements, just wanted to make sure you saw this: Make release a descendant of stable by LiberalArtist · Pull Request #4460 · racket/racket · GitHub

@jbclements

Yes. Did my response (sent on a plane so who knows) actually get to you?

@LiberalArtist

Yes, I've just written back. Tl;dr the simple part should be simple, but you were right about:

@jbclements

One challenge in testing this manual makefile update are that the catalog itself must be available in order to test it, which is why this update occurs after testing is complete.

From the off-list email chain with @jbclements:

@jbclements

D’oh! I can’t accept this PR using the GitHub interface, since merge commits are disabled for our repo in the GitHub settings …

So I decided to just run your commands on my own release branch.

I noticed … [t]he repo graph after the merge with stable is is totally horrible. But AFAICT it’s horrible because of what we’ve done to stable in the past, and not any worse than the existing graph of stable.

We dropped the notion of delaying creation of the Git tag after this comment by @elibarzilay, plus some complications @jbclements pointed out in those off-list emails, so we focused just on organizing the Git history for the release and stable branches:

@LiberalArtist

@elibarzilay

• Re your suggestion of not creating the tag until some testing, the original motivation for creating the tag as soon as the version is bumped (for releases) is to avoid a situation where there are two builds with the same version number that are not the same code, so there's no chance of confusion. There was even at some point a bit in the release where I faked the new version number so I can update things like screenshots (and some Shriram-related things too?). Probably mostly moot now.

This makes sense. (@jbclements also pointed me to some complications off-list.) If we try to revisit this idea next release, one thing I noticed is that release-catalog/scripts/add-tags.rkt at master · racket/release-catalog · GitHub currently creates tags using the GitHub API: maybe creating tags locally could keep some benefits but be more flexible? But I also think it might be a better use of effort to just do as @samth suggested elsewhere:

@samth

In general it seems like we should consider figuring out how to make point releases easier to make, since downstream consumers are unhappy when we change things after the release.

Interestingly, it seems we did discussing merging stable into master in a way that would address the git describe quirk:

@LiberalArtist

@elibarzilay

• By some cosmic coincidence, I went over this branching approach in the last few weeks (due to $work), and implemented roughly the same strategy, automated via a github action. One thing that I've noticed (~two days ago) is that due to merges going only from master to release to stable but not the other way, GH shows stable as being way ahead of master. I figured that an improvement there would be exactly the kind of -s ours fake merge to avoid that confusion.

Interesting! To be concrete, are you suggesting that, after tagging the release and updating stable, we should git checkout master && git merge -s ours stable? That seems like a nice enhancement! In addition to showing which branch is "ahead", it would also record where we were in the history of master when we released a given version.

That is the process illustrated in the last version of a #lang slideshow diagram I made of this process, which I found very helpful in thinking it through: Racket release branch diagram · GitHub

@jbclements raised the point that any merge of stable into master would break the property that master currently has a linear history. The last discussion I found on that is:

@LiberalArtist

Re:

@jbclements

Thinking about this a bit more. I kind of like the fact that currently, the master branch has a single thread. I realize that merge commits that are made with merge -s ours don't represent "real" dependencies, but that's not obvious unless you dig into the commits and see that the merge has no diff. So I'm inclined to drop back to the "merge-with-the-release-branch-when-its-created" strategy. I'd really like to hear from others, though. @samth ? @mflatt ?

I don't have a strong opinion; I can see advantages on both sides. If we did opt to merge stable into master, you have a good point that we should probably edit the commit message to be explicit that it's a "fake" merge.

But actually, I think we don't need to make a final decision until after this release. Once you push v8.7 and stable, we'll presumably see this effect @elibarzilay pointed out:

@elibarzilay

One thing that I've noticed (~two days ago) is that due to merges going only from master to release to stable but not the other way, GH shows stable as being way ahead of master.

If that creates noise in the UI or it otherwise seems desirable to do the "merge-stable-into-master-after-release" variant, we could do git switch master && git merge -s ours stable && git push any time before branching for the 8.8 release. Otherwise, we do nothing and stick with the "merge-with-the-release-branch-when-its-created" strategy.

I don't think we ever followed up on that, and I know I personally was otherwise tied up and couldn't follow the 8.8 release process closely.

It does seem that no one has complained of any strangeness in the GitHub UI. But git describe could arguably be considered UI. It did come up in adjust build stamp for development and snapshots by LiberalArtist · Pull Request #4871 · racket/racket · GitHub (prompted by @benknoble).

@benknoble

Perhaps an interim solution would be to tag the "release cuts" like v8.18-cutoff or something

That does seem like a possible path to better git describe output by default while maintaining the linear history on master. (I don't have a strong opinion myself on how to balance these competing desiderata, though.)

If we did that, I think the tags should point to commits like Post-release version for the v8.18 release · racket/racket@02afeae · GitHub, and I'd suggest a naming convention like post/8.18.0.1. In particular, I'd want the names not to start with v and to be clear to both humans and tools that they are not release versions.