Summary: Ref T4788. This moves everything except "merge" to the new code.
Test Plan:
- Edited relationships in Differential, Diffusion, and Pholio.
- Uninstalled Pholio, made sure "Edit Mocks..." actions vanished.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16193
Summary:
Ref T9838.
Add a Properties field to Revision, and update a `wasAcceptedBeforeClose` when closing a revision.
Test Plan:
A quick run through the obvious steps (Close with commit/manually, with or w/o accept) and calling `differential.query` shows the `wasAcceptedBeforeClose` property was setup correctly.
Pushing closed + accepted passes the relevant herald, which was my immediate issue; Pushing un-accepted is blocked.
Test the "commit" rule (Different from "pre-commit") by hacking the DB and running the "has accepted revision" rule in a test-console.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T9838
Differential Revision: https://secure.phabricator.com/D15085
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:
Ref T11034. This seems a little more promising. Two problems at the moment:
- This doesn't actually provide any useful information at all right now.
- Many object types have no profile images.
Test Plan:
{F1695254}
{F1695255}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11034
Differential Revision: https://secure.phabricator.com/D16155
Summary: Ref T9028. When selecting refs, pretend refs in "refs/remotes/" that we don't otherwise recognize don't exist, since it looks like these are probably remotes //of the remote// we're observing, and who knows what state they're in.
Test Plan: Used `bin/repository discover --verbose` to verify that these named refs no longer appear in the list.
Reviewers: chad, joshuaspence
Reviewed By: joshuaspence
Maniphest Tasks: T9028
Differential Revision: https://secure.phabricator.com/D16136
Summary: Ref T9028. Ref T6878. This rule should probably be refined in the long term, but for now just ignore "phabricator/diff/12424" and similar staging area tags.
Test Plan: Ran `bin/repository discover --verbose` on a repository with staging area refs, saw Phabricator ignore those refs as untracked.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6878, T9028
Differential Revision: https://secure.phabricator.com/D16134
Summary:
Ref T9028. This is the easy part of dealing with deleted commits:
- Add a flag for unreachable commits (nothing sets this flag yet).
- Ignore unreachable commits when querying for known commits during discovery, so we pretend they do not exist.
- When recording a commit, try just reviving an existing unreachable commit first. If that works, bail out.
Test Plan:
- Artificially marked a commit as unreachable with raw SQL.
- Verified it said "deleted: unreachable" in the UI.
- Ran `repository discover --trace --verbose`.
- Saw the discovery process ignore the commit when filling the cache.
- Saw the discovery process revive the commit instead of trying to record it again.
- Web UI now shows the commit as normal.
- Running `repository discover` again doesn't make any further changes.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9028
Differential Revision: https://secure.phabricator.com/D16130
Summary:
Ref T9028. Fixes T6878. Currently, we only fetch and discover branches. This is fine 99% of the time but sometimes commits are pushed to just a tag, e.g.:
```
git checkout <some hash>
nano file.c
git commit -am '...'
git tag wild-wild-west
git push origin wild-wild-west
```
Through a similar process, commits can also be pushed to some arbitrary named ref (we do this for staging areas).
With the current rules, we don't fetch tag refs and won't discover these commits.
Change the rules so:
- we fetch all refs; and
- we discover ancestors of all refs.
Autoclose rules for tags and arbitrary refs are just hard-coded for now. We might make these more flexible in the future, or we might do forks instead, or maybe we'll have to do both.
Test Plan:
Pushed a commit to a tag ONLY (`vegetable1`).
<cf508b8de6>
On `master`, prior to the change:
- Used `update` + `refs` + `discover`.
- Verified tag was not fetched with `git for-each-ref` in local working copy and the web UI.
- Verified commit was not discovered using the web UI.
With this patch applied:
- Used `update`, saw a `refs/*` fetch instead of a `refs/heads/*` fetch.
- Used `git for-each-ref` to verify that tag fetched.
- Used `repository refs`.
- Saw new tag appear in the tags list in the web UI.
- Saw new refcursor appear in refcursor table.
- Used `repository discover --verbose` and examine refs for sanity.
- Saw commit row appear in database.
- Saw commit skeleton appear in web UI.
- Ran `bin/phd debug task`.
- Saw commit fully parse.
{F1689319}
Reviewers: chad
Reviewed By: chad
Subscribers: avivey
Maniphest Tasks: T6878, T9028
Differential Revision: https://secure.phabricator.com/D16129
Summary:
When having lots of repos, seeing "all revisions in this project" is hard, and we ended up adding herald rules to basically copy project tags to the revisions on a per-project basis. Adding a "tagged: project" function to the Repositories search field allows users to find differentials within a project.
Fix T10850.
Test Plan: search differentials by tagging project and repository in the Repository field
Reviewers: avivey, epriestley, #blessed_reviewers
Reviewed By: avivey, epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T10850
Differential Revision: https://secure.phabricator.com/D16096
Summary: Fixes T11139. We missed this years ago when we moved to PhutilUTF8StringTruncator.
Test Plan: {F1686072}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11139
Differential Revision: https://secure.phabricator.com/D16105
Summary: Ref T10227. When we perform `git` http operations (fetch, mirror) check if we should use a proxy; if we should, set `http_proxy` or `https_proxy` in the environment to make `git` have `curl` use it.
Test Plan:
- Configured a proxy extension to run stuff through a local instance of Charles.
- Ran `repository pull` and `repository mirror`.
- Saw `git` HTTP requests route through the proxy.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10227
Differential Revision: https://secure.phabricator.com/D16092
Summary:
Ref T4103. This just adds a single global default setting group, not full profiles.
Primarily, I'm not sure how administrators are supposed to set profiles for users, since most ways user accounts get created don't really support setting roles.. When we figure that out, it should be reasonably easy to extend this. There also isn't much of a need for this now, since pretty much everyone just wants to turn off mail.
Test Plan:
- Edited personal settings.
- Edited global settings.
- Edited a bot's settings.
- Tried to edit some other user's settings.
- Saw defaults change appropriately as I edited global and personal settings.
{F1677266}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16048
Summary: Ref T4103. Fully modernize the filetree show/hide, durable column show/hide, and profile menu collapse/wide settings.
Test Plan:
- Toggled filetree on/off, reloaded page, setting stuck.
- Same with conpherence column and profile menus.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16034
Summary: Ref T4103. Modernize the blame/color toggles in Diffusion. These have no separate settings UI.
Test Plan: Toggled blame and colors, reloaded pages, settings stuck.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16026
Summary:
Ref T4103. Settings panels are grouped into categories of similar panels (like "Email" or "Sessions and Logs").
Currently, this is done informally, by just grouping and ordering by strings. This won't work well with translations, since it means the ordering is entirely dependent on the language order, so the first settings panel you see might be something irrelvant or confusing. We'd also potentially break third-party stuff by changing strings, but do so in a silent hard-to-detect way.
Provide formal objects and modularize the panel groups completely.
Test Plan: Verified all panels still appear properly and in the same groups and order.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16020
Summary:
Ref T4103. This doesn't get everything, but takes care of most of the easy stuff.
The tricky-ish bit here is that I need to move timezones, pronouns and translations to proper settings. I expect to pursue that next.
Test Plan:
- Grepped for `loadPreferences` to identify callsites.
- Changed start-of-week setting, loaded Calendar, saw correct start.
- Visited welcome page, read "Adjust Settings" point.
- Loaded Conpherence -- I changed behavior here slightly (switching threads drops the title glyph) but it wasn't consistent to start with and this seems like a good thing to push to the next version of Conpherence.
- Enabled Filetree, toggled in Differential.
- Disabled Filetree, no longer visible in Differential.
- Changed "Unified Diffs" preference to "Small Screens" vs "Always".
- Toggled filetree in Diffusion.
- Edited a task, saw sensible projects in policy dropdown.
- Viewed user profile, uncollapsed/collapsed side nav, reloaded page, sticky'd.
- Toggled "monospaced textareas", used a comment box, got appropriate fonts.
- Toggled durable column.
- Disabled title glyphs.
- Changed monospaced font to 18px/36px impact.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16004
Summary:
Ref T4292. For hosted, clustered repositories we have a good way to increment the internal version of the repository: every time a user pushes something, we increment the version by 1.
We don't have a great way to do this for observed/remote repositories because when we `git fetch` we might get nothing, or we might get some changes, and we can't easily tell //what// changes we got.
For example, if we see that another node is at "version 97", and we do a fetch and see some changes, we don't know if we're in sync with them (i.e., also at "version 97") or ahead of them (at "version 98").
This implements a simple way to version an observed repository:
- Take the head of every branch/tag.
- Look them up.
- Pick the biggest internal ID number.
This will work //except// when branches are deleted, which could cause the version to go backward if the "biggest commit" is the one that was deleted. This should be OK, since it's rare and the effects are minor and the repository will "self-heal" on the next actual push.
Test Plan:
- Created an observed repository.
- Ran `bin/repository update` and observed a sensible version number appear in the version table.
- Pushed to the remote, did another update, saw a sensible update.
- Did an update with no push, saw no effect on version number.
- Toggled repository to hosted, saw the version reset.
- Simulated read traffic to out-of-sync node, saw it do a remote fetch.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15986
Summary:
Ref T11051. This is still not as clear as it should be, but is at least working as intended now.
I believe this part of the code just never worked. The test plan on D10489 didn't specifically cover it.
Test Plan:
Did this sort of thing in a repository:
```
$ git checkout -b featurex
$ echo x >> y
$ git commit -am wip
$ arc diff
```
Then I simulated just pushing it (this flow is a little more involved than necessary):
```
$ arc land --hold
$ git commit --amend
$ # remove all metadata -- particularly, "Differential Revision"!
$ git push HEAD:master
```
I got a not-great but more-useful dialog:
{F1667318}
Prior to this change, the hash match was incorrectly not reported at all.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11051
Differential Revision: https://secure.phabricator.com/D15989
Summary:
Fixes T11020. I think this resolves things -- `$new_version` (set above) should be used, not `$new_log` directly.
Specifically, we would get into trouble if the initial push failed for some reason (working copy not initialized yet, commit hook rejected, etc).
Test Plan: Made a bad push to a new repository. Saw it freeze before the patch and succeed afterwards.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11020
Differential Revision: https://secure.phabricator.com/D15969
Summary:
Ref T10939. Fixes T10174. We can currently trigger "uninteresting" auditors in two ways:
- Packages with auditing disabled ("NONE" audits).
- Packages with auditing enabled, but they don't need an audit (e.g., author is a pacakge owner; "NOT REQUIRED" audits).
These audits aren't interesting (we only write them so we can list "commits in this package" from other UIs) but right now they take up the audit slot. In particular:
- They show in the UI, but are generally useless/confusing nowadays. The actual table of contents does a better job of just showing "which packages do these paths belong to" now, and shows all packages for each path.
- They block Herald from adding real auditors.
Change this:
- Don't show uninteresting auditors.
- Let Herald upgrade uninteresting auditors into real auditors.
Test Plan:
- Ran `bin/repository reparse --owners <commit> --force`, and `--herald` to trigger Owners and Herald rules.
- With a package with auditing disabled, triggered a "None" audit and saw it no longer appear in the UI with the patch applied.
- With a package with auditing disabled, added a Herald rule to trigger an audit. With the patch, saw it go through and upgrade the audit to "Audit Required".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10174, T10939
Differential Revision: https://secure.phabricator.com/D15940
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
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
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
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
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
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
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
Summary:
Ref T10923. We sort of dead-end new users creating repositories right now, by dumping them into the manage UI without an obvious way forward.
You can click the crumb to get to the repository, but by default it will say something like `R1` which isn't very obvious.
Add a more obvious navigational link to get to the main view.
Test Plan: {F1308196}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15889
Summary:
Ref T10923. The old behavior was to show a full command in SVN, Mercurial, and Git, like this:
- `git clone <uri>`
- `hg clone <uri>`
- `svn checkout <uri> <directory>`
In Git and Mercurial, the `<uri>` ends in something like `/nice-repository-name.git` so the default directory it creates is called `nice-repository-name/`.
In Subversion, we don't (and can't easily) do that for various reasons so we provide an explicit `<directory>` with the nice name.
In the update, I've changed things to just show the URI. I often found that I wanted the URI alone, not the whole clone command (for example, to `fetch`, `remote-add`, etc). This is also consistent with GitHub. Because we have nice URIs for Git and Mercurial, `git clone <uri>` has good behavior.
In Subversion, `svn checkout <uri>` has bad beahvior (you get a directory named `47/` or whatever). So continue showing the whole command there.
We can possibly tailor this after T4245 finishes up and we get access to `/source/nice-repository-name/` URIs.
Test Plan:
- Viewed a Subversion repository, saw a full command.
- Viewed a Git repository, saw only a clone URI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15888
Summary: Fixes T10941. This avoids a confusing dead end when configuring Subversion hosting, where `svnserve` will fail to execute hooks if the CWD isn't readable by the vcs-user.
Test Plan:
- Updated and committed in a hosted SVN repository.
- Ran some git operations, too.
- @dpotter confirmed this locally in T10941.
Reviewers: chad
Reviewed By: chad
Subscribers: dpotter
Maniphest Tasks: T10941
Differential Revision: https://secure.phabricator.com/D15879
Summary: Ref T10923. Although I'd ideally like to get rid of this eventually, keep it around for now.
Test Plan:
- Edited value for an SVN repository.
- Observed no panel present for a Git repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15883
Summary: Fixes T10815. We already recovered reasonably from this for cluster repositories, but not for non-cluster repositories.
Test Plan:
- Viewed cluster and non-cluster empty Git repository.
- Viewed cluster and non-cluster empty Mercurial repository.
- Viewed cluster and non-clsuter empty hosted SVN repository.
- Viewed cluster and non-cluster empty observed SVN repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10815
Differential Revision: https://secure.phabricator.com/D15878
Summary:
Ref T10923. Fixes T9554.
When hosting a repository, we currently have a heuristic that tries to detect when you're doing an initial import: if you push more than 7 commits to an empty repository, it counts as an import and we disable mail/feed/etc.
Do something similar for observed repositories: if the repository is empty and we discover more than 7 commits, switch to import mode until we catch up.
This should align behavior with user expectation more often when juggling hosted vs imported repositories.
Test Plan:
- Created a new hosted repository.
- Activated it and allowed it to fully import.
- Added an "Observe URI".
- Saw it automatically drop into "Importing" mode until the import completed.
- Swapped it back to hosted mode.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9554, T10923
Differential Revision: https://secure.phabricator.com/D15877
Summary: Ref T10923. Walk users through the "create, configure, activate" workflow a little better and set expectations more clearly.
Test Plan:
- Created a new repository, saw new UI help.
- Activated repository, saw onboarding help disappear.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15875
Summary:
Ref T10923.
- Hide "Automation", "Staging" and "Branches" in repositories where they do nothing.
- Fix SVN SSH URIs to read "svn+ssh://" and have proper paths.
Test Plan:
- Verified irrelevant sections did not appear in Subversion in Manage UI.
- Checked out a new hosted SVN repository.
Reviewers: chad, avivey
Reviewed By: avivey
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15874
Summary:
Ref T10923. Fixes T10406. This brings most of the guidance/instructions forward:
- Some remained as instructions.
- Some moved to documentation.
Test Plan: Went through all of the sections and hit the help.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10406, T10923
Differential Revision: https://secure.phabricator.com/D15873
Summary: Ref T10923. This provides a little guidance about hosted vs observed, and points at the `diffusion.ssh-*` options.
Test Plan: Poked around in the web UI, saw useful guidance.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15872
Summary: Ref T10923. This cleans up the remaining "pro" mess left by the cutover.
Test Plan: Viewed, managed, edited a repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15871
Summary:
Ref T10923. Primarily documents the process for creating repositories via the API.
Also fixes a couple of issues with `repositoryPHID` not being set yet when creating URIs via the API.
Test Plan:
- Followed all documented steps to create a new repository.
- Created and edited some new URIs from the web workflow, too.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15870
Summary: Ref T10923. This isn't complete yet, but reduces lies and increases truths.
Test Plan: Read documentation, clicked new "Documentation" nav item.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15868
Summary:
Ref T10923. Some of the dialogs ("Deactivate Repository", "Test Automation", etc.) had cancel or redirect URIs which I missed originally.
Go through them and make sure they all point to the right places.
Also removed one unused controller which I missed the first time around.
Test Plan:
- Opened all these dialogs in a new tab with Command-Click.
- Clicked every "cancel" and "submit" button on all of these dialogs.
- Got consistently sent to the place I came from.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15867
Summary:
Ref T10923.
- The "Policy" edit form currently goes "Push, View, Edit". Reorder the defaults to "View, Edit, Push".
- Editing Spaces doesn't currently work: the element appears in the UI, but isn't actually processed when handling transactions. Make that work.
Test Plan:
- Edited a repository policies, saw "View, Edit, Push".
- Moved a repository between Spaces.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15866
Summary:
Ref T10927. Pretty sure the issue is:
- User makes an HTTPS request.
- Load balancer terminates it, but with an `X-Forwarded-Proto` header.
- `secure001` (or whatever; acting as web host) proxies it to `secure002` (or whatever; acting as a repository host). **This** connection is plain HTTP.
- Since this proxied connection is plain HTTP, we check if the repository can serve over "http", but it can't: only "https". So we fail incorrectly, even though the original user request was HTTPS.
In the long run we should probably forward the `X-Forwarded-Proto` header, but that has some weird implications and it's broadly fine to allow either protocol to serve as long as the other one is active: configuration like `security.require-https` is already stronger than these settings.
Test Plan: This is likely only observable in production, but normal cloning still works locally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10927
Differential Revision: https://secure.phabricator.com/D15856
Summary:
Ref T10748. This needs more extensive testing and is sure to have some rough edges, but seems to basically work so far.
Throwing this up so I can work through it more deliberately and make notes.
Test Plan:
- Ran migration.
- Used `bin/repository list` to list existing repositories.
- Used `bin/repository update <repository>` to update various repositories.
- Updated a migrated, hosted Git repository.
- Updated a migrated, observed Git repository.
- Converted an observed repository into a hosted repository by toggling the I/O mode of the URI.
- Conveted a hosted repository into an observed repository by toggling it back.
- Created and activated a new empty hosted Git repository.
- Created and activated an observed Git repository.
- Updated a mirrored repository.
- Cloned and pushed over HTTP.
- Tried to HTTP push a read-only repository.
- Cloned and pushed over SSH.
- Tried to SSH push a read-only repository.
- Updated several Mercurial repositories.
- Updated several Subversion repositories.
- Created and edited repositories via the API.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15842
Summary:
Ref T10748. This migrates and swaps mirroring to `PhabricatorRepositoryURI`, obsoleting `PhabricatorRepositoryMirror`.
This prevents you from editing, adding or disabling mirrors unless you know a secret URI (until the UI cuts over fully), but existing mirroring is not affected.
Test Plan:
- Added a mirroring URI to an old repository.
- Verified it worked with `bin/repository mirror`.
- Migrated forward.
- Verified it still worked with `bin/repository mirror`.
- Wow, mirroring: https://github.com/epriestley/locktopia-mirror
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15841
Summary:
Ref T4039. Long ago these were more freely editable and there were some security concerns around creating a repository, then setting its local path to point somewhere it shouldn't.
Local paths are no longer editable so there's no real reason we need to provide a uniqueness guarantee anymore, but you could still make a mistake with `bin/repository move-paths` by accident, and it's a little cleaner to pull them out into their own column with a key.
(We still don't -- and, largely can't -- guarantee that two paths aren't //equivalent// since one might be symlinked to the other, or symlinked only on some hosts, or whatever, but the primary value here is as a sanity check that you aren't goofing things up and pointing a bunch of repositories at the same working copy by mistake.)
Test Plan:
- Ran migrations.
- Grepped for `local-path`.
- Listed and moved paths with `bin/repository`.
- Created a new repository, verified its local path populated correctly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4039
Differential Revision: https://secure.phabricator.com/D15837
Summary:
Ref T10748. These:
- Look nice.
- Hint at panel contents / effects.
- Hint which panels have been customized.
- Allow panels with issues or errors to be highlighted with an alert/attention icon.
Test Plan: {F1256156}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15836
Summary: Ref T10748. This copies existing code in the `CreateController` which will eventually be removed.
Test Plan:
- Created a new repository with the EditPro workflow.
- Saw it come up into the cluster properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15835
Summary:
Ref T10748. This allows an EditEngine form to be broken up into pages.
This is less powerful than `PHUIPagedFormView`, because the pages are not sequential / stateful. Each form saves immediately once it's submitted, and can not take you to a new form or back/forward in a series of forms.
For example, you can't create a workflow where the user fills out 5 pages of information before we create an object, like the current repository workflow does.
However, the only place we've ever wanted to do this is repositories and it's fairly bad there, so I feel reasonably confident we aren't going to miss this in the future.
(We do "choose a type of service/repository/rule -> fill out one page of info" fairly often, but can do this without the full-power paging stuff.)
Test Plan:
- Created a repository usin the new Manage UI, filling out only a handful of fields.
- Edited a repository using the new Manage UI.
- All forms are now EditEngine forms offering paged views of the big huge underlying form:
{F1254371}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15832
Summary:
Ref T10748. Ref T10366. Allows users to set credential for new URIs.
- Ref T7221. Our handling of the "git://" protocol is currently incorrect. This protocol is not authenticated, but is considered an SSH protocol. In the new UI, it is considered an anonymous/unauthenticated protocol instead.
- Ref T10241. This fixes the `PassphraseCredentialControl` so it doesn't silently edit the value if the current value is not visible to you and/or not valid.
Test Plan:
Performed a whole lot of credential edits, removals, and adjustments. I'll give this additional vetting before cutting over to it.
{F1253207}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7221, T10241, T10366, T10748
Differential Revision: https://secure.phabricator.com/D15829
Summary:
Ref T10748.
- Allow users to add new URIs by clicking a button instead of knowing a secret URI.
- Validate that URIs are actually valid URIs.
- Add enable/disable action and strings.
Test Plan:
- Created a new URI.
- Tried to create a nonsense URI, created a good URI.
- Enabled/disabled a URI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15825
Summary: Ref T10748. Adds a "uris" attachment with URI information.
Test Plan: Queried URI information via Conduit, saw reasonable looking information.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15822
Summary: Ref T10748. Brings the rest of the transactions to EditEngine, supports creating via API.
Test Plan:
- Created a URI via API.
- Created a URI via web.
- Tried to apply sneaky transactions, got rejected with good error messages. <_< >_>
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15821
Summary:
Ref T10748.
- New View page for repository URIs.
- Make display and I/O behavior (observe, mirror, read, read/write) editable.
- Add a bunch of checks to prevent you from completely screwing up a repository by making it writable from a bunch of differnet sources.
Test Plan:
{F1249866}
{F1249867}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15816
Summary:
Ref T10748. Ref T10366. This adds a new EditEngine, EditController, Editor, Query, and Transaction for RepositoryURIs.
None of these really do anything helpful yet, and these URIs are still unused in the actual application.
Test Plan: {F1249794}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10366, T10748
Differential Revision: https://secure.phabricator.com/D15815
Summary:
Ref T10748. Allow the new EditEngine workflow to create repositories by giving the user a modal repository type choice upfront.
(The rest of this flow is still confusing/weird, though.)
Test Plan:
- Created a new repository.
{F1249626}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15813
Summary: Ref T10748. This brings the "Actions" items (publish/notify + autoclose enabled) into the new UI.
Test Plan:
- Edited this stuff via EditEngine and Conduit.
- Viewed via new Manage UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15811
Summary: Ref T10748. Makes a "Branches" panel, enables these transactions in the EditEngine.
Test Plan:
- Edited via EditEngine + Conduit.
- Viewed via manage UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15809
Summary: Ref T10748. Port this, add EditEngine support, add some type validation to the transaction.
Test Plan:
- Edited via EditEngine.
- Edited via Conduit.
- Viewed via Management UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15808
Summary: Ref T10748. Brings this forward in the UI and EditEngine.
Test Plan:
- Edited via Conduit.
- Viewed via Manage UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15805
Summary: Ref T10748. Ports this UI and exposes it on the EditEngine.
Test Plan:
- Edited via EditEngine.
- Viewed new manage UI.
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15804
Summary: Ref T10748. Brings this over and adds EditEngine support for it.
Test Plan: Viewed and edited staging area information.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15801
Summary: Ref T10748. This merges "Storage" and "Cluster" into a single UI which combines the information of both.
Test Plan: {F1246882}
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15800
Summary:
Ref T10860. This allows us to recover if the connection to the database is lost during a push.
If we lose the connection to the master database during a push, we would previously freeze the repository. This is very safe, but not very operator-friendly since you have to go manually unfreeze it.
We don't need to be quite this aggressive about freezing things. The repository state is still consistent after we've "upgraded" the lock by setting `isWriting = 1`, so we're actually fine even if we lost the global lock.
Instead of just freezing the repository immediately, sit there in a loop waiting for the master to come back up for a few minutes. If it recovers, we can release the lock and everything will be OK again.
Basically, the changes are:
- If we can't release the lock at first, sit in a loop trying really hard to release it for a while.
- Add a unique lock identifier so we can be certain we're only releasing //our// lock no matter what else is going on.
- Do the version reads on the same connection holding the lock, so we can be sure we haven't lost the lock before we do that read.
Test Plan:
- Added a `sleep(10)` after accepting the write but before releasing the lock so I could run `mysqld stop` and force this issue to occur.
- Pushed like this:
```
$ echo D >> record && git commit -am D && git push
[master 707ecc3] D
1 file changed, 1 insertion(+)
# Push received by "local001.phacility.net", forwarding to cluster host.
# Waiting up to 120 second(s) for a cluster write lock...
# Acquired write lock immediately.
# Waiting up to 120 second(s) for a cluster read lock on "local001.phacility.net"...
# Acquired read lock immediately.
# Device "local001.phacility.net" is already a cluster leader and does not need to be synchronized.
# Ready to receive on cluster host "local001.phacility.net".
Counting objects: 3, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 254 bytes | 0 bytes/s, done.
Total 3 (delta 1), reused 0 (delta 0)
BEGIN SLEEP
```
- Here, I stopped `mysqld` from the CLI in another terminal window.
```
END SLEEP
# CRITICAL. Failed to release cluster write lock!
# The connection to the master database was lost while receiving the write.
# This process will spend 300 more second(s) attempting to recover, then give up.
```
- Here, I started `mysqld` again.
```
# RECOVERED. Link to master database was restored.
# Released cluster write lock.
To ssh://local@localvault.phacility.com/diffusion/26/locktopia.git
2cbf87c..707ecc3 master -> master
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10860
Differential Revision: https://secure.phabricator.com/D15792
Summary:
Ref T10860. At least in Git over SSH, we can freely echo a bunch of stuff to stderr and Git will print it to the console, so we can tell users what's going on.
This should make debugging, etc., easier. We could tone this down a little bit once things are more stable if it's a little too chatty.
Test Plan:
```
$ echo D >> record && git commit -am D && git push
[master ca5efff] D
1 file changed, 1 insertion(+)
# Push received by "local001.phacility.net", forwarding to cluster host.
# Waiting up to 120 second(s) for a cluster write lock...
# Acquired write lock immediately.
# Waiting up to 120 second(s) for a cluster read lock on "local001.phacility.net"...
# Acquired read lock immediately.
# Device "local001.phacility.net" is already a cluster leader and does not need to be synchronized.
# Ready to receive on cluster host "local001.phacility.net".
Counting objects: 3, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 256 bytes | 0 bytes/s, done.
Total 3 (delta 1), reused 0 (delta 0)
To ssh://local@localvault.phacility.com/diffusion/26/locktopia.git
8616189..ca5efff master -> master
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10860
Differential Revision: https://secure.phabricator.com/D15791
Summary: Ref T10860. This doesn't change anything, it just separates all this stuff out of `PhabricatorRepository` since I'm planning to add a bit more state to it and it's already pretty big and fairly separable.
Test Plan: Pulled, pushed, browsed Diffusion.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10860
Differential Revision: https://secure.phabricator.com/D15790
Summary:
Ref T4292. When the daemons make a query for repository information, we need to make sure the working copy on disk is up to date before we serve the response, since we might not have the inforamtion we need to respond otherwise.
We do this automatically for almost all Diffusion methods, but this particular method is a little unusual and does not get this check for free. Add this check.
Test Plan:
- Made this code throw.
- Ran `bin/repository reparse --message ...`, saw the code get hit.
- Ran `bin/repository lookup-user ...`, saw this code get hit.
- Made this code not throw.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15783
Summary:
Ref T10751. Make the UI more useful and explain what failure states mean and how to get out of them.
The `bin/repository thaw` command does not exist yet, I'll write that soon.
Test Plan: {F1238241}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10751
Differential Revision: https://secure.phabricator.com/D15766
Summary: Ref T10751. This cleans this up so it's a little more modern, and fixes a possible bad access on the log detail page.
Test Plan: Viewed push log list, viewed push log detail.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10751
Differential Revision: https://secure.phabricator.com/D15765
Summary: Ref T4292. When we write a push log, also log which node received the request.
Test Plan: {F1230467}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15759
Summary:
Ref T4292. We currently synchronize hosted, clustered, Git repositories when we receive an SSH pull or push.
Additionally:
- Synchronize before HTTP reads and writes.
- Synchronize reads before Conduit requests.
We could relax Conduit eventually and allow Diffusion to say "it's OK to give me stale data".
We could also redirect some set of these actions to just go to the up-to-date host instead of connecting to a random host and synchronizing it. However, this potentially won't work as well at scale: if you have a larger number of servers, it sends all of the traffic to the leader immediately following a write. That can cause "thundering herd" issues, and isn't efficient if replicas are in different geographical regions and the write just went to the east coast but most clients are on the west coast. In large-scale cases, it's better to go to the local replica, wait for an update, then serve traffic from it -- particularly given that writes are relatively rare. But we can finesse this later once things are solid.
Test Plan:
- Pushed and pulled a Git repository over HTTP.
- Browsed a Git repository from the web UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15758
Summary:
Ref T4292. Before we write or read a hosted, clustered Git repository over SSH, check if another version of the repository exists on another node that is more up-to-date.
If such a version does exist, fetch that version first. This allows reads and writes of any node to always act on the most up-to-date code.
Test Plan: Faked my way through this and got a fetch via `bin/repository update`; this is difficult to test locally and needs more work before we can put it in production.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15757
Summary:
Ref T4292. When you run `git fetch` and connect to, say, `repo001.west.company.com`, we'll look at the current version of the repository in other nodes in the cluster.
If `repo002.east.company.com` has a newer version of the repository, we'll fetch that version first, then respond to your request.
To do this, we need to run `git fetch repo002.east.company.com ...` and have that connect to the other host and be able to fetch data.
This change allows us to run `PHABRICATOR_AS_DEVICE=1 git fetch ...` to use device credentials to do this fetch. (Device credentials are already supported and used, they just always connect as a user right now, but these fetches should be doable without having a user. We will have a valid user when you run `git fetch` yourself, but we won't have one if the daemons notice that a repository is out of date and want to update it, so the update code should not depend on having a user.)
Test Plan:
```
$ PHABRICATOR_AS_DEVICE=1 ./bin/ssh-connect local.phacility.com
Warning: Permanently added 'local.phacility.com' (RSA) to the list of known hosts.
PTY allocation request failed on channel 0
phabricator-ssh-exec: Welcome to Phabricator.
You are logged in as device/daemon.phacility.net.
You haven't specified a command to run. This means you're requesting an interactive shell, but Phabricator does not provide an interactive shell over SSH.
Usually, you should run a command like `git clone` or `hg push` rather than connecting directly with SSH.
Supported commands are: conduit, git-lfs-authenticate, git-receive-pack, git-upload-pack, hg, svnserve.
Connection to local.phacility.com closed.
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15755
Summary:
Ref T4292. This consolidates code for figuring out which user we should connect to hosts with.
Also narrows a lock window.
Test Plan: Browsed Diffusion, pulled and pushed through an SSH proxy.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15754
Summary:
Ref T4292. Ref T10366. Depends on D15751. Today, generating repository commands is purely a function of the repository, so they use protocols and credentials based on the repository configuration.
For example, a repository with an SSH "remote URI" always generate SSH "remote commands".
This needs to change in the future:
- After T10366, repositories won't necessarily just have one type of remote URI. They can only have one at a time still, but the repository itself won't change based on which one is currently active.
- For T4292, I need to generate intracluster commands, regardless of repository configuration. These will have different protocols and credentials.
Prepare for these cases by separating out command construction, so they'll be able to generate commands in a more flexible way.
Test Plan:
- Added unit tests.
- Browsed diffusion.
- Ran `bin/phd debug pull` to pull a bunch of repos.
- Ran daemons.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292, T10366
Differential Revision: https://secure.phabricator.com/D15752
Summary: Ref T4292. This will let the UI and future `bin/repository` tools give administrators more tools to understand problems when reporting or resolving them.
Test Plan:
- Pushed fully clean repository.
- Pushed previously-pushed repository.
- Forced write to abort, inspected useful information in the database.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15748
Summary:
Ref T4292. Small fixes:
- There was a bug with the //first// write, where we'd write 1 but expect 0. Fix this.
- Narrow the window where we hold the `isWriting` lock: we don't need to wait for the client to finish.
- Release the lock even if something throws.
- Use a more useful variable name.
Test Plan:
- Made new writes to a fresh cluster repository.
- Made sequential writes.
- Made concurrent writes.
- Made good writes and bad writes.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15747
Summary:
Ref T10748. This supports more transaction types in the modern editor and improves validation so Conduit benefits.
You can technically create repositories via `diffusion.repository.edit` now, although they aren't very useful.
Test Plan:
- Used `diffusion.repository.edit` to create and edit repositories.
- Used `/editpro/` to edit repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15740
Summary: Ref T10748. Ref T10366. No support for editing and no impact on the UI, but get some of the basics in place.
Test Plan: {F1223279}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10366, T10748
Differential Revision: https://secure.phabricator.com/D15742
Summary: Ref T10748. Pretty straightforward. I'd like to put a little "!" icon in the menu if there's a warning/error eventually, but can deal with that latre.
Test Plan: {F1223096}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15741
Summary:
Ref T10748. This is roughly where I'm headed, if it makes some kind of sense? The "Edit" links in sub-sections don't work yet since I haven't built the thing.
Probably depends on D15736.
Test Plan: Manually navigated to `/manage/`, clicked around.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15737
Summary: Ref T10748. Ref T10337. This technically implements this stuff, but it does not do anything useful yet. This skips all the hard stuff.
Test Plan:
- Technically used `diffusion.repository.search` to get repository information.
- Technically used `diffusion.repository.edit` to change a repository name.
- Used `editpro/` to edit a repository name.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10337, T10748
Differential Revision: https://secure.phabricator.com/D15736
Summary: Fixes T10816. The way these work is a little unusual since these chunks of file-rendering code are unusuall performance-sensitive, so the Differential version doesn't adapt directly to Diffusion. Both can possibly be unified at some point in the future, although they do slightly different things.
Test Plan: {F1220170}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10816
Differential Revision: https://secure.phabricator.com/D15719
Summary:
Ref T10809. Currently, both the proxy and target may mutate URIs (rewriting "svn+ssh://x/diffusion/Y/" to a path on disk).
I believe this previously worked by fate/chance/luck since both URI variants contain the repository information, but the algorithms were tightened up recently with callsign removal.
Stop rewriting them if we're the intracluster proxy -- they only need to be rewritten on the target host.
Test Plan:
- Checked out a proxied SVN repository, with and without a callsign.
- Checked out an unproxied SVN repository, with and without a callsign.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10809
Differential Revision: https://secure.phabricator.com/D15712
Summary:
Ref T2783. This allows this worker to run on a machine different to the one that stores the repository, by routing the execution of Git over Conduit calls.
This API method is super gross, but fixing it isn't straightforward and it runs into other complicated considerations. We can fix it later; for now, just define it as "internal" to limit how much mess this creates.
"Internal" methods do not appear on the console.
Test Plan: Ran `bin/repository reparse --change <commit> --trace` on several commits, saw daemons make a Conduit call instead of running a `git` command.
Reviewers: hach-que, chad
Reviewed By: chad
Subscribers: joshuaspence, Korvin, epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11874
Summary: Fixes T10797. This seems to fix things on my local system.
Test Plan:
- Cloned with a username, got prompted for a password.
- Cloned with a username + password.
- Cloned with a username + bad password (error).
Reviewers: chad
Reviewed By: chad
Subscribers: Grimeh
Maniphest Tasks: T10797
Differential Revision: https://secure.phabricator.com/D15706
Summary:
Ref T4292. This mostly implements the locking/versioning logic for multi-master repositories. It is only active on Git SSH pathways, and doesn't actually do anything useful yet: it just does bookkeeping so far.
When we read (e.g., `git fetch`) the logic goes like this:
- Get the read lock (unique to device + repository).
- Read all the versions of the repository on every other device.
- If any node has a newer version:
- Fetch the newer version.
- Increment our version to be the same as the version we fetched.
- Release the read lock.
- Actually do the fetch.
This makes sure that any time you do a read, you always read the most recently acknowledged write. You may have to wait for an internal fetch to happen (this isn't actually implemented yet) but the operation will always work like you expect it to.
When we write (e.g., `git push`) the logic goes like this:
- Get the write lock (unique to the repository).
- Do all the read steps so we're up to date.
- Mark a write pending.
- Do the actual write.
- Bump our version and mark our write finished.
- Release the write lock.
This allows you to write to any replica. Again, you might have to wait for a fetch first, but everything will work like you expect.
There's one notable failure mode here: if the network connection between the repository node and the database fails during the write, the write lock might be released even though a write is ongoing.
The "isWriting" column protects against that, by staying locked if we lose our connection to the database. This will currently "freeze" the repository (prevent any new writes) until an administrator can sort things out, since it'd dangerous to continue doing writes (we may lose data).
(Since we won't actually acknowledge the write, I think, we could probably smooth this out a bit and make it self-healing //most// of the time: basically, have the broken node rewind itself by updating from another good node. But that's a little more complex.)
Test Plan:
- Pushed changes to a cluster-mode repository.
- Viewed web interface, saw "writing" flag and version changes.
- Pulled changes.
- Faked various failures, got sensible states.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15688
Summary:
Ref T4292. This adds some very basic cluster/device data to the new management view. Nothing interesting yet.
Also deal with disabled bindings a little more cleanly.
Test Plan: {F1214619}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15685
Summary:
Ref T4292. This puts a very rough skeleton in place for the new "Manage Repository" UI, somewhat similar to the "Settings" UI.
Right now, it has one panel with no content, and is not reachable from the UI.
Test Plan: {F1214525}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15683
Summary: Going to render these all normal case instead of all caps, and bump up the font size. Should be more consistent. Yellow if you green anything orange.
Test Plan: grep, lint
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15645
Summary:
Ref T10262. This removes one-time tokens and makes file data responses always-cacheable (for 30 days).
The URI will stop working once any attached object changes its view policy, or the file view policy itself changes.
Files with `canCDN` (totally public data like profile images, CSS, JS, etc) use "cache-control: public" so they can be CDN'd.
Files without `canCDN` use "cache-control: private" so they won't be cached by the CDN. They could still be cached by a misbehaving local cache, but if you don't want your users seeing one anothers' secret files you should configure your local network properly.
Our "Cache-Control" headers were also from 1999 or something, update them to be more modern/sane. I can't find any evidence that any browser has done the wrong thing with this simpler ruleset in the last ~10 years.
Test Plan:
- Configured alternate file domain.
- Viewed site: stuff worked.
- Accessed a file on primary domain, got redirected to alternate domain.
- Verified proper cache headers for `canCDN` (public) and non-`canCDN` (private) files.
- Uploaded a file to a task, edited task policy, verified it scrambled the old URI.
- Reloaded task, new URI generated transparently.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10262
Differential Revision: https://secure.phabricator.com/D15642
Summary:
Fixes T10721. When trying to load commits by identifier, we would take some bad pathways in Subversion if the repository had no callsign and end up missing the commits.
Fix this logic so it works for either callsigns (e.g., if passed `rXyyy`) or with PHIDs if passed repositories.
Test Plan:
- Viewed SVN commit in a Subversion repository with no callsign.
- Added a callsign, looked at it again.
- Viewed non-SVN commits in callsign and non-callsign repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10721
Differential Revision: https://secure.phabricator.com/D15607
Summary:
Fixes T10385. Two issues:
- `$show_blame` and `$show_color` were improperly swapped.
- Code to hide these columns got dropped somewhere, probably in my recent-ish rewrite.
Test Plan:
- Showed/hid blame.
- Showed/hid colors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10385
Differential Revision: https://secure.phabricator.com/D15528
Summary:
Ref T10537. For Nuance, I want to introduce new sources (like "GitHub" or "GitHub via Nuance" or something) but this needs to modularize eventually.
Split ContentSource apart so applications can add new content sources.
Test Plan:
This change has huge surface area, so I'll hold it until post-release. I think it's fairly safe (and if it does break anything, the breaks should be fatals, not anything subtle or difficult to fix), there's just no reason not to hold it for a few hours.
- Viewed new module page.
- Grepped for all removed functions/constants.
- Viewed some transactions.
- Hovered over timestamps to get content source details.
- Added a comment via Conduit.
- Added a comment via web.
- Ran `bin/storage upgrade --namespace XXXXX --no-quickstart -f` to re-run all historic migrations.
- Generated some objects with `bin/lipsum`.
- Ran a bulk job on some tasks.
- Ran unit tests.
{F1190182}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15521
