Branches & backporting

[justinbeaty]

This is probably something that can be discussed in the Discord developer meeting tomorrow, but I was just trying to think of ways to make managing two branches easier.

My first thought is that perhaps 20.0 (or 22.0) should become the default branch. Then, perhaps 1.9.4.x should only receive security fixes or other very basic commits, but no new features. So instead of merging 1.9.4.x into 20.0 every release, we only need to cherry-pick commits from 20.0 that are security patches or bugfixes.

A tool such as this could help too: https://github.com/sqren/backport-github-action. Ideally, it looks like you can just add an PR label such as backport-to-1.9.4.x and it does it automatically (I guess unless there's merge conflicts).

I think the benefit is that we are not risking breaking anything in 1.9.4.x any longer with features that may or may not be needed. This seems like the very definition of LTS to me.

I also do not think this conflicts with any goals we've already had, such as every 2-4 years or so tagging a new LTS release. So say in 2023 then branch 23.0 becomes LTS and security patches/bugfixes are backported from 24.0. Or even depending on how well that tool works, it might not be too difficult to continue to backport into both 1.9.4.x and 23.0.

Just throwing some thoughts out there, I think we do need to go through the formal RFC process for this so we really all agree with what we are doing.

[justinbeaty]

Some more food for thought. Maybe the ideal scenario is even years are LTS and odd years are mainline. This is like both nginx (as @Flyingmana points out in his RFC draft) and nodejs. So maybe we support 1-2 prior stable releases with patches which should be easy enough if we are only backporting security or bug fixes. Or maybe always one recent LTS and the 1.9.4.x branch for people who just want an upgrade from Magento 1.9.

[justinbeaty]

Sorry for just dumping ideas here, but one other thought:

It may be better to use a main branch instead, and every even year create a 22.0 (or 24.0, 26.0, etc) branch from the last tagged release. So, all new commits target main and optionally are backported to other branches via a github action.

I say this because it avoids having to change the base branches of PRs every year. I.e. from 20.0 to 22.0.

[fballiano]

this ❤️

[Flyingmana]

