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

507 commits

Author SHA1 Message Date
Aviv Eyal
5d1489a3ce Search repos by Project
Summary:
Ref T3102

In diffusion, add "In Any Project" to search options.

Test Plan: use it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3102

Differential Revision: https://secure.phabricator.com/D8113
2014-01-30 11:45:43 -08:00
epriestley
c41b4cfac0 Allow Git and Mercurial repositories to be cloned with names in the URI
Summary:
Ref T4175. This allows these URIs to all be valid for Git and Mercurial:

  /diffusion/X/
  /diffusion/X/anything.git
  /diffusion/X/anything/

This mostly already works, it just needed a few tweaks.

Test Plan: Cloned git and hg working copies using HTTP and SSH.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T4175

Differential Revision: https://secure.phabricator.com/D8098
2014-01-30 11:42:25 -08:00
epriestley
ffeee37810 Add "Clone As" to repositories and generate full clone commands in UI
Summary:
Ref T4175.

  - Add a configurable name for the clone-as directory, so you can have "Bits & Pieces" clone as "bits~n~pieces/" or simliar.
  - By default, use "reasonable" heruistics to choose such a name.
  - Generate a copy/pasteable clone commmand with this directory name.

Test Plan: Looked at some repositories.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T4175

Differential Revision: https://secure.phabricator.com/D8097
2014-01-30 11:42:10 -08:00
epriestley
96dd530c44 Distinguish between "Remote URI" and "Clone URI" in Repositories
Summary:
Hosted repositories have muddied this distinction somewhat. In some cases, we only want to use the real remote URI, and the call is only relevant for imported repositories.

In other cases, we want the URI we'd plug into `git clone`.

Move this logic into `PhabricatorRepository` and make the distinction more clear.

Test Plan: Viewed SVN, Git, and Mercurial hosted and remote repositories, all the URIs looked reasonable.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, dctrwatson

Differential Revision: https://secure.phabricator.com/D8096
2014-01-30 11:41:21 -08:00
epriestley
f585f6fddd Enable "Allow Dangerous Changes" config for Mercurial repositories
Summary:
Fixes T4357. The Mercurial commit hooks enforce dangerous change protection, but the UI doesn't actually let you configure it.

This was just an oversight (I never went back and enabled it) -- allow it to be configured in the UI.

Test Plan: Clicked "Edit Repository" on a hosted Mercurial repository, saw option to enable dangerous changes.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T4357

Differential Revision: https://secure.phabricator.com/D8108
2014-01-30 09:45:03 -08:00
epriestley
4a6239d53c Fix an issue which could prevent new repositories from being marked imported. 2014-01-30 09:43:53 -08:00
Richard van Velzen
28fcb5711b Add basic support for mirroring Mercurial repositories
Summary:
Ref T4338. Mercurial exits with exit code 1 and "no changes found" in stdout
when there's no changes. I've split up the `pushRepositoryToMirror` to make
the code a tad more readable.

It isn't perfect, but it works for me.

Test Plan:
pushed some changes to my hosted repo. Saw them appearing in the
mirrored repo

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4338

Differential Revision: https://secure.phabricator.com/D8107
2014-01-30 08:37:12 -08:00
epriestley
e293d39b5f Split the difference on remote URIs.
This is slightly trickier than D8082.

Auditors: btrahan
2014-01-27 19:44:39 -08:00
epriestley
f2bc293a2a Use remote URI, not display URI, to match repositories
Summary:
I derped this up, and it passed my tests because the two URIs are the same for hosted repositories.

Match repositories against their remote URI (like `https://github.com/user/repo.git`), not their display URI (like `/diffusion/X/`).

Test Plan: i r dums

Reviewers: talshiri, btrahan

Reviewed By: talshiri

CC: aran

Differential Revision: https://secure.phabricator.com/D8082
2014-01-27 18:49:35 -08:00
epriestley
014a873773 Update DifferentialDiff: add repositoryPHID, drop parentRevisionID
Summary:
Moves away from ArcanistProjects:

  - Adds storage for diffs to be directly associated with a repository (instead of indirectly, through arcanist projects). Not really populated yet.
  - Drops `parentRevisionID`, which is obsoleted by the "Depends On" edge. This is not exposed in the UI anywhere and doesn't do anything. Resolves TODO.

