git strategy for geth (upstream) sync

[ARR4N]

Tip

TL;DR A merge strategy is my recommended approach because it (a) preserves history in full; while (b) encoding merge-conflict resolution in the tree such that it can be reviewed with a single command. A rebase strategy loses history and makes it difficult to review conflict resolution because it (a) isn't encoded in the tree; and (b) requires multiple steps.

Background

This document opens a discussion around possible git strategies for updating libevm to incorporate the latest upstream geth commits. It analyses a merge strategy and a rebase strategy.

Requirements

1. A fully inspectable history;
   a. libevm and geth commits in their original state; and
   b. Modifications due to resolution of merge conflicts.
2. Via PR to allow for review and public discussion.
3. Honours branch-protection rules.

Branches and tags

• master tracks the ethereum/go-ethereum equivalent branch;
  • A sync from upstream is trivial and can be achieved through the GitHub UI but diligence is required ⚠️ see CAUTION ⚠️
• libevm is our default branch.
• libevm-base is a floating tag that we update to point to the latest geth version incorporated in libevm;
  • The phrase "incorporated" is deliberately vague as it is the topic of discussion for this document; currently it means all commits up to the initial libevm-base and in the future it will mean all commits up to the last one included in an update.
  • This tag exists primarily to enable the libevm delta workflow but is useful in this discussion as a reference point.
• For the purpose of this document,sync refers to some intermediate branch being used to bring commits from master into libevm via a PR for review.

Caution

When performing a sync of master, triple check the branch. GitHub displays the option for our libevm branch as well and this would be a destructive action that loses all history!

Current state

We branched off geth at some historical version, tagged as libevm-base. Both we and they have since committed to our respective branches and their latest release is v1.14.11 (the specific value is irrelevant). We wish to incorporate this release into libevm.

---
config:
  gitGraph:
    mainBranchName: master
---
gitGraph
    commit id: "G-1"
    commit id: "G0" tag: "libevm-base"
    branch libevm
    commit id: "L1"
    checkout master
    commit id: "G1"
    checkout libevm
    commit id: "L..."
    checkout master
    commit id: "G..."
    checkout libevm
    commit id: "Ln"
    checkout master
    commit id: "Gm" tag: "geth-v1.14.11"

Loading

Merge strategy

A rough outline of the (untested) git recipe:

$ git checkout libevm
$ git checkout -b sync
$ git merge master
# Resolve conflicts
$ git push --set-upstream origin sync

Then open a PR to merge sync into libevm.

---
config:
  gitGraph:
    mainBranchName: master
    mainBranchOrder: 0
---
gitGraph
    commit id: "G-1"
    commit id: "G0" tag: "libevm-base"
    branch libevm order: 2
    commit id: "L1"
    checkout master
    commit id: "G1"
    checkout libevm
    commit id: "L..."
    checkout master
    commit id: "G..."
    checkout libevm
    commit id: "Ln"
    checkout master
    commit id: "Gm" tag: "geth-v1.14.11"
    checkout libevm
    branch sync order: 1
    merge master id: "M" type: HIGHLIGHT tag: "PR"

Loading

Note

All "PR" tags should be interpreted as labels for easy reference, not as literal tags.

Reviewing a merge strategy

1. The GitHub UI will show the entire proposed change. This allows for review of the upstream code.
2. The merge commit M carries the merge-conflict resolutions.
   a. These can be inspected with git show, which uses git diff-tree -cc under the hood for merge commits.
   b. A UI for inspection +/- code review is an open question.

Post review

Incorporating commit M into libevm in a non-destructive fashion is important. A squash commit MUST NOT be used.

Merge + fast-forward (preferred)

Note that since development is paused on libevm, the branch can be fast-forwarded to M. Whether GitHub's PR merging via merge commit will perform a fast-forward is an open question. If this were to be performed locally, the recipe would be as follows, which apparently results in automatic PR merging (GitHub documentation) while respecting branch-protection rules:

$ git checkout libevm
$ git merge --ff-only sync
$ git push

That would result in M simply being the next commit in the libevm branch before further development recommences with Ln+1:

---
config:
  gitGraph:
    mainBranchName: master
