1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 00:02:41 +01:00
Commit graph

132 commits

Author SHA1 Message Date
Mukunda Modell
c0bf08058b Check for empty output from git ls-tree
Summary: Fixes T10155

Test Plan: View an empty repository in diffusion, check for the exception.
See T10155 for steps to reproduce

Reviewers: epriestley

Subscribers:
2016-09-10 06:02:48 -05:00
epriestley
c55de86f0e Return Diffusion diffs through Files, not directly over Conduit
Summary:
Fixes T10423. Ref T11524. This changes `diffusion.rawdiffquery` to return a file PHID instead of a blob of data.

This is better in general, but particularly better for huge diffs (as in T10423) and diffs with non-utf8 data (as in T10423).

Test Plan:
  - Used `bin/differential extract` to extract a latin1 diff, got a clean diff.
  - Used `bin/repository reparse --herald` to rerun herald on a latin1 diff, got a clean result.
  - Pushed latin1 diffs to test commit hooks.
  - Triggered the the too large / too slow logic.
  - Viewed latin1 diffs in Diffusion.
  - Used "blame past this change" in Diffusion to hit the `before` logic.

Reviewers: chad

Reviewed By: chad

Subscribers: eadler

Maniphest Tasks: T10423, T11524

Differential Revision: https://secure.phabricator.com/D16460
2016-08-27 09:11:03 -07:00
epriestley
771579496f Make logic for streaming VCS stuff directly to Files more reusable
Summary:
Ref T11524. Ref T10423. Earlier, I converted `diffusion.filecontentquery` to put the actual file content in Files, then return a PHID for the file, instead of trying to send the content over Conduit.

In T11524, we have a similar set of problems with diffs that contain non-UTF8 data (and, in T10423, diffs that are simply enormous).

I want to provide an API method to do the same sort of thing with diff output (like from `git diff`), so we call the method, it shoves the data in Files, and then we go pull it out of Files.

To support this, take the "shove the output of a Future into Files" logic and put it in a new base `FileFuture` query. This will let me make `RawDiffQuery` share the logic more easily.

Test Plan: Browsed Diffusion, ran `diffusion.filecontentquery` to fetch file content.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10423, T11524

Differential Revision: https://secure.phabricator.com/D16458
2016-08-27 09:10:20 -07:00
epriestley
d44a5fa933 In Git, only use "--find-copies-harder" on small diffs
Summary:
Ref T10423. This flag can cause `git diff` to take an enormously long time (the problem case was a 5M line, 20K file commit).

Instead:

  - Run without the flag first.
  - If that shows that the diff is definitely small, try again with the flag.
  - If that works, return the slower, better output.
  - If the fast diff affects too many paths or generating the slow diff takes too long, return the faster, slightly worse output.

The quality of the output differs in how well Git is able to detect "M" and "C" (moves and copies of files).

For example, if you copy `src/` to `srcpro/`, the fast output may not show that you copied files. The slow output will.

I think this is rarely useful for large copies anyway: it's interesting if a 1-2 file diff is a copy, but usually obvious/uninteresting if a 500-file diff is a copy.

Test Plan:
  - Ran `bin/repository reparse --change rXnnn` on Git changes.
  - Saw fast and slow commands execute normally.
  - Tried on a large diff, saw only the fast command execute.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10423

Differential Revision: https://secure.phabricator.com/D16266
2016-07-10 08:03:57 -07:00
epriestley
2a1393c008 Fix impropery history graph trace in Mercurial
Summary: Fixes T11267. This data was coming back weird (in reverse order relative to the graph itself). Previously it worked OK anyway, but the new logic is a little more sensitive to the input.

Test Plan: Viewed a Mercurial repository with linear history, saw linear history.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11267

