Summary:
Ref T7931. I'm going to do this separate from existing infrastructure because:
- events start at different times for different users;
- I like the idea of being able to batch stuff (send one email about several upcoming events);
- triggering on ghost/recurring events is a real complicated mess.
This puts a skeleton in place that finds all the events we need to notify about and writes some silly example bodies to stdout, marking that we notified users so they don't get notified again.
Test Plan:
Ran `bin/calendar notify`, got a "great" notification in the command output.
{F1891625}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7931
Differential Revision: https://secure.phabricator.com/D16783
Summary:
Ref T11469. This isn't directly related, but has been on my radar for a while: building SSH keyfiles (particular for installs with a lot of keys, like ours) can be fairly slow.
At least one cluster instance is making multiple clone requests per second. While that should probably be rate limited separately, caching this should mitigate the impact of these requests.
This is pretty straightforward to cache since it's exactly the same every time, and only changes when users modify SSH keys (which is rare).
Test Plan:
- Ran `bin/auth-ssh`, saw authfile generate.
- Ran it again, saw it read from cache.
- Changed an SSH key.
- Ran it again, saw it regenerate.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11469
Differential Revision: https://secure.phabricator.com/D16744
Summary: Ref T11589. Provide a way for scripts to say "just continue if database config fails", and use it in `bin/config` and `bin/storage`.
Test Plan:
- Broke database config.
- Ran `bin/config`, worked fine.
- Ran `bin/storage`, got helpful "set up the database" message.
- Ran `bin/repository`, got fatal.
- Ran normal site with valid/invalid config, got proper feedback.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11589
Differential Revision: https://secure.phabricator.com/D16502
Summary:
This updates the eye logo and removes the formal wordmark "Phabricator" as an image. Instead we'll use the new updated eye logo and plain text for "Phabricator", both of which are more friendly and less industrial.
Installs that already use the `header-logo` customization setting will need to rebuild their logo to 80px x 80px. They will then also get to use plain text to whitebox their install as they see fit.
Test Plan:
Tested new logo at desktop, tablet, and mobile sizes. Set a random instance name, saw new wordmark. Created a really long wordmark of MMMMMMMMMMMM, saw text cut off so UI doesn't break. May need some additional tweaking, but I think we covered the most edge cases here.
{F1751791, size=full}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: edibiase, bjshively, yelirekim, Korvin
Maniphest Tasks: T4214, T11096
Differential Revision: https://secure.phabricator.com/D16373
Summary:
Ref T11208. See that task for a more detailed description of revprops.
This allows revprop changes in a hosted Subversion repository if the repository has the "allow dangerous changes" flag set.
In the future, we could expand this into real Herald support, but the only use case we have for now is letting `svnsync` work.
Test Plan:
Edited revprops with `svn propset --revprop -r 2 propkey propvalue repositoryuri`:
- Tried before patch, got a "configure a commit hook" error.
- Tried after patch, got a "dangerous change" error.
- Allowed dangerous changes.
- Did a revprop edit.
- Prevented dangerous changes.
- Got an error again.
- Made a normal commit to an SVN repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11208
Differential Revision: https://secure.phabricator.com/D16174
Summary:
Fixes T11203. Subversion handling of `SVN_SSH` commands requires some additional finesse for nonstandard remote SSH ports.
We get `domain.com:port` in the command, so parse it out if it's present.
Test Plan: @enckse confirmed this locally in T11203.
Reviewers: chad
Reviewed By: chad
Subscribers: enckse
Maniphest Tasks: T11203
Differential Revision: https://secure.phabricator.com/D16172
Summary:
In D16167 I required users to be logged in to be "activated", but this logic doesn't account for that properly when the user is an Almanac device (a cluster host connecting to another cluster host).
Don't do this check for device connections, they can always establish sessions.
Test Plan: Will push.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16170
Summary:
Ref T10547. This has been around for a while but I was never able to reproduce it. I caught a repro case in the cluster recently and I think this is the right fix.
We tell Subversion to run `ssh-connect` instead of `ssh` so we can provide options and credentials, by using `SVN_SSH` in the environment. Subversion will sometimes kill the SSH tunnel subprocess aggressively with SIGTERM -- as of writing, you can search for `SIGTERM` in `make_tunnel()` here:
http://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_ra_svn/client.c
By default, when a PHP process gets SIGTERM it just exits immediately, without running destructors or shutdown functions. Since destructors/shutdown functions don't run, `TempFile` doesn't get a chance to remove the file.
I don't have a clear picture of //when// Subversion sends SIGTERM to the child process. I can't really get this to trigger locally via `svn`, although I was able to get it to trigger explicitly. So I'm only about 95% sure this fixes it, but it seems likely.
Test Plan:
Locally, I couldn't get this to reproduce "normally" even knowing the cause (maybe Subversion doesn't do the SIGTERM stuff on OSX?) but I was able to get it to reproduce reliabily by adding `posix_kill(getmypid(), SIGTERM);` to the body of the script.
With that added, running the script with `PHABRICATOR_CREDENTIAL=PHID-CDTL-...` in the environment reliably left straggler temporary files.
Adding `declare()` and a signal handler fixed this: the script now runs the `TempFile` destructor and longer leaves the stragglers around.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10547
Differential Revision: https://secure.phabricator.com/D16102
Summary:
Ref T11098. This primarily fixes Conduit calls to `*.edit` methods failing when trying to access user preferences.
(The actual access is a little weird, since it seems like we're building some UI stuff inside a policy query, but that's an issue for another time.)
To fix this, consolidate the "we're about to run some kind of request with this user" code and run it consistently for web, conduit, and SSH sessions.
Additionally, make sure we swap things to the user's translation.
Test Plan:
- Ran `maniphest.edit` via `arc call-conduit`, no more settings exception.
- Set translation to ALL CAPS, got all caps output from `ssh` and Conduit.
Reviewers: avivey, chad
Reviewed By: chad
Maniphest Tasks: T11098
Differential Revision: https://secure.phabricator.com/D16066
Summary: Ref T10811. This is a companion change for D16053, but affects the Phabricator version of this script.
Test Plan: Started daemons, ^C'd them, saw them handle the signal.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10811
Differential Revision: https://secure.phabricator.com/D16054
Summary:
Ref T10917. Currently, when you delete an SSH key, we really truly delete it forever.
This isn't very consistent with other applications, but we built this stuff a long time ago before we were as rigorous about retaining data and making it auditable.
In partiular, destroying data isn't good for auditing after security issues, since it means we can't show you logs of any changes an attacker might have made to your keys.
To prepare to improve this, stop destoying data. This will allow later changes to become transaction-oriented and show normal transaction logs.
The tricky part here is that we have a `UNIQUE KEY` on the public key part of the key.
Instead, I changed this to `UNIQUE (key, isActive)`, where `isActive` is a nullable boolean column. This works because MySQL does not enforce "unique" if part of the key is `NULL`.
So you can't have two rows with `("A", 1)`, but you can have as many rows as you want with `("A", null)`. This lets us keep the "each key may only be active for one user/object" rule without requiring us to delete any data.
Test Plan:
- Ran schema changes.
- Viewed public keys.
- Tried to add a duplicate key, got rejected (already associated with another object).
- Deleted SSH key.
- Verified that the key was no longer actually deleted from the database, just marked inactive (in future changes, I'll update the UI to be more clear about this).
- Uploaded a new copy of the same public key, worked fine (no duplicate key rejection).
- Tried to upload yet another copy, got rejected.
- Generated a new keypair.
- Tried to upload a duplicate to an Almanac device, got rejected.
- Generated a new pair for a device.
- Trusted a device key.
- Untrusted a device key.
- "Deleted" a device key.
- Tried to trust a deleted device key, got "inactive" message.
- Ran `bin/ssh-auth`, got good output with unique keys.
- Ran `cat ~/.ssh/id_rsa.pub | ./bin/ssh-auth-key`, got good output with one key.
- Used `auth.querypublickeys` Conduit method to query keys, got good active keys.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10917
Differential Revision: https://secure.phabricator.com/D15943
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:
Fixes T10758.
- Adds a "--host" flag. If you specify this, we read your cluster config. This lets you dump from a replica.
- Adds a "--for-replica" flag to `storage dump`. This makes `mysqldump` include a `CHANGE MASTER ...` statement in the output, which is useful when setting up a replica for the first time.
Test Plan:
- Dumped master and replica cluster databases.
- Dumped non-cluster databases.
- Ran various other commands (help, status, etc).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10758
Differential Revision: https://secure.phabricator.com/D15714
Summary:
Ref T4571. Ref T10759. Ref T10758. This isn't complete, but gets most of the job done:
- When `cluster.databases` is set up, most things ignore `mysql.host` now.
- You can `bin/storage upgrade` and stuff works.
- You can browse around in the web UI and stuff works.
There's still a lot of weird tricky stuff to navigate, and this has real no advantages over configuring a single server yet (no automatic failover, etc).
Test Plan:
- Configured `cluster.databases` to point at my `t1.micro` hosts in EC2 (master + replica).
- Ran `bin/storage upgrade`, got a new install setup on them properly.
- Survived setup warnings, browsed around.
- Switched back to local config, ran `bin/storage upgrade`, browsed around, went through setup checks.
- Intentionally broke config (bad hosts, no masters) and things seemed to react reasonably well.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4571, T10758, T10759
Differential Revision: https://secure.phabricator.com/D15668
Summary:
Ref T10537. More infrastructure:
- Put a `bin/nuance` in place with `bin/nuance import`. This has no useful behavior yet.
- Allow sources to be searched by substring. This supports `bin/nuance import --source whatever` so you don't have to dig up PHIDs.
Test Plan:
- Applied migrations.
- Ran `bin/nuance import --source ...` (no meaningful effect, but works fine).
- Searched for sources by substring in the UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15436
Summary: Ref T4245. Accept identifiers instead of callsigns in these scripts so things continue to work in a future callsign-optional world.
Test Plan: Ran these scripts with both valid and invalid arguments; saw success and errors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D15300
Summary:
Ref T4245. Two effects:
- First, let hooks work for future repositories without callsigns.
- Second, provide a better error when users push directly to hosted repositories.
Test Plan: Ran `bin/commit-hook PHID-REPO-xxx`.
Reviewers: chad, avivey
Reviewed By: avivey
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D15293
Summary: Fixes T10317. If we failed to load `$device`, it will be null, so we won't be able to call `getName()` on it.
Test Plan: `SSH_CLIENT='127.0.0.1' ./bin/ssh-exec --phabricator-ssh-device xyz` no longer fatals
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10317
Differential Revision: https://secure.phabricator.com/D15235
Summary: These are old project image choices, remove and only go with FontAwesome related images.
Test Plan: Project -> Edit Picture -> Save
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15051
Summary: Removes header gradient images for flat, CSS controlled colors. I didn't convert the "pony" colors over, going with few options for easier theme-ability.
Test Plan:
Test each color choice.
{F1063828}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15052
Summary:
Ref T9319. When we discover a commit, we sometimes update the corresponding revision with a "this is the actual committed change" diff and send out a link to the changes between review and commit.
This is currently very difficult to test, because it only happens the first time and you have to either go set up a bunch of objects or add a bunch of special casing to the parser to hit the workflow.
I'm making some changes to how it pulls file content. To make those changes easier to test, first start extracting this stuff so the code can be run with `bin/differential extract ...` instead of needing to do a bunch of more complicated setup steps.
Test Plan:
- Ran `bin/differential extract ...` to extract diffs from commits.
- Forced my way through the daemon workflow by faking out a bunch of flags, got a clean extract + attach + update. After this patch, this should rarely be necessary.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9319
Differential Revision: https://secure.phabricator.com/D14967
Summary: Ref T9156. This makes the UX a little more modern/standard/safe.
Test Plan:
```
epriestley@orbital ~/dev/phabricator $ ./bin/lipsum generate
Choose which type or types of test data you want to generate, or select "all".
- Differential Revisions
- Files
- Maniphest Tasks
- Pastes
- Pholio Mocks
- Projects
- User Accounts
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9156
Differential Revision: https://secure.phabricator.com/D14873
Summary: Fixes T9715. Adds a MySQL-based lock to ensure that schema migrations are not applied on multiple hosts simultaneously.
Test Plan: Ran `./bin/storage upgrade` concurrently. One invocation was successful whilst the other hit a `PhutilLockException`.
Reviewers: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9715
Differential Revision: https://secure.phabricator.com/D14463
Summary:
Fixes T9494. This:
- Removes all the random GC.x.y.z config.
- Puts it all in one place that's locked and which you use `bin/garbage set-policy ...` to adjust.
- Makes every TTL-based GC configurable.
- Simplifies the code in the actual GCs.
Test Plan:
- Ran `bin/garbage collect` to collect some garbage, until it stopped collecting.
- Ran `bin/garbage set-policy ...` to shorten policy. Saw change in web UI. Ran `bin/garbage collect` again and saw it collect more garbage.
- Set policy to indefinite and saw it not collect garabge.
- Set policy to default and saw it reflected in web UI / `collect`.
- Ran `bin/phd debug trigger` and saw all GCs fire with reasonable looking queries.
- Read new docs.
{F857928}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9494
Differential Revision: https://secure.phabricator.com/D14219
Summary:
This enables CORGI.
Currently, `AphrontSite` subclasses can't really have their own routes. They can do this sort of hacky rewriting of paths, but that's a mess and not desirable in the long run.
Instead, let subclasses build their own routing maps. This will let CORP and ORG have their own routing maps.
I was able to get rid of the `PhameBlogResourcesSite` since it can really just share the standard resources site.
Test Plan:
- With no base URI set, and a base URI set, loaded main page and resources (from main site).
- With file domain set, loaded resources from main site and file site.
- Loaded a skinned blog from a domain.
- Loaded a skinned blog from the main site.
- Viewed "Request" tab of DarkConsole to see site/controller info.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14008
Summary: Use `PhutilClassMaQuery` instead of `PhutilSymbolLoader`, mostly for consistency. Depends on D13588.
Test Plan: Poked around a bunch of pages.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13589
Summary: We've decomissioned this host.
Test Plan: `git grep www.phabricator.com`
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D13585
Summary: Ref T8099, hashtag#yolo. Adds back the original gradients plus a 'light' theme. Unclear which should be default, but we can play with it until a decision needs to be made.
Test Plan: Change colors a lot, turn on durable column.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13146
Summary:
Ref T8387. Adds new mailing list users.
This doesn't migrate anything yet. I also need to update the "Email Addresses" panel to let administrators change the list address.
Test Plan:
- Created and edited a mailing list user.
- Viewed profile.
- Viewed People list.
- Searched for lists / nonlists.
- Grepped for all uses of `getIsDisabled()` / `getIsSystemAgent()` and added relevant corresponding behaviors.
- Hit the web/api/ssh session blocks.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, tycho.tatitscheff, epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13123
Summary: Ref T8099. Adds back basic header color options, which change the logo color instead. Also RAINBOW.
Test Plan:
tested each of the new colors.
{F442284}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13040
Summary: Ref T8099, this removes the gradient headers and their edge cases in Pinboard View, Setup Issues, Exceptions, and Document Heiriarchy.
Test Plan:
Tested each of these, grep for "gradient" and verify all states are gone.
{F410708}
{F410709}
{F410710}
{F410714}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D12944
Summary: Ref T7604. Change `DiffusionLintSaveRunner` to use repositories instead of Arcanist Projects.
Test Plan: Ran the `save_lint.php` script and queried results using the `diffusion.getlintmessages` Conduit endpoint.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T7604
Differential Revision: https://secure.phabricator.com/D12893
Summary: Ref T7977. Fix a few issues that I forgot to fix up.
Test Plan: Run the script.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7977
Differential Revision: https://secure.phabricator.com/D12668
Summary: Fixes T7220. Ref T7977. Changes symbols from being bound to an Arcanist project to being bound to a repository.
Test Plan:
- Added symbols and then applied migrations, symbols seemed to be migrated successfully.
- Tested the `/diffusion/symbol/$SYMBOL_NAME` endpoint.
- Tested the `/diffusion/symbol/$SYMBOL_NAME` endpoint with the `?repositories=$REPOSITORY_PHID` parameter.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: avivey, Korvin, epriestley
Maniphest Tasks: T7977, T7220
Differential Revision: https://secure.phabricator.com/D12608
Summary:
Fixes T7712. Currently, files sent via email get default policies, like they were dragged and dropped onto the home page.
User expectation is better aligned with giving files more restrictive policies, like they were draggged and dropped directly onto an object.
Make files sent via email have restricted default visibility. Once we identify the sender, set them as the file author. Later, the file will become visible to other users via attachment to a task, revision, etc.
Test Plan: Sent some files via email; verified they got restrictive policies, correct authorship, and appropriate object attachment.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7712
Differential Revision: https://secure.phabricator.com/D12255
Summary:
Ref T7352. This changes `phd` to pass configuration to overseers over stdin. We still run one overseer per daemon.
The "status" stuff needs some cleanup, but it's mostly just UI/cosmetic.
Test Plan:
- Ran `phd debug`, `phd launch`, `phd start`, `phd status`, `phd stop`, etc.
- Verified PID files write in a reasonable format.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7352
Differential Revision: https://secure.phabricator.com/D11855
Summary:
Ref T6941. In the cluster (and in other reasonable setups) we've separated SSH load balancers from HTTP load balancers.
In particular, ELBs will not let you load balance port 22, so this is likely a reasonable/common issue in larger clusters in AWS.
Allow users to specify an alternate host for SSH traffic.
Test Plan: Set host to someting different, saw it reflected in UI.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6941
Differential Revision: https://secure.phabricator.com/D11800
Summary:
Ref T7119. Three users have now reported that this causes their systems to hang, so it's looking like the cure is worse than the disease.
Revert this until we can eventually reproduce and understand the hang.
Test Plan: Users running into issues have reported that this resolves things.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7119
Differential Revision: https://secure.phabricator.com/D11644
Summary: Use `PhutilXHPASTBinary` methods instead of `xhpast_parse` functions. Depends on D11517.
Test Plan: N/A, this is a direct swap.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11612
Summary:
Ref T6881. This adds the worker, and a script to make it easier to test. It doesn't actually invoice anything.
I'm intentionally allowing the script to double-bill since it makes testing way easier (by letting you bill the same period over and over again), and provides a tool for recovery if billing screws up.
(This diff isn't very interesting, just trying to avoid a 5K-line diff at the end.)
Test Plan: Used `bin/phortune invoice ...` to get the worker to print out some date ranges which it would theoretically invoice.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6881
Differential Revision: https://secure.phabricator.com/D11577
Summary: Fixes T7034. Like HTTP, proxy requests to the correct host if a repository has an Almanac service host.
Test Plan: Ran VCS requests through the proxy.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7034
Differential Revision: https://secure.phabricator.com/D11543
Summary:
Ref T7034.
In a cluster environment, when a user connects with a VCS request over SSH (like `git pull`), the receiving server may need to proxy it to a server which can actually satisfy the request.
In order to proxy the request, we need to know which repository the user is interested in accessing.
Split the SSH workflow into two steps:
# First, identify the repository.
# Then, execute the operation.
In the future, this will allow us to put a possible "proxy the whole thing somewhere else" step in the middle, mirroring the behavior of Conduit.
This is trivially easy in `git` and `hg`. Both identify the repository on the commmand line.
This is fiendishly complex in `svn`, for the same reasons that hosting SVN was hard in the first place. Specifically:
- The client doesn't tell us what it's after.
- To get it to tell us, we have to send it a server capabilities string //first//.
- We can't just start an `svnserve` process and read the repository out after a little while, because we may need to proxy the request once we figure out the repository.
- We can't consume the client protocol frame that tells us what the client wants, because when we start the real server request it won't know what the client is after if it never receives that frame.
- On the other hand, we must consume the second copy of the server protocol frame that would be sent to the client, or they'll get two "HELLO" messages and not know what to do.
The approach here is straightforward, but the implementation is not trivial. Roughly:
- Start `svnserve`, read the "hello" frame from it.
- Kill `svnserve`.
- Send the "hello" to the client.
- Wait for the client to send us "I want repository X".
- Save the message it sent us in the "peekBuffer".
- Return "this is a request for repository X", so we can proxy it.
Then, to continue the request:
- Start the real `svnserve`.
- Read the "hello" frame from it and throw it away.
- Write the data in the "peekBuffer" to it, as though we'd just received it from the client.
- State of the world is normal again, so we can continue.
Also fixed some other issues:
- SVN could choke if `repository.default-local-path` contained extra slashes.
- PHP might emit some complaints when executing the commit hook; silence those.
Test Plan: Pushed and pulled repositories in SVN, Mercurial and Git.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7034
Differential Revision: https://secure.phabricator.com/D11541
Summary: Ref T7034. This is a second special case, like commit hooks, where we need some help from Phabricator to make instance identity knowable.
Test Plan: Connected to an instance and ran SSH commands.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7034
Differential Revision: https://secure.phabricator.com/D11539
