Mbed-os: Question: development workflow (master vs. release branches/tags)

[bmcdonnell-ionx]

Description

• Type: Question
• Related issue: #2602
• Priority: Minor

---

Background

I think I have most of the general idea of the Mbed OS development workflow in as it relates to the branching and merging.

• The master branch is kept stable, and that's where features and bugfixes are merged to once approved.
• The release-candidate branch splits off from master in preparation for each "minor" version release (e.g. mbed-os-5.7.0, but not mbed-os-5.7.1).
• There are "release" branches like mbed-os-5.7, where you merge the release-candidate branch into it for each release, which you tag as e.g. mbed-os-5.7.4.
• These are done as non-fast-forward merges, for clarity and consistency in the history.

For reference, I use git log --all --graph --decorate --pretty=oneline --abbrev-commit to see the DAG.

Question

Is all of the above correct?

The big thing that I don't understand is - if I'm interpreting the history correctly - why don't you merge master (or master~N) into release-candidate? It looks to me like you're cherry-picking everything from master into the release-candidate branch. Do I have that correct? If so, why?

I would've expected merging master (or master~N) into release-candidate, and then maybe adding a few cherry-picks / hotfixes before tagging a release. Isn't this non-merging method creating extra duplication? And doesn't it make it tougher to keep release-candidate and master in sync?

[bmcdonnell-ionx]

Also, in #2602, @0xc0170 said,

The branching model will be explained soon.

Has it been explained? I haven't found it yet.

Link please, if this exists.
[Mirrored to Jira]

[0xc0170]

There's one mention of feature branches in the workflow document https://github.com/ARMmbed/mbed-os-5-docs/blob/new_engine/docs/reference/contributing/guidelines/workflow.md#features but lot of info from above not yet there (branching model).

@ARMmbed/mbed-os-maintainers Please review
[Mirrored to Jira]

[adbridge]

https://os.mbed.com/docs/v5.9/introduction/how-we-release-arm-mbed-os.html

You can see in the above on selected changes go to a patch release and hence why we use cherry picking of commits rather than merges from Master.

[Mirrored to Jira]

[bmcdonnell-ionx]

@adbridge, cherry picking as a part of a git workflow, especially as frequently as Mbed OS does, breaks the DAG model, and makes it harder for people to see what changes are part of which branches. (I think there are some effects on Git tools for history searching, too, but those escape me at the moment.)

This emphatically-titled article describes some of the problems, and proposes a solution:
If you cherry pick, your branch model is wrong.

Has the Mbed OS team considered using a workflow with less or no cherry picking, such as the so-called git flow? In particular, have a look at the Hotfix branches section. Note, I'd say the Mbed OS master branch equates to the "git flow" develop branch. I'm not sure if the "git flow" master branch has a current equivalent in Mbed OS.

I think something like this modified git flow might work. The key difference is the base of the hotfix branches.

[Mirrored to Jira]

[cmonr]

@bmcdonnell-ionx Trying to answer this without definitively knowing the history behind our branching model led me to a back and forth with myself caused me to lose a whole bunch of time, so I'll leave it to @adbridge and/or @0xc0170 to chime in here (maybe even @sg-?).

The gist of it is that 1) switching models would be a huge endeavor that might even be too big for a feature release (famous last words), 2) what we have has its sore points (one of which may be that we don't have a DAG model that is easily usable, _but_ it is still possible to pull commit author history for cherry-picked commits), but it works and still seems to be working with few exceptions popping up, and 3) having said 1 and 2, I definitely think it's a good idea to review processes every once in a while to make sure we're not stagnating and see if there aren't better ways of doing things.

Oh, and thanks for the modified flow diagram, but if we wanted to switch over to that model, a couple of things would need to change. At least, if we wanted to make the change as painless as possible :)
[Mirrored to Jira]

[adbridge]

Internal Jira reference: https://jira.arm.com/browse/IOTCORE-131

[bmcdonnell-ionx]

Tools for implementing git flow:
https://github.com/petervanderdoes/gitflow-avh

[0xc0170]

I think we still have to do documentation review if all is explained, we will check documentation to be able to resolve this.

Regarding the git flow, it was considered (some internal teams were using it previously) but the current model was chosen - trunk based development (easier to understand - all goes to master, having tools to do releases) and it's being used.