Differential Revision: https://secure.phabricator.com/D16229
2016-07-04 10:24:14 -07:00
epriestley
f5f784f4c1 Version clustered, observed repositories in a reasonable way (by largest discovered HEAD)
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
2016-05-30 09:53:01 -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
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
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
d87c500002 Synchronize (hosted, clustered, Git) repositories over Conduit + HTTP
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
2016-04-19 13:05:45 -07:00
epriestley
575c01373e Extract repository command construction from Repositories
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
2016-04-19 04:51:48 -07:00
epriestley
adf42db5ea Trivially implement RepositoryEditEngine and API methods
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
2016-04-17 16:02:13 -07:00
June Rhodes
7150aa8e19 Use Conduit in PhabricatorRepositoryGitCommitChangeParserWorker
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
2016-04-14 04:53:03 -07:00
epriestley
601aaa5a86 Modularize content sources
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
2016-03-26 11:59:45 -07:00
Vlad Albulescu
130e1d1f68 Unbreak regex filename search
Summary:
D9087 adds a nice typeahead but breaks the existing regex
search by quoting the pattern. Ideally, this change won't break the
typeahead, which as far as I can tell doesn't use the `pattern`
argument.

Test Plan:
Not yet.
RFC as to whether this change makes sense, will fix my local setup and resend if so.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15500
2016-03-20 10:15:21 -07:00
epriestley
58c2141ffd Fix limit calculation for largeish Mercurial repsositories
Summary:
Fixes T10304. In Mercurial, we must enumerate the whole file tree. Currently, we incorrectly count files within directories (which won't be shown) toward the "100 file" limit at top level, so directories with more than 100 subpaths are truncated improperly.

This is approxiately the same as @richardvanvelzen's fix.

Test Plan: Viewed a large Mercurial repository, saw a complete directory listing.

Reviewers: chad

Reviewed By: chad

Subscribers: richardvanvelzen

Maniphest Tasks: T10304

Differential Revision: https://secure.phabricator.com/D15282
2016-02-16 14:14:59 -08:00
epriestley
b51a859636 Allow diffusion.filecontentquery to load data for arbitrarily large files
Summary:
Fixes T10186. After D14970, `diffusion.filecontentquery` puts the content in a file and returns the file PHID.

However, it does this in a way that doesn't go through the chunking engine, so it will fail for files larger than the chunk threshold (generally, 8MB).

Instead, stream the file from the underlying command directly into chunked storage.

Test Plan:
  - Made a commit including a really big file: 4dcd4c492b
  - Used `diffusion.filecontentquery` to load file content.
  - Parsed/imported commit locally.
  - Used `diffusion.filecontentquery` to load content for smaller files (README, etc).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10186

Differential Revision: https://secure.phabricator.com/D15072
2016-01-21 09:52:43 -08:00
epriestley
a061bd2d09 Parse and display commit authorship date in Git in Diffusion
Summary: Fixes T8826. Git tracks an "author date", which may be different from the "committed date". We don't currently extract/show this; do so.

Test Plan: {F1059235}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8826

Differential Revision: https://secure.phabricator.com/D14995
2016-01-11 09:32:37 -08:00
epriestley
d1fb2f7fb9 Make diffusion.filecontentquery return file PHIDs instead of raw content
Summary:
Fixes T9319. Proxied requests (e.g., in the cluster) for binary files (like images) currently fail because we can not return binary data over Conduit in JSON.

Although Conduit will eventually support binary-safe encodings, a cleaner approach to this is just to return a `filePHID` instead of the raw content. This is generally faster and more flexible, and gives us more opportunities to add caching later.

After making the call, the client pulls the file data separately.

We also no longer need to return a complex data structure because we don't do blame over this call any longer.

Test Plan:
  - Viewed images in Diffusion.
  - Viewed READMEs in Diffusion.
  - Used `bin/differential attach-commit rX Dy` to hit attach pathway.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9319

Differential Revision: https://secure.phabricator.com/D14970
2016-01-08 09:29:16 -08:00
epriestley
741118a08f Improve Diffusion behavior for directories with impressive numbers of files
Summary:
Fixes T4366. Two years ago, Facebook put 16,000 files in a directory. Today, the page has nearly loaded.

Paginate large directories.

Test Plan:
  - Viewed home and browse views in Git, Mercurial and Subversion.

I put an artificially small page size (5) on home:

{F1055653}

I pushed 16,000 files to a directory and paged through them. Here's the last page, which rendered in about 200ms:

{F1055655}

Our behavior is a bit better than GitHub here, which shows only the first 1,000 files, disables pagination, and can't retrieve history for the files:

{F1055656}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4366

Differential Revision: https://secure.phabricator.com/D14956
2016-01-06 14:19:55 -08:00
epriestley
9728c65e93 Drive blame generation through diffusion.blame
Summary:
Ref T2450. Ref T9319. This is still a bit messy, but not quite so bad as it was: instead of using a single call to get both blame information and file content, use `diffusion.blame` for blame information.

This will make optimizations to both blame and file content easier.

Test Plan: Viewed a bunch of blame (color on/off, blame on/off).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2450, T9319

Differential Revision: https://secure.phabricator.com/D14958
2016-01-06 09:24:21 -08:00
epriestley
f561dc172d Implement a dedicated "diffusion.blame" API method
Summary:
Fixes T2451. Several motivations here, from strongest to weakest:

  - Currently, getting blame and file content are closely entwined. This makes fixing T9319 more difficult, and I want to fix it. I want to separate blame from content so there's more flexibility in how we approach this issue.
  - This makes pursuing T2450 easier, if it turns out to be a meaningful win.
  - If we can get a win on blame performance, we can do `arc blame` eventually if we want.

Test Plan:
  - Blamed in SVN, Git and Mercurial.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2451

Differential Revision: https://secure.phabricator.com/D14957
2016-01-06 09:24:03 -08:00
epriestley
f1c298203a Return no results from grep repository queries on error
Summary: Fixes T7852. Although `1` could also indicate other kinds of problems, assume it means "no results".

Test Plan: Searched for nonsense strings in Git and Mercurial. Searched for valid strings in Git and Mercurial.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7852

Differential Revision: https://secure.phabricator.com/D14943
2016-01-05 13:59:12 -08:00
epriestley
9febfb26a0 Convert diffusion.looksoon to use repository identifiers instead of callsigns
Summary:
Ref T4245. Like everything else, accept more identifiers.

This needs a change in `arc`, which I've made a note about elsewhere.

Test Plan: Used "Update Now" from web UI, saw update get scheduled.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14932
2016-01-04 06:54:20 -08:00
epriestley
b1388c5ca1 Remove diffusion.getcommits Conduit API method
Summary:
Ref T4245. This was obsoleted long ago and has no callers in Phabricator or Arcanist.

Also some minor cleanup.

Test Plan: `grep` for callers everywhere.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14930
2016-01-04 06:53:45 -08:00
epriestley
35e7a1f3c0 Continue reducing callers to getCallsign()
Summary: Ref T4245. More of the same, just narrowing down the easy cases.

Test Plan:
- Called `diffusion.querycommit`.
- Browsed branches.
- Browsed repository.
- Browsed directory.
- Searched for stuff.
- Viewed a commit.
- Viewed a file diff.
- Edited a commit.
- Viewed history.
- Viewed tags.
- Viewed push log.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14929
2016-01-04 06:53:32 -08:00
epriestley
9d84eb4c74 Allow Conduit API methods in Diffusion to accept any repository identifier
Summary:
Ref T4245. Broaden support to include "ABCD", "rABCD", "1234", "R1234", etc.

This doesn't change the old behavior, just accepts more stuff.

Test Plan:
  - Browsed Diffusion.
  - Made various calls via API console.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14928
2016-01-04 06:51:04 -08:00
David Fisher
c589af51e8 add "update" mode to Diffusion coverage Conduit
Summary:
This diff adds a new mode argument to the Diffusion Conduit API with two options:
- "overwrite": the default, maintains the current behavior of deleting all coverage
  in the specified branch before uploading the new coverage
- "update": does not delete old coverage, but will overwrite previous
  coverage information if it's for the same file and commit

`DiffusionRequest::loadCoverage` already loads a file's coverage from the
latest available commit, so uploading coverage for different files in different
commits with "update" will result in seeing the latest uploaded coverage in
Diffusion.

Test Plan: manual local verification

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14428
2015-11-09 16:52:34 -08:00
epriestley
b2e89a9e48 Fix several error handling issues with Subversion commits in Diffusion
Summary:
Ref T9513. I checked this briefly but didn't do a very thorough job of it.

  - Don't try to query merges for Subversion, since it doesn't support them.
  - Fix up "existsquery" to work properly (and efficiently) for both hosted and imported repositories.
  - Fix up "parentsquery" to have similar behavior on invalid commits to other VCSes (throw an exception).

Test Plan:
  - No more merges warning on SVN.
  - Hosted SVN gets the right exists result now.
  - Visiting "r23980283789287" now 404's instead of "not parsed yet".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9513

Differential Revision: https://secure.phabricator.com/D14239
2015-10-05 15:57:41 -07:00
epriestley
e96cd29eff Reduce badness in viewing large files in the Diffusion web UI
Summary:
Fixes T8597. Second issue there is that if you look at a huge file in Diffusion (like `/path/to/300MB.pdf`) we pull the whole thing over Conduit upfront, then try to shove it into file storage.

Instead, pull no more than the chunk limit (normally 4MB) and don't spend more than 10s pulling data.

If we get 4MB of data and/or time out, just fail with a message in the vein of "this is a really big file".

Eventually, we could improve this:

  - We can determine the //size// of very large files correctly in at least some VCSes, this just takes a little more work. This would let us show the true filesize, at least.
  - We could eventually stream the data out of the VCS, but we can't stream data over Conduit right now and this is a lot of work.

This is just "stop crashing".

Test Plan: Changed limits to 0.01 seconds and 8 bytes and saw reasonable errors. Changed them back and got normal beahvior.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8597

Differential Revision: https://secure.phabricator.com/D13348
2015-06-18 13:06:01 -07:00
Joshua Spence
36e2d02d6e phtize all the things
Summary: `pht`ize a whole bunch of strings in rP.

Test Plan: Intense eyeballing.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12797
2015-05-22 21:16:39 +10:00
Joshua Spence
f8b7f03145 Change "lint save" to not use Arcanist Projects
Summary: Ref T7604. Change `DiffusionLintSaveRunner` to use repositories instead of Arcanist Projects.

Test Plan: Ran the `save_lint.php` script and queried results using the `diffusion.getlintmessages` Conduit endpoint.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12893
2015-05-19 00:07:47 +10:00
epriestley
7f8ee4e329 Fix a bug with DiffusionMergedCommitsQueryConduitAPIMethod
Summary: We were adding "+1" to PHP_INT_MAX, which could cause us to wrap and run `git log -n -9223372036854775808 ...`. Git, unable to give us negative nine trillion zillion results, would then exit without producing results.

Test Plan: Got results out of this method.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12726
2015-05-05 15:44:34 -07:00
Joshua Spence
2cc898a63f Add repository parameter to diffusion.findsymbols method
Summary: Ref T7977. Add a repository parameter to the `diffusion.findsymbols` Conduit method to allow restricting search results to the specified repository.

Test Plan: Queried the Conduit endpoint.

Reviewers: avivey, epriestley, #blessed_reviewers

Reviewed By: avivey, epriestley, #blessed_reviewers

Subscribers: eadler, avivey, Korvin, epriestley

Maniphest Tasks: T7977

Differential Revision: https://secure.phabricator.com/D12663
2015-05-05 08:27:08 +10:00
Joshua Spence
2483f6f120 Move symbols to be repository-based
Summary: Fixes T7220. Ref T7977. Changes symbols from being bound to an Arcanist project to being bound to a repository.

Test Plan:
- Added symbols and then applied migrations, symbols seemed to be migrated successfully.
- Tested the `/diffusion/symbol/$SYMBOL_NAME` endpoint.
- Tested the `/diffusion/symbol/$SYMBOL_NAME` endpoint with the `?repositories=$REPOSITORY_PHID` parameter.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: avivey, Korvin, epriestley

Maniphest Tasks: T7977, T7220

Differential Revision: https://secure.phabricator.com/D12608
2015-05-03 13:23:07 +10:00
epriestley
2a37459a5f Only resolve branch names to branches
Summary: Fixes T7100. In the bizarre case that a Git repository has a branch and tag with the same name, don't resolve branch names into tag names.

Test Plan: Test repo with branch and tag both named "git" no longer reports ambiguity.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7100

Differential Revision: https://secure.phabricator.com/D12553
2015-04-27 03:51:53 -07:00
epriestley
d3436c256c Ignore closed branch heads by default in Mercurial
Summary:
Fixes T6160. Ref T7100.

  - When resolving ambiguous branch references, ignore closed heads unless there are no other options.
  - Hide closed heads by default on the main page.
  - Show branch open/closed state in Mercurial.

Test Plan: Browsed a previously-ambiguous Mercurial repository because of multiple branch heads, no longer ambiguous.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6160, T7100

Differential Revision: https://secure.phabricator.com/D12552
2015-04-27 03:51:21 -07:00
epriestley
156b156e77 Give Conduit params/return/errors protected visibility
Summary:
Ref T7803. Ref T5873. I want to drive Conduit through more shared infrastructure, but can't currently add parameters automatically.

Put a `getX()` around the `defineX()` methods so the parent can provide default behaviors.

Also like 60% of methods don't define any special error types; don't require them to implement this method. I want to move away from this in general.

Test Plan:
  - Ran `arc unit --everything`.
  - Called `conduit.query`.
  - Browsed Conduit UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: hach-que, epriestley

Maniphest Tasks: T5873, T7803

Differential Revision: https://secure.phabricator.com/D12380
2015-04-13 11:58:35 -07:00
Bob Trahan
3fcc3fdedf Diffusion - be sure to properly unserialize result from conduit query
Summary: Fixes T7256.

Test Plan: Looked at rXPRF0a7a5f69f5d7 in a local instance. things looked great both pre and post patch.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7256

Differential Revision: https://secure.phabricator.com/D11790
2015-02-17 13:54:59 -08:00
Bob Trahan
e1dcbc4386 Policy - lock down DiffusionSymbolQuery repo-loading code
Summary: Ref T7094.

Test Plan: couldn't really test this - how does one get symbols going nowadays given they are acanist project based?

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7094

Differential Revision: https://secure.phabricator.com/D11584
2015-01-31 18:36:36 -08:00
Bob Trahan
bdb3adeee4 Policy - clean up the deprecated diffusion.getcommits
Summary: Ref T7094. Could just delete this end point too I guess? Needed to add "withCommitPHIDs" to the differentialrevisionquery to get this done.

Test Plan: used diffusion.getcommits from conduit console and got a sensible result for a query for two commits, one with a diff and one without.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7094

Differential Revision: https://secure.phabricator.com/D11581
2015-01-30 11:51:16 -08:00
epriestley
8d087ae738 Remove 'initFromConduit' option from Diffusion
Summary:
Ref T2783. I think this served two purposes:

  - Improving performance in cases where we "know" a repository is local.
  - Preventing loops.

It is now obsolete:

  - After D11476, refs can almost always resolve on a fast path.
  - As T2783 moves forward, we can usually no longer know when a repository is local without actually looking it up -- almost everything is allowed to run anywhere.
  - The cluster behavior in D11475 now prevents loops.

Test Plan: `grep`, browsed around. This didn't really do much of anything anymore.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2783

Differential Revision: https://secure.phabricator.com/D11477
2015-01-23 13:31:45 -08:00
epriestley
d94d1da610 Proxy Diffusion Conduit API calls
Summary:
Fixes T7020. When an external user makes a Conduit request to Diffusion but the repository isn't hosted locally, we need to proxy it.

This also adds a guard layer to prevent requests from getting infinitely proxied inside the cluster.

In "trivial" configurations (where the repository is a service repository, but the service is on the local device) I'm making us always proxy anyway. This basically makes it reasonable to test this stuff (otherwise you'd have to set up two different installs) and this configuration doesn't make much sense in real life (if you're using multiple machines, making one a dedicating daemons+repo box is almost certainly the most reasonable configuration, even for a cluster size of 2).