Test Plan: Ran storage upgrades, browsed around, lots of `grep`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8072
2014-01-26 15:29:22 -08:00
epriestley
e34a44a7ed Allow repository lookup by remote URI
Summary: This helps us move away from arcanist projects in normal use, by allowing us to identify repositories based on the remote URI.

Test Plan: Used `repository.query` to query some repos by remote URI.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8069
2014-01-25 14:02:38 -08:00
epriestley
ce45c57057 Add more options to repository.query
Summary: Expose more options on `repository.query`. My broad goal here is to move forward on suppressing Arcanist Projects.

Test Plan: Used Conduit console to execute queries.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8068
2014-01-25 14:02:28 -08:00
epriestley
31e11a97d2 If repository mirroring fails, keep trying the other mirrors
Summary: Ref T4338. Currently, if you have several mirrors and the first one fails, we won't try the other mirrors (since we'll throw and that will take us out of the mirroring process). Instead, try each mirror even if one fails, and then throw an AggregateException with all the failures.

Test Plan:
  - Ran `bin/repository mirror` normally.
  - Faked an exception, ran again, got the AggregateException I expected.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4338

Differential Revision: https://secure.phabricator.com/D8067
2014-01-25 14:02:09 -08:00
epriestley
dd944f7d83 Separate repository mirroring into an Engine and provide bin/repository mirror
Summary:
Ref T4338. Currently, there's no diagnostic command to execute mirroring (so I can't give users an easy command to run), and it's roughly the last piece of real logic left in the PullLocal daemon.

Separate mirroring out, and provide `bin/repository mirror`.

Test Plan:
  - Ran `bin/repository mirror` to mirror a repository.
  - Ran PullLocalDaemon and verified it also continued mirroring normally.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4338

Differential Revision: https://secure.phabricator.com/D8066
2014-01-25 14:01:58 -08:00
Brecht Van Lommel
f007ed6263 Allow repository mirrors even if the repository is hosted elsewhere.
Summary:
I'm not sure if this is desired functionality, but we happen to need
mirroring of our repository which is not hosted by phabricator, and as far as
I can tell the mirroring code does not depend on the hosting code.

Test Plan:
Setup mirror with SSH credentials to github, pushed changes to
elsewhere hosted repository, commits got mirrored to github.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4338

Differential Revision: https://secure.phabricator.com/D7637
2014-01-25 12:02:50 -08:00
epriestley
c21a8d31dd When repairing Git remote URIs, include credentials
Summary: See IRC. Currently, if a fetch fails, we may repair the remote URI incorrectly, replacing it with one without any credentials. Instead, retain credentials.

Test Plan: Faked it locally, got le-boom to verify on IRC.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8055
2014-01-23 17:23:38 -08:00
epriestley
be59578794 Fix bin/repository importing for CLOSEABLE flag
Summary: After adding the CLOSEABLE flag, `bin/repository importing` reports CLOSEABLE commits with no information. Just don't report these, for consistency with the old behavior. Adding flags to show them might be nice at some point if we run into issues where that output would be useful.

Test Plan: Ran `bin/repositroy importing` on a repository with CLOSEABLE commits.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8024
2014-01-21 14:09:51 -08:00
epriestley
c9a0ffa1cf Verify that SVN repository roots really are repository roots
Summary: Fixes T3238. Ref T4327. Although the instructions are fairly clear on this, it's easy to miss them. Make sure the root the user enters matches the real root.

Test Plan: Added unit tests. Used `bin/repository discover` to hit the check explicitly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3238, T4327

Differential Revision: https://secure.phabricator.com/D8020
2014-01-21 14:02:58 -08:00
epriestley
ef2c9861ef Add yet more unit tests for Subversion
Summary: Ref T4327. This adds a subpath/partial repository where we import only a subdirectory, and adds tests for it.

