Summary:
Ref T10923. We sort of dead-end new users creating repositories right now, by dumping them into the manage UI without an obvious way forward.
You can click the crumb to get to the repository, but by default it will say something like `R1` which isn't very obvious.
Add a more obvious navigational link to get to the main view.
Test Plan: {F1308196}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15889
Summary:
Ref T10923. The old behavior was to show a full command in SVN, Mercurial, and Git, like this:
- `git clone <uri>`
- `hg clone <uri>`
- `svn checkout <uri> <directory>`
In Git and Mercurial, the `<uri>` ends in something like `/nice-repository-name.git` so the default directory it creates is called `nice-repository-name/`.
In Subversion, we don't (and can't easily) do that for various reasons so we provide an explicit `<directory>` with the nice name.
In the update, I've changed things to just show the URI. I often found that I wanted the URI alone, not the whole clone command (for example, to `fetch`, `remote-add`, etc). This is also consistent with GitHub. Because we have nice URIs for Git and Mercurial, `git clone <uri>` has good behavior.
In Subversion, `svn checkout <uri>` has bad beahvior (you get a directory named `47/` or whatever). So continue showing the whole command there.
We can possibly tailor this after T4245 finishes up and we get access to `/source/nice-repository-name/` URIs.
Test Plan:
- Viewed a Subversion repository, saw a full command.
- Viewed a Git repository, saw only a clone URI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15888
Summary: Fixes T10941. This avoids a confusing dead end when configuring Subversion hosting, where `svnserve` will fail to execute hooks if the CWD isn't readable by the vcs-user.
Test Plan:
- Updated and committed in a hosted SVN repository.
- Ran some git operations, too.
- @dpotter confirmed this locally in T10941.
Reviewers: chad
Reviewed By: chad
Subscribers: dpotter
Maniphest Tasks: T10941
Differential Revision: https://secure.phabricator.com/D15879
Summary: Ref T10923. Although I'd ideally like to get rid of this eventually, keep it around for now.
Test Plan:
- Edited value for an SVN repository.
- Observed no panel present for a Git repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15883
Summary: Fixes T10815. We already recovered reasonably from this for cluster repositories, but not for non-cluster repositories.
Test Plan:
- Viewed cluster and non-cluster empty Git repository.
- Viewed cluster and non-cluster empty Mercurial repository.
- Viewed cluster and non-clsuter empty hosted SVN repository.
- Viewed cluster and non-cluster empty observed SVN repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10815
Differential Revision: https://secure.phabricator.com/D15878
Summary:
Ref T10923. Fixes T9554.
When hosting a repository, we currently have a heuristic that tries to detect when you're doing an initial import: if you push more than 7 commits to an empty repository, it counts as an import and we disable mail/feed/etc.
Do something similar for observed repositories: if the repository is empty and we discover more than 7 commits, switch to import mode until we catch up.
This should align behavior with user expectation more often when juggling hosted vs imported repositories.
Test Plan:
- Created a new hosted repository.
- Activated it and allowed it to fully import.
- Added an "Observe URI".
- Saw it automatically drop into "Importing" mode until the import completed.
- Swapped it back to hosted mode.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9554, T10923
Differential Revision: https://secure.phabricator.com/D15877
Summary: Ref T10923. Walk users through the "create, configure, activate" workflow a little better and set expectations more clearly.
Test Plan:
- Created a new repository, saw new UI help.
- Activated repository, saw onboarding help disappear.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15875
Summary:
Ref T10923.
- Hide "Automation", "Staging" and "Branches" in repositories where they do nothing.
- Fix SVN SSH URIs to read "svn+ssh://" and have proper paths.
Test Plan:
- Verified irrelevant sections did not appear in Subversion in Manage UI.
- Checked out a new hosted SVN repository.
Reviewers: chad, avivey
Reviewed By: avivey
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15874
Summary:
Ref T10923. Fixes T10406. This brings most of the guidance/instructions forward:
- Some remained as instructions.
- Some moved to documentation.
Test Plan: Went through all of the sections and hit the help.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10406, T10923
Differential Revision: https://secure.phabricator.com/D15873
Summary: Ref T10923. This provides a little guidance about hosted vs observed, and points at the `diffusion.ssh-*` options.
Test Plan: Poked around in the web UI, saw useful guidance.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15872
Summary: Ref T10923. This cleans up the remaining "pro" mess left by the cutover.
Test Plan: Viewed, managed, edited a repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15871
Summary:
Ref T10923. Primarily documents the process for creating repositories via the API.
Also fixes a couple of issues with `repositoryPHID` not being set yet when creating URIs via the API.
Test Plan:
- Followed all documented steps to create a new repository.
- Created and edited some new URIs from the web workflow, too.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15870
Summary: Ref T10923. This isn't complete yet, but reduces lies and increases truths.
Test Plan: Read documentation, clicked new "Documentation" nav item.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15868
Summary:
Ref T10923. Some of the dialogs ("Deactivate Repository", "Test Automation", etc.) had cancel or redirect URIs which I missed originally.
Go through them and make sure they all point to the right places.
Also removed one unused controller which I missed the first time around.
Test Plan:
- Opened all these dialogs in a new tab with Command-Click.
- Clicked every "cancel" and "submit" button on all of these dialogs.
- Got consistently sent to the place I came from.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15867
Summary:
Ref T10923.
- The "Policy" edit form currently goes "Push, View, Edit". Reorder the defaults to "View, Edit, Push".
- Editing Spaces doesn't currently work: the element appears in the UI, but isn't actually processed when handling transactions. Make that work.
Test Plan:
- Edited a repository policies, saw "View, Edit, Push".
- Moved a repository between Spaces.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15866
Summary:
Ref T10927. Pretty sure the issue is:
- User makes an HTTPS request.
- Load balancer terminates it, but with an `X-Forwarded-Proto` header.
- `secure001` (or whatever; acting as web host) proxies it to `secure002` (or whatever; acting as a repository host). **This** connection is plain HTTP.
- Since this proxied connection is plain HTTP, we check if the repository can serve over "http", but it can't: only "https". So we fail incorrectly, even though the original user request was HTTPS.
In the long run we should probably forward the `X-Forwarded-Proto` header, but that has some weird implications and it's broadly fine to allow either protocol to serve as long as the other one is active: configuration like `security.require-https` is already stronger than these settings.
Test Plan: This is likely only observable in production, but normal cloning still works locally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10927
Differential Revision: https://secure.phabricator.com/D15856
Summary:
Ref T10748. This needs more extensive testing and is sure to have some rough edges, but seems to basically work so far.
Throwing this up so I can work through it more deliberately and make notes.
Test Plan:
- Ran migration.
- Used `bin/repository list` to list existing repositories.
- Used `bin/repository update <repository>` to update various repositories.
- Updated a migrated, hosted Git repository.
- Updated a migrated, observed Git repository.
- Converted an observed repository into a hosted repository by toggling the I/O mode of the URI.
- Conveted a hosted repository into an observed repository by toggling it back.
- Created and activated a new empty hosted Git repository.
- Created and activated an observed Git repository.
- Updated a mirrored repository.
- Cloned and pushed over HTTP.
- Tried to HTTP push a read-only repository.
- Cloned and pushed over SSH.
- Tried to SSH push a read-only repository.
- Updated several Mercurial repositories.
- Updated several Subversion repositories.
- Created and edited repositories via the API.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15842
Summary:
Ref T10748. This migrates and swaps mirroring to `PhabricatorRepositoryURI`, obsoleting `PhabricatorRepositoryMirror`.
This prevents you from editing, adding or disabling mirrors unless you know a secret URI (until the UI cuts over fully), but existing mirroring is not affected.
Test Plan:
- Added a mirroring URI to an old repository.
- Verified it worked with `bin/repository mirror`.
- Migrated forward.
- Verified it still worked with `bin/repository mirror`.
- Wow, mirroring: https://github.com/epriestley/locktopia-mirror
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15841
Summary:
Ref T4039. Long ago these were more freely editable and there were some security concerns around creating a repository, then setting its local path to point somewhere it shouldn't.
Local paths are no longer editable so there's no real reason we need to provide a uniqueness guarantee anymore, but you could still make a mistake with `bin/repository move-paths` by accident, and it's a little cleaner to pull them out into their own column with a key.
(We still don't -- and, largely can't -- guarantee that two paths aren't //equivalent// since one might be symlinked to the other, or symlinked only on some hosts, or whatever, but the primary value here is as a sanity check that you aren't goofing things up and pointing a bunch of repositories at the same working copy by mistake.)
Test Plan:
- Ran migrations.
- Grepped for `local-path`.
- Listed and moved paths with `bin/repository`.
- Created a new repository, verified its local path populated correctly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4039
Differential Revision: https://secure.phabricator.com/D15837
Summary:
Ref T10748. These:
- Look nice.
- Hint at panel contents / effects.
- Hint which panels have been customized.
- Allow panels with issues or errors to be highlighted with an alert/attention icon.
Test Plan: {F1256156}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15836
Summary: Ref T10748. This copies existing code in the `CreateController` which will eventually be removed.
Test Plan:
- Created a new repository with the EditPro workflow.
- Saw it come up into the cluster properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15835
Summary:
Ref T10748. This allows an EditEngine form to be broken up into pages.
This is less powerful than `PHUIPagedFormView`, because the pages are not sequential / stateful. Each form saves immediately once it's submitted, and can not take you to a new form or back/forward in a series of forms.
For example, you can't create a workflow where the user fills out 5 pages of information before we create an object, like the current repository workflow does.
However, the only place we've ever wanted to do this is repositories and it's fairly bad there, so I feel reasonably confident we aren't going to miss this in the future.
(We do "choose a type of service/repository/rule -> fill out one page of info" fairly often, but can do this without the full-power paging stuff.)
Test Plan:
- Created a repository usin the new Manage UI, filling out only a handful of fields.
- Edited a repository using the new Manage UI.
- All forms are now EditEngine forms offering paged views of the big huge underlying form:
{F1254371}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15832
Summary:
Ref T10748. Ref T10366. Allows users to set credential for new URIs.
- Ref T7221. Our handling of the "git://" protocol is currently incorrect. This protocol is not authenticated, but is considered an SSH protocol. In the new UI, it is considered an anonymous/unauthenticated protocol instead.
- Ref T10241. This fixes the `PassphraseCredentialControl` so it doesn't silently edit the value if the current value is not visible to you and/or not valid.
Test Plan:
Performed a whole lot of credential edits, removals, and adjustments. I'll give this additional vetting before cutting over to it.
{F1253207}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7221, T10241, T10366, T10748
Differential Revision: https://secure.phabricator.com/D15829
Summary:
Ref T10748.
- Allow users to add new URIs by clicking a button instead of knowing a secret URI.
- Validate that URIs are actually valid URIs.
- Add enable/disable action and strings.
Test Plan:
- Created a new URI.
- Tried to create a nonsense URI, created a good URI.
- Enabled/disabled a URI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15825
Summary: Ref T10748. Adds a "uris" attachment with URI information.
Test Plan: Queried URI information via Conduit, saw reasonable looking information.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15822
Summary: Ref T10748. Brings the rest of the transactions to EditEngine, supports creating via API.
Test Plan:
- Created a URI via API.
- Created a URI via web.
- Tried to apply sneaky transactions, got rejected with good error messages. <_< >_>
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15821
Summary:
Ref T10748.
- New View page for repository URIs.
- Make display and I/O behavior (observe, mirror, read, read/write) editable.
- Add a bunch of checks to prevent you from completely screwing up a repository by making it writable from a bunch of differnet sources.
Test Plan:
{F1249866}
{F1249867}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15816
Summary:
Ref T10748. Ref T10366. This adds a new EditEngine, EditController, Editor, Query, and Transaction for RepositoryURIs.
None of these really do anything helpful yet, and these URIs are still unused in the actual application.
Test Plan: {F1249794}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10366, T10748
Differential Revision: https://secure.phabricator.com/D15815
Summary:
Ref T10748. Allow the new EditEngine workflow to create repositories by giving the user a modal repository type choice upfront.
(The rest of this flow is still confusing/weird, though.)
Test Plan:
- Created a new repository.
{F1249626}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15813
Summary: Ref T10748. This brings the "Actions" items (publish/notify + autoclose enabled) into the new UI.
Test Plan:
- Edited this stuff via EditEngine and Conduit.
- Viewed via new Manage UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15811
Summary: Ref T10748. Makes a "Branches" panel, enables these transactions in the EditEngine.
Test Plan:
- Edited via EditEngine + Conduit.
- Viewed via manage UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15809
Summary: Ref T10748. Port this, add EditEngine support, add some type validation to the transaction.
Test Plan:
- Edited via EditEngine.
- Edited via Conduit.
- Viewed via Management UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15808
Summary: Ref T10748. Brings this forward in the UI and EditEngine.
Test Plan:
- Edited via Conduit.
- Viewed via Manage UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15805
Summary: Ref T10748. Ports this UI and exposes it on the EditEngine.
Test Plan:
- Edited via EditEngine.
- Viewed new manage UI.
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15804
Summary: Ref T10748. Brings this over and adds EditEngine support for it.
Test Plan: Viewed and edited staging area information.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15801
Summary: Ref T10748. This merges "Storage" and "Cluster" into a single UI which combines the information of both.
Test Plan: {F1246882}
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15800
Summary:
Ref T10860. This allows us to recover if the connection to the database is lost during a push.
If we lose the connection to the master database during a push, we would previously freeze the repository. This is very safe, but not very operator-friendly since you have to go manually unfreeze it.
We don't need to be quite this aggressive about freezing things. The repository state is still consistent after we've "upgraded" the lock by setting `isWriting = 1`, so we're actually fine even if we lost the global lock.
Instead of just freezing the repository immediately, sit there in a loop waiting for the master to come back up for a few minutes. If it recovers, we can release the lock and everything will be OK again.
Basically, the changes are:
- If we can't release the lock at first, sit in a loop trying really hard to release it for a while.
- Add a unique lock identifier so we can be certain we're only releasing //our// lock no matter what else is going on.
- Do the version reads on the same connection holding the lock, so we can be sure we haven't lost the lock before we do that read.
Test Plan:
- Added a `sleep(10)` after accepting the write but before releasing the lock so I could run `mysqld stop` and force this issue to occur.
- Pushed like this:
```
$ echo D >> record && git commit -am D && git push
[master 707ecc3] D
1 file changed, 1 insertion(+)
# Push received by "local001.phacility.net", forwarding to cluster host.
# Waiting up to 120 second(s) for a cluster write lock...
# Acquired write lock immediately.
# Waiting up to 120 second(s) for a cluster read lock on "local001.phacility.net"...
# Acquired read lock immediately.
# Device "local001.phacility.net" is already a cluster leader and does not need to be synchronized.
# Ready to receive on cluster host "local001.phacility.net".
Counting objects: 3, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 254 bytes | 0 bytes/s, done.
Total 3 (delta 1), reused 0 (delta 0)
BEGIN SLEEP
```
- Here, I stopped `mysqld` from the CLI in another terminal window.
```
END SLEEP
# CRITICAL. Failed to release cluster write lock!
# The connection to the master database was lost while receiving the write.
# This process will spend 300 more second(s) attempting to recover, then give up.
```
- Here, I started `mysqld` again.
```
# RECOVERED. Link to master database was restored.
# Released cluster write lock.
To ssh://local@localvault.phacility.com/diffusion/26/locktopia.git
2cbf87c..707ecc3 master -> master
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10860
Differential Revision: https://secure.phabricator.com/D15792
Summary:
Ref T10860. At least in Git over SSH, we can freely echo a bunch of stuff to stderr and Git will print it to the console, so we can tell users what's going on.
This should make debugging, etc., easier. We could tone this down a little bit once things are more stable if it's a little too chatty.
Test Plan:
```
$ echo D >> record && git commit -am D && git push
[master ca5efff] D
1 file changed, 1 insertion(+)
# Push received by "local001.phacility.net", forwarding to cluster host.
# Waiting up to 120 second(s) for a cluster write lock...
# Acquired write lock immediately.
# Waiting up to 120 second(s) for a cluster read lock on "local001.phacility.net"...
# Acquired read lock immediately.
# Device "local001.phacility.net" is already a cluster leader and does not need to be synchronized.
# Ready to receive on cluster host "local001.phacility.net".
Counting objects: 3, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 256 bytes | 0 bytes/s, done.
Total 3 (delta 1), reused 0 (delta 0)
To ssh://local@localvault.phacility.com/diffusion/26/locktopia.git
8616189..ca5efff master -> master
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10860
Differential Revision: https://secure.phabricator.com/D15791
Summary: Ref T10860. This doesn't change anything, it just separates all this stuff out of `PhabricatorRepository` since I'm planning to add a bit more state to it and it's already pretty big and fairly separable.
Test Plan: Pulled, pushed, browsed Diffusion.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10860
Differential Revision: https://secure.phabricator.com/D15790
Summary:
Ref T4292. When the daemons make a query for repository information, we need to make sure the working copy on disk is up to date before we serve the response, since we might not have the inforamtion we need to respond otherwise.
We do this automatically for almost all Diffusion methods, but this particular method is a little unusual and does not get this check for free. Add this check.
Test Plan:
- Made this code throw.
- Ran `bin/repository reparse --message ...`, saw the code get hit.
- Ran `bin/repository lookup-user ...`, saw this code get hit.
- Made this code not throw.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15783
Summary:
Ref T10751. Make the UI more useful and explain what failure states mean and how to get out of them.
The `bin/repository thaw` command does not exist yet, I'll write that soon.
Test Plan: {F1238241}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10751
Differential Revision: https://secure.phabricator.com/D15766
Summary: Ref T10751. This cleans this up so it's a little more modern, and fixes a possible bad access on the log detail page.
Test Plan: Viewed push log list, viewed push log detail.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10751
Differential Revision: https://secure.phabricator.com/D15765
Summary: Ref T4292. When we write a push log, also log which node received the request.
Test Plan: {F1230467}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15759
Summary:
Ref T4292. We currently synchronize hosted, clustered, Git repositories when we receive an SSH pull or push.
Additionally:
- Synchronize before HTTP reads and writes.
- Synchronize reads before Conduit requests.
We could relax Conduit eventually and allow Diffusion to say "it's OK to give me stale data".
We could also redirect some set of these actions to just go to the up-to-date host instead of connecting to a random host and synchronizing it. However, this potentially won't work as well at scale: if you have a larger number of servers, it sends all of the traffic to the leader immediately following a write. That can cause "thundering herd" issues, and isn't efficient if replicas are in different geographical regions and the write just went to the east coast but most clients are on the west coast. In large-scale cases, it's better to go to the local replica, wait for an update, then serve traffic from it -- particularly given that writes are relatively rare. But we can finesse this later once things are solid.
Test Plan:
- Pushed and pulled a Git repository over HTTP.
- Browsed a Git repository from the web UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15758
Summary:
Ref T4292. Before we write or read a hosted, clustered Git repository over SSH, check if another version of the repository exists on another node that is more up-to-date.
If such a version does exist, fetch that version first. This allows reads and writes of any node to always act on the most up-to-date code.
Test Plan: Faked my way through this and got a fetch via `bin/repository update`; this is difficult to test locally and needs more work before we can put it in production.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15757
Summary:
Ref T4292. When you run `git fetch` and connect to, say, `repo001.west.company.com`, we'll look at the current version of the repository in other nodes in the cluster.
If `repo002.east.company.com` has a newer version of the repository, we'll fetch that version first, then respond to your request.
To do this, we need to run `git fetch repo002.east.company.com ...` and have that connect to the other host and be able to fetch data.
This change allows us to run `PHABRICATOR_AS_DEVICE=1 git fetch ...` to use device credentials to do this fetch. (Device credentials are already supported and used, they just always connect as a user right now, but these fetches should be doable without having a user. We will have a valid user when you run `git fetch` yourself, but we won't have one if the daemons notice that a repository is out of date and want to update it, so the update code should not depend on having a user.)
Test Plan:
```
$ PHABRICATOR_AS_DEVICE=1 ./bin/ssh-connect local.phacility.com
Warning: Permanently added 'local.phacility.com' (RSA) to the list of known hosts.
PTY allocation request failed on channel 0
phabricator-ssh-exec: Welcome to Phabricator.
You are logged in as device/daemon.phacility.net.
You haven't specified a command to run. This means you're requesting an interactive shell, but Phabricator does not provide an interactive shell over SSH.
Usually, you should run a command like `git clone` or `hg push` rather than connecting directly with SSH.
Supported commands are: conduit, git-lfs-authenticate, git-receive-pack, git-upload-pack, hg, svnserve.
Connection to local.phacility.com closed.
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15755
Summary:
Ref T4292. This consolidates code for figuring out which user we should connect to hosts with.
Also narrows a lock window.
Test Plan: Browsed Diffusion, pulled and pushed through an SSH proxy.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15754
Summary:
Ref T4292. Ref T10366. Depends on D15751. Today, generating repository commands is purely a function of the repository, so they use protocols and credentials based on the repository configuration.
For example, a repository with an SSH "remote URI" always generate SSH "remote commands".
This needs to change in the future:
- After T10366, repositories won't necessarily just have one type of remote URI. They can only have one at a time still, but the repository itself won't change based on which one is currently active.
- For T4292, I need to generate intracluster commands, regardless of repository configuration. These will have different protocols and credentials.
Prepare for these cases by separating out command construction, so they'll be able to generate commands in a more flexible way.
Test Plan:
- Added unit tests.
- Browsed diffusion.
- Ran `bin/phd debug pull` to pull a bunch of repos.
- Ran daemons.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292, T10366
Differential Revision: https://secure.phabricator.com/D15752
Summary: Ref T4292. This will let the UI and future `bin/repository` tools give administrators more tools to understand problems when reporting or resolving them.
Test Plan:
- Pushed fully clean repository.
- Pushed previously-pushed repository.
- Forced write to abort, inspected useful information in the database.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15748
Summary:
Ref T4292. Small fixes:
- There was a bug with the //first// write, where we'd write 1 but expect 0. Fix this.
- Narrow the window where we hold the `isWriting` lock: we don't need to wait for the client to finish.
- Release the lock even if something throws.
- Use a more useful variable name.
Test Plan:
- Made new writes to a fresh cluster repository.
- Made sequential writes.
- Made concurrent writes.
- Made good writes and bad writes.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15747
Summary:
Ref T10748. This supports more transaction types in the modern editor and improves validation so Conduit benefits.
You can technically create repositories via `diffusion.repository.edit` now, although they aren't very useful.
Test Plan:
- Used `diffusion.repository.edit` to create and edit repositories.
- Used `/editpro/` to edit repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15740
Summary: Ref T10748. Ref T10366. No support for editing and no impact on the UI, but get some of the basics in place.
Test Plan: {F1223279}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10366, T10748
Differential Revision: https://secure.phabricator.com/D15742
Summary: Ref T10748. Pretty straightforward. I'd like to put a little "!" icon in the menu if there's a warning/error eventually, but can deal with that latre.
Test Plan: {F1223096}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15741
Summary:
Ref T10748. This is roughly where I'm headed, if it makes some kind of sense? The "Edit" links in sub-sections don't work yet since I haven't built the thing.
Probably depends on D15736.
Test Plan: Manually navigated to `/manage/`, clicked around.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15737
Summary: Ref T10748. Ref T10337. This technically implements this stuff, but it does not do anything useful yet. This skips all the hard stuff.
Test Plan:
- Technically used `diffusion.repository.search` to get repository information.
- Technically used `diffusion.repository.edit` to change a repository name.
- Used `editpro/` to edit a repository name.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10337, T10748
Differential Revision: https://secure.phabricator.com/D15736
Summary: Fixes T10816. The way these work is a little unusual since these chunks of file-rendering code are unusuall performance-sensitive, so the Differential version doesn't adapt directly to Diffusion. Both can possibly be unified at some point in the future, although they do slightly different things.
Test Plan: {F1220170}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10816
Differential Revision: https://secure.phabricator.com/D15719
Summary:
Ref T10809. Currently, both the proxy and target may mutate URIs (rewriting "svn+ssh://x/diffusion/Y/" to a path on disk).
I believe this previously worked by fate/chance/luck since both URI variants contain the repository information, but the algorithms were tightened up recently with callsign removal.
Stop rewriting them if we're the intracluster proxy -- they only need to be rewritten on the target host.
Test Plan:
- Checked out a proxied SVN repository, with and without a callsign.
- Checked out an unproxied SVN repository, with and without a callsign.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10809
Differential Revision: https://secure.phabricator.com/D15712
Summary:
Ref T2783. This allows this worker to run on a machine different to the one that stores the repository, by routing the execution of Git over Conduit calls.
This API method is super gross, but fixing it isn't straightforward and it runs into other complicated considerations. We can fix it later; for now, just define it as "internal" to limit how much mess this creates.
"Internal" methods do not appear on the console.
Test Plan: Ran `bin/repository reparse --change <commit> --trace` on several commits, saw daemons make a Conduit call instead of running a `git` command.
Reviewers: hach-que, chad
Reviewed By: chad
Subscribers: joshuaspence, Korvin, epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11874
Summary: Fixes T10797. This seems to fix things on my local system.
Test Plan:
- Cloned with a username, got prompted for a password.
- Cloned with a username + password.
- Cloned with a username + bad password (error).
Reviewers: chad
Reviewed By: chad
Subscribers: Grimeh
Maniphest Tasks: T10797
Differential Revision: https://secure.phabricator.com/D15706
Summary:
Ref T4292. This mostly implements the locking/versioning logic for multi-master repositories. It is only active on Git SSH pathways, and doesn't actually do anything useful yet: it just does bookkeeping so far.
When we read (e.g., `git fetch`) the logic goes like this:
- Get the read lock (unique to device + repository).
- Read all the versions of the repository on every other device.
- If any node has a newer version:
- Fetch the newer version.
- Increment our version to be the same as the version we fetched.
- Release the read lock.
- Actually do the fetch.
This makes sure that any time you do a read, you always read the most recently acknowledged write. You may have to wait for an internal fetch to happen (this isn't actually implemented yet) but the operation will always work like you expect it to.
When we write (e.g., `git push`) the logic goes like this:
- Get the write lock (unique to the repository).
- Do all the read steps so we're up to date.
- Mark a write pending.
- Do the actual write.
- Bump our version and mark our write finished.
- Release the write lock.
This allows you to write to any replica. Again, you might have to wait for a fetch first, but everything will work like you expect.
There's one notable failure mode here: if the network connection between the repository node and the database fails during the write, the write lock might be released even though a write is ongoing.
The "isWriting" column protects against that, by staying locked if we lose our connection to the database. This will currently "freeze" the repository (prevent any new writes) until an administrator can sort things out, since it'd dangerous to continue doing writes (we may lose data).
(Since we won't actually acknowledge the write, I think, we could probably smooth this out a bit and make it self-healing //most// of the time: basically, have the broken node rewind itself by updating from another good node. But that's a little more complex.)
Test Plan:
- Pushed changes to a cluster-mode repository.
- Viewed web interface, saw "writing" flag and version changes.
- Pulled changes.
- Faked various failures, got sensible states.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15688
Summary:
Ref T4292. This adds some very basic cluster/device data to the new management view. Nothing interesting yet.
Also deal with disabled bindings a little more cleanly.
Test Plan: {F1214619}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15685
Summary:
Ref T4292. This puts a very rough skeleton in place for the new "Manage Repository" UI, somewhat similar to the "Settings" UI.
Right now, it has one panel with no content, and is not reachable from the UI.
Test Plan: {F1214525}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15683
Summary: Going to render these all normal case instead of all caps, and bump up the font size. Should be more consistent. Yellow if you green anything orange.
Test Plan: grep, lint
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15645
Summary:
Ref T10262. This removes one-time tokens and makes file data responses always-cacheable (for 30 days).
The URI will stop working once any attached object changes its view policy, or the file view policy itself changes.
Files with `canCDN` (totally public data like profile images, CSS, JS, etc) use "cache-control: public" so they can be CDN'd.
Files without `canCDN` use "cache-control: private" so they won't be cached by the CDN. They could still be cached by a misbehaving local cache, but if you don't want your users seeing one anothers' secret files you should configure your local network properly.
Our "Cache-Control" headers were also from 1999 or something, update them to be more modern/sane. I can't find any evidence that any browser has done the wrong thing with this simpler ruleset in the last ~10 years.
Test Plan:
- Configured alternate file domain.
- Viewed site: stuff worked.
- Accessed a file on primary domain, got redirected to alternate domain.
- Verified proper cache headers for `canCDN` (public) and non-`canCDN` (private) files.
- Uploaded a file to a task, edited task policy, verified it scrambled the old URI.
- Reloaded task, new URI generated transparently.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10262
Differential Revision: https://secure.phabricator.com/D15642
Summary:
Fixes T10721. When trying to load commits by identifier, we would take some bad pathways in Subversion if the repository had no callsign and end up missing the commits.
Fix this logic so it works for either callsigns (e.g., if passed `rXyyy`) or with PHIDs if passed repositories.
Test Plan:
- Viewed SVN commit in a Subversion repository with no callsign.
- Added a callsign, looked at it again.
- Viewed non-SVN commits in callsign and non-callsign repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10721
Differential Revision: https://secure.phabricator.com/D15607
Summary:
Fixes T10385. Two issues:
- `$show_blame` and `$show_color` were improperly swapped.
- Code to hide these columns got dropped somewhere, probably in my recent-ish rewrite.
Test Plan:
- Showed/hid blame.
- Showed/hid colors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10385
Differential Revision: https://secure.phabricator.com/D15528
Summary:
Ref T10537. For Nuance, I want to introduce new sources (like "GitHub" or "GitHub via Nuance" or something) but this needs to modularize eventually.
Split ContentSource apart so applications can add new content sources.
Test Plan:
This change has huge surface area, so I'll hold it until post-release. I think it's fairly safe (and if it does break anything, the breaks should be fatals, not anything subtle or difficult to fix), there's just no reason not to hold it for a few hours.
- Viewed new module page.
- Grepped for all removed functions/constants.
- Viewed some transactions.
- Hovered over timestamps to get content source details.
- Added a comment via Conduit.
- Added a comment via web.
- Ran `bin/storage upgrade --namespace XXXXX --no-quickstart -f` to re-run all historic migrations.
- Generated some objects with `bin/lipsum`.
- Ran a bulk job on some tasks.
- Ran unit tests.
{F1190182}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15521
Summary:
Fixes T10665. See that task for discussion.
Because `$head_map` is not properly re-initialized for each ref we check, pushes which affect multiple branches (say, "A" and "B") can have information bleed from the first branch check to the second branch.
To trigger a problem behavior, you can push one commit which updates an existing branch, plus one commit which creates a new branch. If they process in the right order, the `$head_map` from the updated branch will bleed into the `$head_map` for the new branch and trigger an incorrect head split detection.
Test Plan:
- Pushed a set of changes which updated `branch-a` and created `branch-b`.
- Before change: improper detection of split heads.
- After change: clean push.
- Pushed a set of changes which split the head of `branch-d`.
- Correct detection of split heads.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10665
Differential Revision: https://secure.phabricator.com/D15522
Summary:
D9087 adds a nice typeahead but breaks the existing regex
search by quoting the pattern. Ideally, this change won't break the
typeahead, which as far as I can tell doesn't use the `pattern`
argument.
Test Plan:
Not yet.
RFC as to whether this change makes sense, will fix my local setup and resend if so.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15500
Summary: Ref T7789. Implement proper detection for read-only requests. Previously, we assumed every request was read/write and required lots of permissions, but we don't need "Can Push" permission if you're only cloning/fetching/pulling.
Test Plan:
- Set push policy to "no one".
- Fetched, got clean data out of LFS.
- Tried to push, got useful error.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7789
Differential Revision: https://secure.phabricator.com/D15499
Summary:
- Fix spacing on InfoView inside collasped boxes
- Fix spacing on stacked PropertyLists in TwoColumn
- Fix spacing on Readmes on Tablets
- Fix unset variable on importing commits
Test Plan: Review each of the above cases.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15496
Summary: Ref T7789. This isn't the most perfect UI imaginable, but it's similar to what GitHub does and seems reasonable.
Test Plan:
{F1180271}
{F1180272}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7789
Differential Revision: https://secure.phabricator.com/D15494
Summary:
Ref T7789. Ref T10604. This implements the `upload` action, which streams file data into Files.
This makes Git LFS actually work, at least roughly.
Test Plan:
- Tracked files in an LFS repository.
- Pushed LFS data (`git lfs track '*.png'; git add something.png; git commit -m ...; git push`).
- Pulled LFS data (`git checkout master^; rm -rf .git/lfs; git checkout master; open something.png`).
- Verified LFS refs show up in the gitlfsref table.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7789, T10604
Differential Revision: https://secure.phabricator.com/D15492
Summary: I think I like this better -- but maybe right-aligned?
Test Plan:
{F1180295}
{F1180296}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15495
Summary:
Ref T7789. This implements:
- A new table to store the `<objectHash, filePHID>` relationship between Git LFS files and Phabricator file objects.
- A basic response to `batch` commands, which return actions for a list of files.
Test Plan:
Ran `git lfs push origin master`, got a little further than previously:
```
epriestley@orbital ~/dev/scratch/poemslocal $ git lfs push origin master
Git LFS: (2 of 1 files) 174.24 KB / 87.12 KB
Git LFS operation "upload/b7e0aeb82a03d627c6aa5fc1bbfd454b6789d9d9affc8607d40168fa18cf6c69" is not supported by this server.
Git LFS operation "upload/b7e0aeb82a03d627c6aa5fc1bbfd454b6789d9d9affc8607d40168fa18cf6c69" is not supported by this server.
```
With `GIT_TRACE=1`, this shows the batch part of the API going through.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7789
Differential Revision: https://secure.phabricator.com/D15489
Summary: This updates (all?) of Diffusion/Audit to new UI, included edit and other extra form pages. It's fairly complete but I don't know all the nooks and crannies so to speak to fully verify I didn't mess anything up.
Test Plan: Tested creating new repositories, browsing, searching, auditing. Need more eyes.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15487
Summary:
Ref T7789. This builds on top of `git-lfs-authenticate` to detect LFS requests, read LFS tokens, and route them to a handler which can do useful things.
This handler promptly drops them on the floor with an error message.
Test Plan:
Here's a transcript showing the parts working together so far:
- `git-lfs` connects to the server with SSH, and gets told how to connect with HTTP to do uploads.
- `git-lfs` uses HTTP, and authenticates with the tokens properly.
- But the server tells it to go away, and that it doesn't support anything, so the operation ultimately fails.
```
$ GIT_TRACE=1 git lfs push origin master
12:45:56.153913 git.c:558 trace: exec: 'git-lfs' 'push' 'origin' 'master'
12:45:56.154376 run-command.c:335 trace: run_command: 'git-lfs' 'push' 'origin' 'master'
trace git-lfs: Upload refs origin to remote [master]
trace git-lfs: run_command: git rev-list --objects master --not --remotes=origin
trace git-lfs: run_command: git cat-file --batch-check
trace git-lfs: run_command: git cat-file --batch
trace git-lfs: run_command: 'git' config -l
trace git-lfs: tq: starting 3 transfer workers
trace git-lfs: tq: running as batched queue, batch size of 100
trace git-lfs: prepare upload: b7e0aeb82a03d627c6aa5fc1bbfd454b6789d9d9affc8607d40168fa18cf6c69 lfs/dog1.jpg 1/1
trace git-lfs: tq: sending batch of size 1
trace git-lfs: ssh: local@localvault.phacility.com git-lfs-authenticate diffusion/18/poems.git upload
trace git-lfs: api: batch 1 files
trace git-lfs: HTTP: POST http://local.phacility.com/diffusion/POEMS/poems.git/info/lfs/objects/batch
trace git-lfs: HTTP: 404
trace git-lfs: HTTP: {"message":"Git LFS operation \"objects\/batch\" is not supported by this server."}
trace git-lfs: HTTP:
trace git-lfs: api: batch not implemented: 404
trace git-lfs: run_command: 'git' config lfs.batch false
trace git-lfs: tq: batch api not implemented, falling back to individual
trace git-lfs: ssh: local@localvault.phacility.com git-lfs-authenticate diffusion/18/poems.git upload b7e0aeb82a03d627c6aa5fc1bbfd454b6789d9d9affc8607d40168fa18cf6c69
trace git-lfs: api: uploading (b7e0aeb82a03d627c6aa5fc1bbfd454b6789d9d9affc8607d40168fa18cf6c69)
trace git-lfs: HTTP: POST http://local.phacility.com/diffusion/POEMS/poems.git/info/lfs/objects
trace git-lfs: HTTP: 404
trace git-lfs: HTTP: {"message":"Git LFS operation \"objects\" is not supported by this server."}
trace git-lfs: HTTP:
trace git-lfs: tq: retrying 1 failed transfers
trace git-lfs: ssh: local@localvault.phacility.com git-lfs-authenticate diffusion/18/poems.git upload b7e0aeb82a03d627c6aa5fc1bbfd454b6789d9d9affc8607d40168fa18cf6c69
trace git-lfs: api: uploading (b7e0aeb82a03d627c6aa5fc1bbfd454b6789d9d9affc8607d40168fa18cf6c69)
trace git-lfs: HTTP: POST http://local.phacility.com/diffusion/POEMS/poems.git/info/lfs/objects
trace git-lfs: HTTP: 404
trace git-lfs: HTTP: {"message":"Git LFS operation \"objects\" is not supported by this server."}
trace git-lfs: HTTP:
Git LFS: (0 of 1 files) 0 B / 87.12 KB
Git LFS operation "objects" is not supported by this server.
Git LFS operation "objects" is not supported by this server.
```
Reviewers: chad
Reviewed By: chad
Subscribers: eadler
Maniphest Tasks: T7789
Differential Revision: https://secure.phabricator.com/D15485
Summary:
Ref T7789. This implements a (probably) usable "git-lfs-authenticate" on top of the new temporary token infrastructure.
This won't actually do anything yet, since nothing reads the tokens.
Test Plan:
```
$ ./bin/ssh-exec --phabricator-ssh-user admin --ssh-command 'git-lfs-authenticate'
phabricator-ssh-exec: Expected `git-lfs-authenticate <path> <operation>`, but received too few arguments.
```
```
$ ./bin/ssh-exec --phabricator-ssh-user admin --ssh-command 'git-lfs-authenticate x'
phabricator-ssh-exec: Unrecognized repository path "x". Expected a path like "/diffusion/X/" or "/diffusion/123/".
```
```
$ ./bin/ssh-exec --phabricator-ssh-user admin --ssh-command 'git-lfs-authenticate diffusion/22'
Exception: Expected `git-lfs-authenticate <path> <operation>`, but received too few arguments.
```
```
$ ./bin/ssh-exec --phabricator-ssh-user admin --ssh-command 'git-lfs-authenticate diffusion/22 y'
Exception: Git LFS operation "y" is not supported by this server.
```
```
$ ./bin/ssh-exec --phabricator-ssh-user admin --ssh-command 'git-lfs-authenticate diffusion/22 upload'
{"header":{"Authorization":"Basic QGdpdC1sZnM6NmR2bDVreWVsaXNuMmtnNXBtbnZwM3VlaWhubmI1bmI="},"href":"http:\/\/local.phacility.com\/diffusion\/22\/new-callsign-free-repository.git\/info\/lfs"}
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7789
Differential Revision: https://secure.phabricator.com/D15482
Summary:
First pass at converting Differential, I likely have some buggy-poos but thought I'd toss this up now in case very bad bugs present.
To do:
- Need to put status back on Hovercards
- "Diff Detail" probably needs a better design
Test Plan: Looking at lots of diffs, admittedly I dont have harbormaster, etc, running locally. Checked Diffusion for Table of Content changes on small and large commits.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15463
Summary:
Two minor changes here:
- Replace `get/setUser()` with `get/setViewer()` for consistency with everything else.
- `getViewer()` now throws if no viewer is set. We had a lot of code that either "should" check this but didn't, or did check it in an identical way, duplicating work. In contrast, very little code checks for a viewer but works if one is not present.
Test Plan:
- Grepped for `->user`.
- Attempted to fix all callsites inside `*View` classes.
- Browsed around a bunch of applications, particularly Calendar, Differential and Diffusion, which seemed most heavily affected.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15412
Summary:
Ref T10449. Currently, we store classes (like "AlmanacClusterRepositoryServiceType") in the database.
Instead, store types (like "cluster.repository").
This is a small change, but types are a little more flexible (they let us freely reanme classes), a little cleaner (fewer magic strings in the codebase), and a little better for API usage (they're more human readable).
Make this minor usability change now, before we unprototype.
Also make services searchable by type.
Also remove old Almanac API endpoints.
Test Plan:
- Ran migration, verified all data migrated properly.
- Created, edited, rebound, and changed properties of services.
- Searched for services by service type.
- Reviewed available Conduit methods.
Reviewers: chad
Reviewed By: chad
Subscribers: yelirekim
Maniphest Tasks: T10449
Differential Revision: https://secure.phabricator.com/D15346
Summary: Fixes T10432. I missed these in making properties non-default.
Test Plan: Diffusion now works again in a cluster configuration.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10432
Differential Revision: https://secure.phabricator.com/D15337
Summary:
These trip me up every time because Differential has:
> Comment, Accept, Request Changes, Resign, Commandeer, Add Reviewers, Add Subscribers
while audits currently show:
> Comment, Add Subscribers, Add Auditors, Accept, Raise Concern, Resign
Now they're more or less in the same order which helps with muscle memory.
Test Plan: Careful inspection.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15323
Summary: Swapping out to PHUIDocumentProView to remove all calls to PHUIDocumentView.
Test Plan: Review the Phabricator Readme.MD in Diffusion
Reviewers: epriestley, avivey
Reviewed By: avivey
Subscribers: avivey, Korvin
Differential Revision: https://secure.phabricator.com/D15308
Summary:
Ref T4245. This could still use a little UI smoothing, but:
- Don't require a callsign on the create flow (you can add one later in "Edit Basic Information" if you want).
- Allow existing callsigns to be removed.
Test Plan:
- Created a new repository with no callsign.
- Cloned it; pushed to it.
- Browsed around Diffusion a bunch.
- Visited a commit URI.
- Added a callsign to it.
- Removed the callsign again.
- Referenced it with `R22` in remarkup.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D15305
Summary:
Ref T4245. This is a prelude to removing them from the "create" screen.
Currently, if you try to delete the callsign you get an unceremonious database error, but the next diff (or maybe two) will permit that, so I didn't put any "this is required yada yada" text in.
This could also maybe use some big flashing warning lights and a "if you edit this, all your URIs break" but I'll save that for later.
Test Plan: Changed the callsign for a repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D15304
Summary: Ref T4245. When creating new repositories, set a default local path based on the repository ID instead of callsign.
Test Plan:
- Created a new repository.
- Saw it get a reasonable, ID-based local path.
- Edited a repository to make sure the `applyFinalEffects()` wasn't doing anything whacky.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D15303
Summary: Ref T4245. Consolidates the URI parsing/rewriting logic so that repositories can be served from either `/diffusion/XYZ/` or `/diffusion/123/`, over both HTTP and SSH.
Test Plan:
- Pulled a Git repository by ID and callsign over HTTP and SSH.
- Pulled a Mercurial repository by ID and callsign over HTTP and SSH.
- Pulled a Subversion repository by ID and callsign over SSH (no HTTP support for SVN).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D15302
Summary:
Ref T4245. Make `/diffusion/123/` work, but redirect the user to `/diffusion/XYZ/` if the repository has a callsign.
(Right now, every repository has a callsign, so this always redirects.)
Also redirect `/R123:abcdef` if the repository has a callsign.
Also also, move the Pull garbage collector somewhere more sensible.
Test Plan:
- Added test coverage.
- Visited `/diffusion/1/`, was redirected.
- Visited `/diffusion/R1:abcdef`, was redirected.
- Browsed Diffusion normally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D15301
Summary: Ref T4245. This has no callers.
Test Plan:
- Ran `git grep -i 'getCallsign('` and visually verified that no callers could reasonably be `DiffusionRequest` objects (there are only 23 remaining sites, and about half are `$this->...` in `PhabricatorRepository`.
- Browsed around directory/file/branch/content/diff/etc pages in Diffusion.
Reviewers: chad, avivey
Reviewed By: avivey
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D15295
Summary:
Ref T4245. We pass this exclusively for use by additional third-party hooks.
This is technically a backward compatibility break, but I suspect it doesn't affect anyone:
- Probably almost no one is using this (there are few reasons to, even for the tiny number of installs with custom commit hooks).
- If they are, there's a good chance the PHID will work anyway, since nearly all scripts and Conduit methods will accept it in place of a callsign now, and if it's in logging or debugging code the PHID is a reasonable substitute
- Even if it doesn't just keep working, the break should be very obvious in most reasonable cases.
I'll call this out explicitly in the changelog, though -- almost everything else will just continue working, but this is a strict compatibility break.
Test Plan:
- Ugh.
- Picked a hosted Git repo out of Diffusion.
- Went to the path on disk.
- Went into `hooks/`.
- Went into `pre-receive-phabricator.d/`.
- Wrote this hook and gave it `chmod +x`:
```name=stuff.sh
#!/bin/sh
echo $PHABRICATOR_REPOSITORY >> /tmp/stuff.log
```
- Pushed to the repository.
- Saw a PHID show up in the log:
```
$ cat /tmp/stuff.log
PHID-REPO-bqkcdp47euwnwlasrsrh
```
Reviewers: chad, avivey
Reviewed By: avivey
Subscribers: avivey
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D15294
Summary: Fixes T9562. We already do this for tags, but didn't have similar logic for branches. Implement that logic.
Test Plan:
- Set limit to 1, saw "More branches", clicked it, got the correct results.
- Verified that branch table with no specified commit still works properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9562
Differential Revision: https://secure.phabricator.com/D15284
Summary:
Fixes T10304. In Mercurial, we must enumerate the whole file tree. Currently, we incorrectly count files within directories (which won't be shown) toward the "100 file" limit at top level, so directories with more than 100 subpaths are truncated improperly.
This is approxiately the same as @richardvanvelzen's fix.
Test Plan: Viewed a large Mercurial repository, saw a complete directory listing.
Reviewers: chad
Reviewed By: chad
Subscribers: richardvanvelzen
Maniphest Tasks: T10304
Differential Revision: https://secure.phabricator.com/D15282
Summary: Moves all the one off object calls to PHUIRemarkupView, adds a "Document" call as well (future plans).
Test Plan: Visited most pages I could get access to, but may want extra careful eyes on this diff.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15281
Summary:
Fixes T10264. I'm reasonably confident that this is the chain of events here:
First, prior to 8269fd6e, we would ignore "Content-Encoding" when reading inbound bodies. So if a request was gzipped, we would read a gzipped body, then give `git-http-backend` a gzipped body with "Content-Encoding: gzip". Everything matched normally, so that was fine, except in the cluster.
In the cluster, we'd accept "gzip + compressed body" and proxy it, but not tell cURL that it was already compressed. cURL would think it was raw data, so it would arrive on the repository host with a compressed body but no "Content-Encoding: gzip". Then we'd hand it to git in the same form. This caused the issue in 8269fd6e: handing it compressed data, but no "this is compressed" header.
To fix this, I made us decompress the encoding when we read the body, so the cluster now proxies raw data instead of proxying gzipped data. This fixed the issue in the cluster, but created a new issue on non-cluster hosts. The new issue is that we accept "gzip + compressed body" and decompress the body, but then pass the //original// header to `git-http-backend`. So now we have the opposite problem from what we originally had: a "gzip" header, but a raw body.
To fix //this//, we could do two things:
- Revert 8269fd6e, then change the proxy request to preserve "Content-Encoding" instead.
- Stop telling `git-http-backend` that we're handing it compressed data when we're handing it raw data.
I did the latter here because it's an easier change to make and test, we'll need to interact with the raw data later anyway, to implement repository virtualization in connection with T8238.
Test Plan: See T10264 for users confirming this fix.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10264
Differential Revision: https://secure.phabricator.com/D15258
Summary: Also adds the commit to the header underneath the title. Ref T7628
Test Plan: Review a few Diffusion pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T7628
Differential Revision: https://secure.phabricator.com/D15246
Summary: These are not needed I think? and handy for cut and paste. Fixes T7628
Test Plan: cut and paste easier from commit hash.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T7628
Differential Revision: https://secure.phabricator.com/D15245
Summary:
Ref T10289. This probably doesn't cover everything but should do a little bit better.
Although we should mabye just exlude milestones from this menu completely?
Test Plan: {F1093937}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10289
Differential Revision: https://secure.phabricator.com/D15191
Summary: No UI changes, just some search and replace for UI consistency.
Test Plan: Test person and object hovercards still work. UIExamples too.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15172
Summary: Fixes T10259. There was no real reason to do this `ip2long()` stuff in the first place -- it's very slightly smaller, but won't work with ipv6 and the savings are miniscule.
Test Plan:
- Ran migration.
- Viewed logs in web UI.
- Pulled and pushed.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10259
Differential Revision: https://secure.phabricator.com/D15165
Summary: I believe this got clobbered in rP8b6edaa4e238a809fe78e6d14ad0705545f8179f. This index doesn't seem to be present in the line dictionary and we're now relying on `$line_index` for the current position.
Test Plan:
before {F1085522}
after {F1085521}
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D15156
Summary: Ref T10228. Commands like `git-http-backend` can emit errors with raw bytes in the output. Sanitize these if present so we can log them in JSON format.
Test Plan: Edited this into production. >_> sneaky sneaky <_<
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10228
Differential Revision: https://secure.phabricator.com/D15144
Summary: Fixes T10226. I just made a mistake here when rewriting this recently.
Test Plan: {F1079166}
Reviewers: chad, avivey
Reviewed By: avivey
Maniphest Tasks: T10226
Differential Revision: https://secure.phabricator.com/D15131
Summary:
Ref T10228. This is currently quite limited:
- No UI.
- No SSH support.
My primary goal is to debug the issue in T10228. In the long run we can expand this to be a bit fancier.
Test Plan:
Made various valid and invalid clones, got sucess responses and not-so-successful responses, viewed the log table for general corresponding messages and broad sanity.
Ran GC via `bin/phd debug trigger`, no issues.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10228
Differential Revision: https://secure.phabricator.com/D15127
Summary: Mostly for consistency, we're not using other forms of icons and this makes all classes that use an icon call it in the same way.
Test Plan: tested uiexamples, lots of other random pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15125
Summary: Fixes T10212. This method was removed in D14990, but I missed a callsite.
Test Plan: Disabling blame now works nicely.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10212
Differential Revision: https://secure.phabricator.com/D15100
Summary:
Fixes T10186. After D14970, `diffusion.filecontentquery` puts the content in a file and returns the file PHID.
However, it does this in a way that doesn't go through the chunking engine, so it will fail for files larger than the chunk threshold (generally, 8MB).
Instead, stream the file from the underlying command directly into chunked storage.
Test Plan:
- Made a commit including a really big file: 4dcd4c492b
- Used `diffusion.filecontentquery` to load file content.
- Parsed/imported commit locally.
- Used `diffusion.filecontentquery` to load content for smaller files (README, etc).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10186
Differential Revision: https://secure.phabricator.com/D15072
Summary:
Ref T4705 (there are also some other adjacent related tasks dealing with URIs).
Currently, we issue a "get repositories matching URIs: ..." query by loading every possible repository and then checking their URIs in PHP.
Instead, put URIs in a separate table. I plan for each repository to potentially have multiple URIs soon, so this prepares for that.
Test Plan:
- Ran migrations.
- Looked at index table, made sure it appeared sensible.
- Ran some queries by `uri` to find repositories, found the repositories I expected.
- Updated the remote URI of a repository, saw queries / index update appropriately.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4705
Differential Revision: https://secure.phabricator.com/D15005
Summary: Fixes T8826. Git tracks an "author date", which may be different from the "committed date". We don't currently extract/show this; do so.
Test Plan: {F1059235}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8826
Differential Revision: https://secure.phabricator.com/D14995
Summary: Ref T4245. Full commit display names (like `rPaaaa`) are going to be obnoxious soon in some cases (e.g., `rPaaaa` becomes `R123:aaaa`, which is much uglier) so reduce how often we show the repository in cases where it isn't really necessary to include it.
Test Plan:
- Saw no more `rX` on repository list view for Git/Mercurial (still present for Subversion).
- Saw no more `rX` on various repository detail views, except when referencing other commits (e.g., mentions).
- Grepped for removed `getShortName()`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14990
Summary:
Ref T4245.
- Rename "Clone/Checkout As" to "Short Name" in the UI.
- Allow any repository to have a short name, not just hosted repositories.
Test Plan:
- Ran migration.
- Reviewed old transactions, saw they looked good.
- Edited an existing repository's short name.
- Gave an imported repository a new short name.
- Removed a repository's short name.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14989
Summary:
Fixes T7938.
- Primarily, users can currently shoot themselves in the foot by putting `../../etc/passwd` and other similar nonsense in these fields (this is not dangerous, but also does not work). Require sensible names.
- Enforce uniqueness so these names can be used in URIs and as identifiers in the future.
- (This doesn't start actually using them for anything fancy yet.)
Test Plan:
- Gave several repositories clone names: a valid name, two duplicate names, an invalid, name, some with no names.
- Ran migrations.
- Got clean conversion for valid names, appropriate errors for invalid/duplicate names.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7938
Differential Revision: https://secure.phabricator.com/D14986
Summary:
Fixes T9323. Two minor fixes:
- On the first commit, don't render a downward line.
- Clean up a 1px spacing issue that had cropped up a while ago when we added icons or something, I think.
Test Plan:
Before:
{F1057248}
After:
{F1057249}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9323
Differential Revision: https://secure.phabricator.com/D14974
Summary:
Fixes T9319. Proxied requests (e.g., in the cluster) for binary files (like images) currently fail because we can not return binary data over Conduit in JSON.
Although Conduit will eventually support binary-safe encodings, a cleaner approach to this is just to return a `filePHID` instead of the raw content. This is generally faster and more flexible, and gives us more opportunities to add caching later.
After making the call, the client pulls the file data separately.
We also no longer need to return a complex data structure because we don't do blame over this call any longer.
Test Plan:
- Viewed images in Diffusion.
- Viewed READMEs in Diffusion.
- Used `bin/differential attach-commit rX Dy` to hit attach pathway.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9319
Differential Revision: https://secure.phabricator.com/D14970
Summary: I was looking at some random un-revisioney repository for most of my testing and missed these.
Test Plan: Viewed blame of a file with some revisions.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14965
Summary: Fixes T2450. If we spend more than 15 seconds in blame, just cut it off.
Test Plan:
- Changed timeout to 0.01 seconds.
- Did blame on a non-highlighted file, got no blame, saw warning.
- Did blame on a highlighted file, got no blame.
- Note: you don't get a warning here because of Ajax stuff. It'd be kind of tricky to add and doesn't seem like a big deal so I'm planning to leave it as-is for now.
Reviewers: chad
Reviewed By: chad
Subscribers: 20after4, chasemp
Maniphest Tasks: T2450
Differential Revision: https://secure.phabricator.com/D14964
Summary:
Ref T2450. This reorganizes code to improve performance.
Mostly, there are a lot of things which are unique per commit (author name, links, short name, etc), but we were rendering them for every line.
This often meant we'd render the same author's name thousands of times. This is slower than rendering it only once.
In 99% of interfaces this doesn't matter, but blame is weird and it's significant on big files.
Test Plan:
Locally, `__phutil_library_map__.php` now has costs of roughly:
- 550ms for main content (from 650ms before the patch).
- 1,500ms for blame content (frrom 1,800ms before the patch).
So this isn't huge, is a decent ~20%-ish performance gain for shuffling some stuff around.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2450
Differential Revision: https://secure.phabricator.com/D14963
Summary:
Ref T2450. File blame tends to have the same commit a lot of times, and we don't do lookups like this efficiently right now.
In particular, for a file like `__phutil_library_map__.php`, we would issue a query with ~9,000 clauses like this:
```
(repositoryID = 1 AND commitIdentifier LIKE "XYZ%")
```
...but only a few hundred of those identifiers were unique. Instead, issue only one clause per unique identifier.
MySQL also seems to do a little better on "commitIdentifier = X" if we have the full hash, so special case that slightly.
Test Plan:
- Issuing a query for only unique identifiers dropped the cost from 400ms to 100ms locally.
- Swapping to `=` if we have the full hash dropped the cost from 100ms to 75ms locally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2450
Differential Revision: https://secure.phabricator.com/D14962
Summary:
Fixes T4366. Two years ago, Facebook put 16,000 files in a directory. Today, the page has nearly loaded.
Paginate large directories.
Test Plan:
- Viewed home and browse views in Git, Mercurial and Subversion.
I put an artificially small page size (5) on home:
{F1055653}
I pushed 16,000 files to a directory and paged through them. Here's the last page, which rendered in about 200ms:
{F1055655}
Our behavior is a bit better than GitHub here, which shows only the first 1,000 files, disables pagination, and can't retrieve history for the files:
{F1055656}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4366
Differential Revision: https://secure.phabricator.com/D14956
Summary:
When looking at a large file in Diffusion:
- disable highlighting if it's huge and show a note about why;
- pick up a few other optimizations.
Test Plan: Locally, this improves the main render of `__phutil_library_map__.php` from 3,200ms to 600ms for me, at the cost of syntax highlighting (we can eventually add view options and let users re-enable it).
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14959
Summary:
Ref T2450. Ref T9319. This is still a bit messy, but not quite so bad as it was: instead of using a single call to get both blame information and file content, use `diffusion.blame` for blame information.
This will make optimizations to both blame and file content easier.
Test Plan: Viewed a bunch of blame (color on/off, blame on/off).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2450, T9319
Differential Revision: https://secure.phabricator.com/D14958
Summary:
Fixes T2451. Several motivations here, from strongest to weakest:
- Currently, getting blame and file content are closely entwined. This makes fixing T9319 more difficult, and I want to fix it. I want to separate blame from content so there's more flexibility in how we approach this issue.
- This makes pursuing T2450 easier, if it turns out to be a meaningful win.
- If we can get a win on blame performance, we can do `arc blame` eventually if we want.
Test Plan:
- Blamed in SVN, Git and Mercurial.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2451
Differential Revision: https://secure.phabricator.com/D14957
Summary: Ref T4245. This is the last of it, and covers the clone/push stuff.
Test Plan:
- Cloned git.
- Pushed git.
- Cloned mercurial.
- Pushed mercurial.
- Visited a `blah.git` URL in my browser just because; got redirected to a human-facing UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14949
Summary: Ref T4245. On their best day these don't work all that well, but I'm pretty sure I didn't make anything worse.
Test Plan:
- Viewed global lint.
- Viewed lint for a repository.
- Viewed lint details for a particular message.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14948
Summary: Ref T4245. This gets everything else except serving HTTP requests (complicated) and lint (quite weird).
Test Plan:
- Viewed a diff.
- Viewed externals.
- Viewed history table to see last modified.
- Did path completion and validation in Owners.
- Did tree path search in Diffusion.
- Viewed a repository.
- Created a new repository.
- Looked up symbols.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14947
Summary: Fixes T7852. Although `1` could also indicate other kinds of problems, assume it means "no results".
Test Plan: Searched for nonsense strings in Git and Mercurial. Searched for valid strings in Git and Mercurial.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7852
Differential Revision: https://secure.phabricator.com/D14943
Summary:
Ref T4245. Browsing is huge and currently split across 5 files using controller delegation.
Although having a huge file isn't great, I think the way it is split up is currently worse, and it gets weird with more flexible repository identifiers.
So this is mostly merging five controllers into one, then a bit of modernization.
I think this can probably be split up better by pulling some of it out into views, instead of using delegation.
Test Plan: Browsed files, directories, and search results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14942
Summary: Ref T4245. Prepares these controllers to accept alternate identifers, plus minor spacing and layout fixes.
Test Plan: Viewed tags, viewed branches.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14941
Summary:
Ref T4245. This adds support for both ID-based and callsign-based routes, although the ID-based routes don't occur anywhere.
Also moves toward simplifying the DiffusionRequest stuff.
Test Plan: Visited normal callsign-based commit pages; visited new ID-based commit pages.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14940
Summary: Ref T4245. This further reduces the reliance on callsigns in Diffusion.
Test Plan:
- Pretty reasonable test coverage already exists.
- Browsed repository list, browse view, history view, content view, change view, commit view, tag view, branch view of repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14937
Summary: Ref T4245. These mostly relate to building URIs.
Test Plan: Tried to hunt down as many of these in the UI as I could. Some are a bit tricky but they should be low-risk.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14933
Summary:
Ref T4245. Like everything else, accept more identifiers.
This needs a change in `arc`, which I've made a note about elsewhere.
Test Plan: Used "Update Now" from web UI, saw update get scheduled.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14932
Summary: Ref T4245. Pass the whole repository in so it can do something else in a future change.
Test Plan: Loaded changesets in Diffusion.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14931
Summary:
Ref T4245. This was obsoleted long ago and has no callers in Phabricator or Arcanist.
Also some minor cleanup.
Test Plan: `grep` for callers everywhere.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14930
Summary: Ref T4245. More of the same, just narrowing down the easy cases.
Test Plan:
- Called `diffusion.querycommit`.
- Browsed branches.
- Browsed repository.
- Browsed directory.
- Searched for stuff.
- Viewed a commit.
- Viewed a file diff.
- Edited a commit.
- Viewed history.
- Viewed tags.
- Viewed push log.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14929
Summary:
Ref T4245. Broaden support to include "ABCD", "rABCD", "1234", "R1234", etc.
This doesn't change the old behavior, just accepts more stuff.
Test Plan:
- Browsed Diffusion.
- Made various calls via API console.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14928