[adbridge]

Tools for implementing git flow:
https://github.com/petervanderdoes/gitflow-avh

I think the key difference from the above flow and ours is that Master is our development branch, but we still make sure it doesn't get broken. We also do not make releases from Master accordingly. All patch releases come from specific release branches (a new one of which is created for each Minor release). As we have strict (with a few occasional exceptions) rules over what is allowed to go into a patch release we pick and choose which fixes, that have gone onto the development branch (ie Master) , go to a specific patch release.
The release-candidate branch is really just a temporary branch that we use to pull all the patches we want for the specific release and then create a PR from there to the actual release branch in order to run these patches through our CI before merging them and making the release. The majority of this is automated. This step is likely to change in the future and we will update our documentation as and when this happens.
This model has now been in use for nearly 2 years and generally works very smoothly and meets our needs very well.

[bmcdonnell-ionx]

@0xc0170,

trunk based development (easier to understand - all goes to master, having tools to do releases)

Git flow is "trunk-based", too, right? By default develop is the trunk. I don't think it's important to use their names--you could implement git flow with master as the "trunk".

[bmcdonnell-ionx]

@adbridge,

I think the key difference from the above flow and ours is that Master is our development branch,

This is just a naming difference (your master is canonically develop). I think the key difference is the cherry-picking instead of merging to your release.

but we still make sure it doesn't get broken.

I don't think this differs from git flow.

As we have strict (with a few occasional exceptions) rules over what is allowed to go into a patch release we pick and choose which fixes, that have gone onto the development branch (ie Master) , go to a specific patch release.

If you were to implement a more git-flow-like workflow, I think you'd make those decisions sooner, i.e. before merging to your "trunk" (master or develop).

The release-candidate branch is really just a temporary branch that we use to pull all the patches we want for the specific release and then create a PR from there to the actual release branch in order to run these patches through our CI before merging them and making the release. The majority of this is automated. ... This model has now been in use for nearly 2 years and generally works very smoothly and meets our needs very well.

This doesn't address my reasons noted above to consider moving away from the cherry-picking-based model, AFAICT.

This step is likely to change in the future and we will update our documentation as and when this happens.

My suggestion is above. 😃

[kjbracey-arm]

The mbed OS model is very close to the Linux kernel model - work done on master, minor releases basically come off of master (more-or-less), with the patch releases being made of backported cherry-picks from master.

We don't follow all these rules for our patches, but they're in the same spirit: https://github.com/torvalds/linux/blob/master/Documentation/process/stable-kernel-rules.rst

I think both models are equally viable, and have their pros and cons - the only fundamental distinction is this choice:
1) all changes are made on master first, so there's no new work on other patch or release branches, so those other branches never need to be merged - cherry-pick master->patch only
2) changes can be made on other branches, so they all need to be merged in to make sure master holds everything - merge patch->master only

The important thing is to never mix merging and cherry-picking, as it can cause conflict grief, and I've never worked with Git flow enough to understand how you'd manage to avoid cherry-picking if maintaining multiple stable branches in practice. (It can be done in principle, but you need to really understand how to base your topic branches on oldest applicable version, something people tend not to do).

[cmonr]

Sharing something I came across recently: https://stxnext.com/blog/2018/02/28/escape-merge-hell-why-i-prefer-trunk-based-development-over-feature-branching-and-gitflow/

[bmcdonnell-ionx]

@cmonr, thanks for your link. I'll check it out.

I noticed from it that I apparently inferred an incorrect meaning for "trunk-based development". So apparently the answer to my question above

Git flow is "trunk-based", too, right?

is "no".

[0xc0170]

@bmcdonnell-ionx Thanks for questioning our release process. As @cmonr shared the link above, it nicely explains pro/cons both approaches, teams like one or another. We are not considering switching our release process.

I hope we answered all the questions and this can be closed? By the way, we will add some illustration to our release process documentation.

[bmcdonnell-ionx]

Do the "Changes to [your] branching strategy" described in the recent blog post (link below) mean that you won't have the big stacks of duplicated commits from cherry-picking anymore?

Change of Mbed OS release process

[adbridge]

@bmcdonnell-ionx Yes we won't need to be cherry-picking for every patch release going forward in January.