Page MenuHomePhabricator

How to rebrand Phabricator
Open, Needs TriagePublic

Description

With Phacility winding down operations, maintenance of Phabricator no longer receiving regular updates, and Phabricator being an open source project, there are a number of discussions around creating forks.

The Phabricator application contains a large number of references to the product and organization names which are both trademarked. To make rebranding easier I've been investigating what these changes might look like and would like to discuss the approach with Phacility, and if agreeable submit these changes for upstream. These findings are based only on where "Phabricator" or "Phacility" appears in the user interface, and not changing the 20,000 files/classes/variables under the sea.

In reviewing where "Phabricator" appears in code the set of changes I believe would need to be made are

  1. Create something like src/infrastructure/Platform.php with getProjectName() to return "Phabricator" and getOrgName() to return "Phacility".
  2. Update the the ~500 "Phabricator" and "Phacility" references used in translated text (appearing in pht()) to swap out for %s and the appropriate new function to be called
    • Invalidates ~500 * 6 translations, update these
  3. Create Platform::getProjectRepo() to return "phabricator", or instead detect the repository name. There are a number of places which assume the project is checked out under phabricator, both functionally as well as when displayed to the user, e.g. "Run this command to change configuration, phabricator/ $ ./bin/config ....
    • There are a number of places which invoke phutil_get_library_root('phabricator') which would be updated to use this new function.
    • There are an assortment of examples displayed to the user in the style of http://phabricator.company.com which could be updated to use this instead.
  4. Update Diviner document sources to remove "Phabricator" and "Phacility" naming. I'm not yet clear with Diviner on whether these would be able to make use of dynamic naming based on getProjectName().
    • In the application code there are a number of places which retrieve the location of diviner docs to link to using PhabricatorEnv::getDoclink() which reference documents that currently use "Phabricator" in the document name.
  5. There are an assortment of references or links to phurl.io and Tnnnn for further information on things. My best guess for this is producing new Diviner documents on these topics and linking to them via getDocLink().
  6. PHUIBadgeExample.php indicates Lead Developer badge was awarded by epriestley - this could be updated to George Washington or similar if it's preferred to not have this. Although in the UI this text does not appear to show (possible bug?).

There are a number of preferences with phabricator in the name. This current approach would leave these alone for now.

Event Timeline

cspeckmim updated the task description. (Show Details)

Create Platform::getProjectRepo() to return "phabricator", or instead detect the repository name.

I suspect having this method do anything nontrivial may lead to some weird boostrapping issues, where it gets called very early during startup or with a partially built environment or during exception handling or whatever and not everything you expect to be available is actually available, so I suspect you'll have the best luck by just hard-coding this.

(For example, Phabricator reads PHABRICATOR_INSTANCE from the environment very early.)

Update the the ~500 "Phabricator" and "Phacility" references used in translated text

At least some of these can probably be made platform-name-agnostic.

I'm not yet clear with Diviner on whether these would be able to make use of dynamic naming based on getProjectName().

I think adding some kind of variable-replacement to Remarkup and then using ${{{ platform.name }}} or similar is reasonable for documentation.

this could be updated to George Washington or similar if it's preferred to not have this.

I don't have any issue with this, but it's also probably reasonable to remove, especially if it doesn't do anything. A lot of the UIExample stuff is more useful in theory than in practice, I think.

There are a number of preferences with phabricator in the name. This current approach would leave these alone for now.

We could probably alias/deprecate these, although I don't think there's a formal config alias system right now. At first glance, none of these prefixes seem especially useful, e.g. phabricator.timezone could reasonably just be timezone.

...to return "Phacility".

I suspect essentially all references to Phacility can be scrubbed. I just did a quick look at these but they mostly seem irrelevant for a fork (e.g., the "how to contribute" documentation will obviously be different) or test cases, comments, etc.


Given the amount of work involved here, I would perhaps maybe suggest you identify and implement, like, 3-5 useful patches which aren't upstreamable before moving forward with this, just to make sure you don't end up sinking a ton of hours into something that's sort of moot. (But maybe there's already a clear case for doing this legwork.)

This is a bit out there, but: PHP once had something called classkit and later runkit, which perhaps exists as runkit7 now, that let you dynamically rename classes at runtime. In theory, you could use this as a compatibility layer to support mass renaming of classes: if code asks for PhabricatorXYZ, look for GenericXYZ (or XYZ, or consult a mapping), then make a runtime copy of the class. This narrow capability may be in PHP's core as class_alias(), now.

I don't know how class_alias() interacts with static (see also T13637) or with introspection, but it's possible that some clever aliasing in the autoloader could let you rename classes one-by-one with no loss of backward compatibility.

Proof-of-concept only for class_alias():