---
gitGraph
    commit id: "G-1"
    commit id: "G0" tag: "(old) libevm-base"
    branch libevm
    commit id: "L1"
    checkout master
    commit id: "G1"
    checkout libevm
    commit id: "L..."
    checkout master
    commit id: "G..."
    checkout libevm
    commit id: "Ln"
    checkout master
    commit id: "Gm" tag: "geth-v1.14.11" tag: "(new) libevm-base"
    checkout libevm
    merge master id: "M" type: HIGHLIGHT tag: "Approved PR"
    commit id: "Ln+1"
    checkout master
    commit id: "Gm+1"

Loading

Although this doesn't result in a linear history, that is actually the true history of concurrent development on the master and libevm branches.

Merge M + merge-commit MC

This is now redundant as merge + fast-forward has been demonstrated to work.

If pushing after local fast-forward doesn't work as expected, a regular PR merge can be performed, but it would result in an effectively empty MC commit being added. This is ugly but still preserves all history.

---
title: "Redundant strategy: do not use"
config:
  gitGraph:
    mainBranchName: master
    mainBranchOrder: 0
---
gitGraph
    commit id: "G-1"
    commit id: "G0" tag: "(old) libevm-base"
    branch libevm order: 2
    commit id: "L1"
    checkout master
    commit id: "G1"
    checkout libevm
    commit id: "L..."
    checkout master
    commit id: "G..."
    checkout libevm
    commit id: "Ln"
    checkout master
    commit id: "Gm" tag: "geth-v1.14.11" tag: "(new) libevm-base"
    checkout libevm
    branch sync order: 1
    merge master id: "M" type: HIGHLIGHT tag: "PR"
    checkout libevm
    merge sync id:"MC" tag: "Approved PR"
    commit id: "Ln+1"
    checkout master
    commit id: "Gm+1"

Loading

Reverting a merge strategy

If the incorporated changes need to be reverted, either the M or MC commits are simple targets for git revert, assuming no further development on the branch.

Rebase strategy

A completely different approach is to rebase all of the L* commits to instead branch off the Gm commit that would have otherwise been merged. Each of the Li commits now has a respective Ri commit that introduces the same functionality/fix but with conflicts resolved on a per-commit basis.

Opening a PR to merge sync (Rn) into libevm:

---
config:
  gitGraph:
    mainBranchName: master
    mainBranchOrder: 0
---
gitGraph
    commit id: "G-1"
    commit id: "G0" tag: "libevm-base"
    branch libevm order: 2
    commit id: "L1"
    checkout master
    commit id: "G1"
    checkout libevm
    commit id: "L..."
    checkout master
    commit id: "G..."
    checkout libevm
    commit id: "Ln"
    checkout master
    commit id: "Gm" tag: "geth-v1.14.11"
    checkout master
    branch sync order: 1
    commit id: "R1"
    commit id: "R..."
    commit id: "Rn" tag: "PR"

Loading

Reviewing a rebase strategy

1. As with the merge strategy, the GitHub UI shows the entire proposed change, allowing for review of the upstream code.
2. Unlike the merge strategy, there is no single carrier of conflict resolution.
   a. n comparisons via git diff Li Ri would have to be performed.
   i. This requires external knowledge of the PR pairings, not captured in the commit tree.
   b. A UI for inspection +/- code review is an open question.

Post review

Rebase + merge is ambiguous

Even if there were no conflicts when merging sync into libevm, future commits would have an ambiguous history. If there were conflicts and they had to be resolved, this would indicate a broken process because resolution was already performed during the rebase.

---
config:
  gitGraph:
    mainBranchName: master
    mainBranchOrder: 0
---
gitGraph
    commit id: "G-1"
    commit id: "G0" tag: "(old) libevm-base"
    branch libevm order: 2
    commit id: "L1"
    checkout master
    commit id: "G1"
    checkout libevm
    commit id: "L..."
    checkout master
    commit id: "G..."
    checkout libevm
    commit id: "Ln"
    checkout master
    commit id: "Gm" tag: "geth-v1.14.11" tag: "(new) libevm-base"
    checkout master
    branch sync order: 1
    commit id: "R1"
    commit id: "R..."
    commit id: "Rn" tag: "PR"
    checkout libevm
    merge sync id: "M" type: REVERSE tag: "Approved PR"
    commit id: "Ln+1"