Test Plan:
  - With a service-hosted repository, made Diffusion conduit calls and browsed the UI. Verified requests got proxied once, then resovled.
  - With a non-service repository, made Diffusion conduit calls and browsed UI. Verified requests were handled in-process immediately.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7020

Differential Revision: https://secure.phabricator.com/D11475
2015-01-23 13:30:52 -08:00
epriestley
cae8c49745 Fix diffusion.readmequery to work in a cluster enviroment
Summary:
Ref T2783. This method is kind of goofballs:

  - We send a big list of paths to it.
  - It sends back a giant blob of HTML.

Instead, just figure out the path we want locally, then fetch the content with `diffusion.filecontentquery`.

Test Plan:
  - Viewed main view and directory view, saw a README.
  - See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2783

Differential Revision: https://secure.phabricator.com/D11099
2014-12-31 11:54:52 -08:00
Joshua Spence
39ca2fdf64 Use new FutureIterator instead of Futures
Summary: Ref T6829. Deprecate the `Futures()` function.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6829

Differential Revision: https://secure.phabricator.com/D11077
2014-12-30 23:13:38 +11:00
Joshua Spence
3cf9a5820f Minor formatting changes
Summary: Apply some autofix linter rules.

Test Plan: `arc lint` and `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D10585
2014-10-08 08:39:49 +11:00
epriestley
ac4247ea59 Provide more information from diffusion.querycommits
Summary:
Ref T2783. Fixes T6039.

  - Provide `authorPHID` and `committerPHID` to resolve T6039.
  - In message parser, store author/email strings.
  - In cached results, emit author/email strings.

Test Plan: Called method with and without bypassCache. Used `reparse.php` to repopulate data on an old commit.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2783, T6039

Differential Revision: https://secure.phabricator.com/D10424
2014-09-05 12:27:55 -07:00
James Rhodes
d7f51325e3 Populate results of DiffusionQueryCommitsConduitAPIMethod with DiffusionLowLevelCommitQuery
Summary:
Ref T2783.  This populates the following fields in DiffusionQueryCommitsConduitAPIMethod using DiffusionLowLevelCommitQuery when `bypassCache` is set to true:

  * `authorName`
  * `authorEmail`
  * `committerName`
  * `committerEmail`
  * `message`
  * `hashes`

The original outline called for `authorPHID` and `committerPHID` as well (but no `message` field).  As far as I can tell, the PHIDs aren't actual a property on `DiffusionCommitRef`, and since the intention of this is to be able to populate a `DiffusionCommitRef`, I haven't included them.  Let me know if we really do need the PHIDs here.

Test Plan: Tested using 3 Phabricator instances (one web, one taskmaster and one storage).  The web and taskmaster tiers are directed at the Conduit API of the storage tier.  Made a `diffusion.querycommits` from the Conduit app on the web tier instance and saw the data populated from the raw VCS data (located on the storage tier).

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T2783

Differential Revision: https://secure.phabricator.com/D10399
2014-09-03 22:49:44 +10:00
Bob Trahan
d1936711a0 Diffusion - replace last hg manifest call with hg locate
Summary: Fixes T4387.

Test Plan: Setup a mercurial repository for rabbitmq-server. Browsed around it and things looked good.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4387

Differential Revision: https://secure.phabricator.com/D10380
2014-08-28 13:08:42 -07:00
Bob Trahan
c1e8d97069 Diffusion - re-jigger how README files get rendered
Summary: be more aggressive about assuming plain-text, use remarkup for no extension, .remarkup, and .md, and last but not least use rainbow for .rainbow. Fixes T5818.

Test Plan: my README rendered just fine post these changes

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: asherkin, epriestley, Korvin

Maniphest Tasks: T5818

Differential Revision: https://secure.phabricator.com/D10340
2014-08-22 15:49:03 -07:00