Patching strategy

sbt

(Eugene Yokota) #1

There’s been several discussions around where bug fix pull requests should target. There are two options. First option is targeting stable branch (1.2.x), and the other option is dev (1.x) branch. There are some pro/cons.

Merge to Stable branch (for example 1.2.x) first

  1. A bug fix goes into the Stable branch, for example 1.2.x during sbt 1.2.x series.
  2. Patch releases are released in short window (around 2-week wait?).
  3. Periodically 1.2.x branch is merged up to 1.x containing potentially multiple patches.

sbt and its submodules (io, util, lm, zinc) have been using this strategy.

  • Pro: For the most part, the merge PRs are straight forward and it’s a semi-automatic process.
  • Con: We have to do the “hey could you target 1.2.x instead” dance with contributors. GitHub does allow retarget but won’t work if they branched off of 1.x.
  • Con: Maintainers have to periodically create “merge 1.2.x” pull request. If they don’t we could end up either missing bug fixes or merge conflict (coding on top of source that’s already changed).

Merge to Development branch (1.x) first

  1. All pull requests regardless of bug fix or not go into the Development branch 1.x.
  2. If the bug fix is worthy of patch release, either the maintainer or an advanced contributor can resend the patch against the stable branch.
  3. Otherwise, the changes are released in the next feature release (around 3 months wait?)
  • Pro: Contributors are going to be less confused.
  • Pro: 1.x would contain all commits as soon as they are available, which is good for integration testing and downstream project (Bloop).
  • Con: The cherry picking process is annoying for the maintainers. This probably should go with policy on squashing commits to as few commits as possible per PR.

Zinc and the rest

Should the strategy be different for Zinc, or should we pick a consistent strategy across all sbt submodules?

Notes


(Jorge) #2

Thanks for moving this discussion to a more official venue, Eugene.

I cannot speak for the rest of the sbt modules, but for Zinc I prefer the second approach for one reason: most of the changes that are made to the repository make it to both 1.x and 1.2.x. Rarely some contribution is not deemed stable enough to skip 1.2.x (or whatever release branch), and those cases should be handled as exceptional.

By the way, thinking more about the way you branch out for concrete release branches (1.2.x), we could probably avoid the cherry-picking process (which I’m fine with in scalacenter/zinc) with an append-only strategy where you revert the merge commits of PRs that should not make it to the release. This will bring more sanity to this approach. Note that there’s no need to squash commits here as the only thing you revert is the merge commits.


(Eugene Yokota) #3

If the majority of the PRs in Zinc are ok-to-patch PRs, wouldn’t that favor the first approach? We just have to tell contributors of Zinc to always target 1.2.x branch, and occasionally the maintainers can forward it to 1.x branch via GitHub UI.

I think the only reason we should consider “Merge to Development branch ( 1.x ) first” is if we are not backporting most of the commits, and limit the patch releases to serious bug fixes only.

Are you saying when we release 1.2.(1, 2, 3, 4, …) I take 1.x branch and back out individual commits that I don’t like? That sounds like a non-standard, error-prone method. I like having 1.2.x branch around so we know exactly what the patch release is going to look like.


(Jorge) #4

That could be an argument in favor of it, but my argument in favor of merging first to 1.x is that the restricted set of changes that make it to a release should be carefully picked by the maintainers. 1.2.x is a subset of 1.x, and therefore most of the changes to the codebase will go to 1.x so that branches don’t become too different and then end up with a lot of conflicts whenever you want to make a change in 1.x in some code that was changed in 1.2.x.

Agreed I just made this workflow up – I have no idea if it’s been used in production somewhere, but I don’t think it’s that bad. The most important drawback is that you cannot preview what the changes of 1.2.x will be. But you cannot either if you go with my preferred approach, where you’ll need to cherry-pick the changes for the release branches, unless you do this very often and are disciplined to keep doing it in the future. Note, however, that an easy way to fix this is to label those PRs you want to use in 1.2.x with a special tag so that you can go to the previous release tag, branch out, apply those PRs and finish the release.

I think a strong argument in favor of “merge to release branch (1.2.x)” (your first approach) could be made if you had an automatic way of submitting PRs to keep the branches synced. But otherwise, I prefer the second approach because it makes my life easier as a maintainer of sbt/zinc and scalacenter/zinc.


(Eugene Yokota) #5

Found something that might be helpful - https://backstroke.co/


(Jorge) #6

Any more ideas re patching? Otherwise we should converge towards a solution, at least for sbt/zinc.


(Nepomuk Seiler) #7

Hi,

IMHO the confusion comes from the arbitrary branch 1.x. I would prefer the first solution
and remove the 1.x branch completely. If a pull request needs to be ported to an older
version, then it should be back ported via a new pull request. The workflow would be

  1. checkout latest 1.1.x branch
  2. create a new branch
  3. cherry-pick the commit(s) we want to backport
  4. open a pull request (ideally referencing the initial implementation)