Loading

Rebase + force push loses history

The libevm branch could be forced to point to Rn.

---
config:
  gitGraph:
    mainBranchName: master
---
gitGraph
    commit id: "G-1"
    commit id: "G0" tag: "(old) libevm-base"
    commit id: "G1"
    commit id: "G..."
    commit id: "Gm" tag: "geth-v1.14.11" tag: "(new) libevm-base"
    branch libevm
    commit id: "R1"
    commit id: "R..."
    commit id: "Rn" tag: "Approved PR"
    commit id: "Ln+1"

Loading

There are a lot of resources online explaining the problems with force pushes. Using the commit tree alone, history regarding the original development of the L* branches will be lost, which doesn't meet our requirements.

Reverting a rebase strategy

Reverting a merged rebase can be done with git revert M. Reverting a force-pushed rebase requires knowledge of the old libevm branch, which would itself be force-pushed.

[darioush]

Maintaining Git Branches to Represent a Patchset

The goal is to maintain a set of branches where each branch represents a cohesive feature or patch. This allows us to:

• Periodically update the patches by incorporating changes from the upstream repository.
• Maintain clear histories and branch structures to support GitHub reviews (avoiding disruptive rebases).

Workflow Overview

1. Upstream repository: We track an upstream repository (e.g., ethereum/go-ethereum) and define patch branches (patch0, patch1, etc.), where each patch builds on the previous one.
2. Patch branches: Each branch adds a single, cohesive feature.
3. Periodic rebase: To keep patches updated, we periodically rebase them onto the latest upstream version.
4. Merge strategy for GitHub reviews: Instead of rebasing directly on the patch branches (which would rewrite history and confuse GitHub reviews), we create intermediate rebased branches and merge them back into the original branches, keeping review history intact.

Git Workflow Steps

1. Initial Setup

• Clone the upstream repository and set up the patch branches.

  # Clone upstream repository
  git clone https://github.com/ethereum/go-ethereum.git
  cd go-ethereum
  
  # Set upstream to track the upstream repository
  git remote add upstream https://github.com/ethereum/go-ethereum.git
  git fetch upstream
  
  # Create the patch branches, starting from upstream/master
  git checkout -b patch0 upstream/master
  # Make changes for patch0
  
  git checkout -b patch1 patch0
  # Make changes for patch1
  
  git checkout -b patch2 patch1
  # Make changes for patch2

2. Ongoing Development

When adding a new patch (patchN), branch off the last patch branch in the chain. For example, if you're working on patch2, branch off patch1 to ensure the patch sequence is preserved.

3. Periodic Upstream Updates (Expanded Example)

You will periodically want to rebase your patchset branches on the latest upstream/master. Here's a detailed example involving both patch0 and patch1.

1. Fetch latest upstream changes:

   git fetch upstream

   This ensures you have the latest upstream changes locally.

2. Rebase patch0 onto upstream/master:

   # Checkout patch0 and create a rebased branch
   git checkout patch0
   git checkout -b rebased-patch0
   
   # Rebase patch0 onto upstream/master
   git rebase upstream/master

   During the rebase, you'll resolve any conflicts that arise. After resolving conflicts and completing the rebase, you'll have a rebased-patch0 branch based on the latest upstream.

3. Rebase patch1 onto rebased-patch0:

   Since patch1 builds on patch0, you need to rebase patch1 onto rebased-patch0:

   # Checkout patch1 and create a rebased branch
   git checkout patch1
   git checkout -b rebased-patch1
   
   # Rebase patch1 onto rebased-patch0
   git rebase rebased-patch0

   Again, resolve any conflicts that come up during the rebase. Now, rebased-patch1 is based on the new rebased-patch0, which itself is based on the latest upstream/master.

4. Rebase additional patches:

   Repeat the process for patch2 and further patches, rebasing each onto its predecessor.

4. Merging Back to Preserve Review History

After rebasing the patches, you’ll want to merge the rebased branches back into the original patch branches (patch0, patch1, etc.) without losing GitHub review history.