diff --git a/src/init/init-library.php b/src/init/init-library.php
index 1041d80b..543415a5 100644
--- a/src/init/init-library.php
+++ b/src/init/init-library.php
@@ -22,6 +22,33 @@ function __phutil_autoload($class_name) {
       ->setName($class_name)
       ->selectAndLoadSymbols();
 
+    if (!$symbols) {
+
+      $map = array(
+        'PhabricatorEnv' => 'TheEnvironment',
+      );
+
+      if (isset($map[$class_name])) {
+        $modern_class_name = $map[$class_name];
+        $loader = new PhutilSymbolLoader();
+        $symbols = $loader
+          ->setType('class')
+          ->setName($modern_class_name)
+          ->selectAndLoadSymbols();
+        if ($symbols) {
+          $ok = class_alias($modern_class_name, $class_name, false);
+          if (!$ok) {
+            throw new Exception(
+              sprintf(
+                'While autoloading reference to legacy class "%s", failed '.
+                'to alias modern class "%s".',
+                $class_name,
+                $modern_class_naem));
+          }
+        }
+      }
+    }
+
     if (!$symbols) {
       throw new PhutilMissingSymbolException(
         $class_name,

With that applied, Phabricator runs fine with this patch that incrementally renames PhabricatorEnv to TheEnvironment without updating any references in the codebase:

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 663c64fff9..a18e7fbf30 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3343,7 +3343,6 @@ phutil_register_library_map(array(
     'PhabricatorEmptyQueryException' => 'infrastructure/query/exception/PhabricatorEmptyQueryException.php',
     'PhabricatorEnterHisecUserLogType' => 'applications/people/userlog/PhabricatorEnterHisecUserLogType.php',
     'PhabricatorEnumConfigType' => 'applications/config/type/PhabricatorEnumConfigType.php',
-    'PhabricatorEnv' => 'infrastructure/env/PhabricatorEnv.php',
     'PhabricatorEnvTestCase' => 'infrastructure/env/__tests__/PhabricatorEnvTestCase.php',
     'PhabricatorEpochEditField' => 'applications/transactions/editfield/PhabricatorEpochEditField.php',
     'PhabricatorEpochExportField' => 'infrastructure/export/field/PhabricatorEpochExportField.php',
@@ -5975,6 +5974,7 @@ phutil_register_library_map(array(
     'SlowvoteSearchConduitAPIMethod' => 'applications/slowvote/conduit/SlowvoteSearchConduitAPIMethod.php',
     'SubscriptionListDialogBuilder' => 'applications/subscriptions/view/SubscriptionListDialogBuilder.php',
     'SubscriptionListStringBuilder' => 'applications/subscriptions/view/SubscriptionListStringBuilder.php',
+    'TheEnvironment' => 'infrastructure/env/TheEnvironment.php',
     'TokenConduitAPIMethod' => 'applications/tokens/conduit/TokenConduitAPIMethod.php',
     'TokenGiveConduitAPIMethod' => 'applications/tokens/conduit/TokenGiveConduitAPIMethod.php',
     'TokenGivenConduitAPIMethod' => 'applications/tokens/conduit/TokenGivenConduitAPIMethod.php',
@@ -9860,7 +9860,6 @@ phutil_register_library_map(array(
     'PhabricatorEmptyQueryException' => 'Exception',
     'PhabricatorEnterHisecUserLogType' => 'PhabricatorUserLogType',
     'PhabricatorEnumConfigType' => 'PhabricatorTextConfigType',
-    'PhabricatorEnv' => 'Phobject',
     'PhabricatorEnvTestCase' => 'PhabricatorTestCase',
     'PhabricatorEpochEditField' => 'PhabricatorEditField',
     'PhabricatorEpochExportField' => 'PhabricatorExportField',
@@ -12998,6 +12997,7 @@ phutil_register_library_map(array(
     'SlowvoteSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
     'SubscriptionListDialogBuilder' => 'Phobject',
     'SubscriptionListStringBuilder' => 'Phobject',
+    'TheEnvironment' => 'Phobject',
     'TokenConduitAPIMethod' => 'ConduitAPIMethod',
     'TokenGiveConduitAPIMethod' => 'TokenConduitAPIMethod',
     'TokenGivenConduitAPIMethod' => 'TokenConduitAPIMethod',
diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/TheEnvironment.php
similarity index 99%
rename from src/infrastructure/env/PhabricatorEnv.php
rename to src/infrastructure/env/TheEnvironment.php
index 4806289f52..34da59e83b 100644
--- a/src/infrastructure/env/PhabricatorEnv.php
+++ b/src/infrastructure/env/TheEnvironment.php
@@ -48,7 +48,7 @@
  * @task test     Unit Test Support
  * @task internal Internals
  */
-final class PhabricatorEnv extends Phobject {
+final class TheEnvironment extends Phobject {
 
   private static $sourceStack;
   private static $repairSource;

This may or may not be interesting.

Create Platform::getProjectRepo() to return "phabricator", or instead detect the repository name.

I suspect having this method do anything nontrivial may lead to some weird boostrapping issues, where it gets called very early during startup or with a partially built environment or during exception handling or whatever and not everything you expect to be available is actually available, so I suspect you'll have the best luck by just hard-coding this.

(For example, Phabricator reads PHABRICATOR_INSTANCE from the environment very early.)

Ah I did have a suspicion that would be tricky but was thinking to give it a shot. Hardcoding is probably just going to be easiest.

Update the the ~500 "Phabricator" and "Phacility" references used in translated text

At least some of these can probably be made platform-name-agnostic.

That's a good point. I had started looking at swapping out the names but it might be better to swap out with "the platform" or "the service" or something.

I'm not yet clear with Diviner on whether these would be able to make use of dynamic naming based on getProjectName().

I think adding some kind of variable-replacement to Remarkup and then using ${{{ platform.name }}} or similar is reasonable for documentation.

Ooh that's an interesting idea. I'll take a look at that approach.

this could be updated to George Washington or similar if it's preferred to not have this.

I don't have any issue with this, but it's also probably reasonable to remove, especially if it doesn't do anything. A lot of the UIExample stuff is more useful in theory than in practice, I think.

I would be fine leaving it as-is since it's an example but wanted to bring it up since it belongs to you, or considered PII.

There are a number of preferences with phabricator in the name. This current approach would leave these alone for now.

We could probably alias/deprecate these, although I don't think there's a formal config alias system right now. At first glance, none of these prefixes seem especially useful, e.g. phabricator.timezone could reasonably just be timezone.

Yea I was thinking in the long-term something like this but figured it would be fine to initially leave these alone. I'm familiar with a similar config/preference migration strategy for another product but I wasn't entirely sure how preferences are stored in phab -- I wasn't sure if clustering (which I know nothing about) might be involved somehow.


Given the amount of work involved here, I would perhaps maybe suggest you identify and implement, like, 3-5 useful patches which aren't upstreamable before moving forward with this, just to make sure you don't end up sinking a ton of hours into something that's sort of moot. (But maybe there's already a clear case for doing this legwork.)

Ah that was my expectation, trying a few different attempts to see what works. I dove in head-first to swap out instances of Phabricator in translated text and got through a hundred or so making notes of these other types of references.

This is a bit out there, but: PHP once had something called classkit and later runkit, which perhaps exists as runkit7 now, that let you dynamically rename classes at runtime. In theory, you could use this as a compatibility layer to support mass renaming of classes: if code asks for PhabricatorXYZ, look for GenericXYZ (or XYZ, or consult a mapping), then make a runtime copy of the class. This narrow capability may be in PHP's core as class_alias(), now.

I don't know how class_alias() interacts with static (see also T13637) or with introspection, but it's possible that some clever aliasing in the autoloader could let you rename classes one-by-one with no loss of backward compatibility.

With that applied, Phabricator runs fine with this patch that incrementally renames PhabricatorEnv to TheEnvironment without updating any references in the codebase:
This may or may not be interesting.

Ooh interesting indeed

With that test did you not have to update the initial class definition in TheEnvironment.php? I wonder if this would also work so if something extends/implements PhabricatorEnvironment then that also goes through translation as well. PHP probably has some way of getting the current stack frame as well so we could log the call site when these occur to be markers for where to update.


Thanks for all the suggestions! I plan to take a second approach sometime next week or so. One thing I forgot to mention in the description is Arcanist. I was originally planning to have the getProjectName() call a method in Arcanist with the constant defined, since I believe Arcanist also has references which will need updated and Phab already depends on Arcanist.

One other case where "Phabricator" appears is in HTTP and Email headers, e.g. X-Phabricator-XYZ. For a separate project wanting to update these I think a slow migration approach is needed, to allow recipients currently expecting the existing fields. I'm guessing there isn't a reasonable change here for the upstream.

One other case where "Phabricator" appears is in HTTP and Email headers, e.g. X-Phabricator-XYZ. For a separate project wanting to update these I think a slow migration approach is needed, to allow recipients currently expecting the existing fields. I'm guessing there isn't a reasonable change here for the upstream.

Depending on how many of these we end up with, I think an email-header-prefix sort of config option might be reasonable.

I'm also conceptually open to making "Phabricator" in code generic (e.g., pick some neutral term like "Platform", and rename PhabricatorEnv -> PlatformEnv, etc) but I'm not sure how practical that change is (or how many of the tricky cases it could handle), even with the class_alias() stuff.

This whole thing may also be moot since it looks like Phorge has already landed changes which find/replace hundreds of instances of "Phabricator", so upstream compatibility may not ultimately be a major project goal once the dust settles.

With that test did you not have to update the initial class definition in TheEnvironment.php?

I updated class PhabricatorEnv to class TheEnvironment, but did not need to update existing references to PhabricatorEnv::suchAndSuch(). I believe extends PhabricatorEnv would still work, but I'm not sure what happens if we have this:

Given:
  A extends PhabricatorEnv
  B extends TheEnvironment

Is:
  A instanceof TheEnvironment ?
  B instanceof PhabricatorEnv ?

And various other cases like this. In this particular case, it seems like everything "just works":

$ cat test.php 
<?php

class A {}
class_alias('A', 'B');

$a = new A();
$b = new B();

q("A instanceof B?", $a instanceof B);
q("B instanceof A?", $b instanceof A);

function q($question, $answer) {
  echo $question;
  echo "\n";
  if ($answer) {
    echo "YES";
  } else {
    echo "NO";
  }
  echo "\n\n";
}
$ php -f test.php 
A instanceof B?
YES

B instanceof A?
YES

I wasn't entirely sure how preferences are stored in phab -- I wasn't sure if clustering (which I know nothing about) might be involved somehow.

There's no super-easy mechanism for aliasing config right now, but config itself doesn't have too much magic going on. Generally, processes get full access to config before they start doing stuff like interacting with other nodes: config always comes before clustering, broadly.

One other case where "Phabricator" appears is in HTTP and Email headers, e.g. X-Phabricator-XYZ. For a separate project wanting to update these I think a slow migration approach is needed, to allow recipients currently expecting the existing fields. I'm guessing there isn't a reasonable change here for the upstream.

Depending on how many of these we end up with, I think an email-header-prefix sort of config option might be reasonable.

Oh and then for existing installs this would just default to X-Phabricator-XYZ to continue with existing behavior?

I'm also conceptually open to making "Phabricator" in code generic (e.g., pick some neutral term like "Platform", and rename PhabricatorEnv -> PlatformEnv, etc) but I'm not sure how practical that change is (or how many of the tricky cases it could handle), even with the class_alias() stuff.

I was sorta guessing changing of code using "Phabricator" might not be something accepted upstream so my thought would be forks have to do a slow changeover, but maybe providing some tooling via the class_alias() stuff for doing so.

This whole thing may also be moot since it looks like Phorge has already landed changes which find/replace hundreds of instances of "Phabricator", so upstream compatibility may not ultimately be a major project goal once the dust settles.

I've been working with the Phorge project to try and organize the re-brand. My preferred method is to take the approaches outlined here and submit for upstream, at which point then creating the forked repository. So far I think the only thing that has landed there have been changes to Diviner documents and nothing else in the code. I think the number and size of changes Phorge has done so far are small enough that re-forking would be the preferred approach. This past week or so I've switched focus to Mercurial changes. I plan to get back to the re-brand this weekend if possible.

With that test did you not have to update the initial class definition in TheEnvironment.php?

I updated class PhabricatorEnv to class TheEnvironment, but did not need to update existing references to PhabricatorEnv::suchAndSuch(). I believe extends PhabricatorEnv would still work, but I'm not sure what happens if we have this:

Given:
  A extends PhabricatorEnv
  B extends TheEnvironment

Is:
  A instanceof TheEnvironment ?
  B instanceof PhabricatorEnv ?

And various other cases like this. In this particular case, it seems like everything "just works":

$ cat test.php 
<?php

class A {}
class_alias('A', 'B');

$a = new A();
$b = new B();

q("A instanceof B?", $a instanceof B);
q("B instanceof A?", $b instanceof A);

function q($question, $answer) {
  echo $question;
  echo "\n";
  if ($answer) {
    echo "YES";
  } else {
    echo "NO";
  }
  echo "\n\n";
}
$ php -f test.php 
A instanceof B?
YES

B instanceof A?
YES

Ah okay, I'll do some other testing. I was also wondering if there would be cases in Phabricator where something is doing something like if (get_class($a) == 'PhabricatorEnv') {... which if so I'm guessing would cause failures.

I wasn't entirely sure how preferences are stored in phab -- I wasn't sure if clustering (which I know nothing about) might be involved somehow.

There's no super-easy mechanism for aliasing config right now, but config itself doesn't have too much magic going on. Generally, processes get full access to config before they start doing stuff like interacting with other nodes: config always comes before clustering, broadly.

Ah good to know, ty

Ah okay, I'll do some other testing. I was also wondering if there would be cases in Phabricator where something is doing something like if (get_class($a) == 'PhabricatorEnv') {... which if so I'm guessing would cause failures.

This shouldn't be used very often, but Lisk does use get_class() to generate default table names (e.g., PHP class DifferentialChangeset has default MySQL storage table differential_changeset). Most of these probably don't have Phabricator in their names, though, and this mapping doesn't need to be derived from the class name.