Commit 0c51885c authored by epriestley's avatar epriestley
Browse files

Survive importing Git commits with no commit message and/or no author

Summary:
Ref T13538. See PHI1739. Synthetic Git commits with no author and/or no commit message currently extract `null` and then fail to parse.

Ideally, we would carefully distinguish between `null` and empty string. In practice, that requires significant schema changes (these columns are non-nullable and have indexing requirements) and these cases are degenerate. These commits are challenging to build and can not normally be constructed with `git commit`.

At least for now, merge the `null` cases into the empty string cases so we can survive import.

Test Plan:
  - Constructed a commit with no author and no commit message using the approach described in T13538; pushed and parsed it.
  - Before: fatals during identity selection and storing the commit message (both roughly NULL inserts into non-null columns).
  - After: clean import.

This produces a less-than-ideal UI in Diffusion, but it doesn't break anything:

{F7492094}

Maniphest Tasks: T13538

Differential Revision: https://secure.phabricator.com/D21266
parent f86d822a
......@@ -62,7 +62,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
final protected function updateCommitData(DiffusionCommitRef $ref) {
$commit = $this->commit;
$author = $ref->getAuthor();
$message = $ref->getMessage();
$committer = $ref->getCommitter();
$hashes = $ref->getHashes();
$has_committer = (bool)strlen($committer);
......@@ -73,6 +72,14 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
->setViewer($viewer)
->setSourcePHID($commit->getPHID());
// See T13538. It is possible to synthetically construct a Git commit with
// no author and arrive here with NULL for the author value.
// This is distinct from a commit with an empty author. Because both these
// cases are degenerate and we can't resolve NULL into an identity, cast
// NULL to the empty string and merge the flows.
$author = phutil_string_cast($author);
$author_identity = $identity_engine->newResolvedIdentity($author);
if ($has_committer) {
......@@ -102,6 +109,11 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
'authorPHID',
$author_identity->getCurrentEffectiveUserPHID());
// See T13538. It is possible to synthetically construct a Git commit with
// no message. As above, treat this as though it is the same as the empty
// message.
$message = $ref->getMessage();
$message = phutil_string_cast($message);
$data->setCommitMessage($message);
if ($has_committer) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment