Recently we've moved to the last version of Phabricator. And since that we're experiencing issues because we rely on 'differential.query' search by branches which seems to be broken.

At first we thought that it was our migration mistake, but it turns out that the current installation of phabricator (secure.phabricator.com) has the same symptoms.

Let's try to search by any old enough branch.

POST /api/differential.query HTTP/1.1

params%5Bbranches%5D=%5B%22sha1%22%5D

---

{
  "0": {
    "id": "1000",
    "phid": "PHID-DREV-77397651c68839ac6688",
    "title": "Replace callsites to sha1() that use it to asciify entropy with Filesystem::readRandomCharacters()",
...

OK. Now let's try any recent branch (https://secure.phabricator.com/D12537).

POST /api/differential.query HTTP/1.1

params%5Bbranches%5D=%5B%22go-unit-engine%22%5D

---

{}

Ooops.

Interesting that if we'll search by id, we will get this revision with corresponding branch name.

POST /api/differential.query HTTP/1.1

params%ids%5D=%5B12537%5D

---

{
  "0": {
    "id": "12537",
    "phid": "PHID-DREV-ktn64pmsit44ekup7euu",
    "title": "Add a Go unit test engine (through go and godep).",
...
    "branch": "go-unit-engine",
...

I'm new to phabricator's codebase, so I don't feel lucky enough to fix this bug since I don't even understand how it is supposed to work. But here is the information I've collected while trying to debug it. I'm sorry if something below sounds funny or naive.

The main thing, I suppose, is that the field 'differential_revision.branchName' is not set when creating revision. So I suppose that when fetching a revision its 'branch' field comes from some other source. So I tried to understand why it is not set.

We can take all branches one by one from older to newer and search revisions by them until we get an empty result, and the dateCreated of that reision can help to find a change that broke search by branches. But it's much simpler if someone who can straight access to DB will execute the following query

mysql> SELECT * FROM `differential_revision` where branchName is not null order by id desc limit 1\G

Then we can try to find the nearest commit using found dateCreated.

The next thing I've found out is that the branch field is filtered out when getting object fields for create operation. I really don't understand how 'branchName' is supposed to set, so I've looked into DifferentialTitleField.

Looks like the branch field is filtered by DifferentialCustomField::shouldEnableForRole, let's take a look

public function shouldEnableForRole($role) {
    switch ($role) {
      case self::ROLE_COMMITMESSAGE:
        return $this->shouldAppearInCommitMessage();
      case self::ROLE_COMMITMESSAGEEDIT:
        return $this->shouldAppearInCommitMessage() &&
               $this->shouldAllowEditInCommitMessage();
    }

    return parent::shouldEnableForRole($role);
  }

Again, I don't understand that roles, but we can see that for any conduit api method the role is DifferentialCustomField::ROLE_COMMITMESSAGEEDIT when getting field list.

(from DifferentialConduitAPIMethod::applyFieldEdit)

$field_list = PhabricatorCustomField::getObjectFields(
      $revision,
      DifferentialCustomField::ROLE_COMMITMESSAGEEDIT);

So to pass the filter we must have $this->shouldAppearInCommitMessage() && $this->shouldAllowEditInCommitMessage(); expression to be true.

DifferentialCustomField::shouldAllowEditInCommitMessage is true by default.

public function shouldAllowEditInCommitMessage() {
    if ($this->getProxy()) {
      return $this->getProxy()->shouldAllowEditInCommitMessage();
    }
    return true;
  }

And DifferentialCustomField::shouldAppearInCommitMessage is false by default.

public function shouldAppearInCommitMessage() {
    if ($this->getProxy()) {
      return $this->getProxy()->shouldAppearInCommitMessage();
    }
    return false;
  }

DifferentialTitleField simply overrides the DifferentialTitleField::shouldAppearInCommitMessage.

public function shouldAppearInCommitMessage() {
    return true;
  }

So I think it's the main reason it's not filtered out. And DifferentialBranchField doesn't override this method and it doesn't have any proxy, so no chance to be set.

Hope it helps.

Oh, we're currently on 75f081aaf2fbbec22847ecdc72a44bb7f0df0cf1.