Page MenuHomePhabricator

Improve repository Staging Areas
Open, NormalPublic

Description

Staging areas currently use hard-coded tags. The desired behavior is that, for hosted repositories, we use virtual refs instead. This primarily depends on:

  • T8093 - Virtualize Git refs.

Once this works, we have a clearer path toward:

  • T11091 - Make arc patch use staging areas.
  • PHI929 - Staging information should be surfaced on builds more cleanly.
  • PHI1181 - Other ref information should be surfaced on builds more cleanly.
  • T10093 - Staging area failures are not surfaced well in the UI.

Event Timeline

hskiba added a subscriber: hskiba.Mon, Apr 8, 7:26 AM
epriestley moved this task from Backlog to Staging Areas on the Diffusion board.Mon, Apr 15, 3:39 PM

With changes in T13277, I think we're ready to start making ref staging the default behavior in hosted repositories even without T8093 (which will give us some tools to improve things later, but seems like it's generally in good shape).

The major barrier here is third-party build systems that don't handle arbitrary refs. We likely need to keep a "push as tags" system in place for the moment to handle these cases. My plan is:

  • Add a "what do we push" mode to staging area configuration, with options "tags" and "refs". Existing repositories with a staging area configured will have their value set to "tags". Other repositories will have the value set to "refs".
  • Add a "where do we push" mode to staging area configuration, with options "default", "none", "self", and "remote". Existing repositories with a staging area configured have the value set to "remote". Other repositories will have value set to "default".
    • For hosted git repositories, "default" means "self".
    • For observed git repositories, "default" means "none".
  • Retain the existing "remote URI" config as-is. This is used when "where" is set to "remote".

It's possible we can remove "push as tags" some day, since CircleCI was the original problem and they no longer support Phabricator, but Buildkite support for tags was uneven last I looked and stuff like Jenkins probably doesn't understand refs.

With ref virtualization, an alternative is to expose ssh://.../src/as/tags/thing.git which rewrites refs/staging/diff/123 into refs/head/staging-diff-123 or similar, so it looks like a branch and non-ref build systems can handle it.

For now, though, I think we can just eat this complexity.

When staging is "self", we don't need to push a base ref.

Then, getting this to actually work is mostly about backward compatibility around arc, APIs, etc., I think.

APIs: Currently, diffusion.repository.search does not expose staging information.

repository.query does, and arc relies on it. It looks like this:

{
  "supported": true,
  "prefix": "phabricator",
  "uri": <some-uri>
}

We actually want to stop tag-only arc from pushing at all, so the fix here is probably:

  • Preserve this structure if the staging area is a "tags" staging area.
  • If the repository is "self", send our own URI (this is a valid configuration today, anyway).
  • If the staging area is not a "tags" staging area, send NULL as the URI.

Then we need to send a newer version of the structure that newer arc (which will know how to push non-tag refs) can read. We could send this in diffusion.repository.query, which I believe already supports all the other information we need (and has for quite a long time).

So the changes here are:

  • Add staging information to diffusion.repository.search.
  • Make repository.query emit staging information only if the client should push tags.
  • Make arc try diffusion.repository.search before repository.query and read the newer staging information if it finds it.

Diff Staging Storage: Diffs store arc.staging information. It looks like this:

{
  "ref": "refs/full/ref/name",
  "type": <diff|base>,
  "commit": <hash>,
  "remote": {
    "uri": <uri>
  }
}

That's perfect and doesn't need changes.


Build APIs: We expose some staging information to builds, notably repository.staging.uri and repository.staging.ref. We can probably retain this behavior.

PHI1181 would like better support for staging refs, which we can provide, but this is essentially supplemental.