Difference between revisions of "Managing Xen Patches with Git"

From Xen
m (Addressing Review Comments)
(Create a branch for your changes: No need to specify staging twice.)
 
(12 intermediate revisions by 2 users not shown)
Line 7: Line 7:
 
Similar documents exist for
 
Similar documents exist for
 
* '''StGit''', a Python application providing similar functionality to Quilt (i.e. pushing/popping patches to/from a stack) on top of Git. You can find instructions at [[Managing Xen Patches with StGit]].
 
* '''StGit''', a Python application providing similar functionality to Quilt (i.e. pushing/popping patches to/from a stack) on top of Git. You can find instructions at [[Managing Xen Patches with StGit]].
  +
* '''git-series''' is a tool on top of Git that tracks changes to a patch series over time, including cover letter for the patch series, formats the series for email, and prepares patch submission. You can find instructions at [[Managing Xen Patches with Git-series]].
   
 
== Generating an initial Patch or Patch Series ==
 
== Generating an initial Patch or Patch Series ==
 
Begin by cloning the git repo from XenProject.org:
 
Begin by cloning the git repo from XenProject.org:
 
<syntaxhighlight lang="sh" highlight="1">
 
<syntaxhighlight lang="sh" highlight="1">
$ git clone git://xenbits.xenproject.org/xen.git xen.git
+
$ git clone git://xenbits.xenproject.org/xen.git
  +
$ cd xen
 
</syntaxhighlight>
 
</syntaxhighlight>
   
 
At this point you should have <code>xenbits</code> set up as the remote repostiory called "origin":
 
At this point you should have <code>xenbits</code> set up as the remote repostiory called "origin":
<syntaxhighlight lang="sh" highlight="1">
+
<syntaxhighlight lang="sh" highlight="1">
 
$ git branch -a
 
$ git branch -a
 
* master
 
* master
  +
staging
remotes/origin/HEAD -> origin/master
 
  +
...
remotes/origin/master
 
remotes/origin/stable-4.0
 
remotes/origin/stable-4.1
 
remotes/origin/stable-4.2
 
remotes/origin/staging
 
remotes/origin/staging-4.0
 
remotes/origin/staging-4.1
 
remotes/origin/staging-4.2
 
 
</syntaxhighlight>
 
</syntaxhighlight>
   
Line 32: Line 27:
   
 
The branch called <code>staging</code> is the bleeding-edge of commits: this branch is tested regularly with the <code>xenproject.org</code> build and regression testing system, and when it passes, changes are pushed to the <code>master</code> branch. However, <code>master</code> can be significantly behind <code>staging</code>.
 
The branch called <code>staging</code> is the bleeding-edge of commits: this branch is tested regularly with the <code>xenproject.org</code> build and regression testing system, and when it passes, changes are pushed to the <code>master</code> branch. However, <code>master</code> can be significantly behind <code>staging</code>.
  +
  +
{{Anchor|staging-master}}
  +
=== Developing against <code>staging</code> or <code>master</code>? ===
   
 
From the contributor's point of view, this gives a choice of
 
From the contributor's point of view, this gives a choice of
* Use <code>staging</code> as a development baseline and have it apply easily to the tree when all changes are approved. This exposes you to the risk of importing showstopper bugs which prevent you from building or testing. This happens very infrequently, which is why we recommend that people '''develop against staging'''.
+
* Use <code>staging</code> as a development baseline and have it apply easily to the tree when all changes are approved. This exposes you to the risk of importing showstopper bugs which prevent you from building or testing.
 
* Use <code>master</code> which may mean that your change wont apply to <code>staging</code>. When this occurs your committer ought to try and resolve merge conflicts: however this does not always happen. Thus, it makes sense to rebase against <code>staging</code> when you are close to completing the code review process.
 
* Use <code>master</code> which may mean that your change wont apply to <code>staging</code>. When this occurs your committer ought to try and resolve merge conflicts: however this does not always happen. Thus, it makes sense to rebase against <code>staging</code> when you are close to completing the code review process.
  +
  +
We would recommend to
  +
* Develop smaller or shorter-lived changes against <code>staging</code>
  +
* Develop a big series which are going to go through multiple rounds against <code>master</code> and deal with rebasing it to staging at the end
   
 
The remainder of this document assumes you develop against the <code>staging</code> branch.
 
The remainder of this document assumes you develop against the <code>staging</code> branch.
Line 41: Line 43:
 
=== Create a branch for your changes ===
 
=== Create a branch for your changes ===
 
