More or less ran into first issue with Q444.

See PHI594, where a real user had the same human name as an external user from another project.

To actually build this:

• Create a new PhabricatorRepositoryIdentity or similar object.
• It should work like PhabricatorRepositoryRefCursor to accommodate non-UTF8 identities (i.e. have raw, hash and encoding columns) and arbitrarily long identities. If you need test cases, the strategy in T11537#192019 seems very likely to let you build commits with arbitrarily long/silly/garbage author and committer identities.
• We store the raw identity string that comes out of the VCS, e.g., Blah Blah <blah@blah.com>. Each sequence of bytes is a unique identity. If you very cleverly encode two different display strings in the same sequence of bytes using Shift-JIS in one repository and ISO-ROFLOL in another repository, too bad, Phabricator doesn't care, they're the same identity.
• We assume an identity always identifies the same user in every repository. You can't have joe be two different users in two different repositories.
• We assume an identity always identifies the same user in a given repository. You can't have joe be one user on some branches and a different user on other branches, or on old commits vs new commits, etc.
• These objects should have real PHIDs.
• Since we're going to let users muck around with them (claim identities, etc) they should also probably have proper transaction support so we can render a log of who messed with stuff and broke everything.
• In PhabricatorReposiotryCommitMessageParserWorker->updateCommitData(), start creating Identity objects for $author and $committer if they do not exist yet.
• Use bin/repository reparse --message ... to test this. It should populate a bunch of stuff into the database.

That should work, just not do anything useful yet.

• Add authorIdentityPHID and committerIdentityPHID to PhabricatorRepositoryCommit.
• Start setting those in updateCommitData().
• At some point we're going to have to do some painful migrations here but probably not for a bit.

Before we migrate, we can do this:

• Add a UI in Diffusion to list/manage identies (standard search/list/view/edit transaction stuff), just with no way to create a new identity.
• I think we probably add three columns to Identity, like automaticGuessedUserPHID, manuallySetUserPHID, and currentEffectiveUserPHID.

These new columns work like this:

• When we create an identity in MessageParserWorker, we use DiffusionResolveUserQuery to use the current rules (or maybe just the "email address" rule, without the "username" or "real name" rules) to guess which Phabricator user this identity resolves to.
• When a user adds a new email address, we re-resolve any identities which contain it. I think it's fine to just do this with LIKE ..., although ideally we'd actually do the re-resolution in the daemons (just queue a task for them, "re-resolve everything containing this email address"). The goal here is to automatically associate existing commits with users who newly sign up. We might need a little fiddling here with uppercase/lowercase, etc.
• There's probably some kind of script to update all the guesses again explicitly.
• When users use the edit UI to "claim" or "release" an identity, they update the manuallySetUserPHID.
• After the guessed or manual PHID is updated, the currentEffectiveUserPHID is set to the manual PHID (if it exists), or the guessed PHID (if it exists), or null (if neither exist). This is the column we'll actually JOIN/WHERE on, etc.
• There should be a way to set the manual PHID to "this identity does not correspond to any Phabricator user" and force the effective identity to null, even if we guessed that it does correspond to a user.

To make this do more stuff:

• In the UI, we can start doing stuff like if ($commit->getAuthorIdentityPHID()) { ... } and marking the older getAuthorPHID() stuff as obsolete.

Then:

• Big scary migration. I don't really see a way around this.

Then we make all the rest of the UI and query stuff use the new Identity stuff, get rid of the old stuff, and hopefully we're in good shape?

From T13152 -- none of this is too important, just didn't want to lose it when I close that:

A sub-issue here is that rebuild-identities (and, likewise, CommitMessageParserWorker) uses getAuthorName() from the CommitData.

This may not be the original string: in particular, it has been converted to utf8 and truncated to 255 characters.

For some data on secure, it is incomplete (probably because of different behavior in older versions of Phabricator), e.g. jack instead of jack <jack@jacksdomain.com>.

It may also be mangled somewhat by DiffusionCommitRef, which splits the author apart rather than preserving it completely faithfully.

Under the hood, DiffusionCommitRef is (possibly) transported in JSON which can not transport non-utf8 values. Fixing this is probably out of scope until wire encodings in T5955, though.

These hosts ultimately timed out on the initial rebuild-identities: db010, db024, db025.

The latter two are somewhat special; the former might have just stalled in a mundane way. I'm going to make a couple of tweaks to the script to make re-running it faster (basically, skip writes if they'd have no effect) and finish those three up manually.

I deployed D19484 and db010, db024 and db025 finished up with no issues.

just wanted to give you some reference data (your mileage may vary), I have 2 Phabricator instances with multiple git repos

one has ~300,000 commits ( a mixture of read-only (mirrors) and hosted repos), the other ~200,000 (all hosted)

in both cases I ran rebuild-identities after doing the Week25 upgrade

time ./bin/repository rebuild-identities --all

it took

21 minutes on the one with 300,000 commits and
16 minutes on the one with 200,000

Can't help but think that this was most likely limited by the speed of the terminal output.

Hope that helps others

In T12164#239595, @mydeveloperday wrote:

just wanted to give you some reference data (your mileage may vary), I have 2 Phabricator instances with multiple git repos

one has ~300,000 commits ( a mixture of read-only (mirrors) and hosted repos), the other ~200,000 (all hosted)

