Jump to content

Infrastructure/Phabricator: Difference between revisions

From KDE Community Wiki
Connecting to KDE: change quickgit.k.o link to cgit.k.o
Ngraham (talk | contribs)
Mention that Phab is still used for task tracking for the time being
 
(91 intermediate revisions by 24 users not shown)
Line 1: Line 1:
= Phabricator User Guide =
[http://phabricator.kde.org/ Phabricator] is KDE's task management system. It was used for patch review and other functions in the past, but KDE has since transitioned to GitLab, at https://invent.kde.org. Phabricator is still used for task tracking until this functionality is migrated to GitLab.
[http://phabricator.org/ Phabricator] is a task management system which KDE is transitioning toward. It is written in php (hence the ph) and structured as a collection of applications, most of which take the form of web modules. These modules can be seen on the left hand side of the [https://phabricator.kde.org/ KDE Phabricator], with names like Differential, Maniphest, and Phriction.


Phabricator is under very active development, but is already an excellent tool. 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. If you are not happy, you can submit bug reports to to [https://secure.phabricator.com/ Phabricator's own Phabricator] to request changes.  Note this may only address Phabricator-related unhappiness.
{{Info|Do not submit patches using Phabricator; instead use https://invent.kde.org. See [[Infrastructure/GitLab]] for details.<br/><br/>Do not submit bug reports using Phabricator; instead use https://bugs.kde.org See [[Get_Involved/Issue_Reporting]] for details.}}


The modular structure allows projects to flexibly create their own workflows. Project maintainers should keep developers and contributors up to date with more specific guidelines on their pages in the KDE Community Wiki or Phriction.


== Basic Tasks ==
=== Logging in ===
The first challenge posed by Phab is logging in.  You will use your KDE Identity account for this.  If you don't have one, you can [https://identity.kde.org/index.php?r=registration/index sign up for one here ].  At the Phabricator login screen, enter that username and password in the "Login or Register with LDAP," which is the lower form. Hopefully this will be clarified and simplified in the future.


=== Getting help ===
= Basic Tasks =
The official documentation is in the [https://secure.phabricator.com/book/phabricator/ Phabricator book] and [https://phacility.com/phabricator/ on their website] -- note the official website is full of puns -- but 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 [https://secure.phabricator.com/ Phab upstream]. The search is in the upper right hand corner. In the future the official documentation may be hosted on the KDE site.
== Logging in ==
Log in to [http://phabricator.kde.org/ Phabricator] with your KDE Identity account. If you don't have one, you can [https://identity.kde.org/index.php?r=registration/index 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:


=== Phab Apps ===
[[File:Phabricator login 2.png ]]
Luckily the use of most of the web applications is described in a small tag line. Differential and Maniphest, the code review and task management applications, are where you should expect to spend most of your time. Calligra intends to collect information for developers in Phriction. You can also create your own to-do list in Dashboard. In addition to the pages which are listed there are more applications available. Some others not listed on the main page by default are Pholio, for discussing mockups, Slowvote, for conducting user polls, and Paste, for sharing text snippets.
If your KDE Identity account works on http://identity.kde.org but not on http://phabricator.kde.org, please contact the KDE sysadmins at sysadmin@kde.org.


=== Posting a Patch ===
== Getting help ==
First, create a diff file using <tt>git diff</tt>. (Do not use <tt>git format-patch</tt>.) Log in to Phab and enter Differential. Click the <tt>+Create Diff</tt> 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.  
The official documentation is in the [https://secure.phabricator.com/book/phabricator/ Phabricator book] and [https://phacility.com/phabricator/ 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 [https://secure.phabricator.com/ Phabricator upstream].


For a reviewer, the default is to select the project itself. That means all users who are members of the project will be notified, and any user can approve the patch. In some cases, you will want to request a review from particular people instead.
== Posting a Patch using the website ==
Once you have [https://community.kde.org/Get_Involved/development#Set_up_your_development_environment set up your development environment] and [https://community.kde.org/Get_Involved/development#Submit_a_patch created a patch], you can submit it using Phabricator!


{{Note|1= You '''must''' specify at least one reviewer for your diff. If you upload a revision with no reviewers, nobody will get notified.}}
Log in to Phabricator and click on <tt>Code Review</tt> in the list on the left. Then, click the <tt>+Create Diff</tt> button in the upper-right corner of the screen. Paste the text of the diff or upload the file using the file dialog. Reviewers are mostly added automatically, but if your patch includes any visual or user interface changes, please add <tt>#vdg</tt> as a reviewer to make sure the [[Get Involved/design | KDE Visual Design Group]] sees it! Please make sure to add a screenshot, too.


== Using Arcanist ==
==Formatting your patch ==
=== What is Arc? ===
The Title of your patch will become the git commit message, so please follow [https://chris.beams.io/posts/git-commit/#seven-rules commit message best practices] when creating a title for the patch.
Arcanist is Phabricator's fully featured command line tool to interface with your local Git repository. It can be used to submit your patches for review. It tries hard to do the right thing and make sure it is doing the right thing. Like the rest of Phabricator it is written in PHP. After you have installed Arc, you can learn more using <tt>man arc</tt> or <tt>arc --help</tt>. Another command useful for getting a feel for Phabricator's style is <tt>arc anoid</tt>.


=== Installing on Linux ===
In the Summary section, write at least one sentence describing your change and why it is necessary, adding more details if necessary.
Although Arc is provided in the official Ubuntu repository and presumably others, at the time of this writing, the development is too fast paced for this version to be up-to-date. A good option is to install directly from upstream. You will need the php command line and CURL packages: in Ubuntu, php5-cli and php5-curl. Then you can download the repositories and add the arc command line tool to your command line.  [https://gist.github.com/thomas-barthelemy/882e6e19405ebecb14be A script to do this automatically in Ubuntu is here.]


=== Installing on Windows ===
=== Add special keywords ===
Downloading the Arcanist web scripts on Windows is no different than on Linux, but getting php is a little more involved, since there is no package manager. Phabricator has a more complete installation guide.
If your patch is intended to fix a Bugzilla ticket, include one of the following on its own line in the Summary section:
<pre>
BUG: 385942
</pre>
or
<pre>
FEATURE: 384936
</pre>
(Just the Bugzilla ticket number, '''not the full URL''')


* [https://secure.phabricator.com/book/phabricator/article/arcanist_windows/ Arc installation guide] 
Use <tt>BUG:</tt> If the Bugzilla ticket describes a bug, and <tt>FEATURE:</tt> 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.
* [http://php.net/manual/en/install.windows.manual.php Php installation guide]


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 <tt>extension_dir = "ext"</tt> and add <tt>php_curl</tt> to the list of extensions.
If you added the <tt>BUG:</tt> or <tt>FEATURE:</tt> tag, also add another tag indicating what version receives the fix or new feature:
<pre>
FIXED-IN: [version]
</pre>
Replace <tt>[version]</tt> 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.


After adding php.exe to your path and installing arcanist/libphutil, you can run arc by defining a function in your Powershell profile:
[https://techbase.kde.org/Development/Git/Configuration#Commit_Template Here is more information] about other special messages that interact with Bugzilla tickets. You can also add special messages that interact with [https://secure.phabricator.com/T5132 other Phabricator tools] (e.g. Maniphest tasks).
<pre> function arc { php -f "C:\path\to\arcanist.php" -- $args }</pre>


=== Connecting to KDE ===
{{Note|These keywords will only work if the email address in your <tt>~/.config/git/config</tt> file matches the email address used for your https://bugs.kde.org account (See [https://techbase.kde.org/Development/Git/Configuration#Basic_Settings this page] for more information).}}
Your project repo should contain the file <tt>.arcconfig</tt> in the root directory.  If not, [https://cgit.kde.org/calligra.git/tree/.arcconfig here is a simple template].  The only additional information Arc needs is your login identifier.  This is as simple as going to the link https://phabricator.kde.org/conduit/login/ and installing your API token with <tt>arc install-certificate.</tt>


=== Posting Patches ===
=== Include some screenshots ===
The basic command to interface with Differential, the patch review system, is <tt>arc diff</tt>. By default, this command will try to take your current git patch and create a new Differential to submit for upstream review. It will reformat Git commit message into something like this:  
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.
 
== What happens next? ==
After you've submitted your patch, KDE developers who work with the software in question will review it and provide feedback. This can often take a few days. However, if nobody has responded after a week, it's likely that the review was overlooked (sorry about that!) and it's appropriate to make a comment saying, "Ping!" or something to that effect.
 
Once the patch is accepted, KDE Developers will land it for you!
 
 
 
= Using Arcanist to post patches =
{{warning|To repeat the caution at the top of this page, use https://invent.kde.org to submit new patches, not Phabricator and Arcanist. See [[Infrastructure/GitLab]] for details.}}
After you've posted a few patches, using the web UI to post patches gets tiresome. Arcanist is a tool to simplify and speed up the process of posting, updating, and merging Phabricator patches. Setting it up is easy:
 
== Installing Arcanist ==
=== Debian/Ubuntu/KDE Neon ===
{{Input|1=<nowiki>
sudo apt install arcanist
</nowiki>}}
 
=== Fedora ===
kanarip/phabricator is not an official Fedora repository, so errors may occur.
{{Input|1=<nowiki>
sudo dnf copr enable kanarip/phabricator
sudo dnf install arcanist
</nowiki>}}
 
Alternatively, install arc manually from the github repository:
{{Input|1=<nowiki>
sudo dnf install php-{common,cli}
mkdir somewhere/
cd somewhere/
git clone https://github.com/phacility/libphutil.git
git clone https://github.com/phacility/arcanist.git
ln -s $(pwd)/arcanist/bin/arc ~/.local/bin/arc
</nowiki>}}
 
=== openSUSE ===
Tumbleweed:
{{Input|1=<nowiki>
sudo zypper ar -f https://download.opensuse.org/repositories/devel:tools:scm/openSUSE_Tumbleweed/devel:tools:scm.repo
sudo zypper install arcanist
</nowiki>}}
 
Leap 15:
{{Input|1=<nowiki>
sudo zypper ar -f https://download.opensuse.org/repositories/devel:tools:scm/openSUSE_Leap_15.0/devel:tools:scm.repo
sudo zypper install arcanist
</nowiki>}}
 
=== Arch / Manjaro ===
{{Input|1=<nowiki>
trizen -S arcanist-git
</nowiki>}}
 
=== Gentoo ===
The default portage tree does not have an ebuild, so unless you already have it, you need to add an overlay, e.g. the kde one:
 
{{Input|1=<nowiki>
sudo layman -a kde
</nowiki>}}
 
Once that is done, you can emerge it:
 
{{Input|1=<nowiki>
sudo emerge -av arcanist
</nowiki>}}
 
You might have to install and possibly unmask a few php related packages, so follow the instructions portage gives you.
 
Keep in mind that it is a vcs version (9999), which means it doesn't have release updates and you are in charge of keeping it up to date.
 
=== Windows ===
[https://secure.phabricator.com/book/phabricator/article/arcanist_windows/ Arcanist User Guide: Windows]
 
The most non-obvious step is that you will need to configure your [http://php.net/manual/en/install.windows.manual.php PHP installation] to use Curl. This requires editing the <tt>php.ini</tt> configuration file to add the line <tt>extension_dir = "ext"</tt> and add <tt>php_curl</tt> to the list of extensions. After adding <tt>php.exe</tt> to your <tt>PATH</tt> and installing arcanist/libphutil, you can run <tt>arc</tt> by defining a function in your Powershell profile:
<pre>
function arc { php -f "C:\path\to\arcanist.php" -- $args }
</pre>
 
<br/>
 
After you have installed Arc, you can learn more using <tt>man arc</tt> or <tt>arc --help</tt>. Another command useful for getting a feel for Phabricator's style is <tt>arc anoid</tt>.
 
 
=== Other operating systems ===
 
Follow the instructions from the [https://secure.phabricator.com/book/phabricator/article/arcanist_quick_start/ Arcanist Quick Start] guide.
 
== Perform one-time setup steps ==
Before you are able to use <tt>arc</tt> for work you will need to install a certificate that will allow it to login to Phabricator:
{{Input|1=<nowiki>
arc install-certificate https://phabricator.kde.org
</nowiki>}}
Just follow the instructions it provides: you will have to visit the page it indicates and copy the API token you find on that page back to the shell. (If the command fails with a cURL SSL error, you may need to set <tt>curl.cainfo</tt> in <tt>/etc/php.ini</tt> to the path to a certificate authority file on your computer, for example <tt>/etc/pki/tls/certs/ca-bundle.crt</tt> .)
 
Next, you will need to tell git who you really are, so your patches will include correct authorship information and can be attributed to yourself:
{{Input|1=<nowiki>
git config --global user.name "<Your real name>"
git config --global user.email "<Your identity.kde.org email address>"
</nowiki>}}
Both of these steps only need to be done once.
 
== Workflow ==
The sanest and easiest way to use <tt>arc</tt> 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.
 
The following commands all need to be executed in your source directory. The hidden file/directory "<tt>.arcconfig</tt>" and "<tt>.git</tt>" tell <tt>arc</tt> what to do so you don't have to.
 
=== Step 1: Create a new diff ===
Before editing anything, create a new branch for your patch:
{{Input|1=<nowiki>
arc feature  <name-for-your-new-branch>
</nowiki>}}(For Git experts: this is equivalent to {{Inline-code|git checkout -t -b <name-for-your-new-branch>}})
 
Now make changes on the feature branch. When you're ready to have your changes reviewed, enter the following command:
{{Input|1=<nowiki>
arc diff    # this will do git-add and git-commit for you; if it asks about ignoring untracked files, enter 'y'
</nowiki>}}
When you run <tt>arc diff</tt>, you will go through a series of dialogs. At the end, you will be asked to rewrite your Git commit message to fit the standard Differential format, like so:  
<pre>
<pre>
<first line of original git commit message>
<first line of original git commit message>
Line 51: Line 172:
Summary: <rest of original commit message>
Summary: <rest of original commit message>


Reviewers:  
Test Plan:


Differential Revision: <URL to the new diff>
Reviewers:
 
Subscribers:  
</pre>
</pre>


The only thing you will have to fill in is the reviewer. As mentioned above, the default is to choose the whole project as the reviewer. This can be done in the text by by writing one of the project hashtags in the reviewer field. For example, [https://phabricator.kde.org/project/profile/8/ the default reviewer for Krita is #krita] and [https://phabricator.kde.org/project/profile/34/ the default hashtag for KDE PIM is #kde_pim]. Additionally, when you create your diff, you can [https://secure.phabricator.com/rP2427d317b2f0957b0c4917c5c321e07d94c7e2b9 associate it with an existing Maniphest task] by adding a line like
<tt><first line of original git commit message></tt> will become the commit message, so please follow [https://chris.beams.io/posts/git-commit/#seven-rules commit message best practices].
<pre>Maniphest Tasks: T50</pre>
 
As when using the web UI, enter any special Bugzilla keywords (such as <tt>BUG: 385942</tt>) on their own lines in the "Summary" section.
 
As when using the web UI, there is no need to specifically add anyone under the "Reviewers"  or "Subscribers" sections unless your patch includes any visual or user interface changes. In this case, please add <tt>#vdg</tt> as a reviewer to make sure the [[Get Involved/design | KDE Visual Design Group]] sees it!
 
=== 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 run <tt>arc diff</tt> to update the Phabricator revision. Your commit message will be added to the Phabricator revision as a new comment, so you can use it to explain your changes.
{{Input|1=<nowiki>
[implement changes based on review comments]
arc diff
</nowiki>}}
 
=== 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!"
 
First, make sure that the world is sane, and that only your patch will be landed:
{{Input|1=<nowiki>
[make sure you are on your feature branch]
arc land --preview
</nowiki>}}
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:
{{Input|1=<nowiki>
arc land
</nowiki>}}


==== 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>release/19.12</tt>, arc will land on that branch instead, and so on.


Phabricator does not require Arcanist for every upstream transaction.  In the same way the KDE bugtracker will [https://techbase.kde.org/Development/Git/Configuration#Commit_Template read from the commit template] and automatically close bugs, Phabricator reads commit messages and will update Maniphest and Diff automatically.  [https://secure.phabricator.com/T5132 Here is more information] about the special messages it looks for.
If you branched your patch from <tt>master</tt> but it is a relatively low-risk bugfix, you will often be a asked to land it on the "stable branch" instead of <tt>master</tt>. The easiest way to do this is to cherry-pick the commit from your feature branch onto the appropriate stable branch and then merge forward.


=== Workflow ===
{{warning|If at any time you feel nervous or run into trouble, ask your reviewers for help. This can be tricky, and undoing bad merges is a pain in the neck.}}
The basic workflow I have found successful with Arc is a feature-branch workflow. I keep a master branch synchronized with the upstream, and make all of my changes on separate branches.
Here is an example of how to do this with a patch branched from <tt>master</tt> that needs to go into the <tt>release/19.12</tt> stable branch (replace <tt>release/19.12</tt> with the appropriate stable branch name when you do this yourself, obviously).


'''Step 1: Creating a new diff.'''
{{Input|1=<nowiki>
[make sure you are on your feature branch]
git log -n 1 --pretty=format:"%H"
[copy that commit hash]
git checkout release/19.12
git pull
git cherry-pick [the commit hash]
git push
git branch -D [the name of your feature branch]
</nowiki>}}


Before editing anything, create a new branch for your new feature.
Note that after committing to the stable branch you are expected to merge that branch to <tt>master</tt> afterwards:
<pre>
{{Input|1=<nowiki>
$ git checkout -b <branchname> origin/master
git checkout master
</pre>  
git pull
Make or cherry-pick changes into this feature branch. When you're ready to have your changes reviewed:
git merge -s recursive -Xours release/19.12
<pre>
git push
$ git commit
</nowiki>}}
$ arc diff
 
</pre>
==== Landing someone else's diff ====
When you run arc diff, you will go through a series of dialogues. You will be asked to rewrite your Git commit message to fit the standard Differential format. Remember that you must enter a reviewer, either by entering a project hashtag or an individual user.
If you have a contributor account and you are helping someone without one through the process, you will need to land their diff for them once it's been accepted. Here's how:
{{Input|1=<nowiki>
arc patch <revision ID>
git show HEAD
</nowiki>}}
 
At this point, you need to verify that the authorship information shown is correct. If it's not, you will need to ask the patch author for their real name and email address. Then you use that information to update the local commit for the patch like so:
{{Input|1=<nowiki>
# Make sure you're on the branch that corresponds to the patch!
git commit --amend --author="firstname lastname <email address>"
</nowiki>}}
 
At this point, you can land the diff normally, [[#Step 3: Land your diff|as described above]].
 
 
 
== Arcanist Tips & Tricks ==
 
=== Look before you diff ===
You can check with <tt>arc which</tt> what Arcanist will do before performing the actual upload with <tt>arc diff</tt> 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 <tt>arc diff --update D12345</tt>. See <tt>arc help diff</tt>.
 
=== 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:
{{Input|1=<nowiki>
arc diff --edit --verbatim
</nowiki>}}
 
=== 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:
{{Input|1=<nowiki>
arc amend
</nowiki>}}
 
Note that in general Arcanist will do this automatically for you once you <tt>arc land</tt>.
 
== Advanced Tasks ==
Once in a while, a reviewer will tell you to do specific things. This section will help you 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:
{{Input|1=<nowiki>
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
</nowiki>}}
 
If you mixed different changes into a single commit on your local branch:
{{Input|1=<nowiki>
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
</nowiki>}}
 
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 a new feature to KIO, and then submit another patch for Dolphin that uses that feature. Here's what you do:
<ol>
<li>Create your first patch as above</li>
<li>When creating the second patch, add the following to its own line in the "Summary" section:
{{Input|1=<nowiki>Depends on DXXXX</nowiki>}}
(Replace "DXXXX" with the ID of the dependent patch, '''not the full URL)'''</li>
</ol>
 
==== 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:
<ol>
<li>Create a branch to track the first feature:
{{Input|1=<nowiki>
git checkout -b <branch name for feature 1> --track origin/master
</nowiki>}}
Then implement the feature and make a commit.
</li>
<li>Then create a branch for your second feature, ''tracking the first branch:''
{{Input|1=<nowiki>
git checkout -b <branch name for feature 2> --track <branch name for feature 1>
</nowiki>}}
As above, implement the feature and make a commit. Continue this pattern for any other required dependent features.
</li>
 
<li>
When you're ready to turn your dependency chain feature branches into patches, do the following:
{{Input|1=<nowiki>
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]
</nowiki>}}
...And so on.
</li>
 
<li>After you get comments, you will have to make changes to your patches and re-base dependent patches:
{{Input|1=<nowiki>
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
</nowiki>}}
...And so on.
</li>
 
<li>When you're ready to land any or all of your patches, do it in sequence, starting from the patch with no unmet dependencies:
{{Input|1=<nowiki>
git checkout <branch name for feature 1>
arc land
git checkout <branch name for feature 2>
git rebase origin/<target branch>
arc land
</nowiki>}}
</li>
</ol>
 
= How to review someone else's patch =
Arcanist (<tt>arc</tt>) makes it easy to review someone's patch. But first you'll need a development environment set up. If you haven't done that yet, it's time to do so. See [[Get_Involved/development#Set_up_your_development_environment]]. Follow the instructions to compile and run the program.
 
== Apply the patch and compile the software ==
Find the patch's revision ID. For example, for https://phabricator.kde.org/D11184, the ID is <tt>D11184</tt>.
 
Now check out or enter the source repository for the software that's being patched. The repository is listed on the web UI:
[[File:Konsole repository for patch.png]]
...So this would be a patch for Konsole.
 
If you've never built it before, check it out and build it once first:
{{Input|1=<nowiki>
kdesrc-build konsole
</nowiki>}}
 
Now go to its source directory:
{{Input|1=<nowiki>
cd ~/kde/src/konsole
</nowiki>}}
 
...and apply the patch:
{{Input|1=<nowiki>
arc patch <revision ID>
</nowiki>}}
 
Answer <tt>y</tt> to any questions that are posed. Arc will automatically create a branch named <tt>arcpatch-<revision ID></tt> for the patch, so it won't damage your checkout at all.
 
Now it's time to compile and run the software to make sure that the patch does what it says it does and doesn't cause any regressions! Compile the patched source code:
{{Input|1=<nowiki>
kdesrc-build konsole --no-src --resume-from konsole
</nowiki>}}
Those arguments will tell <tt>kdesrc-build</tt> to not update the source code after you applied the patch, and to not build any dependencies.


'''Step 2: Updating your diff.'''
If it didn't compile, that's reason alone to reject the patch! Go to the web UI and report your findings, and apply a "Request Changes" status.


After you upload the code the reviewer will take a look and give you some comments. If you get a thumbs up, 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, z."  After you make your changes, you can add an extra commit to the git branch.
== Perform QA ==
<pre>
If it did compile, then it's time to perform QA, because it's important to thoroughly test patches to ensure that bad code and regressions don't slip in. '''This is the entire purpose of having a review infrastructure'''; it is very important.
$ git add -u
$ git commit
$ arc diff
</pre>


'''Step 3: Landing your diff.'''
First make sure the unit tests all pass:
{{Input|1=<nowiki>
cd ~kde/build/kde/applications/konsole
ctest
</nowiki>}}


The following needs you to to have a full [[Infrastructure/Get_a_Contributor_Account | KDE Developer Account]]. If this is one of your first patches to KDE projects, so you do not have one yet, you will need your reviewers or someone else to land the diff.
If any tests fail, report this through a comment on the patch's web page (https://phabricator.kde.org/<revision ID>).


After the patch is in an acceptable state, the reply to your new message will be: "Ship it!"
Next, execute the Test Plan that the submitter wrote. If the patch does not have a Test Plan, request one. Does it all still work for you? If not, return to the web UI and Request Changes, writing a detailed comment explaining what didn't work. It is permissible to do this even if you have not been specified as a reviewer! '''Anyone can reject a patch on the grounds that it does not work, does not do what it says it does, or causes regressions.'''
<pre>
$ git checkout <branchname>
$ arc land
</pre>
This will squash all of the commits in your feature branch together. Phabricator aims for a tight correspondence of one commit = one change = one diff. If you don't want to squash your commits, using Arc for the last step is unnecessary, as Phab will monitor the repo and mark the diff as closed when it sees the commit. You can push using <tt>git push</tt> instead.  


If you are new to Arc, you may want to double-check the commit message before actually pushing to make sure the commit looks as intended. You can do so with <tt>arc land --hold</tt>, which will squash your feature-branch in a single commit on master, without pushing it. <tt>arc land</tt> also assumes the upstream branch is called "origin/master." Usually that is the case, but for more complicated workflows some additional configuration is required.
If the original Test Plan succeeds, try to break the patch. Here are some ideas:
# Remove the program's configuration file (<tt>~/.config/<program name>rc</tt> ) and re-open it
# Try the program with a HiDPI scale factor (or without one) or with a different default font size
# If it's a new feature, feed it unexpected input
# Test related functionality


== Review ==
'''Try to break it!''' A good patch will handle corner cases and variations in configuration. The price of configurability is vigilant testing! We owe it to our users to test using many configurations, not just the defaults or our personal settings.


=== Discussions on Diff and Maniphest ===
== Perform code review ==
Diff and Maniphest are used for coordinating changes and pre-reviewing patches. These are more self-explanatory than Arc and mostly follow the layout of a public forum, like Review Board or Github.  Before sharing links to Phabricator pages and diffs on the KDE forums or bug tracker, be sure you have configured them to be "Visible to Public (No Login Required)."
{{Note|1= Need a section on code review here, preferably written by a core KDE developer or another very experienced developer}}


== Customization ==
== Engage with the author and other reviewers ==
After you have run the program and evaluated the patch, it's time to leave some review comments on the webpage (which again is at https://phabricator.kde.org/<revision ID>). If you have been specified as a reviewer, or are a member of a group that has been specified as a reviewer, it is permissible to Accept the patch. But keep in mind that reviewing involves responsibility: you are giving a thumbs-up to code that will be run potentially by millions of people. If you accept and land a patch that causes regressions, you will share some of the blame. It's important to take the reviewer role seriously.


=== Creating custom dashboard feeds ===
= Customization =
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.   
== 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.)
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 that are in 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.)

Latest revision as of 01:54, 12 July 2020

Phabricator is KDE's task management system. It was used for patch review and other functions in the past, but KDE has since transitioned to GitLab, at https://invent.kde.org. Phabricator is still used for task tracking until this functionality is migrated to GitLab.

Information

Do not submit patches using Phabricator; instead use https://invent.kde.org. See Infrastructure/GitLab for details.

Do not submit bug reports using Phabricator; instead use https://bugs.kde.org See Get_Involved/Issue_Reporting for details.



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

Once you have set up your development environment and created a patch, you can submit it using Phabricator!

Log in to Phabricator and click on Code Review in the list on the left. Then, 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. Reviewers are mostly added automatically, but if your patch includes any visual or user interface changes, please add #vdg as a reviewer to make sure the KDE Visual Design Group sees it! Please make sure to add a screenshot, too.

Formatting your patch

The Title of your patch will become the git commit message, so please follow commit message best practices when creating a title for the patch.

In the Summary section, write at least one sentence describing your change and why it is necessary, adding more details if necessary.

Add special keywords

If your patch is intended to fix a Bugzilla ticket, include one of the following on its own line in the Summary section:

BUG: 385942

or

FEATURE: 384936

(Just the Bugzilla ticket number, not the full URL)

Use BUG: If the Bugzilla ticket 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.

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).

Note

These keywords will only work if the email address in your ~/.config/git/config file matches the email address used for your https://bugs.kde.org account (See this page for more information).


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.

What happens next?

After you've submitted your patch, KDE developers who work with the software in question will review it and provide feedback. This can often take a few days. However, if nobody has responded after a week, it's likely that the review was overlooked (sorry about that!) and it's appropriate to make a comment saying, "Ping!" or something to that effect.

Once the patch is accepted, KDE Developers will land it for you!


Using Arcanist to post patches

Warning

To repeat the caution at the top of this page, use https://invent.kde.org to submit new patches, not Phabricator and Arcanist. See Infrastructure/GitLab for details.

After you've posted a few patches, using the web UI to post patches gets tiresome. Arcanist is a tool to simplify and speed up the process of posting, updating, and merging Phabricator patches. Setting it up is easy:

Installing Arcanist

Debian/Ubuntu/KDE Neon

sudo apt install arcanist

Fedora

kanarip/phabricator is not an official Fedora repository, so errors may occur.

sudo dnf copr enable kanarip/phabricator
sudo dnf install arcanist

Alternatively, install arc manually from the github repository:

sudo dnf install php-{common,cli}
mkdir somewhere/
cd somewhere/
git clone https://github.com/phacility/libphutil.git
git clone https://github.com/phacility/arcanist.git
ln -s $(pwd)/arcanist/bin/arc ~/.local/bin/arc

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 / Manjaro

trizen -S arcanist-git

Gentoo

The default portage tree does not have an ebuild, so unless you already have it, you need to add an overlay, e.g. the kde one:

sudo layman -a kde

Once that is done, you can emerge it:

sudo emerge -av arcanist

You might have to install and possibly unmask a few php related packages, so follow the instructions portage gives you.

Keep in mind that it is a vcs version (9999), which means it doesn't have release updates and you are in charge of keeping it up to date.

Windows

Arcanist User Guide: 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.


Other operating systems

Follow the instructions from the Arcanist Quick Start guide.

Perform one-time setup steps

Before you are able to use arc for work you will need to install a certificate that will allow it to login to Phabricator:

arc install-certificate https://phabricator.kde.org

Just follow the instructions it provides: you will have to visit the page it indicates and copy the API token you find on that page back to the shell. (If the command fails with a cURL SSL error, you may need to set curl.cainfo in /etc/php.ini to the path to a certificate authority file on your computer, for example /etc/pki/tls/certs/ca-bundle.crt .)

Next, you will need to tell git who you really are, so your patches will include correct authorship information and can be attributed to yourself:

git config --global user.name "<Your real name>"
git config --global user.email "<Your identity.kde.org email address>"

Both of these steps only need to be done once.

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.

The following commands all need to be executed in your source directory. The hidden file/directory ".arcconfig" and ".git" tell arc what to do so you don't have to.

Step 1: Create a new diff

Before editing anything, create a new branch for your patch:

arc feature  <name-for-your-new-branch>

(For Git experts: this is equivalent to git checkout -t -b <name-for-your-new-branch>)

Now make changes on the feature branch. When you're ready to have your changes reviewed, enter the following command:

arc diff     # this will do git-add and git-commit for you; if it asks about ignoring untracked files, enter 'y'

When you run arc diff, you will go through a series of dialogs. 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>

Test Plan:

Reviewers:

Subscribers: 

<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, there is no need to specifically add anyone under the "Reviewers" or "Subscribers" sections unless your patch includes any visual or user interface changes. In this case, please add #vdg as a reviewer to make sure the KDE Visual Design Group sees it!

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 run arc diff to update the Phabricator revision. Your commit message will be added to the Phabricator revision as a new comment, so you can use it to explain your changes.

[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 release/19.12, arc will land on that branch instead, and so on.

If you branched your patch from master but it is a relatively low-risk bugfix, you will often be a asked to land it on the "stable branch" instead of master. The easiest way to do this is to cherry-pick the commit from your feature branch onto the appropriate stable branch and then merge forward.

Warning

If at any time you feel nervous or run into trouble, ask your reviewers for help. This can be tricky, and undoing bad merges is a pain in the neck.

Here is an example of how to do this with a patch branched from master that needs to go into the release/19.12 stable branch (replace release/19.12 with the appropriate stable branch name when you do this yourself, obviously).

[make sure you are on your feature branch]
git log -n 1 --pretty=format:"%H"
[copy that commit hash]
git checkout release/19.12
git pull
git cherry-pick [the commit hash]
git push
git branch -D [the name of your feature branch]

Note that after committing to the stable branch you are expected to merge that branch to master afterwards:

git checkout master
git pull
git merge -s recursive -Xours release/19.12
git push

Landing someone else's diff

If you have a contributor account and you are helping someone without one through the process, you will need to land their diff for them once it's been accepted. Here's how:

arc patch <revision ID>
git show HEAD

At this point, you need to verify that the authorship information shown is correct. If it's not, you will need to ask the patch author for their real name and email address. Then you use that information to update the local commit for the patch like so:

# Make sure you're on the branch that corresponds to the patch!
git commit --amend --author="firstname lastname <email address>"

At this point, you can land the diff normally, as described above.


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 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 into 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 a new feature to KIO, and then submit another patch for Dolphin that uses that feature. Here's what you do:

  1. Create your first patch as above
  2. 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:

  1. 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.

  2. 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.

  3. 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.

  4. 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.

  5. 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
    

How to review someone else's patch

Arcanist (arc) makes it easy to review someone's patch. But first you'll need a development environment set up. If you haven't done that yet, it's time to do so. See Get_Involved/development#Set_up_your_development_environment. Follow the instructions to compile and run the program.

Apply the patch and compile the software

Find the patch's revision ID. For example, for https://phabricator.kde.org/D11184, the ID is D11184.

Now check out or enter the source repository for the software that's being patched. The repository is listed on the web UI: ...So this would be a patch for Konsole.

If you've never built it before, check it out and build it once first:

kdesrc-build konsole

Now go to its source directory:

cd ~/kde/src/konsole

...and apply the patch:

arc patch <revision ID>

Answer y to any questions that are posed. Arc will automatically create a branch named arcpatch-<revision ID> for the patch, so it won't damage your checkout at all.

Now it's time to compile and run the software to make sure that the patch does what it says it does and doesn't cause any regressions! Compile the patched source code:

kdesrc-build konsole --no-src --resume-from konsole

Those arguments will tell kdesrc-build to not update the source code after you applied the patch, and to not build any dependencies.

If it didn't compile, that's reason alone to reject the patch! Go to the web UI and report your findings, and apply a "Request Changes" status.

Perform QA

If it did compile, then it's time to perform QA, because it's important to thoroughly test patches to ensure that bad code and regressions don't slip in. This is the entire purpose of having a review infrastructure; it is very important.

First make sure the unit tests all pass:

cd ~kde/build/kde/applications/konsole
ctest

If any tests fail, report this through a comment on the patch's web page (https://phabricator.kde.org/<revision ID>).

Next, execute the Test Plan that the submitter wrote. If the patch does not have a Test Plan, request one. Does it all still work for you? If not, return to the web UI and Request Changes, writing a detailed comment explaining what didn't work. It is permissible to do this even if you have not been specified as a reviewer! Anyone can reject a patch on the grounds that it does not work, does not do what it says it does, or causes regressions.

If the original Test Plan succeeds, try to break the patch. Here are some ideas:

  1. Remove the program's configuration file (~/.config/<program name>rc ) and re-open it
  2. Try the program with a HiDPI scale factor (or without one) or with a different default font size
  3. If it's a new feature, feed it unexpected input
  4. Test related functionality

Try to break it! A good patch will handle corner cases and variations in configuration. The price of configurability is vigilant testing! We owe it to our users to test using many configurations, not just the defaults or our personal settings.

Perform code review

Note

Need a section on code review here, preferably written by a core KDE developer or another very experienced developer


Engage with the author and other reviewers

After you have run the program and evaluated the patch, it's time to leave some review comments on the webpage (which again is at https://phabricator.kde.org/<revision ID>). If you have been specified as a reviewer, or are a member of a group that has been specified as a reviewer, it is permissible to Accept the patch. But keep in mind that reviewing involves responsibility: you are giving a thumbs-up to code that will be run potentially by millions of people. If you accept and land a patch that causes regressions, you will share some of the blame. It's important to take the reviewer role seriously.

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 that are in 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.)