Summary:
See PHI24. If you create a hosted Mercurial repository and switch it to observed, you can end up with a hook installed that runs on pulls and complains.
Instead, just bail out if we're running on a pull.
The corresponding Git hook doesn't run on pulls, so there's no issue in Git.
Test Plan: Executed the hook in an observed Mercurial repository, got a clean exit.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18307
Summary: Ref T10319. This adds a basic means of generating default profile images for users. You can generate them for everyone, a group of users, or force updates. This only generated images and stores them in files. It does not assign them to users.
Test Plan:
`bin/people profileimage --all` to generate all images.
`bin/people profileimage --users chad` to generate a user.
`bin/people profileimage --all --force` to force rebuilding all images.
{F3662810}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10319
Differential Revision: https://secure.phabricator.com/D17464
Summary: Ref T12139. Adds sorting by shortname. Also I sorted everything else. No reason. It didn't help
Test Plan: `:star`
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12139
Differential Revision: https://secure.phabricator.com/D17246
Summary:
This adds a more complete emoji datasource, with a typeahead and autocomplete. It works by pulling in a raw datasource from EmojiOne (I chose Unicode 8, but they have a Unicode 9 datasource as well) and transforming it for speed/need. If we build more robustness or an actual picker into the Remarkup bar, having the additional keywords, etc, might be important. When Unicode 9 support is more prevalent, we should only need to update the single file.
Tossing up as a proof of concept on engineering direction. Also I can't quite get the autocomplete to complete.
Test Plan: Test UIExamples, Autocomplete, and TypeaheadSource
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12139
Differential Revision: https://secure.phabricator.com/D17244
Summary: Ref T8475. This gets rid of most of the old "legacy hunk" code. I'll nuke the rest (and drop the old table) once we're more sure that we're in the clear.
Test Plan: Browsed Differential.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8475
Differential Revision: https://secure.phabricator.com/D17040
Summary:
Via HackerOne. A researcher correctly reports that our install scripts use `HTTP`, not `HTTPS`, to fetch resources and execute them as `root`, which is a potentially significant vulnerability.
Instead, use `HTTPS`.
Test Plan: Verified that these URIs function correctly over `HTTPS`.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16958
Summary:
Ref T11893. Previously, we excuted all `bin/storage` queries through `StorageManagementAPI` objects.
After D16848, we execute some queries through `PhabricatorDatabaseRef`. However, the refs we use weren't getting passed the `-u` / `-p` flags correctly, for specifying alternate administrative credentials.
Test Plan:
- Created a second account (`trunk`).
- Ran `bin/storage -u trunk`.
- Made libphutil throw when not connecting as that user.
- Before patch: some connections incorrectly used the default user, "root".
- After patch: all connections correctly used the configured user, "trunk".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11893
Differential Revision: https://secure.phabricator.com/D16901
Summary:
Ref T11818. See that task for a description.
This is like a tiny pinch of hackiness but not really unreasonable. Basically, `aphlict` is already an "uber-server" and one copy can handle a ton of instances.
Test Plan: Started `bin/aphlict` without a valid database connection.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11818
Differential Revision: https://secure.phabricator.com/D16854
Summary: Depends on D16847. Ref T11044. This updates the remaining storage-related workflows from the CLI to accommodate multiple masters.
Test Plan:
- Configured multiple masters.
- Ran all `bin/storage` workflows.
- Ran `arc unit --everything`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11044
Differential Revision: https://secure.phabricator.com/D16848
Summary:
Ref T11044. This moves toward partitioned application databases:
- You can define multiple masters.
- Convert all the easily-convertible code to become multi-master aware.
This doesn't convert most of `bin/storage` or "Config > Database (Stuff)" yet, as both are quite involved. They still work for now, but only operate on the first master instead of all masters.
Test Plan: Configured multiple masters, browsed around, ran `bin/storage` commands, ran `bin/storage --host ...`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11044
Differential Revision: https://secure.phabricator.com/D16115
Summary:
Ref T11809. This came out of Facebook many years ago for computing the number of business days that revisions had been stale.
We removed the little staleness marker a few months ago and haven't seen complaints about it.
If we did holidays now it would make sense to integrate them more directly with Calendar as real events, but I have no plans to pursue this anytime soon. It's easy enough to add the federal holidays manually (~5 minutes of work per year?) if you want them, and they're commentable/editable and you can add local holidays if you're not in the US.
Test Plan:
- Ran `bin/storage upgrade -f`.
- Grepped for `CalendarHoliday`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11809
Differential Revision: https://secure.phabricator.com/D16788
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
Summary:
Fixes T7019. In a cluster environment, pushes currently fail because the commit hook can't identify the instance.
For web processes, the hostname identifies the instance -- but we don't have a hostname in the hook.
For CLI processes, the environment identifies the instance -- but we don't have an environment in the hook under SVN.
Promote the instance identifier into the upstream and pack/unpack it explicitly for hooks. This is probably not useful for anyone but us, but the amount of special-purpose code we're introducing is very small.
I poked at trying to do this in a more general way, but:
- We MUST know this BEFORE we run code, so the normal subclassing stuff is useless.
- I couldn't come up with any other parameter which might ever be useful to pass in.
Test Plan: Used `git push` to push code through proxied HTTP, got a clean push.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7019
Differential Revision: https://secure.phabricator.com/D11495
Summary: Unused at this point
Test Plan: Grep
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11506
Summary: Removes the 1x application icons, and uses the fonticons instead. Feed was only known location.
Test Plan:
feed, dashboards, grep for use
{F275636}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11496
Summary:
Ref T6881. This makes it easier to fire a trigger and make sure it works properly. You can use the `--now` flag to travel through time, and test scheduling conditions with `--last` and `--next`. It will tell you when the trigger would reschedule.
Better than waiting 24 hours to see if things work.
Test Plan: Fired some backups, got useful output which made me think my code probably works correctly.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6881
Differential Revision: https://secure.phabricator.com/D11438
Summary:
Fixes T5966. Accomplishes a few things
- see title
- adds a force-autoclose flag and the plumbing for it
- removes references to some HarborMaster thing that used to key off commits and seems long dead, but forgotten :/
Test Plan:
ran a few commands. These first three had great success:
`./repository reparse --all FIRSTREPO --message --change --herald --owners`
`./repository reparse --all FIRSTREPO --message --change --herald --owners --min-date yesterday`
`./repository reparse --all FIRSTREPO --message --change --herald --owners --min-date yesterday --force-autoclose`
...and these next two showed me some errors as expected:
`./repository reparse --all FIRSTREPO --message --change --herald --owners --min-date garbagedata`
`./repository reparse --all GARBAGEREPO --message --change --herald --owners`
Also, made a diff in a repository with autoclose disabled and commited the diff. Later, reparse the diff with force-autoclose. Verified the diff closed and that the reason "why" had the proper message text.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: joshuaspence, epriestley, Korvin
Maniphest Tasks: T5966
Differential Revision: https://secure.phabricator.com/D10492
Summary: Removes the docs sprite in Conpherence with FontAwesome, adds additional icons. Unsure what happens if someone customized this config option.
Test Plan: Added images and files to a Conpherence, saw new icons.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11028
Summary: Removes unused payments sprite and code, also some unused conpherence generated images. We use images in login (and could use FontAwesome, maybe).
Test Plan: grep codebase, pull up uiexamples icons page
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11025
Summary: These were refactored out a while ago
Test Plan: Grep codebase, use Conpherence on desktop, mobile.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11023
Summary: Ref T6615. Ref T3554. We need better tooling around the queue eventually, so start here.
Test Plan: Added 100K+ tasks locally with `bin/worker flood`. Executed some of them with `bin/phd debug taskmaster` (we already have a TestWorker, used in unit tests).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T3554, T6615
Differential Revision: https://secure.phabricator.com/D10894
Summary:
Ref T5833. This fixes a few weird things with this table:
- A bunch of columns were nullable for no reason.
- We stored an MD5 hash of the key (unusual) but never used it and callers were responsible for manually populating it.
- We didn't perform known-key-text lookups by using an index.
Test Plan:
- Ran migrations.
- Faked duplicate keys, saw them clean up correctly.
- Added new keys.
- Generated new keys.
- Used `bin/auth-ssh` and `bin/auth-ssh-key`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10805
Summary:
Ref T5833. Currently, SSH keys are associated only with users, and are a bit un-modern. I want to let Almanac Devices have SSH keys so devices in a cluster can identify to one another.
For example, with hosted installs, initialization will go something like this:
- A request comes in for `company.phacility.com`.
- A SiteSource (from D10787) makes a Conduit call to Almanac on the master install to check if `company` is a valid install and pull config if it is.
- This call can be signed with an SSH key which identifies a trusted Almanac Device.
In the cluster case, a web host can make an authenticated call to a repository host with similar key signing.
To move toward this, put a proper Query class on top of SSH key access (this diff). In following diffs, I'll:
- Rename `userPHID` to `objectPHID`.
- Move this to the `auth` database.
- Provide UI for device/key association.
An alternative approach would be to build some kind of special token layer in Conduit, but I think that would be a lot harder to manage in the hosting case. This gives us a more direct attack on trusting requests from machines and recognizing machines as first (well, sort of second-class) actors without needing things like fake user accounts.
Test Plan:
- Added and removed SSH keys.
- Added and removed SSH keys from a bot account.
- Tried to edit an unonwned SSH key (denied).
- Ran `bin/ssh-auth`, got sensible output.
- Ran `bin/ssh-auth-key`, got sensible output.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10790
Summary:
Ref T1191. Notable stuff:
- Adds `--disable-utf8mb4` to `bin/storage` to make it easier to test what things will (approximately) do on old MySQL. This isn't 100% perfect but should catch all the major stuff. It basically makes us pretend the server is an old server.
- Require utf8mb4 to dump a quickstart.
- Fix some issues with quickstart generation, notably special casing the FULLTEXT handling.
- Add an `--unsafe` flag to `bin/storage adjust` to let it truncate data to fix schemata.
- Fix some old patches which don't work if the default table charset is utf8mb4.
Test Plan:
- Dumped a quickstart.
- Loaded the quickstart with utf8mb4.
- Loaded the quickstart with `--disable-utf8mb4` (verified that we get binary columns, etc).
- Adjusted schema with `--disable-utf8mb4` (got a long adjustment with binary columns, some truncation stuff with weird edge case test data).
- Adjusted schema with `--disable-utf8mb4 --unsafe` (got truncations and clean adjust).
- Adjusted schema back without `--disable-utf8mb4` (got a long adjustment with utf8mb4 columns, some invalid data on truncated utf8).
- Adjusted schema without `--disable-utf8mb4`, but with `--unsafe` (got truncations on the invalid data).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D10757
Summary:
Ref T4209. This creates storage for public keys against authorized hosts, such that servers can be authorized to make Conduit calls as the omnipotent user.
Servers are registered into this system by running the following command once:
```
bin/almanac register
```
NOTE: This doesn't implement authorization between servers, just the storage of public keys.
Placing this against Almanac seemed like the most sensible place, since I'm imagining in future that the `register` command will accept more information (like the hostname of the server so it can be found in the service directory).
Test Plan: Ran `bin/almanac register` and saw the host (and public key information) appear in the database.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4209
Differential Revision: https://secure.phabricator.com/D10400
Summary: The output from this script is too verbose... all that I care about is the overall progress.
Test Plan:
Ran the script on a large repository.
```lang=bash
./scripts/repository/reparse.php --all XYZ --message
NOTE: This script will queue tasks to reparse the data. Once the tasks have been queued, you need to run Taskmaster daemons to execute them.
QUEUEING TASKS (92,969 Commits):
[ ] 0.1%
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10441
Summary: Fixes T6006. These didn't get caught by D10392.
Test Plan: Forced migration to re-run; ran SSH commands against Phabricator.
Auditors: btrahan
Summary:
Ref T5885. See D10276.
Currently, ActionHeaders can only have minicons, and we don't use them anywhere and they probably don't make much sense in the product anymore.
Instead, allow them to have font icons. Remove minicons, which have no callsites and probably won't in the future.
Test Plan:
{F190925}
- Grepped for `minicons`.
- Grepped for `setHeaderIcon()`.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5885
Differential Revision: https://secure.phabricator.com/D10277
Summary:
Fixes T5770. This error occurs if you run `bin/storage upgrade` before you set up MySQL credentials.
This isn't what the setup guide says to do, but it's an easy mistake to make and should be a permitted install path since there's no reason you can't do things in this order.
Specifically, we use a mixture of "standard" (configured) and "administrative" (`--user` and `--password`) credentials, and if the standard ones are bogus bad things happen. We use the standard credentials to make some initialization order stuff easier, and because there's no `--host` flag and adding one would be silly, and because we only need administrative credentials to issue ALTER / CREATE statements.
Test Plan: Ran with bad standard credentials; ran with bad administrative credentials. Ran with good credentials.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5770
Differential Revision: https://secure.phabricator.com/D10199
Summary:
CanCDN flag indicates that a file can be served + cached
via anonymous content distribution networks.
Once D10054 lands, any files that lack the CanCDN flag
will require a one-time-use token and headers will
prohibit cache to protect sensitive files from
unauthorized access.
This diff separates the CanCDN changes from the code that
enforces these restrictions in D10054 so that the changes
can be tested and refined independently.
Test Plan: Work in progress
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: rush898, qgil, epriestley, aklapper, Korvin
Maniphest Tasks: T5685
Differential Revision: https://secure.phabricator.com/D10166
Summary: Fixes T5336. Currently, `PhabricatorWorkerLeaseQuery` is basically FIFO. It makes more sense for the queue to be a priority-queue, and to assign higher priorities to alerts (email and SMS).
Test Plan: Created dummy tasks in the queue (with different priorities). Verified that the priority field was set correctly in the DB and that the priority was shown on the `/daemon/` page. Started a `PhabricatorTaskmasterDaemon` and verified that the higher priority tasks were executed before lower priority tasks.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5336
Differential Revision: https://secure.phabricator.com/D9871
Summary: Most scripts detect the relevant workflows automatically. Some scripts, however, use a hardcoded list of workflows.
Test Plan: Ran `./bin/aphlict`, `./bin/cache` and `./bin/phd`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9564
Summary: Applied some more linter fixes that I previously missed because my global `arc` install was out-of-date.
Test Plan: Will run `arc unit` on another host.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9443
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.
Test Plan: Eyeballed it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9431
Summary:
The docs are now a little out of date.
Also //possibly// we should call this `bin/notifications` or something, maybe?
Test Plan: read
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9398
Summary:
Replaces the icons with fonts from FontAwesome. Up in the air about the meme icon. Thoughts?
Also removed the second fullscreen/normal state. Seems obvious what it does, but assume someone complained previously?
Test Plan:
Tested all the icon states and made sure they still worked. Test fullscreen and help.
{F163485}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9385
Summary:
Ref T4045. Ref T5179. While we'll eventually need to force a migration, we can let installs (particularly large installs) do an online migration for now. This moves hunks to the new storage format one at a time.
(Note that nothing writes to the new store yet, so this is the only way to populate it.)
WARNING: Installs, don't run this yet! It won't compress the data. Wait until it can also do compression.
Test Plan: Added a `break;` after migrating one row and moved a few rows over. Spot checked them in the database and viewed the affected diffs.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4045, T5179
Differential Revision: https://secure.phabricator.com/D9291
Summary: These both work, but "git://" uses a nonstandard and possibly firewalled port, is less familiar to users, and is not promoted in the GitHub UI.
Test Plan: `grep`, reviewed diff.
Reviewers: chad, avive, avivey
Reviewed By: avivey
Subscribers: epriestley, avivey
Differential Revision: https://secure.phabricator.com/D9360
Summary: Point everything at the new canonical URI.
Test Plan: `grep`
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9328
Summary:
Updates policy, headers, typeaheads to FA over policy icons
Need advice - can't seem to place where icons come from on Typeahead? Wrong icons and wrong colors.... it is late
Test Plan:
- grepped for SPRITE_STATUS
- grepped for sprite-status
- grepped for setStatus for headers
- grepped individual icons names
Browsed numerous places, checked new dropdowns, see pudgy people.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4739
Differential Revision: https://secure.phabricator.com/D9179
Summary: The removes the sprite sheet 'icons' and replaces it with FontAwesome fonts.
Test Plan:
- Grep for SPRITE_ICONS and replace
- Grep for sprite-icons and replace
- Grep for PhabricatorActionList and choose all new icons
- Grep for Crumbs and fix icons
- Test/Replace PHUIList Icon support
- Test/Replace ObjectList Icon support (foot, epoch, etc)
- Browse as many pages as I could get to
- Remove sprite-icons and move remarkup to own sheet
- Review this diff in Differential
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9052
Summary:
Provides a working SMS implementation with support for Twilio.
This version doesn't really retry if we get any gruff at all. Future versions should retry.
Test Plan: used bin/sms to send messages and look at them.
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: aurelijus, epriestley, Korvin
Maniphest Tasks: T920
Differential Revision: https://secure.phabricator.com/D8930
Summary: These have all been obsolete for a reasonable amount of time, or are no longer relevant.
Test Plan: shrug~
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8941
