1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-30 18:52:42 +01:00
Commit graph

11418 commits

Author SHA1 Message Date
epriestley
44057ad269 Consider packages when calculating Differential authority
Summary:
Ref T10939. This has no effect yet since packages can not actually become reviewers, I'm just inching toward support.

  - When searching for "responsible users", include revisions that need review by packages you have authority over.
  - When calculating review authority, include authority over packages you are a member of (these currently never exist).

Test Plan:
This isn't reachable so I just `var_dump()`'d stuff and looked at the generated queries, which appeared correct/reasonable.

I'll vet this more thoroughly once packages can actually become reviewers.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10939

Differential Revision: https://secure.phabricator.com/D15909
2016-05-13 17:17:50 -07:00
epriestley
1793e2f945 Use ye new Englishe in ye docs
Summary: Ah, a fine thing it be.

Test Plan: Pip pip cheerio.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15919
2016-05-13 13:55:57 -07:00
epriestley
dc2d87059b Fix an issue with URI index updates from the daemons
Summary:
Ref T10923. This extension needs to load a little more data (with `needURIs`) to function correctly now.

(There's a recent migration does this, so indexes got updated correctly when it ran, so it hasn't been obvious that they weren't getting updated properly after that.)

Test Plan: Made an arbitrary edit to a repository, observed no more error in daemon logs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10923

Differential Revision: https://secure.phabricator.com/D15908
2016-05-13 06:51:31 -07:00
epriestley
e5f2ccc57f Don't trigger audits for archived packages
Summary:
Ref T10939. This is just a bug. I thought this was what was described in T10174 but that's actually talking about something completely different.

Also make a `<select />` slightly easier to use.

Test Plan:
  - Created a package with auditing enabled.
  - Pushed a change.
  - Saw audit trigger.
  - Disabled the package, pushed a change.
    - Before patch: saw audit trigger improperly.
    - After patch: restarted daemons, then saw audit correctly not trigger.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10939

Differential Revision: https://secure.phabricator.com/D15907
2016-05-13 06:49:42 -07:00
epriestley
1c73ad6a1b Make repository daemon locks more granular and forgiving
Summary:
Ref T4292. Currently, we hold one big lock around the whole `bin/repository update` workflow.

When running multiple daemons on different hosts, this lock can end up being contentious. In particular, we'll hold it during `git fetch` on every host globally, even though it's only useful to hold it locally per-device (that is, it's fine/good/expected if `repo001` and `repo002` happen to be fetching from a repository they are observing at the same time).

Instead, split it into two locks:

  - One lock is scoped to the current device, and held during pull (usually `git fetch`). This just keeps multiple daemons accidentally running on the same host from making a mess when trying to initialize or update a working copy.
  - One lock is scoped globally, and held during discovery. This makes sure daemons on different hosts don't step on each other when updating the database.

If we fail to acquire either lock, assume some other process is legitimately doing the work and bail more quietly instead of fataling. In approximately 100% of cases where users have hit this lock contention, that was the case: some other daemon was running somewhere doing the work and the error didn't actually represent an issue.

If there's an actual problem, we still raise a diagnostically useful message if you run `bin/repository update` manually, so there are still tools to figure out that something is hung or whatever.

Test Plan:
  - Ran `bin/repository update`, `pull`, `discover`.
  - Added `sleep(5)`, forced processes to contend, got lock exceptions and graceful exit with diagnostic message.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15903
2016-05-13 05:17:27 -07:00
epriestley
8cdafb0032 Allow users to set a line-height in their monospaced font preference
Summary: Ref T10959. This does not fix the problem because the `.differential-diff td` rule is still stronger, but it does let you choose a more compact or breezy style for remarkup blocks and pastes.

Test Plan:
  - Set font to `24px / 48px impact`.
  - Viewed a paste, saw lovely readable text.
  - Viewed an inline code block which was very easy on the eyes.

{F1310420}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10959

Differential Revision: https://secure.phabricator.com/D15904
2016-05-13 05:10:27 -07:00
epriestley
984dff0ae3 Provide a more consistent, mostly relaxed severity for updating non-cluster repositories on cluster devices
Summary:
Fixes T10940. Two issues currently:

First, `PullLocal` deamon refuses to update non-cluster repositories on cluster devices. However, this is surprising/confusing/bad because as soon as you enroll a repository host in the cluster, most of the repositories on it stop working until you `clusterize` them. This is especially confusing because the documentation gives you a very nice, gradual walkthrough about going through things slowly and being able to check your work at every step, but we really drop you off a bit of a cliff here. The workflow implied by the documentation is a desirable one.

This operation is generally only unsafe/problematic if the daemon would be creating a //new// working copy. If a working copy already exists, we can reasonably guess that it's almost certainly because you've enrolled a previously un-clustered host into a new cluster. This allows the nice, gradual workflow the documentation describes to proceed as expected, without any weird surprises.

Instead of refusing to update these repositories, only refuse to update them if updating would create a new working copy. This should make transitioning much smoother without any meaningful reduction in safety.

Second, the lower-level `bin/repository update`, `refs`, `mirror`, etc., commands don't apply this same check. However, these commands are potentially just as dangerous. Use the same code to do a similar check there, making sure we only operate on repositories that are either expected to be on the current device, or which already exist here.

Test Plan:
  - Ran `bin/phd debug pull`, saw diagnostic information choose to update most repositories (including some non-cluster repositories) but properly skip non-cluster repositories that do not exist locally.
  - Ran `bin/repository update`, etc., saw the command apply consistent rules to the rules applied by `PullLocal` and refuse to update non-local repositories it would need to create.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10940

Differential Revision: https://secure.phabricator.com/D15902
2016-05-12 15:51:14 -07:00
epriestley
bd9bcaa8ff Improve HTML mail rendering of inline patches
Summary: Fixes T9790. This uses a simple renderer, like the inline context renderer, that emphasizes getting a quick glance at small changes and working reasonably on mobile devices.

Test Plan:
  - Set `inline` setting to `9999`.
  - Created a diff.
  - Saw it render reasonably in HTML mail.
  - Also tested text mail to make sure I didn't break that.

{F1310137, size=full}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9790

Differential Revision: https://secure.phabricator.com/D15901
2016-05-12 12:13:40 -07:00
epriestley
9d196648f5 Prevent users from disabling repository builtin URIs
Summary:
Ref T10923. Currently, users can disable or enable builtin URIs, but this doesn't actually do anything.

The behavior of "disable" has changed a bit over time and might need some further refinement, but it's currently meaningless for builtin URIs. Prevent adjustment of it. If users want to hide a URI, they should set "Display: Hidden" instead.

Test Plan:
  - Disabled/enabled a non-builtin URI.
  - Tried to disable a builtin URI, saw greyed out UI and got a helpful error message.

Reviewers: chad

Reviewed By: chad

Subscribers: eadler

Maniphest Tasks: T10923

Differential Revision: https://secure.phabricator.com/D15899
2016-05-12 12:09:23 -07:00
epriestley
5162f81095 Provide SSH host documentation, tweak/supplement cluster documentation
Summary: Ref T10751. I think this mostly brings us up to date with the state of the world.

Test Plan: Read documentation.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10751

Differential Revision: https://secure.phabricator.com/D15897
2016-05-12 12:09:04 -07:00
epriestley
0faf3dd18d Fix "Contributing" misspelling in documentation
Summary: Fixes T10958.

Test Plan: Dictionary'd.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10958

Differential Revision: https://secure.phabricator.com/D15898
2016-05-12 08:29:53 -07:00
epriestley
5003f21919 Put "Projects" edit field back on Basics management panel for repositories
Summary: Ref T10923. Fixes T10955. This was accidentally excluded when I broke the form into pages.

Test Plan: Saw edit field in panel; changed project tags for a repository.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10923, T10955

Differential Revision: https://secure.phabricator.com/D15896
2016-05-12 07:17:14 -07:00
epriestley
15f14d6c2f Fix improper viewer for Git SSH cluster workflows
Summary: Ref T10751. These workflows have separate `getUser()` and `getViewer()` for weird legacy reasons. `getUser()` is correct.

Test Plan:
  - Did a Git SSH push, verified that "Last Writer" reflected the proper user in the "Storage" UI in repository management.
  - Grepped for other callsites, double-checked that they used correct users.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10751

Differential Revision: https://secure.phabricator.com/D15893
2016-05-11 18:02:02 -07:00
epriestley
54409e7716 Fix an issue with TextAreaEditField affecting Paste
Summary: Fixes T10952. Fixes T10930. I didn't implement this method correctly when I expanded this field for repositories.

Test Plan: Edited a paste without warnings.

Reviewers: avivey, chad

Reviewed By: chad

Maniphest Tasks: T10930, T10952

Differential Revision: https://secure.phabricator.com/D15892
2016-05-11 15:35:17 -07:00
epriestley
b21b43131c Clean up display of clone URIs a little bit
Summary:
Ref T10923. This makes the "Clone URI" UI a little nicer:

  - Show whether each URI is read-only, read-write, or external.
  - Clicking the button selects the URI.
  - Add a link to manage the appropriate credentials.

Test Plan: {F1308302, size=full}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10923

Differential Revision: https://secure.phabricator.com/D15891
2016-05-11 13:14:55 -07:00
epriestley
fc654f8996 Flesh out a few more repository documentation sections
Summary: Ref T10923. This links up some more related documentation about repository management.

Test Plan: Read documentation.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10923

Differential Revision: https://secure.phabricator.com/D15890
2016-05-11 13:01:02 -07:00
Aviv Eyal
dc6d108b26 Paramater type inheritence fix
Summary: These parameters wrongly extend List.

Test Plan:
Used createdStart field for a search - didn't get error about "should be a list".
`git grep 'extends ConduitListParameterType'`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15881
2016-05-11 18:21:14 +00:00
epriestley
ee74fb4cc7 Add a "View Repository" button to the repository manage UI
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
2016-05-11 09:21:14 -07:00
epriestley
6615d76c34 In Subversion, show "svn checkout <uri> <directory>" in Diffusion
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
2016-05-11 09:20:54 -07:00
epriestley
5587d97a7f Tailor Diffusion protocol rules slightly
Summary: Fixes T10948. Ref T10923. Make these rules a little more thorough and document their behavior.

Test Plan: Looked at Diffusion clone URIs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10923, T10948

Differential Revision: https://secure.phabricator.com/D15887
2016-05-11 07:18:09 -07:00
epriestley
de4312bcde Before executing svnserve, change the CWD to a readable directory
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
2016-05-11 06:48:18 -07:00
epriestley
cd86bf0174 Remove metamta.differential.unified-comment-context and explain it in ExtraConfigSetupCheck
Summary: Ref T10694. This setting no longer has any effect: we always show a limited amount of context now.

Test Plan: `grep`

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10694

Differential Revision: https://secure.phabricator.com/D15886
2016-05-11 06:47:58 -07:00
epriestley
97c103fa00 Restore edit UI for "Import Only" in Subversion
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
2016-05-11 06:47:32 -07:00
epriestley
3fdb1a2bc4 Improve behavior for not-yet-created non-cluster repositories
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
2016-05-11 06:38:53 -07:00
epriestley
71a97d8af5 When observing a repository, switch to "importing" mode on a large discovery in an empty repository
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
2016-05-11 06:36:38 -07:00
epriestley
576b73dc53 Index all repository URIs, not just the "primary" repository URI
Summary:
Ref T10923. When regenerating the URI index for a repository, index every URI.

  - Also, make the index slightly stricter (domain + path instead of just path). Excluding the domain made more sense when we were generating only first-party URIs.
  - Make the index smarter about `/diffusion/123/` URIs.
  - Show normalized URIs in `diffusion.repository.search` results.

Test Plan:
  - Ran migration.
  - Verified sensible-looking results in database.
  - Searched for a repository URI by first-party clone URI.
  - Searched for a repository URI by mirror URI.
  - Used `diffusion.repository.search` to get information about repository URIs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10923

Differential Revision: https://secure.phabricator.com/D15876
2016-05-11 06:36:06 -07:00
epriestley
f05fce44aa Provide more UI guidance when creating repositories
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
2016-05-11 06:35:35 -07:00
epriestley
0b5ab2330d Hide irrelevant panels in Mercurial/Subversion, fix Subversion URIs
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
2016-05-10 05:16:08 -07:00
epriestley
e2bbde9675 Bring old repository instructions and guidance forward to new UI
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
2016-05-10 05:15:43 -07:00
epriestley
98b202042e Provide some more context hints for repository URIs
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
2016-05-10 05:14:29 -07:00
epriestley
3328e78a7b Sort out EditController / ManageController / EditproController Diffusion hierarchy
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
2016-05-10 05:14:09 -07:00
epriestley
f191f66f34 Document API management of repositories and fix some issues with creating URIs via API
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
2016-05-10 05:10:35 -07:00
epriestley
34e85aaeb8 Document most of the new Diffusion management panel
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
2016-05-10 05:10:07 -07:00
epriestley
8512f9358e Update redirect/cancel URIs for repository dialogs
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
2016-05-10 05:09:36 -07:00
epriestley
846eec7563 Put "Push Policy" last in Diffusion, make editing Spaces work
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
2016-05-09 06:34:02 -07:00
epriestley
612a93229f Fix some pagination/redirect issues for repositories
Summary:
Ref T10923. Paging wasn't being applied correctly when creating //new// repositories after API changes.

Also, the first redirect after creation wasn't sending users to the right place.

Test Plan:
  - Created a new repostiory, got redirected properly.
  - Verified that new repostiory flow has the correct fields (name, callsign, etc) and Conduit API has the correct fields (everything).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10923

Differential Revision: https://secure.phabricator.com/D15865
2016-05-06 12:40:20 -07:00
epriestley
412fc34557 Improve inline mail snippet rendering, possibly fixing Airmail?
Summary:
Ref T10694. General improvements:

  - Remove leading empty lines from context snippets.
  - Remove trailing empty lines from context snippets.
  - If we removed everything, render a note.
  - Try using `style` instead of `<pre>`? My thinking is that maybe Airmail has weird default rules for `<pre>`, since that's the biggest / most obvious thing that's different about this element to me.

Test Plan: Viewed normal comments locally, faked a comment on an empty line in the middle of a lot of other empty lines.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10694

Differential Revision: https://secure.phabricator.com/D15864
2016-05-06 11:58:33 -07:00
epriestley
371051ff37 Minor tweaks to pre/inline style for inline comments in HTML mail
Summary:
Ref T10694.

  - Shift margins/padding around so inlines with multiple paragraphs get reasonable spacing.
  - Add `text-decoration: none` to the "View Inline" link to kill the underline.

Test Plan: {F1265342}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10694

Differential Revision: https://secure.phabricator.com/D15863
2016-05-06 11:05:23 -07:00
epriestley
053d6111e4 Refine inline style rendering in email
Summary: Ref T10694. Move the inline style more toward a mix of standard`<pre>` style and the web UI style for inlines.

Test Plan: See screenshots in comments.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10694

Differential Revision: https://secure.phabricator.com/D15857
2016-05-06 10:34:24 -07:00
epriestley
cc579be6f1 Use more precise language to describe what mirroring is
Summary: Fixes T10928. This behavior hasn't changed at all and this is what, e.g., `git push --mirror` means, and the other behavior isn't possible, but be more explicit about the inherent destructive potential of mirroring.

Test Plan: Read documentation.

Reviewers: chad, sascha-egerer

Reviewed By: sascha-egerer

Subscribers: sascha-egerer

Maniphest Tasks: T10928

Differential Revision: https://secure.phabricator.com/D15861
2016-05-06 08:04:18 -07:00
epriestley
fde02c4b4e Fix protocol serve detection for clustered repositories that terminate HTTPS
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
2016-05-05 16:25:14 -07:00
epriestley
1baef494c1 Pick context windows for inlines in a slightly smarter way
Summary:
Ref T10694. This mostly prevents us from having a degenerate case if someone leaves a 200-line inline.

  - For one-line inlines, show 1 line of context above and below (3 lines total).
  - For 3+ line inlines, show just the inline.
  - For 7+ line inlines, show only the first part.

Test Plan: Made a bunch of weird long/short/different-sized comments, saw reasonble-appearing context in text and HTML mail output.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10694

Differential Revision: https://secure.phabricator.com/D15853
2016-05-05 11:15:09 -07:00
epriestley
94c7bb605c Highlight inline diff context in HTML mail
Summary:
Ref T10694. Ref T9790. When generating inline diff context, highlight it and then mangle the highlighted output into `style="..."` so it works in HTML.

Also try to tighten up some spacing/formatting stuff.

Test Plan:
Got some output in this vein:

{F1259937}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9790, T10694

Differential Revision: https://secure.phabricator.com/D15852
2016-05-05 11:13:27 -07:00
epriestley
2025ecd3d8 Rough cut of inline comment context
Summary:
Ref T10694. This is still missing some pieces, but seems to get most of the data into the mail in a plausible format:

  - When an inline remarks on code, show the patch inline in the mail body.
  - When an inline replies to another inline, show that other inline in the mail body.
  - Apply remarkup rendering to inline content.
  - Apply basic styling to mail body blocks.

Not covered yet:

  - Syntax highlighting.
  - Diff highlighting.
  - Maybe clearer style/layout hints to connect comments to what they reply to? Current approach might get messy with inlines that have blockquotes and code blocks inside them, for example.
  - I probably want to cap the amount of diff context we ever show to ~7 lines, even if you drag over 200 lines of code.
  - CSS is a generally a bit rough still.
  - The `unified-comment-context` option is effectively always on now, and should be removed.
  - Text section is getting indented right now but probably shouldn't be.
  - Spacing, etc., might be a bit off.

Test Plan:
Rigged Home to render these things, got a plausible-looking render (top is text, bottom is HTML):

{F1259052}

Sent myself some inline comment mail, got a plausible result.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10694

Differential Revision: https://secure.phabricator.com/D15850
2016-05-05 09:23:08 -07:00
epriestley
19aac8e8d3 Pass the new default syntax highlighting map to the remarkup engine
Summary: Ref T9790. This passes the map down so we can generate highlighted mail.

Test Plan:
Generated this relatively respectable-looking HTML mail:

{F1258558}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9790

Differential Revision: https://secure.phabricator.com/D15848
2016-05-05 02:51:19 -07:00
epriestley
01289f3f48 Generate syntax highlighting CSS from a reusable map
Summary:
Ref T9790. This prepares the syntax color rules to be reused in mail.

This goes about halfway toward T5701 by sort-of supporting different styles but not really.

Test Plan:
  - Ran `bin/celerity syntax` to regenerate syntax map.
  - Viewed some highlighted code, didn't see any differences.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9790

Differential Revision: https://secure.phabricator.com/D15846
2016-05-05 02:50:48 -07:00
Chad Little
e422190eb5 Fix create links when no repositories exist
Summary: Fixes T10925. Sends users to /new/ instead of /create/

Test Plan: Visit page, see links, clicky clicky.

Reviewers: epriestley, thoughtpolice

Reviewed By: thoughtpolice

Subscribers: thoughtpolice, Korvin

Maniphest Tasks: T10925

Differential Revision: https://secure.phabricator.com/D15849
2016-05-04 21:13:36 -07:00
epriestley
95c284b95e Fix a Harbormaster build issue with new URI code
Summary: Ref T10748. We're returning a `PhabricatorRepositoryURI` here but the code expects an actual `PhutilURI`.

Test Plan:
This should clear this up in production:

```
Daemon 180278 STDE [Wed, 04 May 2016 23:25:16 +0000] [2016-05-04 23:25:16] EXCEPTION: (PhutilProxyException) Error while executing Task ID 1677075. {>} (RuntimeException) Object of class PhabricatorRepositoryURI could not be converted to string at [<phutil>/src/error/PhutilErrorHandler.php:205]
Daemon 180278 STDE [Wed, 04 May 2016 23:25:16 +0000] arcanist(head=master, ref.master=c58f1b9a2507), libcore(), phabricator(head=master, ref.master=29d1115037b8), phutil(head=master, ref.master=0709cd5cfc26), services(head=master, ref.master=04ae8c8f8e3b)
Daemon 180278 STDE [Wed, 04 May 2016 23:25:16 +0000]   #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:1200]
Daemon 180278 STDE [Wed, 04 May 2016 23:25:16 +0000]   #1 <#2> PhabricatorRepository::getPublicCloneURI() called at [<phabricator>/src/applications/repository/storage/PhabricatorRepositoryCommit.php:395]
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10748

Differential Revision: https://secure.phabricator.com/D15844
2016-05-04 16:46:22 -07:00
epriestley
29d1115037 Swap Repository Edit UI to new code
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
2016-05-04 16:19:57 -07:00
epriestley
42eaa88f80 Cut mirroring over to new URIs
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
2016-05-04 16:16:16 -07:00
epriestley
34e870819c Remove "bin/repository edit" workflow
Summary:
Ref T10748. In D14250#158181, I accepted this conditional on removing it once Conduit could handle it.

Conduit can now handle it, or at least will be able to as soon as T10748 cuts over.

Test Plan: Grepped for `repository edit`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10748

Differential Revision: https://secure.phabricator.com/D15839
2016-05-04 16:10:49 -07:00
epriestley
b8b700c179 Remove "repository.create" Conduit API method
Summary:
Ref T10748. This has had many problems for a long time, can't create hosted repositories, can't create cluster repositories, etc. It is obsoleted by `diffusion.repository.edit`. Remove it.

(Right now `diffusion.repository.edit` isn't a strict replacement, but it will be as soon as the URI stuff cuts over.)

Test Plan: Grepped for `repository.create`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10748

Differential Revision: https://secure.phabricator.com/D15838
2016-05-04 16:10:19 -07:00
epriestley
dd2b10b8f8 Guarantee repositories have unique local paths
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
2016-05-04 16:09:52 -07:00
Chad Little
d85386488b Add "wide" remarkup image support for Documents
Summary: Seems to work ok, if you give `size=wide` to an image file, we blow it out a bit in DocumentPro mode.

Test Plan:
Test in Phame and Maniphest.

{F1256717}

{F1256718}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15840
2016-05-03 17:27:30 -07:00
epriestley
c3afddec9c Add icons to the new repository edit nav
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
2016-05-03 08:01:18 -07:00
epriestley
319a9cefde When creating a repository with EditEngine, allocate it onto a random cluster service
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
2016-05-03 08:00:47 -07:00
epriestley
cac26c8824 Fix errant rules for associating projects when dragging tasks within a milestone column
Summary:
Fixes T10912. When you drag tasks within a milestone, we currently apply an overbroad, API-focused rule and add the parent board's project. This logic was added fairly recently, as part of T6027, to improve the behavior of API-originated moves.

Later on, this causes the task to toggle in and out of the parent project on every alternate drag.

This logic is also partially duplicated in the `MoveController`.

  - Add test coverage for this interaction.
  - Fix the logic so it accounts for subproject / milestone columns correctly.
  - Put all of the logic into the TransactionEditor, so the API gets the exact same rules.

Test Plan:
  - Added a failing test and made it pass.
  - Dragged tasks around within a milestone column:
    - Before patch: they got bogus project swaps on every other move.
    - After patch: projects didn't change (correct).
  - Dragged tasks around between normal and milestone columns.
    - Before patch: worked properly.
    - After patch: still works properly.

Here's what the bad changes look like, the task is swapping projects with every other move:

{F1255957}

The "every other" is because the logic was trying to do this:

  - Add both the parent and milestone project.
  - Whichever one exists already gets dropped from the change list because it would have no effect.
  - The other one then applies.
  - In applying, it forces removal of the first one.
  - Then this process repeats in the other direction the next time through.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10912

Differential Revision: https://secure.phabricator.com/D15834
2016-05-03 07:59:05 -07:00
epriestley
c0d42a8943 Split Repository EditEngine form into smaller pages
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
2016-05-02 08:28:38 -07:00
epriestley
99718b61d8 Fill in new URI credential edit web UI interfaces
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
2016-05-02 04:26:13 -07:00
Chad Little
562d427f09 New icons, colors for per reviewer status
Summary: Fixes T10906, Fixes T10820. Adds new icons, grey-er colors for previous states. Also, I think fixed a few bugs?

Test Plan: Fake each state, verify icon is as intended.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10820, T10906

Differential Revision: https://secure.phabricator.com/D15830
2016-05-01 12:52:40 -07:00
epriestley
c2227d3ed2 Fix "acccess" spelling error
Summary: This is misspelled.

Test Plan: Consulted a dictionary.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15827
2016-04-30 09:07:51 -07:00
epriestley
0ba3939ce3 Flesh out more web UI actions for new URI interface
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
2016-04-29 17:16:15 -07:00
epriestley
e8dca5a4f9 Alternate fix for Firefox triple click selection
Summary:
Fixes T10905. This reverts D15823, which didn't work well for tasks with very long titles (the title would break as a block element).

This is slightly more magic but works with long titles.

Test Plan: Did everything from D15823, but also with long titles. Triple-click, wrapping, and mobile/device worked in Safari, Firefox and Chrome.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10905

Differential Revision: https://secure.phabricator.com/D15824
2016-04-29 16:59:43 -07:00
epriestley
06abeb2010 Allow tall two-column/curtain headers to be triple-clicked in Firefox
Summary:
Fixes T10905. In Firefox, triple clicking the new headers doesn't select the entire line, so you can't easily copy/paste an entire task title or revision name. It works fine in Safari/Chrome.

This seems to fix that without breaking anything.

Test Plan:
  - Viewed headers in Safari, Firefox, Chrome.
  - Triple-clicked headers in Safari, Firefox, Chrome.
  - Viewed tablet/device layouts in Safari, Firefox, Chrome.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10905

Differential Revision: https://secure.phabricator.com/D15823
2016-04-29 16:43:27 -07:00
epriestley
c314a3672f Allow callers to query information about repository URIs from diffusion.repository.search
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
2016-04-29 16:12:41 -07:00
epriestley
da599386f6 Add diffusion.uri.edit for creating and editing repository URIs
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
2016-04-29 13:55:48 -07:00
epriestley
128995f1ac Document all the hypothetical URI features we plan to support soon
Summary: Ref T10748. Ref T10366. This documents how everything is planned to work shortly.

Test Plan: Read documentation.

Reviewers: chad

Reviewed By: chad

Subscribers: eadler, scode

Maniphest Tasks: T10366, T10748

Differential Revision: https://secure.phabricator.com/D15817
2016-04-29 09:24:10 -07:00
epriestley
c8711da5ff Add repository URI view pages and IO/Display edit logic
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
2016-04-29 09:22:16 -07:00
epriestley
616c9ae887 Rough sketch of new repository URI editing
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
2016-04-29 09:21:00 -07:00
epriestley
0459e95242 Give users a modal VCS choice when creating a new repository
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
2016-04-29 09:20:31 -07:00
epriestley
311de580d6 Port "Actions" to new Repository UI
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
2016-04-27 17:35:36 -07:00
epriestley
4c66a92f92 Port Repository "Branches" to new UI
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
2016-04-27 17:35:19 -07:00
epriestley
8f81930b5d Port Repository "Symbols" to Manage/Panel UI
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
2016-04-27 17:35:03 -07:00
epriestley
fbc4967154 Improve cache behaviors for font files and other nonstandard resource types
Summary:
Ref T10843. There are actually two separate notions of cacheability here:

  - Is this cacheable by the browser (e.g., should we emit "Expires: long in the future")?
  - Is this cacheable locally (e.g., should we stick it in APC, or just read it off disk every time)?

These got a little mixed up by D15775, so we aren't currently emitting proper "Expires" headers on font files and a few other resource types.

Straighten this out so that we "Expires" these unusual resources correctly.

Test Plan: Verified that `.woff` files get a proper "Expires" header now, not just CSS/JS.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10843

Differential Revision: https://secure.phabricator.com/D15807
2016-04-27 07:48:40 -07:00
epriestley
63bbe6b129 Port "Allow Dangerous Changes" to new Manage UI
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
2016-04-27 03:58:10 -07:00
epriestley
57a76d8a70 Port "Automation" panel to new Repository Manage UI
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
2016-04-27 03:57:07 -07:00
epriestley
467c4e84e5 Add an edge table to the search database
Summary:
Fixes T10778. This is a result of T10262: when we save a form configuration and adjust the policy, we try to scramble attached file secrets.

There aren't going to be any attached files, but there's also no edge table, so we fail.

We could skip this code, but we'll likely need an edge table here sooner or later so it's probably simpler in the long run to just add an empty one.

Test Plan:
   - Ran `bin/storage upgrade`, got a clean bill of health.
   - Saved a form configuration after making a policy edit, no more `edge` exception.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10778

Differential Revision: https://secure.phabricator.com/D15803
2016-04-26 11:26:26 -07:00
epriestley
dc3a13c5e8 Add bin/repository clusterize and document setup and migration for clusters
Summary: Ref T4292. This provides at least some sort of hint about how to set up cluster repositories.

Test Plan:
  - Read documentation.
  - Ran `bin/repository clusterize` to add + remove clusters.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15798
2016-04-26 10:07:17 -07:00
epriestley
8d9bc401e4 Improve Diffusion hosting setup instructions somewhat?
Summary:
Ref T10866. Fixes T10386. This attempts to make it a little more plausible to follow these directions:

  - Use simpler language in general.
  - Remove language suggesting that HTTP requires no additional configuration.
  - Suggest using a load balancer or an ugly port number instead of swapping SSH to a different port.
  - Be more granular about `sudo` setup.
  - Organize better?

Test Plan: Read documentation.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10386, T10866

Differential Revision: https://secure.phabricator.com/D15796
2016-04-26 10:06:59 -07:00
epriestley
3fda965288 When multiple web hosts are in service, don't require setup warnings to be dismissed on each one
Summary:
Fixes T10876. Currently, we can end up with a setup warning banner sticking on each web device, since the state is stored in local cache.

Instead:

  - When we actually run the setup checks, save the current state in the database.
  - Before we show a cached banner, make sure the database still says the checks are a problem.

This could lead to some inconsistencies if setup checks legitimately pass on some hosts but not on others. For example, if you have `git` installed on one machine but not on another, we may raise a setup warning ("No Git Binary!") about it on one host only.

For now, assume users have their operational environments in some sort of reasonable shape and can install the same stuff everywhere. In the future, we could split the issues into "global" and "per-host" issues if we run into problems with this.

Test Plan:
This is somewhat tricky to test locally since you really need multiple webservers to test it properly, but I:

  - Created some setup issues, saw banner.
  - Ignored/cleared them, saw banner go away.
  - Verified database cache writes were occurring properly.

Then I sort of faked it like this:

  - Created a setup issue.
  - Manually set the database cache value to `[]` ("no issues").
  - Reloaded page.
  - No more banner.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10876

Differential Revision: https://secure.phabricator.com/D15802
2016-04-26 10:03:45 -07:00
epriestley
8606fb588f Port "Staging Area" repository section to new management UI
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
2016-04-26 08:11:53 -07:00
epriestley
8e4a7742eb Port local storage path to new repository Manage UI
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
2016-04-26 07:59:22 -07:00
epriestley
0630fef9fc Prevent web queries from running for more than 30 seconds
Summary:
Ref T10849. This enforces a global 30-second per-query time limit for anything not coming from the CLI.

If we run into another issue with MySQL hanging in the future, this should prevent it from being nearly as bad as it was.

Test Plan:
  - Set value to 0, verified the UI threw an exception immediately.
  - Set value back to 30, browsed around a bunch of pages.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10849

Differential Revision: https://secure.phabricator.com/D15799
2016-04-26 07:59:09 -07:00
epriestley
2c870bad86 Document how to register cluster devices with Almanac
Summary:
Ref T4292. This is a required step in configuring a cluster: document and explain it.

Previously `bin/almanac register` could //also// add and trust keys. I've removed this capability since I think it's needless and complicated. If there's some real use for it eventually, we could add a `bin/almanac add-key` or whatever. The workflow is simpler and has better guard rails that point you in the correct direction now.

Test Plan:
  - Read documentation.
  - Ran `bin/almanac` with various good/bad flags.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15795
2016-04-25 14:58:58 -07:00
epriestley
550a82d438 Fix two minor formatting issues with bin/repository move-paths
Summary: This gets over-escaped instead of bolded right now, but I only ever hit it when exporting/importing and never both cleaning it up.

Test Plan: Ran `bin/repository move-paths`, saw bolded "Move" instead of ANSI escape sequences.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15797
2016-04-25 12:29:15 -07:00
epriestley
892a9a1f07 Make cluster repositories more resistant to freezing
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
2016-04-25 11:37:31 -07:00
epriestley
d0b5dac36b Make cluster repositories more chatty
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
2016-04-25 11:20:57 -07:00
epriestley
dc75b4bd06 Move all cluster locking logic to a separate class
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
2016-04-25 11:20:29 -07:00
epriestley
1c0980a26a Fix two issues with Remarkup in Pholio
Summary:
Fixes T10865.

  - Mock descriptions did not markup.
  - Image descriptions did not get a proper container `<div />`.

Test Plan:
  - Created a mock with remarkup in the mock description and in an image description.
  - Viewed mock detail.
  - Saw list styles render properly in both mock description and image description.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10865

Differential Revision: https://secure.phabricator.com/D15793
2016-04-25 08:16:23 -07:00
epriestley
623ed1f434 Include directory-ownership note in sshd setup instructions
Summary: Fixes T9560. We suggest a root-owned location, but users who choose their own location instead can run into trouble.

Test Plan:
  - Changed parent directory to have an non-root owner, verified that `ssh` no longer worked.
  - Changed parent directory back to a root owner, verified `ssh` worked again.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9560

Differential Revision: https://secure.phabricator.com/D15794
2016-04-25 08:16:14 -07:00
epriestley
aa9395e38f Fix bad variable causing aphlict to fail to start with no "logs" config
Summary: Fixes T10863. See that task for discussion.

Test Plan:
  - Configured `aphlict` with no "logs".
  - Started `aphlict`.
    - Before change: exception.
    - After change: worked.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10863

Differential Revision: https://secure.phabricator.com/D15788
2016-04-24 11:20:42 -07:00
epriestley
9d0891c7e1 Correct Aphlict documentation for Nginx proxying
Summary: Fixes T10857. This documentation did not accurately reflect proper configuration: in the Aphlict config, SSL is inferred from the presence of `ssl.*` configuration.

Test Plan: Read documentation.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10857

Differential Revision: https://secure.phabricator.com/D15787
2016-04-24 06:31:58 -07:00
epriestley
00885edc47 Don't try to synchronize repositories with no working copy
Summary:
Ref T4292. Sometimes, we may not have a working copy for a repository. The easiest way to get into this condition is to deactivate a repository.

We could try to clone + fetch in this case, but that's kind of complex, and there's an easy command that administrators can run manually. For now, just tell them to do that.

This affects the inactive repositories on `secure`, like rGITCOINS.

Test Plan: Removed working copy, got message.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15786
2016-04-22 08:12:19 -07:00
epriestley
ab20f243b3 Improve consistency of file access policies, particularly for LFS
Summary:
Ref T7789. Currently, we use different viewers if you have `security.alternate-file-domain` configured vs if you do not.

This is largely residual from the days of one-time-tokens, and can cause messy configuration-dependent bugs like the one in T7789#172057.

Instead, always use the omnipotent viewer. Knowledge of the secret key alone is sufficient to access a file.

Test Plan:
  - Disabled `security.alternate-file-domain`.
  - Reproduced an issue similar to the one described on T7789.
  - Applied change.
  - Clean LFS interaction.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7789

Differential Revision: https://secure.phabricator.com/D15784
2016-04-22 08:12:08 -07:00
epriestley
711f13660e Synchronize working copies before doing a "bypassCache" commit read
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
2016-04-22 08:11:43 -07:00
epriestley
0f0105e783 Send the aphlict process log to the node log
Summary: I've possibly seen a couple of `aphlict` processes exit under suspicious circumstances (maybe?). Make sure any PHP errors get captured into the log.

Test Plan:
  - Added an exception after forking.
  - Before change: vanished into thin air.
  - After change: visible in the log.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15782
2016-04-21 17:50:47 -07:00
epriestley
43935d5916 Don't cache resources we can't generate properly
Summary:
Fixes T10843. In a multi-server setup, we can do this:

  - Two servers, A and B.
  - You push an update.
  - A gets pushed first.
  - After A has been pushed, but before B has been pushed, a user loads a page from A.
  - It generates resource URIs like `/stuff/new/package.css`.
  - Those requests hit B.
  - B doesn't have the new resources yet.
  - It responds with old resources.
  - Your CDN caches things. You now have a poisoned CDN: old data is saved in a new URL.

To try to avoid this with as little work as possible and generally make it hard to get wrong, check the URL hash against the hash we would generate.

If they don't match, serve our best guess at the resource, but don't cache it. This should make things mostly keep working during the push, but prevent caches from becoming poisoned, and everyone should get a working version of everything after the push finishes.

Test Plan:
  - `curl`'d a resource, got a cacheable one.
  - Changed the hash a little, `curl`'d again. This time: valid resource, but not cacheable.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10843

Differential Revision: https://secure.phabricator.com/D15775
2016-04-21 11:56:54 -07:00
epriestley
9656fe48bc Add a "Repository Servers" cluster administration panel
Summary: Ref T4292. This adds a new high-level overview panel.

Test Plan: {F1238854}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15772
2016-04-21 11:56:44 -07:00
lkassianik
bd8969a23c Calendar event list items 'Attending:' field should only show users who have confirmed attendance
Summary: Fixes T8897

Test Plan: Open any list view of Calendar events, every event should only show "Attending: ..." with users who are attending event.

Reviewers: chad, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T8897

Differential Revision: https://secure.phabricator.com/D15779
2016-04-21 11:06:49 -07:00
epriestley
fb2b88a4a8 Fix Phriction link syntax a little more
Summary: This still wasn't quite right -- a link like `[[ Porcupine Facts ]]` with a space would not lookup correctly, and would render as `porcupine_facts`.

Test Plan: Verified that `[[ Porcupine Facts ]]` now works correctly.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15780
2016-04-21 10:29:08 -07:00