Test Plan: Ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D8019
2014-01-21 14:02:48 -08:00
epriestley
6c860bf850 Add even more Subversion unit tests
Summary:
Ref T4327. Adds additional tests:

  - Property changes on the root directory (which has special handling).
  - Copying a file from a previous revision (this is `svn cp svn://server.com/root/some_file@123 some_file`).
  - "Multicopying" a file: this is where a file is copied to several places and moved in the same commit.

Test Plan: Ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D8018
2014-01-21 14:02:40 -08:00
epriestley
9dde415884 Add more Subversion unit tests
Summary:
Ref T4327. Adds additional SVN tests to cover:

  - replacing a file with a file;
  - replacing a file with a directory;
  - removing a directory with files in it;
  - adding a directory with files;
  - copying a directory and adding and deleting files inside it.

Test Plan: Ran unit tests. One of these has a slightly irregular parse, see inline.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D8017
2014-01-21 14:02:32 -08:00
epriestley
dc74da0abe Add basic test coverage for the Subversion parser
Summary:
Ref T4327. Adds the same basic battery of tests to the SVN parser as the other parsers have.

cowsay {{{ THIS IS AN AMAZING DIFF }}}

Test Plan:
Ran unit tests against SVN change parsing.

bwahaha

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D8016
2014-01-20 17:25:04 -08:00
epriestley
c951d7e13b Add unit tests for the Mercurial change parser
Summary: Ref T4327. This adds a set of test cases for the Mercurial parser. These tests are nearly identical to the git cases, except that the Mercurial parser can't figure out symlinks right now.

Test Plan: Ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D8014
2014-01-20 13:14:18 -08:00
epriestley
f63e7571e5 Add unit tests for Git change parsers
Summary: Ref T4327. Adds some basic tests to the Git parser for a set of common operations (add, change, move, copy, directory, symlink, propchange).

Test Plan: Ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D8012
2014-01-20 13:14:04 -08:00
epriestley
f7a1feea38 Begin making change parsers testable
Summary:
Ref T4327. There are a bunch of other probably-related tasks too, some linked there.

We have some rare/unusual bugs in the change parsers, mostly in Subversion, but it's terrifying to touch them because they're complicated and fragile and have no test coverage.

To fix this stuff, I want to make them more testable. In particular, they basically end with this big INSERT right now. Instead, I'm going to make them return objects representing the data to be inserted, then have the common infrastructure do the insert. This gives us two benefits:

  - Reduced code duplication on the insert;
  - we can stop before the insert and have unit tests examine the objects.

This swaps the Git parser over, but doesn't swap the hg/svn parsers yet. I'll do those separately, the SVN one looks a bit tricky.

Test Plan:
  - Used `scripts/repository/reparse.php` to reparse a Git commit, with `--trace`. Verified it looked the same as before and the SQL that was executed seemed reasonable.
  - Did the same for `hg` / `svn` commits, to make sure I didn't derp anything. These aren't expected to do anything differently.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D7980
2014-01-20 13:12:44 -08:00
epriestley
a9c16fbe4e Reduce parse latency for changes pushed to hosted repositories
Summary:
Currently, we can sit in a sleep() for up to 15 seconds (or longer, in some cases), even if there's a parse request.

Try polling the DB every second to see if we should wake up instead. This might or might not produce significant improvements, but seems OK to try and see.

Also a small fix for logging branch names with `%` in them, etc.

Test Plan: Ran `phd debug pulllocal` and pushed commits, saw them parse within a second.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, dctrwatson

Differential Revision: https://secure.phabricator.com/D7998
2014-01-17 16:09:51 -08:00
epriestley
8520d9e070 Move more discovery responsibilities into DiscoveryEngine
Summary:
Ref T4327. This moves the last pieces of discovery responsibility out of the PullLocal daemon and into the DiscoveryEngine.

