From 44dee47c288202a232be02d893fbf125e1791714 Mon Sep 17 00:00:00 2001 From: J Rhodes Date: Sat, 20 Jun 2015 14:22:23 +1000 Subject: [PATCH 1/5] Show build plan name on Harbormaster plan view controller Summary: Ref T8096. This shows the build plan name on the Harbormaster build plan view controller. Without this, the name is not displayed anywhere on the page when you're viewing a build plan's configuration (which makes things confusing if you're updating a bunch of build plans at once). Test Plan: Viewed a build plan, saw the build plan name on the page. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Projects: #harbormaster Maniphest Tasks: T8096 Differential Revision: https://secure.phabricator.com/D13356 --- .../controller/HarbormasterPlanViewController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php index 7447f2ed7b..8db8eaf118 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -27,10 +27,10 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { new HarbormasterBuildPlanTransactionQuery()); $timeline->setShouldTerminate(true); - $title = pht('Plan %d', $id); + $title = $plan->getName(); $header = id(new PHUIHeaderView()) - ->setHeader($title) + ->setHeader($plan->getName()) ->setUser($viewer) ->setPolicyObject($plan); From 9921cbc41ac3f80c41f455586a2606fcb98318b0 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Fri, 19 Jun 2015 18:06:53 +1000 Subject: [PATCH 2/5] Allow atoms to be queried by book Summary: Ref T4558. Allows querying for atoms from specified books. Depends on D13091. Test Plan: Poked around at `/diviner/query/`. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T4558 Differential Revision: https://secure.phabricator.com/D13303 --- src/__phutil_library_map__.php | 2 + .../diviner/query/DivinerAtomSearchEngine.php | 15 +++++ .../diviner/query/DivinerBookQuery.php | 56 ++++++++++++++++++- .../typeahead/DivinerBookDatasource.php | 37 ++++++++++++ 4 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 src/applications/diviner/typeahead/DivinerBookDatasource.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5d9b7ad113..3454bae920 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -649,6 +649,7 @@ phutil_register_library_map(array( 'DivinerAtomizeWorkflow' => 'applications/diviner/workflow/DivinerAtomizeWorkflow.php', 'DivinerAtomizer' => 'applications/diviner/atomizer/DivinerAtomizer.php', 'DivinerBookController' => 'applications/diviner/controller/DivinerBookController.php', + 'DivinerBookDatasource' => 'applications/diviner/typeahead/DivinerBookDatasource.php', 'DivinerBookEditController' => 'applications/diviner/controller/DivinerBookEditController.php', 'DivinerBookItemView' => 'applications/diviner/view/DivinerBookItemView.php', 'DivinerBookPHIDType' => 'applications/diviner/phid/DivinerBookPHIDType.php', @@ -4016,6 +4017,7 @@ phutil_register_library_map(array( 'DivinerAtomizeWorkflow' => 'DivinerWorkflow', 'DivinerAtomizer' => 'Phobject', 'DivinerBookController' => 'DivinerController', + 'DivinerBookDatasource' => 'PhabricatorTypeaheadDatasource', 'DivinerBookEditController' => 'DivinerController', 'DivinerBookItemView' => 'AphrontTagView', 'DivinerBookPHIDType' => 'PhabricatorPHIDType', diff --git a/src/applications/diviner/query/DivinerAtomSearchEngine.php b/src/applications/diviner/query/DivinerAtomSearchEngine.php index 70f77f3195..23e8416fcc 100644 --- a/src/applications/diviner/query/DivinerAtomSearchEngine.php +++ b/src/applications/diviner/query/DivinerAtomSearchEngine.php @@ -13,6 +13,9 @@ final class DivinerAtomSearchEngine extends PhabricatorApplicationSearchEngine { public function buildSavedQueryFromRequest(AphrontRequest $request) { $saved = new PhabricatorSavedQuery(); + $saved->setParameter( + 'bookPHIDs', + $this->readPHIDsFromRequest($request, 'bookPHIDs')); $saved->setParameter( 'repositoryPHIDs', $this->readPHIDsFromRequest($request, 'repositoryPHIDs')); @@ -27,6 +30,11 @@ final class DivinerAtomSearchEngine extends PhabricatorApplicationSearchEngine { public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { $query = id(new DivinerAtomQuery()); + $books = $saved->getParameter('bookPHIDs'); + if ($books) { + $query->withBookPHIDs($books); + } + $repository_phids = $saved->getParameter('repositoryPHIDs'); if ($repository_phids) { $query->withRepositoryPHIDs($repository_phids); @@ -74,6 +82,13 @@ final class DivinerAtomSearchEngine extends PhabricatorApplicationSearchEngine { } $form->appendChild($type_control); + $form->appendControl( + id(new AphrontFormTokenizerControl()) + ->setDatasource(new DivinerBookDatasource()) + ->setName('bookPHIDs') + ->setLabel(pht('Books')) + ->setValue($saved->getParameter('bookPHIDs'))); + $form->appendControl( id(new AphrontFormTokenizerControl()) ->setLabel(pht('Repositories')) diff --git a/src/applications/diviner/query/DivinerBookQuery.php b/src/applications/diviner/query/DivinerBookQuery.php index dfd236f3d1..8e6726ef5c 100644 --- a/src/applications/diviner/query/DivinerBookQuery.php +++ b/src/applications/diviner/query/DivinerBookQuery.php @@ -5,6 +5,8 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { private $ids; private $phids; private $names; + private $nameLike; + private $namePrefix; private $repositoryPHIDs; private $needProjectPHIDs; @@ -20,11 +22,21 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $this; } + public function withNameLike($name) { + $this->nameLike = $name; + return $this; + } + public function withNames(array $names) { $this->names = $names; return $this; } + public function withNamePrefix($prefix) { + $this->namePrefix = $prefix; + return $this; + } + public function withRepositoryPHIDs(array $repository_phids) { $this->repositoryPHIDs = $repository_phids; return $this; @@ -121,13 +133,27 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { $this->phids); } - if ($this->names) { + if (strlen($this->nameLike)) { + $where[] = qsprintf( + $conn_r, + 'name LIKE %~', + $this->nameLike); + } + + if ($this->names !== null) { $where[] = qsprintf( $conn_r, 'name IN (%Ls)', $this->names); } + if (strlen($this->namePrefix)) { + $where[] = qsprintf( + $conn_r, + 'name LIKE %>', + $this->namePrefix); + } + if ($this->repositoryPHIDs !== null) { $where[] = qsprintf( $conn_r, @@ -144,4 +170,32 @@ final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery { return 'PhabricatorDivinerApplication'; } + public function getOrderableColumns() { + return parent::getOrderableColumns() + array( + 'name' => array( + 'column' => 'name', + 'type' => 'string', + 'reverse' => true, + 'unique' => true, + ), + ); + } + + protected function getPagingValueMap($cursor, array $keys) { + $book = $this->loadCursorObject($cursor); + + return array( + 'name' => $book->getName(), + ); + } + + public function getBuiltinOrders() { + return array( + 'name' => array( + 'vector' => array('name'), + 'name' => pht('Name'), + ), + ) + parent::getBuiltinOrders(); + } + } diff --git a/src/applications/diviner/typeahead/DivinerBookDatasource.php b/src/applications/diviner/typeahead/DivinerBookDatasource.php new file mode 100644 index 0000000000..2584626d6e --- /dev/null +++ b/src/applications/diviner/typeahead/DivinerBookDatasource.php @@ -0,0 +1,37 @@ +getRawQuery(); + + $query = id(new DivinerBookQuery()) + ->setOrder('name') + ->withNamePrefix($raw_query); + $books = $this->executeQuery($query); + + $results = array(); + foreach ($books as $book) { + $results[] = id(new PhabricatorTypeaheadResult()) + ->setName($book->getTitle()) + ->setURI('/book/'.$book->getName().'/') + ->setPHID($book->getPHID()) + ->setPriorityString($book->getName()); + } + + return $results; + } + +} From 85083c88c1049a230cdf61ed4b5c0bd0518d8ddd Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 20 Jun 2015 05:25:08 -0700 Subject: [PATCH 3/5] Update the translations document Summary: Fixes T8616. The rules for contributors have come up a few times recently, so update this document to give more complete advice. Also try to do a better job with "adding new classes" (previously: libphutil libraries blah blah no one cares). Test Plan: Read documents. Reviewers: btrahan, joshuaspence Reviewed By: btrahan, joshuaspence Subscribers: joshuaspence, epriestley Maniphest Tasks: T8616 Differential Revision: https://secure.phabricator.com/D13358 --- src/docs/book/contributor.book | 2 +- src/docs/book/phabricator.book | 2 +- .../contributor/adding_new_classes.diviner | 256 ++++++++++++ .../contributor/internationalization.diviner | 375 ++++++++++++++++-- .../user/configuration/custom_fields.diviner | 2 +- .../configuration/managing_daemons.diviner | 4 +- .../user/userguide/arcanist_lint_unit.diviner | 2 +- .../userguide/arcanist_new_project.diviner | 2 +- src/docs/user/userguide/events.diviner | 4 +- src/docs/user/userguide/libraries.diviner | 155 -------- 10 files changed, 609 insertions(+), 195 deletions(-) create mode 100644 src/docs/contributor/adding_new_classes.diviner delete mode 100644 src/docs/user/userguide/libraries.diviner diff --git a/src/docs/book/contributor.book b/src/docs/book/contributor.book index d5b2ae90ae..10db63c011 100644 --- a/src/docs/book/contributor.book +++ b/src/docs/book/contributor.book @@ -2,7 +2,7 @@ "name": "phabcontrib", "title": "Phabricator Contributor Documentation", "short": "Phabricator Contributor Docs", - "preface": "Information for Phabricator contributors.", + "preface": "Information for Phabricator contributors and developers.", "root": "../../../", "uri.source": "https://secure.phabricator.com/diffusion/P/browse/master/%f$%l", diff --git a/src/docs/book/phabricator.book b/src/docs/book/phabricator.book index ed74ddd012..2b362853b4 100644 --- a/src/docs/book/phabricator.book +++ b/src/docs/book/phabricator.book @@ -2,7 +2,7 @@ "name": "phabdev", "title": "Phabricator Technical Documentation", "short": "Phabricator Tech Docs", - "preface": "Technical documentation intended for Phabricator developers.", + "preface": "Technical reference material for Phabricator developers.", "root": "../../../", "uri.source": "https://secure.phabricator.com/diffusion/P/browse/master/%f$%l", diff --git a/src/docs/contributor/adding_new_classes.diviner b/src/docs/contributor/adding_new_classes.diviner new file mode 100644 index 0000000000..beb4567210 --- /dev/null +++ b/src/docs/contributor/adding_new_classes.diviner @@ -0,0 +1,256 @@ +@title Adding New Classes +@group developer + +Guide to adding new classes to extend Phabricator. + +Overview +======== + +Phabricator is highly modular, and many parts of it can be extended by adding +new classes. This document explains how to write new classes to change or +expand the behavior of Phabricator. + +IMPORTANT: The upstream does not offer support with extension development. + +Fundamentals +============ + +Phabricator primarily discovers functionality by looking at concrete subclasses +of some base class. For example, Phabricator determines which applications are +available by looking at all of the subclasses of +@{class@phabricator:PhabricatorApplication}. It +discovers available workflows in `arc` by looking at all of the subclasses of +@{class@arcanist:ArcanistWorkflow}. It discovers available locales +by looking at all of the subclasses of @{class@libphutil:PhutilLocale}. + +This pattern holds in many cases, so you can often add functionality by adding +new classes with no other work. Phabricator will automatically discover and +integrate the new capabilities or features at runtime. + +There are two main ways to add classes: + + - **Extensions Directory**: This is a simple way to add new code. It is + less powerful, but takes a lot less work. This is good for quick changes, + testing and development, or getting started on a larger project. + - **Creating Libraries**: This is a more advanced and powerful way to + organize extension code. This is better for larger or longer-lived + projects, or any code which you plan to distribute. + +The next sections walk through these approaches in greater detail. + + +Extensions Directory +==================== + +The easiest way to extend Phabricator by adding new classes is to drop them +into the extensions directory, at `phabricator/src/extensions/`. + +This is intended as a quick way to add small pieces of functionality, test new +features, or get started on a larger project. Extending Phabricator like this +imposes a small performance penalty compared to using a library. + +This directory exists in all libphutil libraries, so you can find similar +directories in `arcanist/src/extensions/` and `libphutil/src/extensions/`. + +For example, to add a new application, create a file like this one and add it +to `phabricator/src/extensions/`. + +```name=phabricator/src/extensions/ExampleApplication.php, lang=php + array( + 'libcustom' => 'libcustom/src/', +), +... +``` + +Now, Phabricator will be able to load classes from your custom library. + + +Writing Classes +=============== + +To actually write classes, create a new module and put code in it: + + libcustom/ $ mkdir src/example/ + libcustom/ $ nano src/example/ExampleClass.php # Edit some code. + +Now, run `arc liberate` to regenerate the static resource map: + + libcustom/ $ arc liberate src/ + +This will automatically regenerate the static map of the library. + + +What You Can Extend And Invoke +============================== + +libphutil, Arcanist and Phabricator are strict about extensibility of classes +and visibility of methods and properties. Most classes are marked `final`, and +methods have the minimum required visibility (protected or private). The goal +of this strictness is to make it clear what you can safely extend, access, and +invoke, so your code will keep working as the upstream changes. + +IMPORTANT: We'll still break APIs frequently. The upstream does not support +extension development, and none of these APIs are stable. + +When developing libraries to work with libphutil, Arcanist and Phabricator, you +should respect method and property visibility. + +If you want to add features but can't figure out how to do it without changing +Phabricator code, here are some approaches you may be able to take: + + - {icon check, color=green} **Use Composition**: If possible, use composition + rather than extension to build your feature. + - {icon check, color=green} **Find Another Approach**: Check the + documentation for a better way to accomplish what you're trying to do. + - {icon check, color=green} **File a Feature Request**: Let us know what your + use case is so we can make the class tree more flexible or configurable, or + point you at the right way to do whatever you're trying to do, or explain + why we don't let you do it. Note that we **do not support** extension + development so you may have mixed luck with this one. + +These approaches are **discouraged**, but also possible: + + - {icon times, color=red} **Fork**: Create an ad-hoc local fork and remove + `final` in your copy of the code. This will make it more difficult for you + to upgrade in the future, although it may be the only real way forward + depending on what you're trying to do. + - {icon times, color=red} **Use Reflection**: You can use + [[ http://php.net/manual/en/book.reflection.php | Reflection ]] to remove + modifiers at runtime. This is fragile and discouraged, but technically + possible. + - {icon times, color=red} **Remove Modifiers**: Send us a patch removing + `final` (or turning `protected` or `private` into `public`). We will almost + never accept these patches unless there's a very good reason that the + current behavior is wrong. + + +Next Steps +========== + +Continue by: + + - visiting the [[ https://secure.phabricator.com/w/community_resources/ | + Community Resources ]] page to find or share extensions and libraries. diff --git a/src/docs/contributor/internationalization.diviner b/src/docs/contributor/internationalization.diviner index 401bf4950a..f937ec1ae2 100644 --- a/src/docs/contributor/internationalization.diviner +++ b/src/docs/contributor/internationalization.diviner @@ -9,57 +9,370 @@ Overview Phabricator partially supports internationalization, but many of the tools are missing or in a prototype state. -This document very briefly summarizes some of what exists today. +This document describes what tools exist today, how to add new translations, +and how to use the translation tools to make a codebase translatable. + + +Adding a New Locale +=================== + +To add a new locale, subclass @{class:PhutilLocale}. This allows you to +introduce a new locale, like "German" or "Klingon". + +Once you've created a locale, applications can add translations for that +locale. + +For instructions on adding new classes, see @{article:Adding New Classes}. + + +Adding Translations to Locale +============================= + +To translate strings, subclass @{class:PhutilTranslation}. Translations need +to belong to a locale: the locale defines an available language, and each +translation subclass provides strings for it. + +Translations are separated from locales so that third-party applications can +provide translations into different locales without needing to define those +locales themselves. + +For instructions on adding new classes, see @{article:Adding New Classes}. + Writing Translatable Code -======== +========================= Strings are marked for translation with @{function@libphutil:pht}. -Adding a New Locale -========= +The `pht()` function takes a string (and possibly some parameters) and returns +the translated version of that string in the current viewer's locale, if a +translation is available. -To add a new locale, subclass @{class:PhutilLocale}. +If text strings will ultimately be read by humans, they should essentially +always be wrapped in `pht()`. For example: -Translating Strings -======== +```lang=php +$dialog->appendParagraph(pht('This is an example.')); +``` + +This allows the code to return the correct Spanish or German or Russian +version of the text, if the viewer is using Phabricator in one of those +languages and a translation is available. + +Using `pht()` properly so that strings are translatable can be tricky. Briefly, +the major rules are: + + - Only pass static strings as the first parameter to `pht()`. + - Use parameters to create strings containing user names, object names, etc. + - Translate full sentences, not sentence fragments. + - Let the translation framework handle plural rules. + - Use @{class@libphutil:PhutilNumber} for numbers. + - Let the translation framework handle subject gender rules. + - Translate all human-readable text, even exceptions and error messages. + +See the next few sections for details on these rules. + + +Use Static Strings +================== + +The first parameter to `pht()` must always be a static string. Broadly, this +means it should not contain variables or function or method calls (it's OK to +split it across multiple lines and concatenate the parts together). + +These are good: + +```lang=php +pht('The night is dark.'); +pht( + 'Two roads diverged in a yellow wood, '. + 'and sorry I could not travel both '. + 'and be one traveler, long I stood.'); + +``` + +These won't work (they might appear to work, but are wrong): + +```lang=php, counterexample +pht(some_function()); +pht('The duck says, '.$quack); +pht($string); +``` + +The first argument must be a static string so it can be extracted by static +analysis tools and dumped in a big file for translators. If it contains +functions or variables, it can't be extracted, so translators won't be able to +translate it. + +Lint will warn you about problems with use of static strings in calls to +`pht()`. + + +Parameters +========== + +You can provide parameters to a translation string by using `sprintf()`-style +patterns in the input string. For example: + +```lang=php +pht('%s earned an award.', $actor); +pht('%s closed %s.', $actor, $task); +``` + +This is primarily appropriate for usernames, object names, counts, and +untranslatable strings like URIs or instructions to run commands from the CLI. + +Parameters normally should not be used to combine two pieces of translated +text: see the next section for guidance. + +Sentence Fragments +================== + +You should almost always pass the largest block of text to `pht()` that you +can. Particularly, it's important to pass complete sentences, not try to build +a translation by stringing together sentence fragments. + +There are several reasons for this: + + - It gives translators more context, so they can be more confident they are + producing a satisfying, natural-sounding translation which will make sense + and sound good to native speakers. + - In some languages, one fragment may need to translate differently depending + on what the other fragment says. + - In some languages, the most natural-sounding translation may change the + order of words in the sentence. + +For example, suppose we want to translate these sentence to give the user some +instructions about how to use an interface: + +> Turn the switch to the right. + +> Turn the switch to the left. + +> Turn the dial to the right. + +> Turn the dial to the left. + +Maybe we have a function like this: + +``` +function get_string($is_switch, $is_right) { + // ... +} +``` + +One way to write the function body would be like this: + +```lang=php, counterexample +$what = $is_switch ? pht('switch') : pht('dial'); +$dir = $is_right ? pht('right') : pht('left'); + +return pht('Turn the ').$what.pht(' to the ').$dir.pht('.'); +``` + +This will work fine in English, but won't work well in other languages. + +One problem with doing this is handling gendered nouns. Languages like Spanish +have gendered nouns, where some nouns are "masculine" and others are +"feminine". The gender of a noun affects which article (in English, the word +"the" is an article) should be used with it. + +In English, we say "**the** knob" and "**the** switch", but a Spanish speaker +would say "**la** perilla" and "**el** interruptor", because the noun for +"knob" in Spanish is feminine (so it is used with the article "la") while the +noun for "switch" is masculine (so it is used with the article "el"). + +A Spanish speaker can not translate the string "Turn the" correctly without +knowing which gender the noun has. Spanish has //two// translations for this +string ("Gira el", "Gira la"), and the form depends on which noun is being +used. + +Another problem is that this reduces flexibility. Translating fragments like +this locks translators into a specific word order, when rearranging the words +might make the sentence sound much more natural to a native speaker. + +For example, if the string read "The knob, to the right, turn it.", it +would technically be English and most English readers would understand the +meaning, but no native English speaker would speak or write like this. + +However, some languages have different subject-verb order rules or +colloquisalisms, and a word order which transliterates like this may sound more +natural to a native speaker. By translating fragments instead of complete +sentences, you lock translators into English word order. + +Finally, the last fragment is just a period. If a translator is presented with +this string in an interface without much context, they have no hope of guessing +how it is used in the software (it could be an end-of-sentence marker, or a +decimal point, or a date separator, or a currency separator, all of which have +very different translations in many locales). It will also conflict with all +other translations of the same string in the codebase, so even if they are +given context they can't translate it without technical problems. + +To avoid these issues, provide complete sentences for translation. This almost +always takes the form of writing out alternatives in full. This is a good way +to implement the example function: + +```lang=php +if ($is_switch) { + if ($is_right) { + return pht('Turn the switch to the right.'); + } else { + return pht('Turn the switch to the left.'); + } +} else { + if ($is_right) { + return pht('Turn the dial to the right.'); + } else { + return pht('Turn the dial to the left.'); + } +} +``` + +Although this is more verbose, translators can now get genders correct, +rearrange word order, and have far more context when translating. This enables +better, natural-sounding translations which are more satisfying to native +speakers. -To translate strings, subclass @{class:PhutilTranslation}. Singular and Plural -======== +=================== -Different languages have various rules for using singular and plural. All you -need to do is to call @{function@libphutil:pht} with a text that is suitable for -both forms. Example: +Different languages have various rules for plural nouns. - pht('%d beer(s)', $count); +In English there are usually two plural noun forms: for one thing, and any +other number of things. For example, we say that one chair is a "chair" and any +other number of chairs are "chairs": "0 chairs", "1 chair", "2 chairs", etc. -Translators will translate this text for all different forms the language uses: +In other languages, there are different (and, in some cases, more) plural +forms. For example, in Czech, there are separate forms for "one", "several", +and "many". - // English translation - array('%d beer', '%d beers'); +Because plural noun rules depend on the language, you should not write code +which hard-codes English rules. For example, this won't translate well: - // Czech translation - array('%d pivo', '%d piva', '%d piv'); +```lang=php, counterexample +if ($count == 1) { + return pht('This will take an hour.'); +} else { + return pht('This will take hours.'); +} +``` -The ugly identifier passed to @{function@libphutil:pht} will remain in the text -only if the translation doesn't exist. +This code is hard-coding the English rule for plural nouns. In languages like +Czech, the correct word for "hours" may be different if the count is 2 or 15, +but a translator won't be able to provide the correct translation if the string +is written like this. + +Instead, pass a generic string to the translation engine which //includes// the +number of objects, and let it handle plural nouns. This is the correct way to +write the translation: + +```lang=php +return pht('This will take %s hour(s).', new PhutilNumber($count)); +``` + +If you now load the web UI, you'll see "hour(s)" literally in the UI. To fix +this so the translation sounds better in English, provide translations for this +string in the @{class@phabricator:PhabricatorUSEnglishTranslation} file: + +```lang=php +'This will take %s hour(s).' => array( + 'This will take an hour.', + 'This will take hours.', +), +``` + +The string will then sound natural in English, but non-English translators will +also be able to produce a natural translation. + +Note that the translations don't actually include the number in this case. The +number is being passed from the code, but that just lets the translation engine +get the rules right: the number does not need to appear in the final +translations shown to the user. + +Using PhutilNumber +================== + +When translating numbers, you should almost always use `%s` and wrap the count +or number in `new PhutilNumber($count)`. For example: + +```lang=php +pht('You have %s experience point(s).', new PhutilNumber($xp)); +``` + +This will let the translation engine handle plural noun rules correctly, and +also format large numbers correctly in a locale-aware way with proper unit and +decimal separators (for example, `1000000` may be printed as "1,000,000", +with commas for readability). + +The exception to this rule is IDs which should not be written with unit +separators. For example, this is correct for an object ID: + +```lang=php +pht('This diff has ID %d.', $diff->getID()); +``` Male and Female -======== +=============== -Different languages use different words for talking about males, females and -unknown genders. Callsites have to call @{function@libphutil:pht} passing -@{class:PhabricatorUser} (or other implementation of -@{interface@libphutil:PhutilPerson}) if talking about the user. Example: +Different languages also use different words for talking about subjects who are +male, female or have an unknown gender. In English this is mostly just +pronouns (like "he" and "she") but there are more complex rules in other +languages, and languages like Czech also require verb agreement. - pht('%s wrote', $actor); +When a parameter refers to a gendered person, pass an object which implements +@{interface@libphutil:PhutilPerson} to `pht()` so translators can provide +gendered translation variants. -Translators will create this translations: +```lang=php +pht('%s wrote', $actor); +``` - // English translation - '%s wrote'; +Translators will create these translations: - // Czech translation - array('%s napsal', '%s napsala'); +```lang=php +// English translation +'%s wrote'; + +// Czech translation +array('%s napsal', '%s napsala'); +``` + +(You usually don't need to worry very much about this rule, it is difficult to +get wrong in standard code.) + + +Exceptions and Errors +===================== + +You should translate all human-readable text, even exceptions and error +messages. This is primarily a rule of convenience which is straightforward +and easy to follow, not a technical rule. + +Some exceptions and error messages don't //technically// need to be translated, +as they will never be shown to a user, but many exceptions and error messages +are (or will become) user-facing on some way. When writing a message, there is +often no clear and objective way to determine which type of message you are +writing. Rather than try to distinguish which are which, we simply translate +all human-readable text. This rule is unambiguous and easy to follow. + +In cases where similar error or exception text is often repeated, it is +probably appropriate to define an exception for that category of error rather +than write the text out repeatedly, anyway. Two examples are +@{class@libphutil:PhutilInvalidStateException} and +@{class@libphutil:PhutilMethodNotImplementedException}, which mostly exist to +produce a consistent message about a common error state in a convenient way. + +There are a handful of error strings in the codebase which may be used before +the translation framework is loaded, or may be used during handling other +errors, possibly rasised from within the translation framework. This handful +of special cases are left untranslated to prevent fatals and cycles in the +error handler. + + +Next Steps +========== + +Continue by: + + - adding a new locale or translation file with @{article:Adding New Classes}. diff --git a/src/docs/user/configuration/custom_fields.diviner b/src/docs/user/configuration/custom_fields.diviner index 405633fc39..41adfbd092 100644 --- a/src/docs/user/configuration/custom_fields.diviner +++ b/src/docs/user/configuration/custom_fields.diviner @@ -207,5 +207,5 @@ integrations, see the base class for your application and Continue by: - learning more about extending Phabricator with custom code in - @{article:libphutil Libraries User Guide}; + @{article@contributor:Adding New Classes}; - or returning to the @{article: Configuration Guide}. diff --git a/src/docs/user/configuration/managing_daemons.diviner b/src/docs/user/configuration/managing_daemons.diviner index 36187ca30f..d82421496f 100644 --- a/src/docs/user/configuration/managing_daemons.diviner +++ b/src/docs/user/configuration/managing_daemons.diviner @@ -109,7 +109,7 @@ This daemon will daemonize and run normally. just those started with `phd start`. If you're writing a restart script, have it launch any custom daemons explicitly after `phd restart`. - You can write your own daemons and manage them with `phd` by extending - @{class:PhabricatorDaemon}. See @{article:libphutil Libraries User Guide}. + @{class:PhabricatorDaemon}. See {article@contributor:Adding New Classes}. - See @{article:Diffusion User Guide} for details about tuning the repository daemon. @@ -137,4 +137,4 @@ Continue by: - learning about the repository daemon with @{article:Diffusion User Guide}; or - - writing your own daemons with @{article:libphutil Libraries User Guide}. + - writing your own daemons with {article@contributor:Adding New Classes}. diff --git a/src/docs/user/userguide/arcanist_lint_unit.diviner b/src/docs/user/userguide/arcanist_lint_unit.diviner index e425911fae..00ad4e237b 100644 --- a/src/docs/user/userguide/arcanist_lint_unit.diviner +++ b/src/docs/user/userguide/arcanist_lint_unit.diviner @@ -38,7 +38,7 @@ make this work, you need to do three things: If you haven't created a library for the class to live in yet, you need to do that first. Follow the instructions in -@{article:libphutil Libraries User Guide}, then make the library loadable by +@{article@contributor:Adding New Classes}, then make the library loadable by adding it to your `.arcconfig` like this: { diff --git a/src/docs/user/userguide/arcanist_new_project.diviner b/src/docs/user/userguide/arcanist_new_project.diviner index cc65344f0f..b414a1e997 100644 --- a/src/docs/user/userguide/arcanist_new_project.diviner +++ b/src/docs/user/userguide/arcanist_new_project.diviner @@ -47,7 +47,7 @@ Other options include: - **load**: list of additional Phutil libraries to load at startup. See below for details about path resolution, or see - @{article:libphutil Libraries User Guide} for a general introduction to + @{article@contributor:Adding New Classes} for a general introduction to libphutil libraries. - **https.cabundle**: specifies the path to an alternate certificate bundle for use when making HTTPS connections. diff --git a/src/docs/user/userguide/events.diviner b/src/docs/user/userguide/events.diviner index 888fe2cf06..5bb8eb313a 100644 --- a/src/docs/user/userguide/events.diviner +++ b/src/docs/user/userguide/events.diviner @@ -21,7 +21,7 @@ To install event listeners in Phabricator, follow these steps: - Write a listener class which extends @{class@libphutil:PhutilEventListener}. - Add it to a libphutil library, or create a new library (for instructions, - see @{article:libphutil Libraries User Guide}. + see @{article@contributor:Adding New Classes}. - Configure Phabricator to load the library by adding it to `load-libraries` in the Phabricator config. - Configure Phabricator to install the event listener by adding the class @@ -38,7 +38,7 @@ To install event listeners in Arcanist, follow these steps: - Write a listener class which extends @{class@libphutil:PhutilEventListener}. - Add it to a libphutil library, or create a new library (for instructions, - see @{article:libphutil Libraries User Guide}. + see @{article@contributor:Adding New Classes}. - Configure Phabricator to load the library by adding it to `load` in the Arcanist config (e.g., `.arcconfig`, or user/global config). - Configure Arcanist to install the event listener by adding the class diff --git a/src/docs/user/userguide/libraries.diviner b/src/docs/user/userguide/libraries.diviner deleted file mode 100644 index b0614d7a8f..0000000000 --- a/src/docs/user/userguide/libraries.diviner +++ /dev/null @@ -1,155 +0,0 @@ -@title libphutil Libraries User Guide -@group userguide - -Guide to creating and managing libphutil libraries. - -= Overview = - -libphutil includes a library system which organizes PHP classes and functions -into modules. Some extensions and customizations of Arcanist and Phabricator -require you to make code available to Phabricator by providing it in a libphutil -library. - -For example, if you want to store files in some kind of custom storage engine, -you need to write a class which can interact with that engine and then tell -Phabricator to load it. - -In general, you perform these one-time setup steps: - - - Create a new directory. - - Use `arc liberate` to initialize and name the library. - - Add a dependency on Phabricator if necessary. - - Add the library to your Phabricator config or `.arcconfig` so it will be - loaded at runtime. - -Then, to add new code, you do this: - - - Write or update classes. - - Update the library metadata by running `arc liberate` again. - -= Creating a New Library = - -To **create a new libphutil library**: - - $ mkdir libcustom/ - $ cd libcustom/ - libcustom/ $ arc liberate src/ - -Now you'll get a prompt like this: - - lang=txt - No library currently exists at that path... - The directory '/some/path/libcustom/src' does not exist. - - Do you want to create it? [y/N] y - Creating new libphutil library in '/some/path/libcustom/src'. - Choose a name for the new library. - - What do you want to name this library? - -Choose a library name (in this case, "libcustom" would be appropriate) and it -you should get some details about the library initialization: - - lang=txt - Writing '__phutil_library_init__.php' to - '/some/path/libcustom/src/__phutil_library_init__.php'... - Using library root at 'src'... - Mapping library... - Verifying library... - Finalizing library map... - OKAY Library updated. - -This will write three files: - - - `src/.phutil_module_cache` This is a cache which makes "arc liberate" - faster when you run it to update the library. You can safely remove it at - any time. If you check your library into version control, you can add this - file to ignore rules (like .gitignore). - - `src/__phutil_library_init__.php` This records the name of the library and - tells libphutil that a library exists here. - - `src/__phutil_library_map__.php` This is a map of all the symbols - (functions and classes) in the library, which allows them to be autoloaded - at runtime and dependencies to be statically managed by "arc liberate". - -= Linking with Phabricator = - -If you aren't using this library with Phabricator (e.g., you are only using it -with Arcanist or are building something else on libphutil) you can skip this -step. - -But, if you intend to use this library with Phabricator, you need to define its -dependency on Phabricator by creating a `.arcconfig` file which points at -Phabricator. For example, you might write this file to -`libcustom/.arcconfig`: - - { - "load": [ - "phabricator/src/" - ] - } - -For details on creating a `.arcconfig`, see -@{article:Arcanist User Guide: Configuring a New Project}. In general, this -tells `arc liberate` that it should look for symbols in Phabricator when -performing static analysis. - -NOTE: If Phabricator isn't located next to your custom library, specify a -path which actually points to the `phabricator/` directory. - -You do not need to declare dependencies on `arcanist` or `libphutil`, -since `arc liberate` automatically loads them. - -Finally, edit your Phabricator config to tell it to load your library at -runtime, by adding it to `load-libraries`: - - ... - 'load-libraries' => array( - 'libcustom' => 'libcustom/src/', - ), - ... - -Now, Phabricator will be able to load classes from your custom library. - -= Writing Classes = - -To actually write classes, create a new module and put code in it: - - libcustom/ $ mkdir src/example/ - libcustom/ $ nano src/example/ExampleClass.php # Edit some code. - -Now, run `arc liberate` to regenerate the static resource map: - - libcustom/ $ arc liberate src/ - -This will automatically regenerate the static map of the library. - -= What You Can Extend And Invoke = - -libphutil, Arcanist and Phabricator are strict about extensibility of classes -and visibility of methods and properties. Most classes are marked `final`, and -methods have the minimum required visibility (protected or private). The goal of -this strictness is to make it clear what you can safely extend, access, and -invoke, so your code will keep working as the upstream changes. - -When developing libraries to work with libphutil, Arcanist and Phabricator, you -should respect method and property visibility and extend only classes marked -`@stable`. They are rendered with a large callout in the documentation (for -example: @{class@libphutil:AbstractDirectedGraph}). These classes are external -interfaces intended for extension. - -If you want to extend a class but it is not marked `@stable`, here are some -approaches you can take: - - - Good: If possible, use composition rather than extension to build your - feature. - - Good: Check the documentation for a better way to accomplish what you're - trying to do. - - Good: Let us know what your use case is so we can make the class tree more - flexible or configurable, or point you at the right way to do whatever - you're trying to do, or explain why we don't let you do it. - - Discouraged: Send us a patch removing "final" (or turning "protected" or - "private" into "public"). We generally will not accept these patches, unless - there's a good reason that the current behavior is wrong. - - Discouraged: Create an ad-hoc local fork and remove "final" in your copy of - the code. This will make it more difficult for you to upgrade in the future. - - Discouraged: Use Reflection to violate visibility keywords. From 04516d256bd34b29d525262d9088dac3f764c6e2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 20 Jun 2015 05:25:44 -0700 Subject: [PATCH 4/5] Add an "--importing" flag to bin/repository reparse Summary: Fixes T6839. Sometimes, worker tasks go astray for whatever reason. This automates the step of `bin/repository importing | xargs | mangle mangle | bin/repostiory reparse`. Test Plan: Ran various flavors of the command, got good looking results. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T6839 Differential Revision: https://secure.phabricator.com/D13362 --- .../diffusion/query/DiffusionCommitQuery.php | 44 +++++++++ ...torRepositoryManagementReparseWorkflow.php | 94 ++++++++++++++----- 2 files changed, 114 insertions(+), 24 deletions(-) diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php index 75913ba37e..7098e7416f 100644 --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -17,6 +17,9 @@ final class DiffusionCommitQuery private $auditorPHIDs; private $auditAwaitingUser; private $auditStatus; + private $epochMin; + private $epochMax; + private $importing; const AUDIT_STATUS_ANY = 'audit-status-any'; const AUDIT_STATUS_OPEN = 'audit-status-open'; @@ -141,6 +144,17 @@ final class DiffusionCommitQuery return $this; } + public function withEpochRange($min, $max) { + $this->epochMin = $min; + $this->epochMax = $max; + return $this; + } + + public function withImporting($importing) { + $this->importing = $importing; + return $this; + } + public function getIdentifierMap() { if ($this->identifierMap === null) { throw new Exception( @@ -329,6 +343,36 @@ final class DiffusionCommitQuery $this->authorPHIDs); } + if ($this->epochMin !== null) { + $where[] = qsprintf( + $conn_r, + 'commit.epoch >= %d', + $this->epochMin); + } + + if ($this->epochMax !== null) { + $where[] = qsprintf( + $conn_r, + 'commit.epoch <= %d', + $this->epochMax); + } + + if ($this->importing !== null) { + if ($this->importing) { + $where[] = qsprintf( + $conn_r, + '(commit.importStatus & %d) != %d', + PhabricatorRepositoryCommit::IMPORTED_ALL, + PhabricatorRepositoryCommit::IMPORTED_ALL); + } else { + $where[] = qsprintf( + $conn_r, + '(commit.importStatus & %d) = %d', + PhabricatorRepositoryCommit::IMPORTED_ALL, + PhabricatorRepositoryCommit::IMPORTED_ALL); + } + } + if ($this->identifiers !== null) { $min_unqualified = PhabricatorRepository::MINIMUM_UNQUALIFIED_HASH; $min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH; diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php index e06c6ca089..d36fb3f4ee 100644 --- a/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php @@ -83,12 +83,17 @@ final class PhabricatorRepositoryManagementReparseWorkflow 'instead of deferring them to taskmaster daemons.', '--all'), ), + array( + 'name' => 'importing', + 'help' => pht( + 'Reparse all steps which have not yet completed.'), + ), array( 'name' => 'force-autoclose', 'help' => pht( - 'Only used with __%s, use this to make sure any '. + 'Only used with __%s__, use this to make sure any '. 'pertinent diffs are closed regardless of configuration.', - '--message__'), + '--message'), ), )); @@ -106,6 +111,7 @@ final class PhabricatorRepositoryManagementReparseWorkflow $force = $args->getArg('force'); $force_local = $args->getArg('force-local'); $min_date = $args->getArg('min-date'); + $importing = $args->getArg('importing'); if (!$all_from_repo && !$reparse_what) { throw new PhutilArgumentUsageException( @@ -123,16 +129,31 @@ final class PhabricatorRepositoryManagementReparseWorkflow $commits)); } - if (!$reparse_message && !$reparse_change && !$reparse_herald && - !$reparse_owners) { + $any_step = ($reparse_message || + $reparse_change || + $reparse_herald || + $reparse_owners); + + if ($any_step && $importing) { throw new PhutilArgumentUsageException( pht( - 'Specify what information to reparse with %s, %s, %s, and/or %s.', + 'Choosing steps with %s conflicts with flags which select '. + 'specific steps.', + '--importing')); + } else if ($any_step) { + // OK. + } else if ($importing) { + // OK. + } else if (!$any_step && !$importing) { + throw new PhutilArgumentUsageException( + pht( + 'Specify which steps to reparse with %s, or %s, %s, %s, or %s.', + '--importing', '--message', '--change', '--herald', '--owners')); - } + } $min_timestamp = false; if ($min_date) { @@ -179,27 +200,28 @@ final class PhabricatorRepositoryManagementReparseWorkflow throw new PhutilArgumentUsageException( pht('Unknown repository %s!', $all_from_repo)); } - $constraint = ''; + + + $query = id(new DiffusionCommitQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withRepository($repository); + if ($min_timestamp) { - $console->writeOut("%s\n", pht( - 'Excluding entries before UNIX timestamp: %s', - $min_timestamp)); - $table = new PhabricatorRepositoryCommit(); - $conn_r = $table->establishConnection('r'); - $constraint = qsprintf( - $conn_r, - 'AND epoch >= %d', - $min_timestamp); + $query->withEpochRange($min_timestamp, null); } - $commits = id(new PhabricatorRepositoryCommit())->loadAllWhere( - 'repositoryID = %d %Q', - $repository->getID(), - $constraint); + + if ($importing) { + $query->withImporting(true); + } + + $commits = $query->execute(); + $callsign = $repository->getCallsign(); if (!$commits) { - throw new PhutilArgumentUsageException(pht( - "No commits have been discovered in %s repository!\n", - $callsign)); + throw new PhutilArgumentUsageException( + pht( + 'No commits have been discovered in %s repository!', + $callsign)); } } else { $commits = array(); @@ -250,6 +272,26 @@ final class PhabricatorRepositoryManagementReparseWorkflow $tasks = array(); foreach ($commits as $commit) { + if ($importing) { + $status = $commit->getImportStatus(); + // Find the first missing import step and queue that up. + $reparse_message = false; + $reparse_change = false; + $reparse_owners = false; + $reparse_herald = false; + if (!($status & PhabricatorRepositoryCommit::IMPORTED_MESSAGE)) { + $reparse_message = true; + } else if (!($status & PhabricatorRepositoryCommit::IMPORTED_CHANGE)) { + $reparse_change = true; + } else if (!($status & PhabricatorRepositoryCommit::IMPORTED_OWNERS)) { + $reparse_owners = true; + } else if (!($status & PhabricatorRepositoryCommit::IMPORTED_HERALD)) { + $reparse_herald = true; + } else { + continue; + } + } + $classes = array(); switch ($repository->getVersionControlSystem()) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: @@ -287,9 +329,13 @@ final class PhabricatorRepositoryManagementReparseWorkflow $classes[] = 'PhabricatorRepositoryCommitOwnersWorker'; } + // NOTE: With "--importing", we queue the first unparsed step and let + // it queue the other ones normally. Without "--importing", we queue + // all the requested steps explicitly. + $spec = array( 'commitID' => $commit->getID(), - 'only' => true, + 'only' => !$importing, 'forceAutoclose' => $args->getArg('force-autoclose'), ); From b22fba1ab52078371b2f939282f8dba012dd6ae1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 20 Jun 2015 05:30:17 -0700 Subject: [PATCH 5/5] Provide general documentation on how to use perfomance tools Summary: Ref T8617. Provide general documentation with tools for debugging hangs and slow pages. Update DarkConsole docs and discuss how to use Services and XHProf. Explain what Multimeter is for and how to use it. Update XHProf docs and provide some usage hints. Test Plan: Read documentation. Reviewers: joshuaspence, btrahan Reviewed By: joshuaspence, btrahan Subscribers: joshuaspence, epriestley Maniphest Tasks: T8617 Differential Revision: https://secure.phabricator.com/D13359 --- .../PhabricatorMultimeterApplication.php | 9 + src/docs/book/user.book | 3 + src/docs/contributor/darkconsole.diviner | 60 ------ .../contributor/installing_xhprof.diviner | 54 ------ src/docs/user/field/darkconsole.diviner | 162 ++++++++++++++++ src/docs/user/field/performance.diviner | 179 ++++++++++++++++++ src/docs/user/field/xhprof.diviner | 122 ++++++++++++ src/docs/user/userguide/multimeter.diviner | 99 ++++++++++ 8 files changed, 574 insertions(+), 114 deletions(-) delete mode 100644 src/docs/contributor/darkconsole.diviner delete mode 100644 src/docs/contributor/installing_xhprof.diviner create mode 100644 src/docs/user/field/darkconsole.diviner create mode 100644 src/docs/user/field/performance.diviner create mode 100644 src/docs/user/field/xhprof.diviner create mode 100644 src/docs/user/userguide/multimeter.diviner diff --git a/src/applications/multimeter/application/PhabricatorMultimeterApplication.php b/src/applications/multimeter/application/PhabricatorMultimeterApplication.php index 5675668fc2..fdac449bd2 100644 --- a/src/applications/multimeter/application/PhabricatorMultimeterApplication.php +++ b/src/applications/multimeter/application/PhabricatorMultimeterApplication.php @@ -43,4 +43,13 @@ final class PhabricatorMultimeterApplication ); } + public function getHelpDocumentationArticles(PhabricatorUser $viewer) { + return array( + array( + 'name' => pht('Multimeter User Guide'), + 'href' => PhabricatorEnv::getDoclink('Multimeter User Guide'), + ), + ); + } + } diff --git a/src/docs/book/user.book b/src/docs/book/user.book index e6f44abe36..464ec994a0 100644 --- a/src/docs/book/user.book +++ b/src/docs/book/user.book @@ -28,6 +28,9 @@ }, "userguide": { "name": "Application User Guides" + }, + "fieldmanual": { + "name": "Field Manuals" } } } diff --git a/src/docs/contributor/darkconsole.diviner b/src/docs/contributor/darkconsole.diviner deleted file mode 100644 index cddef89422..0000000000 --- a/src/docs/contributor/darkconsole.diviner +++ /dev/null @@ -1,60 +0,0 @@ -@title Using DarkConsole -@group developer - -Enabling and using the built-in debugging console. - -= Overview = - -DarkConsole is a debugging console built into Phabricator which exposes -configuration, performance and error information. It can help you detect, -understand and resolve bugs and performance problems in Phabricator -applications. - -DarkConsole was originally implemented as part of the Facebook Lite site; its -name is a bit of play on that (and a reference to the dark color palette its -design uses). - -= Warning = - -Because DarkConsole exposes some configuration and debugging information, it is -disabled by default (and **you should not enable it in production**). It has -some simple safeguards to prevent leaking credential information, but enabling -it in production may compromise the integrity of an install. - -= Enabling DarkConsole = - -You enable DarkConsole in your configuration, by setting `darkconsole.enabled` -to `true`, and then turning it on in `Settings` -> `Developer Settings`. Once -DarkConsole is enabled, you can show or hide it by pressing ##`## on your -keyboard. - -Since the setting is not available to logged-out users, you can also set -`darkconsole.always-on` if you need to access DarkConsole on logged-out pages. - -DarkConsole has a number of tabs, each of which is powered by a "plugin". You -can use them to access different debugging and performance features. - -= Plugin: Error Log = - -The "Error Log" plugin shows errors that occurred while generating the page, -similar to the httpd `error.log`. You can send information to the error log -explicitly with the @{function@libphutil:phlog} function. - -If errors occurred, a red dot will appear on the plugin tab. - -= Plugin: Request = - -The "Request" plugin shows information about the HTTP request the server -received, and the server itself. - -= Plugin: Services = - -The "Services" plugin lists calls a page made to external services, like -MySQL and the command line. - -= Plugin: XHProf = - -The "XHProf" plugin gives you access to the XHProf profiler. To use it, you need -to install the corresponding PHP plugin -- see instructions in the -@{article:Installation Guide}. Once it is installed, you can use XHProf to -profile the runtime performance of a page. diff --git a/src/docs/contributor/installing_xhprof.diviner b/src/docs/contributor/installing_xhprof.diviner deleted file mode 100644 index 32f8576402..0000000000 --- a/src/docs/contributor/installing_xhprof.diviner +++ /dev/null @@ -1,54 +0,0 @@ -@title Installing XHProf -@group developer - -Describes how to install XHProf, a PHP profiling tool. - -Overview -======== - -You can install XHProf to activate the XHProf tab in DarkConsole and the -`--xprofile` flag from the CLI. This will allow you to generate performance -profiles of pages and scripts, which can be tremendously valuable in identifying -and fixing slow code. - -Installing XHProf -================= - -XHProf is a PHP profiling tool. You don't need to install it unless you are -developing Phabricator and making performance changes. - -You can install xhprof with: - - $ pecl install xhprof - -If you have a PEAR version prior to 1.9.3, you may run into a `phpize` failure. -If so, you can download the source and build it with: - - $ cd extension/ - $ phpize - $ ./configure - $ make - $ sudo make install - -You may also need to add `extension=xhprof.so` to your php.ini. - -See for more information. - -Using XHProf: Web -================= - -To profile a web page, activate DarkConsole and navigate to the XHProf tab. -Use the **Profile Page** button to generate a profile. - -Using XHProf: CLI -================= - -From the command line, use the `--xprofile ` flag to generate a -profile of any script. - -Next Steps -========== - -Continue by: - - - enabling DarkConsole with @{article:Using DarkConsole}. diff --git a/src/docs/user/field/darkconsole.diviner b/src/docs/user/field/darkconsole.diviner new file mode 100644 index 0000000000..d235f6c1e4 --- /dev/null +++ b/src/docs/user/field/darkconsole.diviner @@ -0,0 +1,162 @@ +@title Using DarkConsole +@group fieldmanual + +Enabling and using the built-in debugging and performance console. + +Overview +======== + +DarkConsole is a debugging console built into Phabricator which exposes +configuration, performance and error information. It can help you detect, +understand and resolve bugs and performance problems in Phabricator +applications. + + +Security Warning +================ + +WARNING: Because DarkConsole exposes some configuration and debugging +information, it is disabled by default and you should be cautious about +enabling it in production. + +Particularly, DarkConsole may expose some information about your session +details or other private material. It has some crude safeguards against this, +but does not completely sanitize output. + +This is mostly a risk if you take screenshots or copy/paste output and share +it with others. + + +Enabling DarkConsole +==================== + +You enable DarkConsole in your configuration, by setting `darkconsole.enabled` +to `true`, and then turning it on in {nav Settings > Developer Settings}. + +Once DarkConsole is enabled, you can show or hide it by pressing ##`## on your +keyboard. + +Since the setting is not available to logged-out users, you can also set +`darkconsole.always-on` if you need to access DarkConsole on logged-out pages. + +DarkConsole has a number of tabs, each of which is powered by a "plugin". You +can use them to access different debugging and performance features. + + +Plugin: Error Log +================= + +The "Error Log" plugin shows errors that occurred while generating the page, +similar to the httpd `error.log`. You can send information to the error log +explicitly with the @{function@libphutil:phlog} function. + +If errors occurred, a red dot will appear on the plugin tab. + + +Plugin: Request +=============== + +The "Request" plugin shows information about the HTTP request the server +received, and the server itself. + + +Plugin: Services +================ + +The "Services" plugin lists calls a page made to external services, like +MySQL and subprocesses. + +The Services tab can help you understand and debug issues related to page +behavior: for example, you can use it to see exactly what queries or commands a +page is running. In some cases, you can re-run those queries or commands +yourself to examine their output and look for problems. + +This tab can also be particularly useful in understanding page performance, +because many performance problems are caused by inefficient queries (queries +with bad query plans or which take too long) or repeated queries (queries which +could be better structured or benefit from caching). + +When analyzing performance problems, the major things to look for are: + +**Summary**: In the summary table at the top of the tab, are any categories +of events dominating the performance cost? For normal pages, the costs should +be roughly along these lines: + +| Event Type | Approximate Cost | +|---|---| +| Connect | 1%-10% | +| Query | 10%-40% | +| Cache | 1% | +| Event | 1% | +| Conduit | 0%-80% | +| Exec | 0%-80% | +| All Services | 10%-75% | +| Entire Page | 100ms - 1000ms | + +These ranges are rough, but should usually be what you expect from a page +summary. If any of these numbers are way off (for example, "Event" is taking +50% of runtime), that points toward a possible problem in that section of the +code, and can guide you to examining the related service calls more carefully. + +**Duration**: In the Duration column, look for service calls that take a long +time. Sometimes these calls are just what the page is doing, but sometimes they +may indicate a problem. + +Some questions that may help understanding this column are: are there a small +number of calls which account for a majority of the total page generation time? +Do these calls seem fundamental to the behavior of the page, or is it not clear +why they need to be made? Do some of them seem like they could be cached? + +If there are queries which look slow, using the "Analyze Query Plans" button +may help reveal poor query plans. + +Generally, this column can help pinpoint these kinds of problems: + + - Queries or other service calls which are huge and inefficient. + - Work the page is doing which it could cache instead. + - Problems with network services. + - Missing keys or poor query plans. + +**Repeated Calls**: In the "Details" column, look for service calls that are +being made over and over again. Sometimes this is normal, but usually it +indicates a call that can be batched or cached. + +Some things to look for are: are similar calls being made over and over again? +Do calls mostly make sense given what the page is doing? Could any calls be +cached? Could multiple small calls be collected into one larger call? Are any +of the service calls clearly goofy nonsense that shouldn't be happening? + +Generally, this column can help pinpoint these kinds of problems: + + - Unbatched queries which should be batched (see + @{article:Performance: N+1 Query Problem}). + - Opportunities to improve performance with caching. + - General goofiness in how service calls are woking. + +If the services tab looks fine, and particularly if a page is slow but the +"All Services" cost is small, that may indicate a problem in PHP. The best +tool to understand problems in PHP is XHProf. + + +Plugin: XHProf +============== + +The "XHProf" plugin gives you access to the XHProf profiler. To use it, you need +to install the corresponding PHP plugin. + +Once it is installed, you can use XHProf to profile the runtime performance of +a page. This will show you a detailed breakdown of where PHP spent time. This +can help find slow or inefficient application code, and is the most powerful +general-purpose performance tool available. + +For instructions on installing and using XHProf, see @{article:Using XHProf}. + + +Next Steps +========== + +Continue by: + + - installing XHProf with @{article:Using XHProf}; or + - understanding and reporting performance issues with + @{article:Troubleshooting Performance Problems}. diff --git a/src/docs/user/field/performance.diviner b/src/docs/user/field/performance.diviner new file mode 100644 index 0000000000..4aa959b228 --- /dev/null +++ b/src/docs/user/field/performance.diviner @@ -0,0 +1,179 @@ +@title Troubleshooting Performance Problems +@group fieldmanual + +Guide to the troubleshooting slow pages and hangs. + +Overview +======== + +This document describes how to isolate, examine, understand and resolve or +report performance issues like slow pages and hangs. + +This document covers the general process for handling performance problems, +and outlines the major tools available for understanding them: + + - **Multimeter** helps you understand sources of load and broad resource + utilization. This is a coarse, high-level tool. + - **DarkConsole** helps you dig into a specific slow page and understand + service calls. This is a general, mid-level tool. + - **XHProf** gives you detailed application performance profiles. This + is a fine-grained, low-level tool. + +Performance and the Upstream +============================ + +Performance issues and hangs will often require upstream involvement to fully +resolve. The intent is for Phabricator to perform well in all reasonable cases, +not require tuning for different workloads (as long as those workloads are +generally reasonable). Poor performance with a reasonable workload is likely a +bug, not a configuration problem. + +However, some pages are slow because Phabricator legitimately needs to do a lot +of work to generate them. For example, if you write a 100MB wiki document, +Phabricator will need substantial time to process it, it will take a long time +to download over the network, and your browser will proably not be able to +render it especially quickly. + +We may be able to improve perfomance in some cases, but Phabricator is not +magic and can not wish away real complexity. The best solution to these problems +is usually to find another way to solve your problem: for example, maybe the +100MB document can be split into several smaller documents. + +Here are some examples of performance problems under reasonable workloads that +the upstream can help resolve: + + - {icon check, color=green} Commenting on a file and mentioning that same + file results in a hang. + - {icon check, color=green} Creating a new user takes many seconds. + - {icon check, color=green} Loading Feed hangs on 32-bit systems. + +The upstream will be less able to help resolve unusual workloads with high +inherent complexity, like these: + + - {icon times, color=red} A 100MB wiki page takes a long time to render. + - {icon times, color=red} A turing-complete simulation of Conway's Game of + Life implented in 958,000 Herald rules executes slowly. + - {icon times, color=red} Uploading an 8GB file takes several minutes. + +Generally, the path forward will be: + + - Follow the instructions in this document to gain the best understanding of + the issue (and of how to reproduce it) that you can. + - In particular, is it being caused by an unusual workload (like a 100MB + wiki page)? If so, consider other ways to solve the problem. + - File a report with the upstream by following the instructions in + @{article:Contributing Bug Reports}. + +The remaining sections in this document walk through these steps. + + +Understanding Performance Problems +================================== + +To isolate, examine, and understand performance problems, follow these steps: + +**General Slowness**: If you are experiencing generally poor performance, use +Multimeter to understand resource usage and look for load-based causes. See +@{article:Multimeter User Guide}. If that isn't fruitful, treat this like a +reproducible performance problem on an arbitrary page. + +**Hangs**: If you are experiencing hangs (pages which never return, or which +time out with a fatal after some number of seconds), they are almost always +the result of bugs in the upstream. Report them by following these +instructions: + + - Set `debug.time-limit` to a value like `5`. + - Reproduce the hang. The page should exit after 5 seconds with a more useful + stack trace. + - File a report with the reproduction instructions and the stack trace in + the upstream. See @{article:Contributing Bug Reports} for detailed + instructions. + - Clear `debug.time-limit` again to take your install out of debug mode. + +If part of the reproduction instructions include "Create a 100MB wiki page", +the upstream may be less sympathetic to your cause than if reproducing the +issue does not require an unusual, complex workload. + +In some cases, the hang may really just a very large amount of processing time. +If you're very excited about 100MB wiki pages and don't mind waiting many +minutes for them to render, you may be able to adjust `max_execution_time` in +your PHP configuration to allow the process enough time to complete, or adjust +settings in your webserver config to let it wait longer for results. + +**DarkConsole**: If you have a reproducible performance problem (for example, +loading a specific page is very slow), you can enable DarkConsole (a builtin +debugging console) to examine page performance in detail. + +The two most useful tabs in DarkConsole are the "Services" tab and the +"XHProf" tab. + +The "Services" module allows you to examine service calls (network calls, +subprocesses, events, etc) and find slow queries, slow services, inefficient +query plans, and unnecessary calls. Broadly, you're looking for slow or +repeated service calls, or calls which don't make sense given what the page +should be doing. + +After installing XHProf (see @{article:Using XHProf}) you'll gain access to the +"XHProf" tab, which is a full tracing profiler. You can use the "Profile Page" +button to generate a complete trace of where a page is spending time. When +reading a profile, you're looking for the overall use of time, and for anything +which sticks out as taking unreasonably long or not making sense. + +See @{article:Using DarkConsole} for complete instructions on configuring +and using DarkConsole. + +**AJAX Requests**: To debug Ajax requests, activate DarkConsole and then turn +on the profiler or query analyzer on the main request by clicking the +appropriate button. The setting will cascade to Ajax requests made by the page +and they'll show up in the console with full query analysis or profiling +information. + +**Command-Line Hangs**: If you have a script or daemon hanging, you can send +it `SIGHUP` to have it dump a stack trace to `sys_get_temp_dir()` (usually +`/tmp`). + +Do this with: + +``` +$ kill -HUP +``` + +You can use this command to figure out where the system's temporary directory +is: + +``` +$ php -r 'echo sys_get_temp_dir()."\n";' +``` + +On most systems, this is `/tmp`. The trace should appear in that directory with +a name like `phabricator_backtrace_`. Examining this trace may provide +a key to understanding the problem. + +**Command-Line Performance**: If you have general performance issues with +command-line scripts, you can add `--trace` to see a service call log. This is +similar to the "Services" tab in DarkConsole. This may help identify issues. + +After installing XHProf, you can also add `--xprofile ` to emit a +detailed performance profile. You can `arc upload` these files and then view +them in XHProf from the web UI. + +Next Steps +========== + +If you've done all you can to isolate and understand the problem you're +experiencing, report it to the upstream. Including as much relevant data as +you can, including: + + - reproduction instructions; + - traces from `debug.time-limit` for hangs; + - screenshots of service call logs from DarkConsole (review these carefully, + as they can sometimes contain sensitive information); + - traces from CLI scripts with `--trace`; + - traces from sending HUP to processes; and + - XHProf profile files from `--xprofile` or "Download .xhprof Profile" in + the web UI. + +After collecting this information: + + - follow the instructions in @{article:Contributing Bug Reports} to file + a report in the upstream. diff --git a/src/docs/user/field/xhprof.diviner b/src/docs/user/field/xhprof.diviner new file mode 100644 index 0000000000..7d032e6865 --- /dev/null +++ b/src/docs/user/field/xhprof.diviner @@ -0,0 +1,122 @@ +@title Using XHProf +@group fieldmanual + +Describes how to install and use XHProf, a PHP profiling tool. + +Overview +======== + +XHProf is a profiling tool which will let you understand application +performance in Phabricator. + +After you install XHProf, you can use it from the web UI and the CLI to +generate detailed performance profiles. It is the most powerful tool available +for understanding application performance and identifying and fixing slow code. + +Installing XHProf +================= + +You are likely to have the most luck building XHProf from source: + + $ git clone https://github.com/phacility/xhprof.git + +From any source distribution of the extension, build and install it like this: + + $ cd xhprof/ + $ cd extension/ + $ phpize + $ ./configure + $ make + $ sudo make install + +You may also need to add `extension=xhprof.so` to your php.ini. + +You can also try using PECL to install it, but this may not work well with +recent versions of PHP: + + $ pecl install xhprof + +Once you've installed it, `php -i` should report it as installed (you may +see a different version number, which is fine): + + $ php -i | grep xhprof + ... + xhprof => 0.9.2 + ... + + +Using XHProf: Web UI +==================== + +To profile a web page, activate DarkConsole and navigate to the XHProf tab. +Use the **Profile Page** button to generate a profile. + +For instructions on activating DarkConsole, see @{article:Using DarkConsole}. + + +Using XHProf: CLI +================= + +From the command line, use the `--xprofile ` flag to generate a +profile of any script. + +You can then upload this file to Phabricator (using `arc upload` may be easiest) +and view it in the web UI. + + +Analyzing Profiles +================== + +Understanding profiles is as much art as science, so be warned that you may not +make much headway. Even if you aren't able to conclusively read a profile +yourself, you can attach profiles when submitting bug reports to the upstream +and we can look at them. This may yield new insight. + +When looking at profiles, the "Wall Time (Inclusive)" column is usually the +most important. This shows the total amount of time spent in a function or +method and all of its children. Usually, to improve the performance of a page, +we're trying to find something that's slow and make it not slow: this column +can help identify which things are slowest. + +The "Wall Time (Exclusive)" column shows time spent in a function or method, +excluding time spent in its children. This can give you hint about whether the +call itself is slow or it's just making calls to other things that are slow. + +You can also get a sense of this by clicking a call to see its children, and +seeing if the bulk of runtime is spent in a child call. This tends to indicate +that you're looking at a problem which is deeper in the stack, and you need +to go down further to identify and understand it. + +Conversely, if the "Wall Time (Exclusive)" column is large, or the children +of a call are all cheap, there's probably something expesive happening in the +call itself. + +The "Count" column can also sometimes tip you off that something is amiss, if +a method which shouldn't be called very often is being called a lot. + +Some general thing to look for -- these aren't smoking guns, but are unusual +and can lead to finding a performance issue: + + - Is a low-level utility method like `phutil_utf8ize()` or `array_merge()` + taking more than a few percent of the page runtime? + - Do any methods (especially high-level methods) have >10,00 calls? + - Are we spending more than 100ms doing anything which isn't loading data + or rendering data? + - Does anything look suspiciously expensive or out of place? + - Is the profile for the slow page a lot different than the profile for a + fast page? + +Some performance problems are obvious and will jump out of a profile; others +may require a more nuanced understanding of the codebase to sniff out which +parts are suspicious. If you aren't able to make progress with a profile, +report the issue upstream and attach the profile to your report. + + +Next Steps +========== + +Continue by: + + - enabling DarkConsole with @{article:Using DarkConsole}; or + - understanding and reporting performance problems with + @{article:Troubleshooting Performance Problems}. diff --git a/src/docs/user/userguide/multimeter.diviner b/src/docs/user/userguide/multimeter.diviner new file mode 100644 index 0000000000..23e66276d0 --- /dev/null +++ b/src/docs/user/userguide/multimeter.diviner @@ -0,0 +1,99 @@ +@title Multimeter User Guide +@group userguide + +Using Multimeter, a sampling profiler. + +Overview +======== + +IMPORTANT: This document describes a prototype application. + +Multimeter is a sampling profiler that can give you coarse information about +Phabricator resource usage. In particular, it can help quickly identify sources +of load, like bots or scripts which are making a very large number of requests. + +Configuring and Using Multimeter +================================ + +To access Multimeter, go to {nav Applications > Multimeter}. + +By default, Multimeter samples 0.1% of pages. This should be a reasonable rate +for most installs, but you can increase or decrease the rate by adjusting +`debug.sample-rate`. Increasing the rate (by setting the value to a lower +number, like 100, to sample 1% of pages) will increase the granualrity of the +data, at a small performance cost. + +Using Multimeter +================ + +Multimeter shows you what Phabricator has spent time doing recently. By +looking at the samples it collects, you can identify major sources of load +or resource use, whether they are specific users, pages, subprocesses, or +other types of activity. + +By identifying and understanding unexpected load, you can adjust usage patterns +or configuration to make better use of resources (for example, rewrite bots +that are making too many calls), or report specific, actionable issues to the +upstream for resolution. + +The main screen of Multimeter shows you everything Phabricator has spent +resources on recently, broken down by action type. Categories are folded up +by default, with "(All)" labels. + +To filter by a dimension, click the link for it. For example, from the main +page, you can click "Web Request" to filter by only web requests. To expand a +grouped dimension, click the "(All)" link. + +For example, suppose we suspect that someone is running a bot that is making +a lot of requests and consuming a lot of resources. We can get a better idea +about this by filtering the results like this: + + - Click {nav Web Request}. This will show only web requests. + - Click {nav (All)} under "Viewer". This will expand events by viewer. + +Recent resource costs for web requests are now shown, grouped and sorted by +user. The usernames in the "Viewer" column show who is using resources, in +order from greatest use to least use (only administrators can see usernames). + +The "Avg" column shows the average cost per event, while the "Cost" column +shows the total cost. + +If the top few users account for similar costs and are normal, active users, +there may be nothing amiss and your problem might lie elsewhere. If a user like +`slowbot` is in the top few users and has way higher usage than anyone else, +there might be a script running under that account consuming a disproportionate +amount of resources. + +Assuming you find a user with unusual usage, you could dig into their usage +like this: + + - Click their name (like {nav slowbot}) to filter to just their requests. + - Click {nav (All)} under "Label". This expands by request detail. + +This will show exactly what they spent those resources doing, and can help +identify if they're making a lot of API calls or scraping the site or whatever +else. + +This is just an example of a specific kind of problem that Multimeter could +help resolve. In general, exploring Multimeter data by filtering and expanding +resource uses can help you understand how resources are used and identify +unexpected uses of resources. For example: + + - Identify a problem with load balancing by filtering on {nav Web Request} + and expanding on {nav Host}. If hosts aren't roughly even, DNS or a load + balancer are misconfigured. + - Identify which pages cost the most by filtering on {nav Web Request} + and expanding on {nav Label}. + - Find outlier pages by filtering on {nav Web Request} and expanding on + {nav ID}. + - Find where subprocess are invoked from by filtering on {nav Subprocesses}, + then expanding on {nav Context}. + + +Next Steps +========== + +Continue by: + + - understanding and reporting performance issues with + @{article:Troubleshooting Performance Problems}.