Infrastructure/Phabricator: Difference between revisions
Add section about git identity |
Tweak and improve the arcanist section |
||
Line 55: | Line 55: | ||
=== Debian/Ubuntu/KDE Neon === | === Debian/Ubuntu/KDE Neon === | ||
<pre> | <pre> | ||
sudo apt install arcanist | $ sudo apt install arcanist | ||
</pre> | </pre> | ||
=== Fedora === | === Fedora === | ||
<pre> | <pre> | ||
sudo dnf copr enable kanarip/phabricator | $ sudo dnf copr enable kanarip/phabricator | ||
sudo dnf install arcanist | $ sudo dnf install arcanist | ||
</pre> | </pre> | ||
Line 67: | Line 67: | ||
Tumbleweed: | Tumbleweed: | ||
<pre> | <pre> | ||
sudo zypper ar -f https://download.opensuse.org/repositories/devel:tools:scm/openSUSE_Tumbleweed/devel:tools:scm.repo | $ sudo zypper ar -f https://download.opensuse.org/repositories/devel:tools:scm/openSUSE_Tumbleweed/devel:tools:scm.repo | ||
sudo zypper install arcanist | $ sudo zypper install arcanist | ||
</pre> | </pre> | ||
Leap 15: | Leap 15: | ||
<pre> | <pre> | ||
sudo zypper ar -f https://download.opensuse.org/repositories/devel:tools:scm/openSUSE_Leap_15.0/devel:tools:scm.repo | $ sudo zypper ar -f https://download.opensuse.org/repositories/devel:tools:scm/openSUSE_Leap_15.0/devel:tools:scm.repo | ||
sudo zypper install arcanist | $ sudo zypper install arcanist | ||
</pre> | </pre> | ||
=== Arch/Antergos/Manjaro === | === Arch/Antergos/Manjaro === | ||
<pre> | <pre> | ||
yaourt -S arcanist-git | $ yaourt -S arcanist-git | ||
</pre> | </pre> | ||
Line 95: | Line 95: | ||
You will need to tell git who you really are, so your patches will include correct authorship information and can be correctly attributed to yourself. This only needs to be done once: | You will need to tell git who you really are, so your patches will include correct authorship information and can be correctly attributed to yourself. This only needs to be done once: | ||
<pre> | <pre> | ||
git config --global user.name <Your real name> | $ git config --global user.name <Your real name> | ||
git config --global user.email <Your identity.kde.org email address> | $ git config --global user.email <Your identity.kde.org email address> | ||
</pre> | </pre> | ||
Line 105: | Line 105: | ||
Before editing anything, create a new branch for your patch: | Before editing anything, create a new branch for your patch: | ||
<pre> | <pre> | ||
$ git checkout -b <branchname> - | $ git checkout -t -b <branchname> # the -t flag makes git "track" the current branch | ||
</pre> | </pre> | ||
Now make changes on the feature branch. When you're ready to have your changes reviewed, enter the following command: | |||
<pre> | <pre> | ||
$ arc diff | $ arc diff | ||
</pre> | </pre> | ||
Line 132: | Line 130: | ||
Don't touch the "Differential Revision:" section. | Don't touch the "Differential Revision:" section. | ||
=== Step 2: Update your diff === | === Step 2: Update your diff in response to review comments === | ||
After <tt>arc</tt> uploads the patch to phabricator, the project's reviewers will take a look and give you some comments. If you get a thumbs up immediately, you can skip this step. But often you will get a review like "looks good, we can take it if you fix problems x, y, and z." Make the necessary changes, add an extra commit to the git branch, and update the Phabricator revision: | After <tt>arc</tt> uploads the patch to phabricator, the project's reviewers will take a look and give you some comments. If you get a thumbs up immediately, you can skip this step. But often you will get a review like "looks good, we can take it if you fix problems x, y, and z." Make the necessary changes, add an extra commit to the git branch, and update the Phabricator revision: | ||
<pre> | <pre> | ||
[implement changes based on review comments] | [implement changes based on review comments] | ||
$ arc diff | $ arc diff | ||
</pre> | </pre> | ||
Line 143: | Line 139: | ||
=== Step 3: Land your diff === | === Step 3: Land your diff === | ||
If you do not have a [[Infrastructure/Get_a_Contributor_Account | KDE Developer Account]], then someone who does will have to land your patch for you. Otherwise, you can do it yourself once the patch has been accepted and reviewers have given you permission to "ship it!" | If you do not have a [[Infrastructure/Get_a_Contributor_Account | KDE Developer Account]], then someone who does will have to land your patch for you. Otherwise, you can do it yourself once the patch has been accepted and reviewers have given you permission to "ship it!" | ||
First make sure that the world is sane, and that only your patch will be landed: | |||
<pre> | |||
$ [make sure you are on your feature branch] | |||
$ arc land --preview | |||
</pre> | |||
The output of that command should show only the commit message for your patch. If you see more than that, stop and ask your reviewers for help. | |||
==== Landing on master ==== | ==== Landing on master ==== | ||
This is simple: | |||
<pre> | <pre> | ||
$ arc land | $ arc land | ||
</pre> | </pre> | ||
==== Landing on the "Stable branch" ==== | |||
By default, arc will land the patch onto whatever branch was the parent . For example, if you branched from master; <tt>arc</tt> will land on master; if you branched from <tt>Applications/18.04</tt>, arc will land on that branch instead. | |||
Often you will find yourself being asked to land your diffl on the "stable branch", even though you originally branched from master. To make this work, rebase your patch on the stable branch: | |||
<pre> | <pre> | ||
$ [make sure you are on your feature branch] | $ [make sure you are on your feature branch] | ||
Line 178: | Line 180: | ||
=== Updating the summary of the Differential from the local Git commit message === | === Updating the summary of the Differential from the local Git commit message === | ||
If you changed the commit message locally and want to update the text in the summary in Differential, call Arc like this: | If you changed the commit message locally and want to update the text in the summary in Differential, call Arc like this: | ||
<pre> | <pre> | ||
Line 185: | Line 186: | ||
=== Updating the local Git commit message from changes done on Phabricator === | === Updating the local Git commit message from changes done on Phabricator === | ||
If you or someone else updated the title, summary, test plan, reviewers or subscribers of a Diff using the web editor in Phabricator, Arcanist allows to sync those changes back to your local Git commit message: | If you or someone else updated the title, summary, test plan, reviewers or subscribers of a Diff using the web editor in Phabricator, Arcanist allows to sync those changes back to your local Git commit message: | ||
<pre> | <pre> | ||
Line 197: | Line 197: | ||
=== "Please do that in a separate commit" === | === "Please do that in a separate commit" === | ||
Should your patch contain an unrelated change, your reviewer will ask you to undo that part and possibly open a new Diff for that. Here is what you can do: | Should your patch contain an unrelated change, your reviewer will ask you to undo that part and possibly open a new Diff for that. Here is what you can do: | ||
Revision as of 15:25, 10 March 2018
Phabricator is KDE's task management and patch review system. This page is intended to serve as a general-purpose introduction to the most important aspects: submitting and reviewing patches. It should not take long before you are happily using Phabricator.
Basic Tasks
Logging in
Log in to Phabricator with your KDE Identity account. If you don't have one, you can sign up for one here. At the Phabricator home page, click the "Log in" button on the top of the page and enter your KDE Identity username and password:
If your KDE Identity account works on http://identity.kde.org, but not on http://phabricator.kde.org, please contact the KDE sysadmins at [email protected]
Getting help
The official documentation is in the Phabricator book and on their website -- note that since everything is under rapid development, most of the documentation is incomplete. A good way to find the information you're looking for is to search Phabricator upstream.
Posting a Patch using the website
First, create a diff file using git diff (or git format-patch) Log in to Phabricator and enter Code Review. Click the +Create Diff button in the upper-right corner of the screen. Paste the text of the diff or upload the file using the file dialog. Select the project at the bottom; if you start typing it will perform a search among active projects. For a reviewer, use the name of the project itself (the text-field will auto-complete valid projects to help you out). In some cases, you will want to request a review from particular people instead of or in addition to the group.
Formatting your patch
Once the patch lands, the Title will become the git commit message, so please follow commit message best practices.
In the Summary section, write at least one sentence describing your change and why it is necessary, adding more detail if necessary.
Add special keywords
If your patch is intended to fix a Bugzilla ticket, include one of the following on its own line, at the end of the Summary section, below all other text:
BUG: 385942
or
FEATURE: 384936
(Just the Bugzilla ticket number, not the full URL)
Use BUG: If the Bugzilla describes a bug, and FEATURE if the Bugzilla ticket describes a feature request. Either of these tags will cause that Bugzilla ticket to be automatically closed when the patch is committed. Note that these tags cannot appear on the first line of the Summary section; there must be some text before them.
If you added the BUG: or FEATURE: tag, also add another tag indicating what version receives the fix or new feature:
FIXED-IN: [version]
Replace [version] with an appropriate version string; see Guidelines and HOWTOs/Write a version string to find out how to write one. If you can't figure it out, don't worry about it and just omit the tag; a KDE developer will help you add it during code review.
Here is more information about other special messages that interact with Bugzilla tickets. You can also add special messages that interact with other Phabricator tools (e.g. Maniphest tasks).
Include some screenshots
For patches that change something about the user interface, it's customary to include a screenshot of how the interface looks with your patch. Bonus points for including a "Before" image too, so reviewers can easily compare them.
Using Arcanist to post patches
What is Arc?
Using the web UI to post patches gets tiresome after a while. Arcanist is a tool to simplify and speed up the process of posting, updating, and merging Phabricator patches.
Installing Arcanist
Debian/Ubuntu/KDE Neon
$ sudo apt install arcanist
Fedora
$ sudo dnf copr enable kanarip/phabricator $ sudo dnf install arcanist
openSUSE
Tumbleweed:
$ sudo zypper ar -f https://download.opensuse.org/repositories/devel:tools:scm/openSUSE_Tumbleweed/devel:tools:scm.repo $ sudo zypper install arcanist
Leap 15:
$ sudo zypper ar -f https://download.opensuse.org/repositories/devel:tools:scm/openSUSE_Leap_15.0/devel:tools:scm.repo $ sudo zypper install arcanist
Arch/Antergos/Manjaro
$ yaourt -S arcanist-git
Windows
The most non-obvious step is that you will need to configure your PHP installation to use Curl. This requires editing the php.ini configuration file to add the line extension_dir = "ext" and add php_curl to the list of extensions. After adding php.exe to your PATH and installing arcanist/libphutil, you can run arc by defining a function in your Powershell profile:
function arc { php -f "C:\path\to\arcanist.php" -- $args }
After you have installed Arc, you can learn more using man arc or arc --help. Another command useful for getting a feel for Phabricator's style is arc anoid.
Set your git identity
You will need to tell git who you really are, so your patches will include correct authorship information and can be correctly attributed to yourself. This only needs to be done once:
$ git config --global user.name <Your real name> $ git config --global user.email <Your identity.kde.org email address>
Workflow
The sanest and easiest way to use arc is to follow a typical feature branch workflow: keep your master branch synchronized with the upstream repository, and make all changes on separate branches. Each patch needs its own private, temporary branch. Once your patch has been merged, delete the feature branch, and make another new branch for the next patch.
Step 1: Create a new diff
Before editing anything, create a new branch for your patch:
$ git checkout -t -b <branchname> # the -t flag makes git "track" the current branch
Now make changes on the feature branch. When you're ready to have your changes reviewed, enter the following command:
$ arc diff
When you run arc diff, you will go through a series of dialogues. At the end, you will be asked to rewrite your Git commit message to fit the standard Differential format, like so:
<first line of original git commit message> Summary: <rest of original commit message> Reviewers: Differential Revision: <URL to the new diff>
<first line of original git commit message> will become the commit message, so please follow commit message best practices.
As when using the web UI, enter any special Bugzilla keywords (such as BUG: 385942) on their own lines in the "Summary" section.
As when using the web UI, for the "Reviewers" section, enter a hashtag for the review group--which is usually equal to the project name. For example, the review group forKrita is #krita and the review group for KDE PIM is #kde_pim. If you can't figure out the correct hashtag, leave the Reviewers section blank for now over arc's protestations and then edit your revision on the web UI after it's been created; you will be able to enter reviewers into a nice text box that auto-completes with valid reviewers, which is much more user-friendly.
Don't touch the "Differential Revision:" section.
Step 2: Update your diff in response to review comments
After arc uploads the patch to phabricator, the project's reviewers will take a look and give you some comments. If you get a thumbs up immediately, you can skip this step. But often you will get a review like "looks good, we can take it if you fix problems x, y, and z." Make the necessary changes, add an extra commit to the git branch, and update the Phabricator revision:
[implement changes based on review comments] $ arc diff
Step 3: Land your diff
If you do not have a KDE Developer Account, then someone who does will have to land your patch for you. Otherwise, you can do it yourself once the patch has been accepted and reviewers have given you permission to "ship it!"
First make sure that the world is sane, and that only your patch will be landed:
$ [make sure you are on your feature branch] $ arc land --preview
The output of that command should show only the commit message for your patch. If you see more than that, stop and ask your reviewers for help.
Landing on master
This is simple:
$ arc land
Landing on the "Stable branch"
By default, arc will land the patch onto whatever branch was the parent . For example, if you branched from master; arc will land on master; if you branched from Applications/18.04, arc will land on that branch instead.
Often you will find yourself being asked to land your diffl on the "stable branch", even though you originally branched from master. To make this work, rebase your patch on the stable branch:
$ [make sure you are on your feature branch] $ git fetch $ git rebase --onto origin/Applications/18.04 master $ arc land --onto Applications/18.04
Note that in most repositories, after committing to the "stable" branch you are expected to merge the "stable" branch to "master" afterwards (check the Git history to be sure):
$ git checkout master $ git merge -Xours origin/Applications/18.04 $ git push
Arcanist Tips & Tricks
Look before you diff
You can check with arc which what Arcanist will do before performing the actual upload with arc diff if you are unsure what will happen. In particular, look for which commits will be included in your Diff.
arc diff: specify a revision manually
Sometimes - if you messed up with your git branches - arc cannot properly determine which revision (D12345) should be updated. In this case you can use arc diff --update D12345. See arc help diff.
Updating the summary of the Differential from the local Git commit message
If you changed the commit message locally and want to update the text in the summary in Differential, call Arc like this:
$ arc diff --edit --verbatim
Updating the local Git commit message from changes done on Phabricator
If you or someone else updated the title, summary, test plan, reviewers or subscribers of a Diff using the web editor in Phabricator, Arcanist allows to sync those changes back to your local Git commit message:
$ arc amend
Note that in general Arcanist will do this automatically for you once you arc land.
Advanced Tasks
Once in a while a reviewer will tell you to do specific things. This section will help you to figure out what is meant to be done.
"Please do that in a separate commit"
Should your patch contain an unrelated change, your reviewer will ask you to undo that part and possibly open a new Diff for that. Here is what you can do:
If the change in question is in a separate commit on your local branch:
$ git revert <unrelated change> $ arc diff # this updates the first Diff $ git checkout -b <new branch name> master $ git cherry-pick <unrelated change> $ arc diff # this creates a new Diff for the unrelated change
If you mixed different changes in a single commit on your local branch:
$ git reset HEAD^ $ git add -p # type "?" for help, then pick all hunks you want to keep $ git stash # the stash now contains the hunks for the second patch $ git commit $ arc diff # this updates the first Diff $ git checkout -b <new branch name> master $ git stash pop $ git commit $ arc diff # this creates a new Diff for the unrelated change
In case this does not work for you, there's always the plain old copy-and-paste. In general, it is best to avoid adding unrelated changes from the beginning. :-)
Marking patches as dependent on other patches
Sometimes you will want to submit a patch that depends on another patch, creating a dependency chain.
If each patch is intended for a different project
Example: you submit a patch to add new feature in KIO, and then submit another patch for Dolphin that uses that feature. Here's what you do:
- Create your first patch as above
- When creating the second patch, add the following to its own line in the "Summary" section:
Depends on DXXXX
(Replace "DXXXX" with the ID of the dependent patch, not the full URL)
If the patches are all for the same project
Example: you are implementing multiple new features for a single project that each depend on the patch for the prior feature. Here's what you do:
- Create a branch to track the first feature:
$ git checkout -b <branch name for feature 1> --track origin/master
Then implement the feature and make a commit.
- Then create a branch for your second feature, tracking the first branch:
git checkout -b <branch name for feature 2> --track <branch name for feature 1>
As above, implement the feature and make a commit. Continue this pattern for any other required dependent features.
-
When you're ready to turn your dependency chain feature branches into patches, do the following:
$ git checkout <branch name for feature 1> $ arc diff [then go through the process of creating the patch normally] $ git checkout <branch name for feature 2> git commit --amend [then add the special text "Depends on DXXXX", replacing DXXXX with the ID of the first patch $ arc diff [then go through the process of creating the patch normally]
...And so on.
- After you get comments, you will have to make changes to your patches and re-base dependent patches:
$ git checkout <branch name for feature 1> [Make changes] $ git add -u $ git commit $ arc diff $ git checkout <branch name for feature 2> $ git rebase <branch name for feature 1> $ git add -u $ git commit $ arc diff
...And so on.
- When you're ready to land any or all of your patches, do it in sequence, starting from the patch with no unmet dependencies:
$ git checkout <branch name for feature 1> $ arc land $ git checkout <branch name for feature 2> $ git rebase origin/<target branch> $ arc land
Customization
Creating custom dashboard feeds
You can customize your Phabricator homepage by creating a new dashboard. However, the selection of what you can post on your dashboard is limited. The defaults will show all tasks from all projects.
To narrow this down, you need to define a custom query to serve as a filter. For example, if you work on Plasma Mobile and want to monitor the to-do list, perhaps you want to show only tasks which are in the Plasma Mobile and are tagged as open. To do that, enter Maniphest, select "advanced search," select the appropriate terms, then click "save custom query." You can give your query a name. Once it is saved, the query will become available as a new filter for creating feeds on your dashboard. (In Differential you seem to need to perform the test search before the "save query" button becomes visible.)