Page MenuHomePhabricator

Accept pushes with arbitrary Git refs
ClosedPublic

Authored by epriestley on Sun, Apr 14, 6:57 PM.

Details

Summary

Depends on D20419. Ref T13277. Fixes T8936. Fixes T9383. Fixes T12300. When you push arbitrary refs to Phabricator, the push log currently complains if those refs are not tags or branches.

Upstream Git now features "notes", and there's no reason to prevent writes to arbitrary refs, particularly beause we plan to start using them soon (see T13278).

Allow these writes as affecting raw refs.

Test Plan
  • Pushed an arbitrary ref.
  • Pushed some Git notes.
  • Wrote a Herald ref rule, saw "ref" in the dropdown.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Sun, Apr 14, 6:57 PM
epriestley requested review of this revision.Sun, Apr 14, 6:58 PM
epriestley edited the summary of this revision. (Show Details)Mon, Apr 15, 4:10 PM
amckinley accepted this revision.Wed, Apr 17, 6:49 PM
amckinley added inline comments.
src/applications/diffusion/engine/DiffusionCommitHookEngine.php
462

So [branch, tag, ref] is exhaustive? And we can't regex on it?

This revision is now accepted and ready to land.Wed, Apr 17, 6:49 PM
epriestley added inline comments.Wed, Apr 17, 7:01 PM
src/applications/diffusion/engine/DiffusionCommitHookEngine.php
462

Everything is a ref, named refs/something/something. I'm pretty sure you can't push anywhere else -- if you try, Git assumes you're pushing to a branch:

$ git push origin HEAD:not-refs/blah/blah
...
$ git ls-remote origin
...
eae726bc814e71b61c3f58c891866647ad6a649f	refs/heads/not-refs/blah/blah
...

Refs beginning with refs/heads/ are branches. Refs beginning with refs/tags/ are tags.

This is not exhaustive. Git supports notes today (in refs/notes/) and various other systems define things that we could arguably say aren't "just" refs, like refs/pull.

All of these are refs, they just may also be some other kind of more convenient/common object that users are more familiar with.

The REFTYPE stuff is just for convenience -- it only affects these things:

  • How the ref is shown in the push logs.
  • The Herald "Ref Type" rule.

We could call everything a "ref", this would just be a little less nice since users would need to interact with "refs/heads/master" instead of "master".

We could also go crazy and label every kind of magic ref we can find in the wild (and it might be reasonable to do this at some point). I'm not sure if they're all refs/<x>/ or not. We may being introducing refs/virtual/staging/... soon, and in some future world could label this as a "Staging Base Commit" or something, I guess?

This revision was automatically updated to reflect the committed changes.