So the main reason to have 1.9.4.x as main branch was to make sure, bugfixes always end up there, as still the majority of bugfixes happens there. and because for a long time merging upstream was easier, because merging makes sure ALL of them end up there, and not just a part.
Merging is not an option for backporting.
This becomes more important for timeframes, when we have more then 2 supported versions (which can happen if you have 1 LTS release, and 1 normal release, as the normal releases have an overlap of at least 3 months, and also the LTS releases will/should have an overlap of several months for customers beeing able to plan an upgrade.

Of course we do not have finished the RFC related to it, but my wish would have been, to have 19 as an LTS release, which gets bugfixes till the next LTS release 4-5 years later happens.
Because many people want low-maintenance, which is one of the main arguments for OpenMage.
But Low Maintenance is hard to achieve with adding new Features. Of course you can have something like rolling releases, or iterating Bugfix and Feature Releases. Upgrades to new Releases always require some work because of changes. Staying 5 years on an LTS version is different.
But Iam also someone who only uses LTS versions of Ubuntu when using Linux, because I cant stand to have to do a big upgrade every year which might break something.

[justinbeaty]

I understand it's been easier to merge all of 1.9.4.x into 20.0 instead of cherry-picking commits from 20.0 to 1.9.4.x. However for an LTS release we probably add too much to 1.9.4.x -- basically anything other than a BC break goes to 1.9.4.x while it really only needs bug/security fixes.

If a tool such as sqren/backport (or the backport github actions) can backport commits based on PR labels I think it might be an interesting approach.

An LTS every 2, 4, or 5 years is all okay. If it's easy enough to have commits backported then it's less overhead to tag an LTS every two years and backport to it for four years. So there'd be two LTS releases maintained at a time (or three for a period of time during the transitions).

[fballiano]

The backport action seems awesome, I searched for something similar some time ago and didn't find it, we could already use it for v19->v20 immediate merge.

But what happens if there's a conflict? I couldn't find any documentation about that. Maybe also protected branches could be problematic?

[justinbeaty]

@Flyingmana One thought would be that 1.9.4.x branch could be the target of less commits. In other words, put it into actual LTS mode where it only needs to receive security fixes or bug patches.

Then say we want to release v22 (or v21 whatever the numbering scheme is) then v20 can also go LTS and we only need to port from v22 -> v20 and v22 -> v19 again just security fixes or bug patches.

It is a departure from what we are doing today, but we also can worry less about breaking things with new features for people who really just want LTS.

[Flyingmana]

put it into actual LTS mode where it only needs to receive security fixes or bug patches
Its in this mode for 2 years already now. Go through the changelog, the majority is bug fixes.
The Reality of OpenMage is, that the primary set of changes are bugfixes for the existing core.

[justinbeaty]

put it into actual LTS mode where it only needs to receive security fixes or bug patches

Its in this mode for 2 years already now. Go through the changelog, the majority is bug fixes.
The Reality of OpenMage is, that the primary set of changes are bugfixes for the existing core.

The way I understand it is currently 1.9.4.x is the target for any PR that is considered non breaking, whether it's a new feature, optimization, or bugfix.

Looking through the 1.9.4.17 changelog, here's my opinion on some of the commits.

These would be good examples of PRs to backport as they fix security issues or serious bugs.

• Clean up empty directories created when file requested does not exist in remote storage. #1338
• Fix Varien_File_Uploader ignored errors with try catch #1277
• Fixed infinite loop of popup alerts in backend attribute set page #1672
• Upgrade GitHub action/checkout to v3 #2141
• Ensure there are no emails without recipient in email queue #1109
• Changed timestamp definition to fix issue #1166 #2189
• bugfix: fix adminhtml translation in config.xml for importexport module #2244
• prevent customer review view from loading foreign reviews  #1970
• Avoid session invalidation when changing session lifetime. #546

These PRs are bugfixes but not necessarily needed for LTS. However if the PR author indicates they are using an LTS version they may be responsible for ensuring there are no merge conflicts in the backport.

• Issue #268: sales_order_create_sidebar_wishlist breaks if wishlist module is disabled #2059
• Fixed admin tabs order not working properly. #1360
• Fix admin menu z-index when debug template paths is enabled #1819
• Fix admin menu z-index for popup message #2142
• Clean review_entity_summary when reviews are deleted #1103
• Better error handling for issues during bulk attribute update #1434
• Fixed issue #2001, EAV datetime attribute can't save time. #2003
• Fix Core Bug: To make "01/01/1970" a valid birthday #1851

And then we have examples of us breaking 1.9.4.x with commits that likely didn't need to be in 1.9.4.x in the first place. The first example was from a commits I mentioned would have been good for backporting, so we still would have broken an LTS release in that case.

• Fixed issue #2212 SQLSTATE[42S22]: Column not found: 1054 Unknown col… #2221
• Revert "Some more room for the category menu" #2254
• Revert "creditmemo bundle inventory calculation error" #2186
• Reverted commit 2ae8c05 #2182
• Revert "Fix wrong payment info template paths" #2179
• Revert "Prevent string * int multiplification issue in PHP 8.0+" #2078

---

I think a good reference for LTS backporting is Ubuntu's Stable Release Updates Policy. Some excerpts:

---

Stable release updates are automatically recommended to a very large number of users, and so it is critically important to treat them with great caution. Therefore, when updates are proposed, they must be accompanied by a strong rationale and present a low risk of regressions.

"It's just a one-line change!"

Even the simplest of changes can cause unexpected regressions due to lurking problems:

This explains why we should not target 1.9.4.x for all commits just because they don't introduce an obvious breaking change.

---

Stable release updates will, in general, only be issued in order to fix high-impact bugs. Examples of such bugs include:

• Bugs which may, under realistic circumstances, directly cause a security vulnerability.

• Bugs which represent severe regressions from the previous release of Ubuntu. This includes packages which are totally unusable, like being uninstallable or crashing on startup.

• Bugs which may, under realistic circumstances, directly cause a loss of user data

• Updates that need to be applied to Ubuntu packages to adjust to changes in the environment, server protocols, web services, and similar, i. e. where the current version just ceases to work.

So basically just security vulns, fixes for regressions, fixes for bugs that cause major features to be broken, and fixes for protocol updates (such as #2191 - except that's in Zend)

---

A few more related thoughts:

• Other examples of commits that don't need to target 1.9.4.x are PHP8 compatibility since 1.9.4.x would have gone LTS before PHP8 was even released. It doesn't seem reasonable to me that a user would use a non-LTS operating system where PHP8 is available but decide to use an LTS Magento release.
• Not forward merging the entire 1.9.4.x branch allows us to have parts of the code that differ between branches, for example the getOpenMageVersionInfo() function in Mage.php which seems like it will end up very messy as we bump major versions.

---

I hope I explain some of my rationalization for starting this discussion in the first place. I understand we don't want to make more work by having to backport individual commits, but my point is that I think there are way less commits that actually need to be backported.

In the end I am just trying to promote a healthy discussion; if nothing changes in the end I am still a happy OpenMage user.

[Flyingmana]

And then we have examples of us breaking 1.9.4.x with commits that likely didn't need to be in 1.9.4.x in the first place. The first example was from a commits I mentioned would have been good for backporting, so we still would have broken an LTS release in that case.

#2221 - PR was older then 2 years, but yes
#2254 - yes
#2186 - older then 2 years
#2182 - PR was older then 2 years, but yes
#2179 - No, its a bug
#2078 - No, Its a bug which prevents php migration

I commented them with how I see it.

[justinbeaty]

I am not sure if "yes" means you agree it should have been merged or you agree it should not have been merged into 1.9.4.x.

Correct some of those are older than 2 years, I am not sure when 20.0 was created. If 20.0 did exist at the time, then my point still stands I think.

For bug fixes, I don't think it needs to be backported unless it's to fix a regression we introduced. If it's just a long standing bug from Magento, it likely wasn't breaking anything in the years a site has been in operation (either on openmage or magento).

For PHP migrations again I don't think we need to backport those fixes. I would say whatever highest PHP version was supported at the time the branch went LTS is all it should support in the future. Because why use Magento LTS if you are not using an LTS operating system from the same time period?

Hopefully I understood what you mean here.

[fballiano]

Hey everybody, I'm creating this new RFC to move this topic forward:
https://github.com/fballiano/openmage-rfcs/blob/main/draft/0000-new-branching-rules-2022.md

Can we make it better? What can we write in the "Rationale and Alternatives" section?

[justinbeaty]

Here is what I think an ideal workflow might look like:

First, we must decide if we want to make 20.0 the main development branch and backport only some commits instead of having 1.9.4.x. be the development branch and forward porting all changes. It is my opinion this is the way to go so for the rest of the post I'll assume we are going that route.

Next, I would suggest that we start a new major version, 21.0 (if we're going sequentially) or 22.0 (if we're following years). For the rest of the post I am going to call it 22.0 because I kind of like the idea of using the year. As an aside, I think we have an opportunity to merge some of the bigger breaking changes into v22 like @luigifab's mysql4 removal PRs and the composer workflow / release builder.

As far as branches, I'm not sure we should rename 1.9.4.x. It's okay to just leave that as a legacy artifact of an old naming scheme. Then what makes sense to me is as @fballiano said in his RFC, rename 20.0 -> main (so PRs that used to target 20.0 now target main) and then also re-create a 20.0 branch from main.

So now we would now target main for all commits unless necessary as mentioned in the RFC (such as fixing a bug only present in an older branch). PRs that need to be backported get a special github label and the backport tool should be able to (mostly) handle the rest.

For releases, we would tag new v22 releases from main and only when a new version (i.e. v23 or v24) is released would be create the 22.0 branch. In other words we only create branches for prior major versions.

This means that right now we would have two LTS branches: 1.9.4.x and 20.0. Figuring that each LTS release gets a 5 year life span and we release one LTS on every even year, there could be 2-3 LTS releases at any given time. Will this be a PITA? I'm not really sure, it all depends on how well a tool such as sqren/backport works.

Also one other thing regarding version numbering. I think we have troubles following semantic versioning since all our releases are patch releases. Maybe we adopt $YEAR.$BC.$PATCH where $BC is incremented for breaking changes. This is pretty similar to how Magento did it I suppose since 1.8.0.x -> 1.9.0.x would have been a major change, but I was only around since the 1.9.x days so I could be wrong.

This is a lot of changes from how the project has been managed, and it really depends on the ease of backporting. If enough people support this direction I'd suggest we spin up some sort of dummy repo to test the backport tool on. I am looking at this from the perspective that I want to be on OpenMage for the next 5-10 years.

[fballiano]

I don't know man, merging takes a lot of time and care.
But also I would really like to have a workign v20 branch at all times.

[Flyingmana]

Maybe the question should be, why do you need a working branch, why are the releases not enough?

[fballiano]

But why should we have the "old and will be phased out" version up to date while our "latest and greatest" is updated only at release time?
I'd like to be able to test (and maybe use) a v20-dev-master (let's call it this way) which is not possible. while for sure nobody needs a v19-dev-master, since being an LTS-must-be-super-stable it wouldn't make sense (in my opinion) to use it directly in a "dev-master" way.

[Flyingmana]

people will use v19 because people want to migrate in their own tempo, and v19 is more stable. But thats distracting from my question as Iam trying to find a middle ground everyone can work with.

and when you say "I'd like to be able to test (and maybe use) a v20-dev ..." then its still the same question. Why would you want to use this one instead of a release?

and for me its not about who might want to use the v19-dev branch, its about the easiest way to make sure bugfixes are in all needed versions on a release. If there is something not working with the possibly automated backporting job, this could mean backporting 50 bugfixes, and checking all of them if backporting is even needed, as not all bugfixes on a v20 branch would apply to a v19 branch, so its probably 40 out of 50 bugfixes which now need to be manually backported before a release can be done.

[justinbeaty]

One positive reason for an always up-to-date 20.0 branch is that during development and when I'm working on major updates, I actually will use the latest 20.0 code. If that includes commits that are not yet in a release, I'm inadvertently testing unreleased features. Once I'm ready to push changes, I don't care about a tagged release of OpenMage because my composer.lock file will install the same commit hash I tested on.

I think more people are likely to run an unreleased version of 20.0 than 1.9.4.x. In other words what I'm trying to say is that if there's a bug in 1.9.4.x people like me won't catch it until after it's merged into 20.0 and a new 20.0 version is tagged. This past release however I think there were a few 1.9.4.x -> 20.0 merges more often, so 20.0 stayed more up-to-date.

If a backport (or even forward port) github action is feasible, then ideally we have up-to-date branches for all versions. I think it helps to catch bugs earlier.

[sreichel]

I'd kepp it, as it is.

Everything thats no BC breaking change wents to 1.9.x and gets forward merged to v20. The v20 was always ment to be a "development branch" for more advanced user that can fix problems on their own.

Possible backports:

(any reason for being only in v20?)

#1525
#1532