I used this strategy for native-packager the last 5 years and it worked out quite well. This
allows for easy fixes in older versions and back port only relevant stuff. It also ensures that
older versions are more stable and people are more motivated to upgrade to newer versions.

Shared, mutable state is always hard to handle. Also in git repositories :sweat_smile:

WDYT?


(Eugene Yokota) #8

1.x is basically 1.3.x that’s in development. Given an idea, it could either be “let’s put that into 1.3.x” or “we can patch to 1.2.x.”


(Eugene Yokota) #9

I just tried to use Backstroke, but it doesn’t seem to work for the two branches within a same repo. Do you guys know of a GitHub bot that can keep two branches in sync?


(Jorge) #10

I like this approach, it’s a subset of what I proposed.


(Eugene Yokota) #11

sbt goes back and forth on that since we want to ship small and safe changes relatively quicker. Patch releases tend to come out in weeks without going through RC cycle. Whereas feature releases tends to be many months of waiting + RC cycle.


(Eugene Yokota) #12

Another way of looking at it is that we currently use Git workflow (https://nvie.com/posts/a-successful-git-branching-model/, https://www.atlassian.com/git/tutorials/comparing-workflows). Our develop is called 1.x, and master is called 1.2.x (or 1.3.x etc).


(Nepomuk Seiler) #13

I totally agree! I think the main obstacle here is a fast and automated release process. I’m currently playing with travis, sbt-dynver and sbt-gpg for native-packager to release by simply pushing a tag.

Back porting commits should be straightforward most of the time. If the releasing is the same, then the burden on maintainers is relatively small. Also back porting can be done by contributors. If there aren’t any merge conflicts and CI passes, then we should be confident enough to put out a small release.

Maybe that’s a got start as well. My intuition always was that branches like 1.x, 1.1.x are meant for maintenance and back porting.

:blush:


(Jorge) #14

Not sure if it’s too late, but I recommend you to use https://github.com/scalacenter/sbt-release-early instead. Release on merge is a workflow I use all the time in my projects and it’s all automated in the plugin. Typing releaseEarly is enough to release all the build. For example, on every merge I cut a version in scalacenter/zinc: https://github.com/scalacenter/zinc/blob/1.x-bloop/project/BloopPlugin.scala#L24-L48 (this is the only extra configuration you need and you’re good to go, though it’s good to complement it with the docs in the wiki).


(Eugene Yokota) #15

2-branch development (Git workflow) vs mostly-1-branch development

Implicit in this conversation (to merge patches to stable first or development first) is distinct development style of 2-branch development (2BD) vs 1-branch development (1BD).

The benefit of 2BD, is that we are open to both “likely-stable” bug fix contributions and “cutting edge” experimental idea contributions at the same time. If it’s “likely-stable” we put it into 1.2.x or master, and it gets shipped in the next patch release. If it’s “cutting edge”, we wait till the next feature release.

As a datapoint, let’s look at the number of non-merge commits that went into 1.1.x series:

# sbt/sbt
$ git shortlog --no-merges -sn v1.1.0...v1.1.6 | awk '{s+=$1}END{print s}'
110
# sbt/zinc
$ git shortlog --no-merges -sn v1.1.0...v1.1.7 | awk '{s+=$1}END{print s}'
40
# sbt/librarymanagement
$ git shortlog --no-merges -sn v1.1.0...v1.1.5 | awk '{s+=$1}END{print s}'
44
# sbt/io
$ git shortlog --no-merges -sn v1.1.0...v1.1.7 | awk '{s+=$1}END{print s}'
65

So that’s 259 commits to cherry pick. If we are doing 1BR, likely we won’t be backporting that much.

I’ve now internalized these names so they are natural to me :slight_smile:, but I am totally open to renaming branches to develop and master to make the intentions clearer, if we are doing 2-branch development.

about automation

One complication at the moment is the fact that we have multiple repositories. When I make a release, I am currently releasing each module to Bintray (https://bintray.com/sbt/maven-releases), sync to Maven Central, wait few hours, send PR to the next module, and repeat.

If we want to drive automation, we might want to consider a semi-automated process of releasing from Dbuild (https://github.com/sbt/sbt-validator), which currently is capable of publishing nightly builds.

I don’t know if automation can completely solve the problem, because even if we had 1.3.0-M5 as opposed to 1.2.5 they are not exactly the same thing since the point of 1.2.5 is that it doesn’t contain all the “cutting edge” features.

trying 1-branch development

It seems like both @muuki88 and @jvican are in support of 1-branch development, why don’t we try 1BD at least for 1.2.x series, and see how it works?


(Nepomuk Seiler) #16

Sounds good :slight_smile: Do we keep the branch names or define 1.2.x as the current master we merge everything?


(Eugene Yokota) #17

“try 1BD at least for 1.2.x series” may have been misleading. I meant that we first merge everything to current 1.x branch, and then manually cherry pick to 1.2.x.

I don’t think the name master is good for 1.x since it’s not stable to release anything off of it, but renaming it to develop might make it clearer?