1. Merge rebased-patch0 back into patch0:

   git checkout patch0
   git merge --no-ff rebased-patch0

   The --no-ff flag ensures that the merge creates a new commit, preserving the history of changes for code review purposes.

2. Merge rebased-patch1 back into patch1:

   git checkout patch1
   git merge --no-ff rebased-patch1

   Repeat this process for all patch branches (patch2, etc.).

By merging instead of rebasing directly on the patch branches, you avoid rewriting history, which keeps the pull request history intact and GitHub’s review system functional.

Visualization with MermaidJS

Here’s how the branch structure looks, including the steps for patch0 and patch1:

gitGraph
   branch upstream/master
   commit id: "upstream/master"
   branch patch0
   commit id: "patch0"
   branch patch1
   commit id: "patch1"
   branch patch2
   commit id: "patch2"
   branch patchN
   commit id: "patchN"
   
   checkout upstream/master
   commit id: "upstream/master (new)" tag: "new upstream commit"
   branch rebased-patch0
   commit id: "rebased-patch0"
   branch rebased-patch1
   commit id: "rebased-patch1"
   branch rebased-patch2
   commit id: "rebased-patch2"
   branch rebased-patchN
   commit id: "rebased-patchN"
   
   checkout patch0
   merge rebased-patch0
   checkout patch1
   merge rebased-patch1
   checkout patch2
   merge rebased-patch2
   checkout patchN
   merge rebased-patchN

Loading

• Original branches (patch0, patch1, etc.) represent the patchset applied to the upstream repository.
• Rebased branches (rebased-patch0, rebased-patch1, etc.) are created to periodically rebase onto the latest upstream code.
• After rebasing, merges (shown by the arrows from rebased-patch0 back to patch0, etc.) keep the original patch branches up-to-date while preserving history for GitHub reviews.

Conclusion

This approach allows you to maintain your patchset efficiently while periodically updating it from upstream. By rebasing onto intermediate branches and merging back into the original branches, you:

• Preserve a clear history for GitHub reviews.
• Keep each patch modular and isolated.
• Ensure that patches are based on the latest upstream code without causing confusion in the review process.

Merge + Patch branches Alternative

gitGraph
   branch upstream/master
   commit id: "upstream/master"
   branch patch0
   commit id: "patch0"
   branch patch1
   commit id: "patch1"
   branch patch2
   commit id: "patch2"
   branch patchN
   commit id: "patchN"
   
   checkout upstream/master
   commit id: "upstream/master (new)" tag: "new upstream commit"

   checkout patch0
   merge upstream/master
   
   checkout patch1
   merge patch0
   
   checkout patch2
   merge patch1
   
   checkout patchN
   merge patch2

Loading

Note both strategies refer to the patchset preserving merge/rebase workflows, not the previous comment

Strategy	Pros	Cons
Merge	- Preserves history - Simple conflict resolution	- Messier history with merge commits - Patches less clean
Rebase	- Clean, linear history - Self-contained patches	- Rewrites history (disrupts GitHub reviews in-progress) - More conflicts

[ARR4N]

Initial questions/comments that come to mind while reading (I'll edit as I go along):

1. Where does the libevm branch fit in?
2. Given that the code isn't already like this, how would we separate out the patch branches to begin with?
3. What is git doing under the hood when merging a rebase back into the same branch? The ambiguous history of the merge commit jumps out as a bit strange to me. [EDIT: see exploratory research below]
4. Every update to patch set k will result in having to rebase all patch sets >k. This adds a lot of unnecessary work and is likely error-prone.

I disagree with some of the points in the final table:

1. Merge cons:
   a. In general, merge commits show what was required to resolve conflicts, so they're just the history but not necessarily messier.
   b. How are the patches less clean? The tip of the branch will be the same either way.
2. Rebase pros:
   a. None of the strategies (patch set or not, merge or not) result in a linear history. I don't think that's a problem because the real-world history wasn't linear, but it's sometimes upheld as an ideal property to have.

Answer to (3)

This was bugging me so I did an in-depth analysis by following the steps and (a) opening the analysis PR; plus (b) continuing with the command-line merge. As I described above, rebase+merge results in an ambiguous history. In practice this results in the rebase conflicts repeating as merge conflicts, which I believe makes this strategy unviable.