[llvm-dev] How to deal with multiple patches to the same file

Mon Aug 10 11:12:56 PDT 2020

I owe you a gala dinner at your favorite restaurant. Really.

A few questions:

Why did you 'git pull --rebase' when the branch was up-to-date? Is this just a safety habit?

I don't understand the pushing upstream. Since we use Phabricator, isn't that the job of the person who commits the patch?

Does git keep all my branches up-to-date with the origin? Say I come in tomorrow and start working on a branch. Should I make sure I do a checkout on it again?

Is it sufficient to do a 'git show' and submit with Arcanist, or is there an intermediate step to format the patch file?

At 8/10/2020 12:19 PM, Robinson, Paul wrote:
>Hi Paul,
>Here we go with a full example.  I'm using git from the command line,
>because that's my comfort zone; you should be able to do equivalent things
>from a GUI.
>
>Let's start with a test tweak one of my colleagues made recently.
>We'll pretend I'm still experimenting with it.
>------------------------------
>D:\upstream\llvm-project>git status
>On branch master
>Your branch is up to date with 'origin/master'.
>
>Changes not staged for commit:
>  (use "git add <file>..." to update what will be committed)
>  (use "git restore <file>..." to discard changes in working directory)
>        modified:   llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>
>no changes added to commit (use "git add" and/or "git commit -a")
>
>D:\upstream\llvm-project>git diff
>diff --git a/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll b/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>index ffe5d8eedf4..ed3e2e6b481 100644
>--- a/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>+++ b/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>@@ -1,6 +1,6 @@
> ; REQUIRES: asserts
>
>-; RUN: opt -newgvn -S %s | FileChecks %s
>+; RUN: opt -newgvn -S %s | FileCheck %s
>
> ; XFAIL: *
>
>
>D:\upstream\llvm-project>git add llvm/test/Transforms
>
>D:\upstream\llvm-project>git status
>On branch master
>Your branch is up to date with 'origin/master'.
>
>Changes to be committed:
>  (use "git restore --staged <file>..." to unstage)
>        modified:   llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>
>
>D:\upstream\llvm-project>git checkout -b mypatch
>Switched to a new branch 'mypatch'
>
>D:\upstream\llvm-project>git status
>On branch mypatch
>Changes to be committed:
>  (use "git restore --staged <file>..." to unstage)
>        modified:   llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>
>
>D:\upstream\llvm-project>git commit -m "fix bad command name"
>[mypatch 1768f568884] fix bad command name
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>D:\upstream\llvm-project>git show
>commit 1768f56888452f2d068eb1ee51f2bab39042808c (HEAD -> mypatch)
>Author: Paul Robinson <paul.robinson at sony.com>
>Date:   Mon Aug 10 08:43:58 2020 -0700
>
>    fix bad command name
>
>diff --git a/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll b/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>index ffe5d8eedf4..ed3e2e6b481 100644
>--- a/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>+++ b/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>@@ -1,6 +1,6 @@
> ; REQUIRES: asserts
>
>-; RUN: opt -newgvn -S %s | FileChecks %s
>+; RUN: opt -newgvn -S %s | FileCheck %s
>
> ; XFAIL: *
>
>
>D:\upstream\llvm-project>
>------------------------------
>Notice that it was okay to create the branch even though I had changes 
>staged; they stayed staged.  Other git operations might not like having
>staged changes present, but you can use `git stash` to push/pop changes
>(an exercise left to the reader).
>You can redirect the output of `git show` to a file and upload that to
>Phabricator as a diff; it doesn't care about branches.
>
>Now let's make a second patch dependent on the first one, using a
>separate branch. This is a foolish irrelevant change just for example.
>------------------------------
>D:\upstream\llvm-project>git checkout -b mypatch2
>Switched to a new branch 'mypatch2'
>
>D:\upstream\llvm-project>notepad llvm\test\Transforms\NewGVN\todo-pr42422-phi-of-ops.ll
>
>D:\upstream\llvm-project>git commit -a -m "a dependent patch"
>[mypatch2 46eb5487e15] a dependent patch
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>D:\upstream\llvm-project>git show
>commit 46eb5487e15027d83dda027ad02823dfb4cf09b6 (HEAD -> mypatch2)
>Author: Paul Robinson <paul.robinson at sony.com>
>Date:   Mon Aug 10 08:47:15 2020 -0700
>
>    a dependent patch
>
>diff --git a/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll b/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>index ed3e2e6b481..9b9ce0ae82b 100644
>--- a/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>+++ b/llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll
>@@ -1,6 +1,6 @@
> ; REQUIRES: asserts
>
>-; RUN: opt -newgvn -S %s | FileCheck %s
>+; RUN: opt -newgvn %s -S | FileCheck %s
>
> ; XFAIL: *
>
>------------------------------
>This patch depends on the first one because it changed the same line,
>and preserves the change made by the first patch.
>
>Again, redirect the output of `git show` to generate a diff for Phab.
>As I mentioned before, Phab has a way to mark it as dependent on the
>previous patch, but offhand I don't know what that is.
>
>Now time has passed, 'master' has been updated, and I should update 
>all my branches.
>Start with 'master' and work your way through the dependent branches.
>The branch dependencies are master -> mypatch -> mypatch2,
>so you update each branch in order by rebasing on the previous.
>'master' is a special case because it needs to be rebased on the
>remote branch; mypatch rebased on master, mypatch2 rebased on 
>mypatch.
>------------------------------
>D:\upstream\llvm-project>git checkout master
>Switched to branch 'master'
>Your branch is up to date with 'origin/master'.
>
>D:\upstream\llvm-project>git pull --rebase
># lots of git output omitted for brevity
>
>D:\upstream\llvm-project>git checkout mypatch
>Updating files: 100% (1730/1730), done.
>Switched to branch 'mypatch'
>
>D:\upstream\llvm-project>git rebase master
>Successfully rebased and updated refs/heads/mypatch.
>
>D:\upstream\llvm-project>git checkout mypatch2
>Updating files: 100% (1731/1731), done.
>Switched to branch 'mypatch2'
>
>D:\upstream\llvm-project>git rebase mypatch
>Successfully rebased and updated refs/heads/mypatch2.
>------------------------------
>You'll note I used `git pull --rebase` to update master.
>The `--rebase` would be mandatory if I had committed to master
>(otherwise my local master branch history would be unusable for
>pushing to upstream).  It's harmless otherwise, so I've trained 
>myself always to use `--rebase` when updating from a remote.
>
>When it comes time to push my patch upstream, I need to push it
>on master, not on my local branch, so I need to move the patch
>from mypatch to master.
>------------------------------
>D:\upstream\llvm-project>git checkout master
>Switched to branch 'master'
>Your branch is up to date with 'origin/master'.
>
>D:\upstream\llvm-project>git merge --ff-only mypatch
>Updating 68330ee0a97..543870614f6
>Fast-forward
> llvm/test/Transforms/NewGVN/todo-pr42422-phi-of-ops.ll | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>------------------------------
>The `--ff-only` is LLVM project policy, we don't allow using
>merge commits; new patches must be made directly on master.
>At this point you could do `git push` of the first patch.
>
>Once that patch is pushed, the mypatch branch can be deleted,
>and mypatch2 can be rebased directly onto master.  Repeat as
>needed for each patch.
>
>HTH,
>--paulr