in both cases I ran rebuild-identities after doing the Week25 upgrade

time ./bin/repository rebuild-identities --all

it took

21 minutes on the one with 300,000 commits and
16 minutes on the one with 200,000

Can't help but think that this was most likely limited by the speed of the terminal output.

Hope that helps others

Thanks for the data point. @epriestley I’m going to add a —quiet flag for that script; diff coming soon.

The "activity" migration went to production just now. I expect all production instances just bailed out, since I ran rebuild-identities some time ago, but I'll verify that:

$ phage remote query --pools db -- --instance-statuses up --query 'SELECT * FROM <INSTANCE>_config.config_manualactivity'

This caught a handful of instances where commits had been missed and the activity had activated.

I spot-checked these and found:

• One unreachable commit never had its message parse before it was marked as unreachable (this is consistent with pushing a branch by accident, then deleting it), so it didn't get author info. rebuild-identities now assigned it an identity corresponding to "" (the empty string). This presumably is a behavioral change. This is possibly a questionable behavior, but the state is such an edge case that I'm just going to mark it done for now.
• Handful of the same on the next instance.
• One on the next instance.
• Two on the next instance.
• The next instance had about 800 of these.

I did rebuild-identities on every instance, then config done identities to mark the activity complete. I also found one instance with some imported/old data that had a reindex activity scheduled. Now, no instances report waiting activities.

So, current notes here for thinking about handling null in the future:

• A commit may legitimately have a null value for authorIdentityPHID between the time it is discovered and the time the message is parsed. This is normally a few seconds, but could be arbitrarily long.
• When commits become unreachable before their messages parse (which is routine if you push a large branch by accident, then immediately delete it) the message parse phase will not activate and the commit will normally never get an authorIdentityPHID.
• The rebuild-identities script will currently generate an empty string Identity for these commits and assign them the PHID for that empty identity, so commits in this state may or may not have an authorIdentityPHID. We might want to make this behavior more consistent as we look at how we'll handle null and how we'll move forward into the era of Identities.

I did rebuild-identities on every instance

Er, not every instance, but the the handful of instances (I think 7 total) which had the "identities" activity end up in queue after the migration ran.

Sorry if this is a stupid question, but now that this is done, how do I actually map between Phabricator users and VCS user strings, in an existing Diffusion repo?

In the general case:

• Go to Diffusion → Identities → Browse Identities (left menu).
• Find the VCS user string you want to map (perhaps by searching with "Identity Contains" if you have a large number of identities).
• Click it, then Edit Identity, type the Phabricator user it should map to, then Save Changes.

If you have a commit by that user handy:

• Click the "Author: Identity <identity@identy.com>" link.
• As above, Edit Identity and select the user to map to.

Perfect; thanks!

Is there a way to find which object(s) an (unmapped) identity was discovered on? After a rebuild-identities, I have an empty string identity and a couple asdf-style garbage ones. I'd like to find the source of them and either fix them there or see the context to understand the appropriate identity mapping.

Is there a way to find which object(s) an (unmapped) identity was discovered on?

Sort of. There's a value in the database:

mysql> SELECT authorPHID FROM repository_identity WHERE identityNameRaw = 'EC2 Default User <ec2-user@ip-10-170-207-10.us-west-1.compute.internal>';
+--------------------------------+
| authorPHID                     |
+--------------------------------+
| PHID-CMIT-st4quw227w3dakcilr6x |
+--------------------------------+
1 row in set (0.01 sec)

However:

• this value currently is not exposed in the UI; and
• this value may not be populated for older identities.

You can also find commits by author identity:

mysql> SELECT phid FROM repository_identity WHERE identityNameRaw = 'EC2 Default User <ec2-user@ip-10-170-207-10.us-west-1.compute.internal>';
+--------------------------------+
| phid                           |
+--------------------------------+
| PHID-RIDT-64vn2gtoyccdok4o66kk |
+--------------------------------+
1 row in set (0.00 sec)

mysql> SELECT * FROM repository_commit WHERE authorIdentityPHID = 'PHID-RIDT-64vn2gtoyccdok4o66kk'\G
*************************** 1. row ***************************
                   id: 244
         repositoryID: 1
                 phid: PHID-CMIT-st4quw227w3dakcilr6x
     commitIdentifier: 31d2790075fa60a682cd626036cd0faac976db3e
                epoch: 1300222313
           authorPHID: NULL
          auditStatus: none
              summary: durf durf sql
         importStatus: 15
   authorIdentityPHID: PHID-RIDT-64vn2gtoyccdok4o66kk
committerIdentityPHID: PHID-RIDT-64vn2gtoyccdok4o66kk
1 row in set (0.01 sec)

However:

• this won't find Committer identities, and they're currently stored in a JSON blob so there's no efficient way to query them.

I imagine exposing the authorPHID value in the UI and supporting "search commits by identity" and "browse commits using this identity" eventually, but I'm not sure when I'll be back in this area of the codebase.

That was sufficient to get what I needed, thanks so much.

Committer identities ... they're currently stored in a JSON blob so there's no efficient way to query them.

...

committerIdentityPHID: PHID-RIDT-64vn2gtoyccdok4o66kk

(I think I remembered this wrong, clearly not in a blob.)