(This makes it easier to discover repositories in unit tests in the future, since we don't need to build a PullLocal daemon and can just build a DiscoveryEngine.)

Test Plan: Ran `phd debug pulllocal`. Ran `repostory discover`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D7997
2014-01-17 16:09:24 -08:00
epriestley
220addb249 Move Git discovery into DiscoveryEngine
Summary:
Ref T4327. Consolidates and simplifies infrastructure:

  - Moves Git discovery into DiscoveryEngine.
  - Collapses a bunch of the Git and Mercurial code related to stream discovery.
  - Removes all cach code from PullLocal daemon (it's no longer called).
  - Adds basic unit tests for Git and Mercurial discovery.

Various cleanup:

  - Makes GitStream and MercurialStream extend a common base.
  - Improves performance of MercurialStream in some cases, by requiring fewer commits be output and parsed.
  - Makes mirroring exceptions easier to debug.
  - Fixes discovery of Mercurial repositories with multiple branch heads.
  - Adds some missing `pht()`.

Test Plan:
I tested this fairly throughly because I think this phase is complete:

  - Made new repositories in multiple VCSes and did full imports.
    - Particularly, I reimported Arcanist to make sure that TODO was resolved. I think it was related to the toposort stuff.
  - Pushed commits to multiple VCSes.
  - Pushed commits to a non-close branch, then pushed a merge commit. Observed commits import initially as non-close, then get flagged for close.
  - Started full daemons and resolved various minor issues that showed up in the daemon log until everything ran cleanly.
  - Basically spent about 30 minutes banging on this in every way I could think of to try to break it. I found and fixed some minor stuff, but it seems solid.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D7987
2014-01-17 11:48:53 -08:00
epriestley
618da2265d Remove all the multi-pass autoclose-branch separate-cache / seenOnBranches junk
Summary:
Ref T4327. Simplify the git discovery process so I can move it to the DiscoveryEngine, so I can make change parsing testable.

In particular:

  - As an optimization, we process closeable branches ("master") first, then process uncloseable branches ("epriestley-devel"). This means that in the common case we can insert a commit as closeable immediately when it is discovered, the first pass through the pipeline will get it right, and the "ref update" step will never need to do any meaningful work.
  - Commits which do not initially appear on a closeable branch, but later move to one (via merges or ref moves) will now be caught in the ref update step, have the closeable flag set, and have a message step re-queued.
  - We no longer need to do a separate discovery step on closable branches.
  - We no longer need to keep track of `seenOnBranches`.

Test Plan: Ran discovery on repositories after pushing commits, got reasonable results.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D7985
2014-01-17 11:48:53 -08:00
epriestley
cec3f44b90 Provide an alternate, more general "closeable" flag for commits
Summary:
Ref T4327. This provides a more general RefCursor-based way to identify closeable commits, and moves away from the messy `seenOnBranches` stuff. Basically:

  - When a closeable ref (like the branch "master") is updated, query the VCS for all commits which are ancestors of the new ref, but not ancestors of the existing closeable heads we previously knew about. This is basically all the commits which have been merged or moved onto the closeable ref.
  - Take these commits and set the "closeable" flag on the ones which don't have it yet, then queue new tasks to reprocess them.

I haven't removed the old stuff yet, but will do that shortly.

Test Plan:
  - Ran `bin/repository discover` and `bin/repository refs` on a bunch of different VCSes and VCS states. The effects seemed correct (e.g., new commits were marked as closeable).

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D7984
2014-01-17 11:48:53 -08:00
epriestley
f4b9efe256 Introduce ref cursors for repository parsing
Summary:
Ref T4327. I want to make change parsing testable; one thing which is blocking this is that the Git discovery process is still part of `PullLocal` daemon instead of being part of `DiscoveryEngine`. The unit test stuff which I want to use for change parsing relies on `DiscoveryEngine` to discover repositories during unit tests.

The major reason git discovery isn't part of `DiscoveryEngine` is that it relies on the messy "autoclose" logic, which we never implemented for Mercurial. Generally, I don't like how autoclose was implemented: it's complicated and gross and too hard to figure out and extend.

Instead, I want to do something more similar to what we do for pushes, which is cleaner overall. Basically this means remembering the old branch heads from the last time we parsed a repository, and figuring out what's new by comparing the old and new branch heads. This should give us several advantages:

  - It should be simpler to understand than the autoclose stuff, which is pretty mind-numbing, at least for me.
  - It will let us satisfy branch and tag queries cheaply (from the database) instead of having to go to the repository. We could also satisfy some ref-resolve queries from the database.
  - It should be easier to extend to Mercurial.

This implements the basics -- pretty much a table to store the cursors, which we update only for Git for now.

Test Plan:
  - Ran migration.
  - Ran `bin/repository discover X --trace --verbose` on various repositories with branches and tags, before and after modifying pushes.
  - Pushed commits to a git repo.
  - Looked at database tables.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D7982
2014-01-17 11:48:53 -08:00
epriestley
0ac58d7db6 Move repository URI normalization out of PullLocalDaemon
Summary: Ref T4327. Moves us one small step forward toward testable change parsers by separating out this unrelated logic from the PullLocal daemon. We will also probably want to run this logic so we can do remote path lookups to limit the role of Arcanist Projects in the future, which is why I made the URI type (here, only "git") a parameter rather than calling this a `GitURINormalizer` or something.

Test Plan:
  - Ran unit tests.
  - Ran `repository discover` on a correctly-configured remote repository.
  - Ran `repository discover` on an incorrectly-configured remote, got an error message.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

Differential Revision: https://secure.phabricator.com/D7981
2014-01-17 11:48:52 -08:00
epriestley
a716fe99f3 Perform search indexing in the worker queue and respect bin/search index --background
Summary: Fixes T3857. Earlier work made this trivial and just left product questions, which I've answered by requiring the daemons to run on reasonable installs.

Test Plan: Ran `bin/search index` and `bin/search index --background`. Observed indexes write in the former case and tasks queue in the latter case. Commented with a unique string on a revision and searched for it a moment later, got exactly one result (that revision), verifying that reindexing works correctly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3857

Differential Revision: https://secure.phabricator.com/D7966
2014-01-14 13:22:56 -08:00
John Watson
2ee4507486 Add Hosted/Remote filtering to Diffusion
Test Plan: Did searches in Diffusion using all 3 Hosted values

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7927
2014-01-10 09:12:50 -08:00
epriestley
efe187d5be Support "Repository's projects" field in Commit and Differential Revision rules
Summary: This also cleans up some code a little bit. Most of the gymnastics are to make sure we call `needProjectPHIDs()` appropriately.

Test Plan: Created new commit and revision rules with this field. Ran commits and revisions through the test console. Field behavior seemed correct.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, dctrwatson

Differential Revision: https://secure.phabricator.com/D7923
2014-01-09 15:56:24 -08:00
epriestley
972dfa7bfc Add 'hook.d/' directories to SVN and Git repositories for custom hooks
Summary:
Fixes T4189. Ref T4151. Allows repositories to have additional custom hooks for operations which can't be expressed with Herald (one such operation is lint).

This adds only local hook directories, since they're easier to use with existing hooks than global directories. I might add global directories eventually.

This doesn't support Mercurial since we have no demand for it and it's more complicated (we lose compatibility and power by just dropping a `hooks.d/` somewhere).

Test Plan:
  - Pulled hosted SVN and Git repos to verify the hook directories generate correctly.
  - Added a variety of hooks to the hook directories (echo + pass, fail).
  - Pushed commits and verified the hooks fired (output expected info, or failed).
  - Verified push log reflected the correct error code ("3", external) and detail ("nope.sh") when rejecting.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4151, T4189

Differential Revision: https://secure.phabricator.com/D7884
2014-01-03 12:26:10 -08:00
epriestley
637e3f38f3 Allow repositories to be associated with projects
Summary: Ref T4264. Ref T2628. Ref T3102. Allows you to associate repositories with projects. In the future, you'll be able to write Herald object rules against projects, use Herald fields like "Repository's projects", and search by project.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3102, T4264, T2628

Differential Revision: https://secure.phabricator.com/D7881
2014-01-03 12:24:09 -08:00
iodragon
f981d50f5d Set php PATH for repository's hook
Summary:
~~Set PATH for repository's hook, so the environment.append-paths can used~~
repository's hook may can't find php path if user's profile like bash_profile is not loaded.

Test Plan: check the hook generated is contain the right path

Reviewers: epriestley, #blessed_reviewers

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7743
2014-01-03 09:39:31 -08:00
epriestley
09341be10f Remove repository shortcuts
Summary:
Repositories currently have a no-UI "shortcut" feature which is only used by Facebook (and I'm not sure it's even used). As implemented, this feature is policy-oblivious and kind of nonsensical. Throw it away.

I'm open to reimplementing this, but I want to see some level of interest in it before I do. The new implementation would add shortcuts to each repository, similar to how mirrors work. My original plan was to follow this up with such an implementation (it's half-implemented in my sandbox), but as I worked through it I'm not sure it's really valuable.

Test Plan: Browsed repository list, grep.

Reviewers: btrahan

Reviewed By: btrahan

CC: FacebookPOC, aran

Differential Revision: https://secure.phabricator.com/D7862
2014-01-02 11:59:27 -08:00
epriestley
140c88e971 Implement basic object rules for Herald
Summary:
Ref T4264. Allows you to create "Object" rules, in addition to Global and Personal rules. If you choose to create an Object rule, you'll be prompted to select an object on a new screen. You must be able to edit and object in order to create rules for it.

Ref T3506. This makes "All" the default filter for the transcript view, which should reduce confusion on smaller installs.

Test Plan:
  - Created non-object rules.
  - Created object rules.
  - Triggered object rules against matching and unmatching objects.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3506, T4264

Differential Revision: https://secure.phabricator.com/D7853
2013-12-30 16:48:14 -08:00
epriestley
e397103bf2 Extend all "ManagementWorkflow" classes from a base class
Summary:
Ref T2015. Not directly related to Drydock, but I've wanted to do this for a bit.

Introduce a common base class for all the workflows in the scripts in `bin/*`. This slightly reduces code duplication by moving `isExecutable()` to the base, but also provides `getViewer()`. This is a little nicer than `PhabricatorUser::getOmnipotentUser()` and gives us a layer of indirection if we ever want to introduce more general viewer mechanisms in scripts.

Test Plan: Lint; ran some of the scripts.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2015

Differential Revision: https://secure.phabricator.com/D7838
2013-12-27 13:15:40 -08:00
epriestley
aad6b57c36 Add bin/harbormaster to make builds easier to debug
Summary:
Ref T1049. Adds `bin/harbormaster` and `bin/harbormaster build` for applying plans from the console. Since this gets `--trace`, it's much easier to debug what's going on.

This doesn't work properly with some of the Drydock steps yet, I need to look at those. I think `setRunAllTasksInProcess` probably obsoletes some of the mechanisms. It might also not work with "Wait for Builds" but I didn't check.

Test Plan: Used `bin/harbormaster` to run a bunch of builds. Ran builds from web UI.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7825
2013-12-26 10:40:52 -08:00
epriestley
81dcf6378d Make repository pull install hooks the first time
Summary:
Ref T4257. Currently, the pull logic looks like this:

  if (new) {
    create();
  } else {
    if (hosted) {
      install_hooks();
    } else {
      update();
    }
  }

This means that the first time you run `repository pull`, hooks aren't installed, which makes debugging trickier. Instead, reorganize the logic:

  if (new) {
    create();
  } else {
    if (!hosted) {
      update();
    }
  }

  if (hosted) {
    install_hooks();
  }

Test Plan: Ran `bin/repository pull` on a new `hg` repo and got hooks installed immediately.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4257

Differential Revision: https://secure.phabricator.com/D7818
2013-12-23 10:43:49 -08:00
epriestley
d9a04fcb53 Minor, make sure we release locks in pull daemon if we fail in unusual ways
Summary: A user is reporting a re-lock in this daemon, which I can't
reproduce, but might be possible if this throws. Stop it from throwing in
a way which evades unlock.

See: <https://github.com/facebook/phabricator/issues/476>

Auditors: btrahan
2013-12-22 08:51:07 -08:00
epriestley
c462713584 Minor cleanup for task rendering in Daemons
Summary:
Fixes two issues:

  - When rendering a task's details, we currently issue a policy-oblivious query. Instead, issue a policy-aware query.
  - The formatting is a little bit weird, with the top half in a box and the bottom half with an older style. Make them consistent.

Test Plan: Looked at the detail pages for several tasks in queue.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7812
2013-12-20 18:02:32 -08:00
epriestley
9c938701c3 Modernize Diffusion commitparentsquery
Summary: Ref T4195. Ref T2783. We have an old-school implementation of this; move it into a LowLevel query and make callers all run through Conduit. I need the LowLevel query for hooks, to implement an "is merge commit" Herald rule.

Test Plan:
  - Ran query via Conduit for SVN, Mercurial, Git.
  - Parsed a commit which closed a revision, attach/closed worked correctly.
  - Browsed Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195, T2783

Differential Revision: https://secure.phabricator.com/D7808
2013-12-20 12:39:21 -08:00
epriestley
d7c4edab28 Move commit message/metadata field query to a separate class
Summary: Ref T4195. I need to query commit metadata to figure out which revision a commit is associated with. Move this out of the MessageParser so the code can be called from the HookEngine.

Test Plan: Used `reparse.php` to reparse a variety of SVN, Mercurial and Git commits. Used `var_dump()` to verify sensible fields were returned.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7805
2013-12-20 12:38:44 -08:00
epriestley
23332241b2 Move commit hash querying to DiffusionLowLevelCommitQuery
Summary: Ref T4195. I need this for the Herald pre-commit rules, and it generally simplifies things.

Test Plan: Used `reparse.php` plus `var_dump()` to inspect refs in Git, Mercurial and SVN repos. They all looked correct and reparsed correctly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7804
2013-12-20 12:38:15 -08:00
epriestley
d667b12206 Provide a standalone query for resolution of commit author/committer into Phabricator users
Summary:
Ref T4195. To implement the "Author" and "Committer" rules, I need to resolve author/committer strings into Phabricator users.

The code to do this is currently buried in the daemons. Extract it into a standalone query.

I also added `bin/repository lookup-users <commit>` to test this query, both to improve confidence I'm getting this right and to provide a diagnostic command for users, since there's occasionally some confusion over how author/committer strings resolve into valid users.

Test Plan:
I tested this using `bin/repository lookup-users` and `reparse.php --message` on Git, Mercurial and SVN commits. Here's the `lookup-users` output:

  >>> orbital ~/devtools/phabricator $ ./bin/repository lookup-users rINIS3
  Examining commit rINIS3...
  Raw author string: epriestley
  Phabricator user: epriestley (Evan Priestley   )
  Raw committer string: null
  >>> orbital ~/devtools/phabricator $ ./bin/repository lookup-users rPOEMS165b6c54f487c8
  Examining commit rPOEMS165b6c54f487...
  Raw author string: epriestley <git@epriestley.com>
  Phabricator user: epriestley (Evan Priestley   )
  Raw committer string: epriestley <git@epriestley.com>
  Phabricator user: epriestley (Evan Priestley   )
  >>> orbital ~/devtools/phabricator $ ./bin/repository lookup-users rINIH6d24c1aee7741e
  Examining commit rINIH6d24c1aee774...
  Raw author string: epriestley <hg@yghe.net>
  Phabricator user: epriestley (Evan Priestley   )
  Raw committer string: null
  >>> orbital ~/devtools/phabricator $

The `reparse.php` output was similar, and all VCSes resolved authors correctly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1731, T4195

Differential Revision: https://secure.phabricator.com/D7801
2013-12-19 11:05:17 -08:00
epriestley
f750d5f8dc Provide a low-level SVN commit query, and merge the VCS query types
Summary: Ref T4195. Even though we use `svnlook` in the hook itself, I need this query elsewhere, so provide it and merge the classes into one which does the right thing.

Test Plan:
  - Used `reparse.php` to reparse messages for Git, SVN and Mercurial commits, using `var_dump()` to examine the commit refs for sanity.
  - Used `reparse.php` to reparse changes for an SVN commit.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7800
2013-12-19 11:05:06 -08:00