[llvm-dev] How to deal with multiple patches to the same file
Robinson, Paul via llvm-dev
llvm-dev at lists.llvm.org
Mon Aug 10 09:19:32 PDT 2020
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
> -----Original Message-----
> From: Paul C. Anagnostopoulos <paul at windfall.com>
> Sent: Monday, August 10, 2020 10:37 AM
> To: Robinson, Paul <paul.robinson at sony.com>
> Cc: 'llvm-dev at lists.llvm.org' <llvm-dev at lists.llvm.org>
> Subject: RE: [llvm-dev] How to deal with multiple patches to the same file
>
> I think I understand the concepts, but I sure would appreciate a specific
> example, and I appreciate the offer. To make your life harder, could you
> start with what I should do given that I have not created a branch for the
> first patch? I just have the six files staged.
>
> I have GitHub Desktop installed, if that makes any of the steps easier.
>
> Thanks again, and no rush!
>
> At 8/10/2020 10:07 AM, Robinson, Paul wrote:
>
> >> -----Original Message-----
> >> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Paul C.
> >> Anagnostopoulos via llvm-dev
> >> Sent: Monday, August 10, 2020 9:30 AM
> >> To: llvm-dev at lists.llvm.org
> >> Subject: [llvm-dev] How to deal with multiple patches to the same file
> >>
> >> I have submitted a patch to Phabricator that includes the TableGen file
> >> TGparser.cpp. Now I want to fix a bug in that file. What is the proper
> >> procedure so that the two patches don't get screwed up, either in my
> >> repository or in the master repository? Please answer as if I'm a
> >> git/Phabricator idiot, because, well, I am.
> >>
> >> I should note that all I did in my repository for the first patch was
> >> stage the files and then do a diff --staged. Those files remain staged
> >> because I'm not sure what to do with them given that we use Phabricator
> >> and not pull requests.
> >
> >(I'm not completely giving you recipes; if you need more explicit steps
> >then let me know and I'll do a full-on example.)
> >
> >Even though we don't use pull requests, I still tend to commit my patches
> >to branches off of master, in my local clone. This allows `git show` to
> >generate the diff. As master advances, you update your master branch and
> >then on the patch branch, a simple `git rebase master` updates it.
> >
> >Assuming the patch is approved, on master you can `git merge --ff-only`
> >to move the patch to your local master, and then push it from there.
> >
> >If the bugfix is not near your other changes, and can be made
> independently,
> >then you can treat it as a fully independent patch on its own branch.
> >
> >If the bugfix is near your other changes or is dependent on it, then I'd
> >create a second patch branch based on the first patch branch. As before,
> >`git show` generates the diff, and in Phabricator there's a way to mark
> >the second patch as dependent on the first patch. I haven't done this
> >personally so I'm unclear about the exact steps, but I see people doing
> >"patch series" all the time.
> >
> >HTH and again let me know if you need a more explicit example.
> >--paulr
> >
> >>
> >> _______________________________________________
> >> LLVM Developers mailing list
> >> llvm-dev at lists.llvm.org
> >> https://urldefense.com/v3/__https://lists.llvm.org/cgi-
> bin/mailman/listinfo/llvm-
> dev__;!!JmoZiZGBv3RvKRSx!ovCHi0NZ2mAcEA88945Pxt61jI_GYQE-
> ML8yCXISbibLG4DbExFZyshtAzXq6WyKFA$
>
>
> ----------------------------------------------------------------
> Windfall Paul C. Anagnostopoulos
> ----------------------------------------------------------
> Software 978 369-0839
>
> https://urldefense.com/v3/__http://www.windfall.com__;!!JmoZiZGBv3RvKRSx!o
> vCHi0NZ2mAcEA88945Pxt61jI_GYQE-ML8yCXISbibLG4DbExFZyshtAzXhmXYM-A$
> ----------------------------------------------------------------
> My life has been filled with calamities,
> some of which actually happened.
> ---Mark Twain
>
> Guga 'mzimba, sala 'nhliziyo
More information about the llvm-dev
mailing list