When you want to introduce a change, start by making a new branch based on the most recent change in the <code>staging</code> branch.
 
When you want to introduce a change, start by making a new branch based on the most recent change in the <code>staging</code> branch.
 
<syntaxhighlight lang="sh" highlight="1">
 
$ git checkout -b staging remotes/origin/staging
 
Branch staging set up to track remote branch staging from origin.
 
Switched to a new branch 'staging'
 
</syntaxhighlight>
 
   
 
<syntaxhighlight lang="sh" highlight="1">
 
<syntaxhighlight lang="sh" highlight="1">
Line 92: Line 88:
 
Every single commit you make on your branch becomes a patch when you submit it. If your change consists of multiple commits, you will be committing a patch series. Information related to Xen Project conventions around patches and patch series can be found [[Submitting_Xen_Project_Patches#What_is_in_a_patch_and_a_patch_series.3F|here]].
 
Every single commit you make on your branch becomes a patch when you submit it. If your change consists of multiple commits, you will be committing a patch series. Information related to Xen Project conventions around patches and patch series can be found [[Submitting_Xen_Project_Patches#What_is_in_a_patch_and_a_patch_series.3F|here]].
   
== Sending a Patch to xen-devel@ ==
+
== Sending a Patch or Patch Series to xen-devel@ ==
 
You can find instructions on how to send patches in our [[Submitting_Xen_Project_Patches#send|Patch Submission Guide]].
 
You can find instructions on how to send patches in our [[Submitting_Xen_Project_Patches#send|Patch Submission Guide]].
   
Line 125: Line 121:
 
During an interactive rebase there are two ways to combine commits: <code>fixup</code> and <code>squash</code>. There are two corresponding options for the <code>[https://git-scm.com/docs/git-commit git-commit]</code> command called <code>--fixup</code> and <code>--squash</code>. These options instruct Git to write a commit message for us, expressing the intention that this new commit will eventually be squashed (or fixed up) with some existing commit.
 
During an interactive rebase there are two ways to combine commits: <code>fixup</code> and <code>squash</code>. There are two corresponding options for the <code>[https://git-scm.com/docs/git-commit git-commit]</code> command called <code>--fixup</code> and <code>--squash</code>. These options instruct Git to write a commit message for us, expressing the intention that this new commit will eventually be squashed (or fixed up) with some existing commit.
   
  +
Note that <code>--fixup</code> is typically not useful for the patch submission workflow, as you are always expected to add a change-log to a patch that has been modified.
For your first fix, there is no need to modify the original commit message. Thus, you can use <code>--fixup</code> and pass the commit that you want the changes to become part of:
 
  +
  +
For your first fix, you can use <code>--squash</code> and pass the commit that you want the changes to become part of:
   
 
<syntaxhighlight lang="sh" highlight="2">
 
<syntaxhighlight lang="sh" highlight="2">
 
$ git add .
 
$ git add .
$ git commit --fixup 8efb660
+
$ git commit --squash 8efb660
[my-feature cf02946] fixup! My first patch
+
[my-feature cf02946] squash! My first patch
  +
</syntaxhighlight>
  +
  +
Then you add the change history, which may look something like this:
  +
  +
<syntaxhighlight lang="sh">
  +
Changes in v2:
  +
- add #define LIBXL_HAVE_MEMORY_POLICY
  +
- ability to part the memory policy parameter even if gfn is not passed
  +
- rename cache_policy to memory policy
  +
- rename MEMORY_POLICY_DEVMEM to MEMORY_POLICY_ARM_DEV_nGRE
 
</syntaxhighlight>
 
</syntaxhighlight>
   
Line 136: Line 144:
   
 
<syntaxhighlight lang="sh" highlight="1,4">
 
<syntaxhighlight lang="sh" highlight="1,4">
cf02946 (HEAD -> my-feature) fixup! My first patch
+
cf02946 (HEAD -> my-feature) squash! My first patch
 
6711b87 (my-feature-v1) My third patch
 
6711b87 (my-feature-v1) My third patch
 
8db8b21 My second patch
 
8db8b21 My second patch
Line 143: Line 151:
 
</syntaxhighlight>
 
</syntaxhighlight>
   
For your second fix, it is necessary to change the commit message. Thus, you can use <code>--squash</code> to record what needs to be changed when you do the final rebase. For the first fix, you had to tell Git which commit the new changes should be merged with by using the respective SHA from the output of the git log. However, you could have referred to the commit in any of the various ways Git supports. One of the more useful one in code reviews is referring to the commit using some text that appears in its commit message: Git will interpret <code>:/foo</code> as ''the most recent commit that contained the string <code>foo</code> in the first line of its commit message''.
+
For the first fix, you had to tell Git which commit the new changes should be merged with by using the respective SHA from the output of the git log. However, you could have referred to the commit in any of the various ways Git supports. One of the more useful one in code reviews is referring to the commit using some text that appears in its commit message: Git will interpret <code>:/foo</code> as ''the most recent commit that contained the string <code>foo</code> in the first line of its commit message''.
   
 
To use this in your second code review, you can use:
 
To use this in your second code review, you can use:
Line 151: Line 159:
 
$ git commit --squash :/second
 
$ git commit --squash :/second
 
[my-feature 5d1ea0a] squash! My second patch
 
[my-feature 5d1ea0a] squash! My second patch
  +
</syntaxhighlight>
  +
  +
And don't forget to add the change history:
  +
  +
<syntaxhighlight lang="sh">
  +
Changes in v2:
  +
- ...
 
</syntaxhighlight>
 
</syntaxhighlight>
   
Line 157: Line 172:
 
$ git log --oneline --decorate
 
$ git log --oneline --decorate
 
5d1ea0a (HEAD -> my-feature) squash! My second patch
 
5d1ea0a (HEAD -> my-feature) squash! My second patch
cf02946 fixup! My first patch
+
cf02946 sqash! My first patch
 
6711b87 (my-feature-v1) My third patch
 
6711b87 (my-feature-v1) My third patch
 
8db8b21 My second patch
 
8db8b21 My second patch
Line 180: Line 195:
 
</syntaxhighlight>
 
</syntaxhighlight>
   
  +
And you can send out version 2 of the series following the steps outlined [[Managing_Xen_Patches_with_Git#Sending_a_Patch_or_Patch_Series_to_xen-devel.40|here]]
Note that it is possible to automatically enable autosquash rebases using the following setting
 
 
<syntaxhighlight lang="sh" highlight="1">
 
$ git config --global rebase.autosquash true
 
</syntaxhighlight>
 
   
 
== References ==
 
== References ==

Latest revision as of 08:15, 8 August 2019

This document assumes that you are familiar with the following documents

This document lays out basic examples and best practice of how to use Git to manage Xen patches as part of the patch submission process.

Similar documents exist for

  • StGit, a Python application providing similar functionality to Quilt (i.e. pushing/popping patches to/from a stack) on top of Git. You can find instructions at Managing Xen Patches with StGit.
  • git-series is a tool on top of Git that tracks changes to a patch series over time, including cover letter for the patch series, formats the series for email, and prepares patch submission. You can find instructions at Managing Xen Patches with Git-series.

Generating an initial Patch or Patch Series

Begin by cloning the git repo from XenProject.org:

$ git clone git://xenbits.xenproject.org/xen.git
$ cd xen

At this point you should have xenbits set up as the remote repostiory called "origin":

$ git branch -a
* master
  staging
  ...

This process automatically creates a local branch called master that will track the XenProject.org branch called master.

The branch called staging is the bleeding-edge of commits: this branch is tested regularly with the xenproject.org build and regression testing system, and when it passes, changes are pushed to the master branch. However, master can be significantly behind staging.

Developing against staging or master?

From the contributor's point of view, this gives a choice of

  • Use staging as a development baseline and have it apply easily to the tree when all changes are approved. This exposes you to the risk of importing showstopper bugs which prevent you from building or testing.
  • Use master which may mean that your change wont apply to staging. When this occurs your committer ought to try and resolve merge conflicts: however this does not always happen. Thus, it makes sense to rebase against staging when you are close to completing the code review process.

We would recommend to

  • Develop smaller or shorter-lived changes against staging
  • Develop a big series which are going to go through multiple rounds against master and deal with rebasing it to staging at the end

The remainder of this document assumes you develop against the staging branch.

Create a branch for your changes

When you want to introduce a change, start by making a new branch based on the most recent change in the staging branch.

$ git checkout -b out/trondle-calls staging
Switched to a new branch 'out/trondle-calls'

Develop and Test your change

Then edit the source files you want to change. You can see which files have changed by using git status.

Commit your change to your branch

When you're done editing, use git add to specify which file changes you want included in the changeset, and then use git commit to make a commit. You will be prompted to make a changset description. The conventions related to what should be in commit messages are described in Submitting Xen Project Patches. The example below is merely intended to explain the necessary git commands: when you submit patches you will likely need more detail than shown in this document.

$ git status
# On branch out/trondle-calls
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#	modified:   foobar/zot.c
#	modified:   foobar/zot.h
#
no changes added to commit (use "git add" and/or "git commit -a")
$ git add foobar/zot.c foobar/zot.h
$ git commit

Alternatively, you can commit all changes using "git commit -a":

$ git commit -a
foobar: Add a new trondle calls

Add a some new trondle calls to the foobar interface to support
the new zot feature.

Signed-off-by: Joe Smith <joe.smith@citrix.com>

Patches vs. Patch Series

Every single commit you make on your branch becomes a patch when you submit it. If your change consists of multiple commits, you will be committing a patch series. Information related to Xen Project conventions around patches and patch series can be found here.

Sending a Patch or Patch Series to xen-devel@

You can find instructions on how to send patches in our Patch Submission Guide.

Addressing Review Comments

Once you sent the initial patch or patch series, you are likely to get review comments on each patch or just some of them. Let's assume you have just sent v1 of a series on my-feature which is made up of the following three patches.

$ git log --oneline --decorate
6711b87 (HEAD -> my-feature) My third patch
8db8b21 My second patch
8efb660 My first patch
e18393d (staging) Old stuff on staging

You got feedback that needs to be addressed on the first patch and the second patch which requires re-working these patches. When re-sending the series, you are expected to retain the order and number of patches in the second revision, unless the maintainer asked you to refactor the series.

You could just commit these two changes with a message like “Fix first patch” and "Fix second patch" and worry about squashing it later, but then you have to remember which commit fixes which patch and manually re-order the list of commits in an interactive rebase. Git can do all of this automatically.

Creating outbound version branches

Generally, it is a good idea to create an outbound branch after you sent a patch for review. This allows you to go back to previous versions easily. If you share a branch of a complex series in your cover letter as suggested here, it is important to do this, such that the branch remains static and code reviewers do not get confused by unexpected changes in the shared git branch.

$ git branch my-feature-v1
$ git branch -a
* my-feature
  my-feature-v1
  staging
  master

Using autosquash to optimize the workflow

During an interactive rebase there are two ways to combine commits: fixup and squash. There are two corresponding options for the git-commit command called --fixup and --squash. These options instruct Git to write a commit message for us, expressing the intention that this new commit will eventually be squashed (or fixed up) with some existing commit.

Note that --fixup is typically not useful for the patch submission workflow, as you are always expected to add a change-log to a patch that has been modified.

For your first fix, you can use --squash and pass the commit that you want the changes to become part of:

$ git add .
$ git commit --squash 8efb660
[my-feature cf02946] squash! My first patch

Then you add the change history, which may look something like this:

Changes in v2:
- add #define LIBXL_HAVE_MEMORY_POLICY
- ability to part the memory policy parameter even if gfn is not passed
- rename cache_policy to memory policy
- rename MEMORY_POLICY_DEVMEM to MEMORY_POLICY_ARM_DEV_nGRE

The git history now looks like:

cf02946 (HEAD -> my-feature) squash! My first patch
6711b87 (my-feature-v1) My third patch
8db8b21 My second patch
8efb660 My first patch
e18393d (staging) Old stuff on staging

For the first fix, you had to tell Git which commit the new changes should be merged with by using the respective SHA from the output of the git log. However, you could have referred to the commit in any of the various ways Git supports. One of the more useful one in code reviews is referring to the commit using some text that appears in its commit message: Git will interpret :/foo as the most recent commit that contained the string foo in the first line of its commit message.

To use this in your second code review, you can use:

$ git add .
$ git commit --squash :/second
[my-feature 5d1ea0a] squash! My second patch

And don't forget to add the change history:

Changes in v2:
- ...

The git history now looks like:

$ git log --oneline --decorate
5d1ea0a (HEAD -> my-feature) squash! My second patch
cf02946 sqash! My first patch
6711b87 (my-feature-v1) My third patch
8db8b21 My second patch
8efb660 My first patch
e18393d (staging) Old stuff on staging

You can now prepare for sending out v2 of the series by rebasing the series with the following command, which only picks up on commits that begin with fixup! or squash!. Git still gives you the chance to move things around in your editor like a regular interactive rebase.

$ git rebase --interactive --autosquash HEAD~5

The git history now looks like:

$ git log --oneline --decorate
7e82927 (HEAD -> my-feature) My third patch
7b4d943 My second patch
32b81e1 My first patch
e18393d (staging) Old stuff on staging

And you can send out version 2 of the series following the steps outlined here

References