Paul C. Anagnostopoulos via llvm-dev
2020-Aug-10 14:36 UTC
[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://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev---------------------------------------------------------------- Windfall Paul C. Anagnostopoulos ---------------------------------------------------------- Software 978 369-0839 www.windfall.com ---------------------------------------------------------------- My life has been filled with calamities, some of which actually happened. ---Mark Twain Guga 'mzimba, sala 'nhliziyo
Robinson, Paul via llvm-dev
2020-Aug-10 16:19 UTC
[llvm-dev] How to deal with multiple patches to the same file
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
Paul C. Anagnostopoulos via llvm-dev
2020-Aug-10 18:12 UTC
[llvm-dev] How to deal with multiple patches to the same file
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