Branches diverging

[delan]

Currently we have three downstream branches: main, 2023-06-14, and 2023-07-23. We landed #4 into 2023-06-14, but that doesn’t show up on main, nor does it show up in the new 2023-07-23 branch.

To fix the first problem, should we change our workflow a bit? My first idea was:

1. Once only: rename 2023-07-23 to main
2. Once only: reland Enable font-variant for Layout 2020 #4 into main
3. Once only: make Servo depend on main
4. Before our next upgrade, cut a 2023-07-23 branch
5. Do our next upgrade on main
6. Land additional downstream patches into main
7. Go to step 4

I don’t think that will break the ability to build old versions of Servo, because their lockfiles depend on a specific commit hash, but rewriting main has other downsides, like making the commits that land pull requests look orphaned. What about this, which ensures that the default branch is never rewritten:

1. Once only: make 2023-07-23 the default branch
2. Once only: reland Enable font-variant for Layout 2020 #4 into 2023-07-23
3. Once only: delete the main branch
4. Do our next upgrade into a new branch like 2024-01-01
5. Make 2024-01-01 the default branch
6. Land additional downstream patches into 2024-01-01
7. Go to step 4

I’m not sure about the second problem though, because I only realised we had a new 2023-07-23 branch after landing #4 into 2023-06-14. It seems with both approaches, we would still need to make sure we avoid landing patches during an upgrade or into the wrong branch.

[mrobinson]

It's a bit funny that we ran into this issue almost immediately! I think it isn't a tremendous problem since hopefully every change to stylo has a matching change in Servo that also runs tests. That said, I think you're correct that we should avoid it.

I think your approach is quite clever though I see one issue. Once you make Servo depend on main, those revisions may stop compiling in the future which is pretty annoying for bisect. I have an alternative proposal:

1. Wait for the Servo change to be approved against a branch called "stylo-upgrade"
2. Lock the old release branch using GitHub branch protection
3. Rename "stylo-upgrade" to "2024-01-01"
4. The upgrader ensures that all changes from "2024-06-14" (the old branch) are in "2024-01-01"
5. Update the Servo PR and send it to the merge queue

The benefit here is that the old branch stays locked in the future to avoid confusion.

[Loirooriol]

Not sure what's the best approach. I just created the 2023-07-23 branch since I wanted to try the style upgrade in Servo.
I have included #4 into it.

[nicoburns]

I'm wondering about how the commits updating the docs, readme, sync scripts, etc will be maintained. It seems a bit awkward to have essentially two histories stacked on top of one another (i.e. once the docs/readme/sync commits reach into the hundreds (or even tens) of commits it will be hard to find the history of the stylo code itself. I guess we could put a link to the "head" upstream commit in the README?

Also, if we were to publish stylo to crates.io would we want to publish the servo version or the upstream version? In theory there seems to be no reason to limit a crates.io release to a version that Servo supports. On the other hand, if we're maintaining patches on top of upstream for version numbers, examples, etc then it seems like it would be hard (/a bunch of work) to do that on top of the upstream version as well as the Servo version.

Is there an alternative sync workflow using something like git subtree that would avoid rebasing everything all the time? I guess that would make it tricky to push work landed here upstream?

[delan]

I think your approach is quite clever though I see one issue. Once you make Servo depend on main, those revisions may stop compiling in the future which is pretty annoying for bisect.

I think the lockfile means that won’t happen, but I agree it would be best to keep the branch resolvable, rather than relying on the lockfile (and potentially orphaned commits), so my first idea is out.

I like your idea about branch protection though! Can we simplify it by skipping the temporary branch name (stylo-upgrade), or do we not know how far we’re upgrading until we’re done?

Unrelated to that, so far I’ve proposed we delete the main branch and just set the latest date-named branch as default, so that there’s always exactly one branch with the current upgrade. But that means every time you want to git switch -c to make a new downstream pull request, you need to check the default branch on GitHub or reclone the repo, otherwise you may end up writing your patch against the wrong version of Stylo.

I think we can solve this by keeping a main branch to use as the default branch, and whenever main changes, we use CI to push main to a branch named after the date of the last upstream commit (now 2023-06-14, soon 2023-07-23).

If we elaborate on your proposal to also describe what happens between upgrades (like the proposals in the description), but keep main around for pull requests, then I think we have something like this:

1. Once only: restrict pushing to date-named branches to CI only
2. Do our upgrade into a new stylo-upgrade branch
3. Wait for the Servo change to be approved against stylo-upgrade
4. Ensure that stylo-upgrade contains all changes from main (which CI has pushed to 2023-06-14)
5. Force push stylo-upgrade to main (which CI will push to 2023-07-23)
6. Between upgrades, land additional downstream patches into main
7. Go to step 2

This should solve all of the problems we’ve discussed:

• Contributors only have one branch they can open pull requests against — thanks to branch protection
• New downstream changes are included in the default branch — because the default branch is main
• New downstream changes are included in the current date-named branch — thanks to CI
• New downstream changes are included in the next date-named branch — thanks to step 4
• Commits that land pull requests will not look orphaned — they will disappear from main after the next upgrade, but they will forever exist on the date-named branch at the time
• Servo’s branch dep will always be resolvable and compatible, even without the lockfile — thanks to CI
• Contributors can start writing new patches without needing to check the default branch — it’s always main

I'm wondering about how the commits updating the docs, readme, sync scripts, etc will be maintained. It seems a bit awkward to have essentially two histories stacked on top of one another (i.e. once the docs/readme/sync commits reach into the hundreds (or even tens) of commits it will be hard to find the history of the stylo code itself. I guess we could put a link to the "head" upstream commit in the README?

So far, we’ve been squashing those supporting materials into one commit, which also marks the boundary between upstream and downstream. That way, we only need to rebase n+1 commits for the n downstream patches we have.

Also, if we were to publish stylo to crates.io would we want to publish the servo version or the upstream version? In theory there seems to be no reason to limit a crates.io release to a version that Servo supports. On the other hand, if we're maintaining patches on top of upstream for version numbers, examples, etc then it seems like it would be hard (/a bunch of work) to do that on top of the upstream version as well as the Servo version.

I think the idea is we’ll catch up with mozilla-central and upstream as many of our patches as possible, leaving only the patches needed to make Stylo publishable (such as removing upstream path dependencies).

[mrobinson]

I like your idea about branch protection though! Can we simplify it by skipping the temporary branch name (stylo-upgrade), or do we not know how far we’re upgrading until we’re done?

Yeah, I think it's totally fine to skip the temporary branch name.

If we elaborate on your proposal to also describe what happens between upgrades (like the proposals in the description), but keep main around for pull requests, then I think we have something like this:

I like your proposal! I say let's do it.

[delan]

Thanks! On second thought, maybe having an upgrade branch is safest, since it lets us push our upgrade progress and run tests on CI without disrupting main (and everyone’s pull requests) until the upgrade is done.

I’ve renamed 2023-07-23 to upgrade to avoid any conflicts with the new branch protection and CI job, which I’ll put up for review on Monday.

[mrobinson]

Thanks! On second thought, maybe having an upgrade branch is safest, since it lets us push our upgrade progress and run tests on CI without disrupting main (and everyone’s pull requests) until the upgrade is done.

That makes sense!

I’ve renamed 2023-07-23 to upgrade to avoid any conflicts with the new branch protection and CI job, which I’ll put up for review on Monday.

I've renamed this back simply because we landed the upgrade in Servo and I think that things might start breaking if we rename it now.

[mrobinson]

I think we've figure this one out, so perhaps the next step is to simply write down our workflow and put it in the README.