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.

In T13658#255994, @epriestley wrote:

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.

In T13658#255996, @epriestley wrote:

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.

In T13658#255999, @epriestley wrote:

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.

Hey @epriestley :

We came up with a flow in Phorge we think would work well, to progress this task (https://we.phorge.it/E1):

• We'll have a bunch of diffs, on a per-application basis, along the style established here and in D21712.
• We'll review each diff on the Phorge
• When it matures, we'll copy it here for for your review and landing in Phabricator.

Will this work will with Phabricator? Would you have time for the final reviews?

Will this work will with Phabricator?

Yep.

Would you have time for the final reviews?

Uhhhhhh... eventually? No promises, but send 'em up for now and if I end up being the bottleneck we can figure something else out.

I don't want to get embroiled in this too much, but:

Phabricator has a PHP AST (PHPAST) and is capable of semantically rewriting its own source code, as it does at review time in many of its own linters. If you want to rewrite a bunch of Phabricator source code as maintainers of a Phabricator fork, becoming familiar with the Phabricator tools and using them to effect the change might be worth considering, particularly vs reaching out to a third-party team for help running a third-party tool. Or, if you evaluate PHPAST and decide it isn't a good tool for this purpose and its shortcomings can't reasonably be remedied (e.g., maybe barrier to entry is simply too high), you should consider removing it entirely: I think you aren't likely to find a better-matched problem, so if it doesn't win for this, it probably never wins in any problem domain.

D21763 adds a 75-line PHPAST-based linter to identify pht() calls which contain a product name. They can then be identified with arc lint --all. It could also rewrite these calls and apply the patches, but this isn't trivial, and there are so few (and many seem like they'd be better off if manually rewritten) that I'd guess I can get through all of them manually in not much longer than the time it would take to write and test the automated rule.

(I suspect I can just do all of these manually right now in less time than it would take me to review patches, but we'll see if the baby wakes up or not.)

Some general guidelines I'm following here:

• In user-facing text, I'm generally preferring to replace "Phabricator" with "server" where relevant. This may sometimes be imprecise, but I think it's probably the clearest word in these contexts (where we don't care if there's a server cluster, etc, and users are just thinking about the remote service as a whole, but might not have as much familiarity with "service" in this context as they do with "server").
  • When I subjectively think this isn't clear enough, I'll use the platform name or a construction like "%s (or compatible software)".
• In contexts where we are talking about multiple different kinds of the same thing (e.g., "VCS user" vs "Phabricator User"; "external user account" vs "Phabricator user account"), I'm retaining the server platform name, since this seems significantly more clear than generic language. There aren't too many of these (mostly in MFA and Auth, so far).
• In technical contexts, I'm being a little less explicit ("This software requires version 13..." vs "Phabricator requires version 13...") since I suspect server administrators won't have any trouble figuring out what the message is talking about.
  • I'm trying to use precise words these contexts, e.g. "software" vs "service" vs "server" vs "host", and take into account clustering, etc., since I expect server administrators to understand the distinctions and possibly be misled by imprecise language.
• I'm generally replacing example URIs with https://devtools.example.com/.

My initial linter implementation didn't actually catch all of these so there were a few more than I expected, but I think I got essentially all of them.

I removed Releeph and Phragment wholesale. They each had only a few product literal strings, but this seems like a reasonable juncture to remove them rather than continuing to maintain them.