How should master relate to dev after drop

[TerryE]

Retraction warning

This was a cock-up on my part so please ignore.

In #3078 we noticed that the naming of telnet files in master and dev were out of sync. After initial investigation, my TL;DR conclusion is that, for whatever reason, there is steady and possibly significant number of commits to 'dev' that are missing being pulled into master.

I believe that the root of the issue is that pull from dev into master is based on a maturity level which triggers the next_release tag, and the implicit assumption here is that any dev commits that are not next_release are in effect implicitly tagged next-but_one_release and will get promoted to next_release after the master drop and so all dev commits will evenutally migrate to master.

I am now sure that this isn't always happening. As a result the overall software configuration in master is one that has never been really tested in dev, and there is a possibility that it will contain fatal bugs. I think that we need to get a better handle on whether is risk is real or even whether it's not a risk because it is a failure that has happened.

A good basis of comparison is the output of git diff --stat master 3.0-master_20200610 and tracking down the missing commits that account for these discrepancies. However IMO, the only differences between master and the dev tag 3.0-master_20190907 should be for those few commits that we've held back until this release.

The major ones that seem I've found that has been omitted are:

Commit	date	Author	Short title
863dfb5	2019-12-27	Nathaniel Wesley Filardo	SSL rampage (#2938)
e7c29fe	2019-07-18	Terry Ellison	Lua 5.1 to 5.3 realignment phase 1
bbeb09b	2020-04-27	Terry Ellison	Squashed updates do get Lua51 and Lua53 working (#3075)
6926c66	2019-12-31	galjonsfigur	Polish Lua examples (#2846)

These 4 account for some 90% of the discrepancies, so there are others that we've missed, and in some cases the same file has been updated by two or more commits but one has been omitted, so just applying the missed commit might not work correctly.

So my conclusion is the master will no work properly.

• We need a one-off exercise to sync master to dev properly.
• We need to review our drop process to ensure that this doesn't happen again

[nwf]

I'd advocate for simply making master point to particular points along a single lineage, of which dev is the head. We should never merge to master in the future.

[pjsg]

We should probably just backed matter into dev and continue from there

…

On Wed, Jun 10, 2020, 20:27 Nathaniel Wesley Filardo < ***@***.***> wrote: I'd advocate for simply making master point to particular points along a single lineage, of which dev is the head. We should never merge to master in the future. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3153 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALQLTLROPCCG6IMPDVT6X3RWAQFFANCNFSM4N22J3NQ> .

[nwf]

Expanding my previous comment: and this means, necessarily, that dev contains only deltas that are ready to go in the next commit. Anything not yet ready lives in a PR on another branch and gets tested there. These branches should be rebased atop dev as necesary.

(ETA: The most recent assignment of master was, indeed, to just take the dev branch wholesale.)

[TerryE]

My suggestion is a slight variation on @nwf 's.

• Repoint master at the labelled commit in dev (8d091c4, I think). @pjsg, there is word or two missing from your last comment, so I am not sure of your intent here, but if it is to add missing commits to master rather than switching, we've got maybe 20-40 files which were updated in multiple commits and some will need manual resolution. Repointing is quick and simple.
• The concepts of dev and master are well understood by the wide group of active contributors, so we should leave these definitions unchanged: master is a stable release; dev is what PRs are raised against.
• We have a temporary branch draft-master, the maybe 4 currently active committers that work on the release process need to know of its existence and how we use it.
• About one month before a drop we create draft-master based on the current dev. Thereafter only agreed "safe and essential" commits to dev get cherry picked into draft-master.
• The drop essentially involves moving draft-master over master. Hence draft-master disappears until a month before the next release.

We rarely rollback commits to dev other than the last commit, and even this is very rare. Our standard practice is just to keep going, and use another commit to resolve issues if needed.

With our current drop process, I can see no advantage in having master as persistent branch evolving in its own right.

I think that we need @marcelstoer and @HHHartmann's views (at a minimum) before we can be quorate on this.

[pjsg]

I think that I was typing on my phone (with auto-correct).

The git model that I am used to has master being the released branch and (one or more) dev branch. Individual features are branched off dev, and PRs done to dev. Then once we are happy with dev, there is a PR to master to release it. There are frequent master back-merges into the various dev branches.

In nodemcu's case, there is only a single dev branch that should always be ahead of master. Thus a merge from dev to master is fast-forward.

If we have commits on master that are not on dev, then I think that we should merge master into `dev' -- this is a one-time operation that will result in all the commits on master also being on dev. There will be extra commits in dev -- but that is the whole point.

I'm never in favor of changing a branch name to be on a different branch.

[marcelstoer]

Sorry Terry, I don't follow. I don't understand yet what the problem is. AFAICS anything that goes to dev has always been pulled into master. We ensure that by merging the tip of dev into master every time we "release".

The only time I remember we "lost" commits (but not software changes) is with the penultimate drop on Sep 7, 2019. By accident you chose to 'squash and merge' #2886 rather than a regular merge. From your examples I only checked for "Lua 5.1 to 5.3 realignment phase 1" i.e. e7c29fe. In the blue title box GitHub confirms that it's on master and included in the most recent release tag.

[TerryE]

@marcelstoer the questions:

• does this happen?
• why does this happen, if it does?

are entirely different. You seem to be saying: I can't think of a reason why Q2 could happen, therefore the answer to Q1 is "no".

Please go back to my original post. The git diff lists off 551 files that are different between the two commits when the difference should be zero. Just going through the git logs, I have identified 4 large PRs that have somehow how not got integrated into master. I also don't know why this happened, but this is easy to check by doing the git diffs, so what I'm asking is for you to do these diffs yourself and validate my finding - or not.

Once we have agreement on that, then working out the "why"s and how to avoid a repeat of this is the next step.

[marcelstoer]

what I'm asking is for you to do these diffs yourself and validate my finding

I did - at first only with one of your four examples. It was integrated into master.

If I diff master and dev now I only see the expected outcome: RC and SQLite3 are gone and mkdocs.yml is corrected; three commits. https://github.com/nodemcu/nodemcu-firmware/compare/dev

[TerryE]

@pjsg, we have a lot of changes in dev that are not in master, and not the other way around. Going from dev to master is not a simple fast forward, as we've introduce this concept of the "next release" tag and we only merge forward tagged commits. This has clearly gone wrong somewhere. I fully admit that I might have been the committer who did some or all of this, but whoever did this, it is obviously possible to do. IMO, the process needs refining and needs to be more robust.

[TerryE]

@marcelstoer, I deleted the two local copies of these branches and refetched them from the NodeMCU repo. From what you say something has gone wrong with this process on my end. Let me do some more spot checks, and I will then close this issue, if it is my cock up.

[HHHartmann]

@marcelstoer @TerryE Something strange is going on here. When I pull my local clean dev branch of nodemcu/nodemcu-firmware from origin I get merges and merge conflicts. In my (limited) understanding of git this should not happen.
On Guthub the master and dev seem to be the same but probably not on my local copy.

There are also lots of unpushed commits (from all sorts of people, so not mine, as I also never check in to that repo)

So second one having problems here. Master branch seems not to have these problems though.

[nwf]

@marcelstoer rebased dev atop master, which is also why our PRs are all out of whack, and I didn't think about it before hitting the "go ahead" button (sorry; overly eager to get #3066 behind us). git does not handle this well. master is fine, because strictly more commits have been added to where it pointed previously. From git's perspective, dev is a wholly new thing and so your previously cached commits and the ones there now are in conflict.

For any of your local branches that stemmed from dev, try running (with the branch checked out) git rebase --autostash origin/dev. If git complains about conflicts while replaying a duplicated commit, use git rebase --skip. Note that rebase is playing with fire so back up your checkout first.

[nwf]

Oh, FWIW, something I wish git made much easier (oh, the list...) is checking that operations have preserved the tree even if the commit history is mutated. You can use git rev-parse HEAD: (note the trailing :!) to extract the tree hash of the current HEAD, before and after rebasing, for example. These tree hashes can also be fed to git diff as if they were commit IDs.

[TerryE]

I don't know what has happened, but I've just repeated exactly the same process: delete my local copies of dev and master, then refetch form the nodemcu repo. The git diff now yields 0 file differences rather than 551 files different. Whatever was going wrong, or I was doing wrong seems to have fixed itself.

Maybe this just proves that I should stick to Lua internals and modules and leave git to the experts. Sorry for the inconvenience Folks. 😢 💩 Back to the error handling PR.

[marcelstoer]

@marcelstoer rebased dev atop master, which is also why our PRs are all out of whack,

Indeed, I was open about it but in hindsight it probably was a total mind fart. It was really long overdue and our Git history is a lot cleaner now but it maybe wasn't worth it.

and I didn't think about it before hitting the "go ahead" button

Neither did I. Very sorry about it.

I wish GitHub gave us a "Rebase" button for the PRs like GitLab does. We wouldn't have to bother the PR authors with rebasing their branches onto dev then - even though in most cases that's absolutely hassle-free.

[HHHartmann]

Tried marcels suggestion in #3153 (comment) on my clean nodemcu/nodemcu-firmware dev branch and it worked.
Should we create a separate issue about this and make it sticky, so the visibility is good?
Adding it to the release notes seems a bit overdue? or is it worth a mention there. Otherwise I would have been stranded with my limited git knowledge. And I am sure many other also are.