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

193 commits

Author SHA1 Message Date
Joshua Spence
14e3c727cc Allow Arcanist Projects to be deleted using ./bin/remove destroy.
Summary: Self-explanatory.

Test Plan: Created (and destroyed) an arcanist project. Verified that the deletion actually happened.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T4749

Differential Revision: https://secure.phabricator.com/D9353
2014-06-16 04:49:51 +10:00
epriestley
d401036bd8 Prevent the use of file:// URIs in Diffusion
Summary:
Via HackerOne. There are two attacks here:

  - Configuring mirroring to a `file://` URI to place files on disk or overwrite another repository. This is not particularly severe.
  - Configuring cloning from a `file://` URI to read repositories you should not have access to. This is more severe.

Historically, repository creation and editing explicitly supported `file://` URIs to deal with use cases where you had something else managing repositories on the same machine. Since there were no permissions, repository management was admin-only, and you couldn't mirror, this was fine.

As we've evolved, this use case is a tiny minority use case and the security implications of `file://` URIs overwhelm the utility it provides. Prevent the use of `file://` URIs. Existing configured repositories won't stop working, you just can't add any new ones.

Also prevent `localPath` from being set via Conduit (see T4039).

Test Plan:
  - Tried to create a `file://` repository.
  - Tried to create a `file://` mirror.
  - Tried to create a `file://` repository via Conduit.
  - Created a non-`file://` repository.
  - Created a non-`file://` mirror.
  - Created a non-`file://` repository via Conduit.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9513
2014-06-13 07:07:00 -07:00
Joshua Spence
0a62f13464 Change double quotes to single quotes.
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.

Test Plan: Eyeballed it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9431
2014-06-09 11:36:50 -07:00
Joshua Spence
0d03bbe43c Allow repositories to be deleted using ./bin/remove.
Summary: Currently, repositories can be deleted using `./bin/repository delete`. It makes sense to expose this operate to the `./bin/remove` script as well, for consistency.

Test Plan: Deleted a repository with `./bin/remove rTEST`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9350
2014-06-02 17:11:58 -07:00
epriestley
a74545c9da Provide a rough, unstable API for reporting coverage into Diffusion
Summary:
Ref T4994. This stuff works:

  - You can dump a blob of coverage information into `diffusion.updatecoverage`. This wipes existing coverage information and replaces it.
  - It shows up when viewing files.
  - It shows up when viewing commits.

This stuff does not work:

  - When viewing files, the Javascript hover interaction isn't tied in yet.
  - We always show this information, even if you're behind the commit where it was generated.
  - You can't do incremental updates.
  - There's no aggregation at the file (this file has 90% coverage), diff (the changes in this commit are 90% covered), or directory (the code in this directory has 90% coverage) levels yet.
  - This is probably not the final form of the UI, storage, or API, so you should expect occasional changes over time. I've marked the method as "Unstable" for now.

Test Plan:
  - Ran `save_lint.php` to check for collateral damage; it worked fine.
  - Ran `save_lint.php` on a new branch to check creation.
  - Published some fake coverage information.
  - Viewed an affected commit.
  - Viewed an affected file.

{F151915}

{F151916}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: jhurwitz, epriestley, zeeg

Maniphest Tasks: T5044, T4994

Differential Revision: https://secure.phabricator.com/D9022
2014-05-17 16:10:54 -07:00
epriestley
436f0563e8 Add a SublimeText-style repository typeahead
Summary:
Allows you to quickly search for files within a repository. Roughly:

  - We build a big tree of everything and ship it to the client.
  - The client implements a bunch of Sublime-ish magic to find paths.

Test Plan: {F154007}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley, zeeg

Differential Revision: https://secure.phabricator.com/D9087
2014-05-13 14:08:21 -07:00
epriestley
95eab2f3b0 Record parent relationships when discovering commits
Summary:
Ref T4455. This adds a `repository_parents` table which stores `<childCommitID, parentCommitID>` relationships.

For new commits, it is populated when commits are discovered.

For older commits, there's a `bin/repository parents` script to rebuild the data.

Right now, there's no UI suggestion that you should run the script. I haven't come up with a super clean way to do this, and this table will only improve performance for now, so it's not important that we get everyone to run the script right away. I'm just leaving it for the moment, and we can figure out how to tell admins to run it later.

The ultimate goal is to solve T2683, but solving T4455 gets us some stuff anyway (for example, we can serve `diffusion.commitparentsquery` faster out of this cache).

Test Plan:
  - Used `bin/repository discover` to discover new commits in Git, SVN and Mercurial repositories.
  - Used `bin/repository parents` to rebuild Git and Mercurial repositories (SVN repos just exit with a message).
  - Verified that the table appears to be sensible.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: jhurwitz, epriestley

Maniphest Tasks: T4455

Differential Revision: https://secure.phabricator.com/D9044
2014-05-12 11:47:22 -07:00
epriestley
692a28b5b2 Unfatal rendering of repository policy transactions
Summary: Fixes T4919. There's some special casing in Diffusion for CAN_PUSH right now, just accommodate that until things get more general.

Test Plan: Viewed a repository edit screen with a custom policy transaction. Clicked the link to view it.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4919

Differential Revision: https://secure.phabricator.com/D8898
2014-04-29 10:57:32 -07:00
Bob Trahan
2ecc04c159 Audit - move over to application search
Summary: ...also kills off "PhabricatorAuditCommitQuery" and "PhabricatorAuditQuery", by moving the work to "DiffusionCommitQuery". Generally cleans up some code around the joint on this too. Also provides policies for audit requests, which is basically the policy for the underlying commit. Fixes T4715. (For the TODO I added about files, I just grabbed T4713.)

Test Plan:
Audit: verified the three default views all showed the correct things, including highligthing. did some custom queries and got the correct results.
Diffusion: verified "blame view" still worked. verified paths were highlighted for packages i owned.
Home: verified audit boxes showed up with proper commits w/ audits
bin/audit: played around with it via --dry-run and got the right audits back

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: chad, epriestley, Korvin

Maniphest Tasks: T4715

Differential Revision: https://secure.phabricator.com/D8805
2014-04-27 09:43:05 -07:00
Chad Little
11fd6afeb1 Move Timeline icons to Fonts
Summary: Throwing this up for testing, swapped out all icons in timeline for their font equivelants. Used better icons where I could as well. We should feel free to use more / be fun with the icons when possible since there is no penalty anymore.

Test Plan: I browsed many, not all, timelines in my sandbox and in IE8. Some of these were just swagged, but I'm expecting we'll do more SB testing before landing.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8827
2014-04-22 08:25:54 -07:00
austinkelleher
2e5065feb5 Update function name to follow naming convention.
See: <http://github.com/facebook/phabricator/pull/575>

Reviewed by: epriestley
2014-04-20 08:37:37 -07:00
Bob Trahan
d5ded805b2 Herald - fix change type bug
Summary: wasn't working due to some type issues. Fixes T4756. I also made it display nicer while I was debugging this.

Test Plan: created a herald rule to block changes that added refs. git tag -a "test" -m "test test"; git push origin test got me blocked!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4756

Differential Revision: https://secure.phabricator.com/D8724
2014-04-08 11:58:28 -07:00
Bob Trahan
c6cbff1997 Differential - modernize "Local Commits" table
Summary: ...also link to commits we know about in "Local Commits" and "Revision Update History" tables. Fixes T4585.

Test Plan: made a repo. made a diff (foo) and committed it (bar). made a new diff that was comprised of two local commits. noted links to (bar) in various commit hashes as expected

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin, chad

Maniphest Tasks: T4585

Differential Revision: https://secure.phabricator.com/D8679
2014-04-02 13:18:11 -07:00
epriestley
1aad40b7bf Allow users to receive email about pushes via Herald
Summary:
Fixes T4677. Implements a "send an email" pre-receive action, which sends push summaries.

For use cases where features are often pushed as a large number of commits (e.g., checkpoint commits are retained), using commit emails means users get a ton of email. Instead, this allows you to get an email about a push, which summarizes what changed.

Overall, this is basically the same as commit email, but more suitable for some workflows.

Test Plan:
Wrote some rules, then made a bunch of pushes. Got email like this:

{F134929}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4677

Differential Revision: https://secure.phabricator.com/D8618
2014-03-26 13:51:15 -07:00
epriestley
75c47c6ae0 Provide an "event" page for push logs, which shows details on all events in a given push
Summary:
Ref T4677. This shows a more detailed view of an entire "git push", "hg push", or "svn commit".

This is mostly to give push summary emails a reasonable, stable URI to link to for T4677.

Test Plan:
  - Pushed into SVN, Git and Mercurial.
  - Viewed partial and imported event records.

{F134864}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4677

Differential Revision: https://secure.phabricator.com/D8616
2014-03-26 13:51:09 -07:00
epriestley
a5f55d506f Provide a real object ("PhabricatorRepositoryPushEvent") to represent an entire push transaction
Summary:
Ref T4677. Currently, we record individual actions in a push as PhabricatorRepositoryPushLogs, but tie them together only loosely with a `transactionKey`.

Provide a real PushEvent object, and move some of the denormalized fields to it. This primarily just gives us more robust infrastructure for building, e.g., email about pushes, for T4677, since we can act on real PHIDs rather than passing awkward identifiers around.

Test Plan:
  - Performed migration.
  - Looked at database for consistency.
  - Browsed/queried push logs.
  - Pushed a bunch of stuff.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4677

Differential Revision: https://secure.phabricator.com/D8615
2014-03-26 13:51:06 -07:00
epriestley
611f670720 Add CustomField to Diffusion and add a "branches" field
Summary:
Ref T4588. Request from @zeeg. Adds a "BRANCHES" field to commit emails, so the branches the commit appears on are shown.

I've implemented this with CustomField, but in a very light way.

Test Plan: Used `scripts/repository/reparse.php --herald` to generate mail, got a BRANCHES section where applicable.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: aran, epriestley, zeeg

Maniphest Tasks: T4588

Differential Revision: https://secure.phabricator.com/D8501
2014-03-12 11:30:33 -07:00
Joshua Spence
e11adc4ad7 Added some additional assertion methods.
Summary:
There are quite a few tests in Arcanist, libphutil and Phabricator that do something similar to `$this->assertEqual(false, ...)` or `$this->assertEqual(true, ...)`.

This is unnecessarily verbose and it would be cleaner if we had `assertFalse` and `assertTrue` methods.

Test Plan: I contemplated adding a unit test for the `getCallerInfo` method but wasn't sure if it was required / where it should live.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8460
2014-03-08 19:16:21 -08:00
epriestley
a3c1dcb928 Produce a more tailored checkout URI for Subversion
Summary:
For imported SVN repositories with an "Import Only" path, we produce a `/path/to/root/` URI, but should produce `/path/to/root/then/to/import/only/`.

As it is, the URI instructs the user to check out the whole repository.

Also, don't show the "Clone As" fragment in the URI for remote repositories, and prevent it from being edited for nonhosted repositories. This is generally more consistent with user expectation.

Test Plan:
  - Created a remote SVN repository with "Import Only", saw path include it.
  - Verified no "Clone As" options, no "Clone As" in URI.
  - Switched it to hosted, saw "Clone As" options appear and work properly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, staticshock

Differential Revision: https://secure.phabricator.com/D8375
2014-02-28 13:04:41 -08:00
epriestley
e28b78f5eb Fix two issues with ref discovery
Summary:
See IRC. I'm having trouble figuring out what's going on with b4taylor's report, but fix two possible issues:

  # The commit query is missing a `repositoryID`, which could cause issues if you import two copies of the same repository.
  # I think we may try to close commits on untracked branches right now, as long as they aren't excluded by other autoclose rules.

Test Plan: Ran `bin/repository refs` on a few repos.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, brennantaylor

Differential Revision: https://secure.phabricator.com/D8373
2014-02-28 12:56:18 -08:00
Joshua Spence
6270114767 Various linter fixes.
Summary:
- Removed trailing newlines.
- Added newline at EOF.
- Removed leading newlines.
- Trimmed trailing whitespace.
- Spelling fix.
- Added newline at EOF

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: hach-que, chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8344
2014-02-26 12:44:58 -08:00
epriestley
a298a79bda Convert Phabricator to handle "%s" / "%B" properly
Summary:
Ref T1191. I believe we only have three meaningful binary fields across all applications:

  - The general cache may contain gzipped content.
  - The file storage blob may contain arbitrary binary content.
  - The Passphrase secret can store arbitrary binary data (although it currently never does).

This adds Lisk config for binary fields, and uses `%B` where necessary.

Test Plan:
  - Added and executed unit tests.
  - Forced file uploads to use MySQL, uploaded binaries.
  - Disabled the CONFIG_BINARY on the file storage blob and tried again, got an appropraite failure.
  - Tried to register with an account containing a G-Clef, and was stopped before the insert.

Reviewers: btrahan, arice

Reviewed By: arice

CC: arice, chad, aran

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D8316
2014-02-23 16:20:46 -08:00
epriestley
88227d26bc Allow CustomField to provide ApplicationTransaction change details
Summary:
Ref T3886. Ref T418. For fields like "Summary" and "Test Plan" where changes can't be summarized in one line, allow CustomField to provide a "(Show Details)" link and render a diff.

Also consolidate some of the existing copy/paste, and simplify this featuer slightly now that we've move to dialogs.

Test Plan:
{F115918}

  - Viewed "description"-style field changes in phlux, pholio, legalpad, maniphest, differential, ponder (questions), ponder (answers), and repositories.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3886, T418

Differential Revision: https://secure.phabricator.com/D8284
2014-02-21 11:53:04 -08:00
epriestley
b96ab5aadf Modernize VCS password storage to use shared hash infrastructure
Summary: Fixes T4443. Plug VCS passwords into the shared key stretching. They don't use any real stretching now (I anticipated doing something like T4443 eventually) so we can just migrate them into stretching all at once.

Test Plan:
  - Viewed VCS settings.
  - Used VCS password after migration.
  - Set VCS password.
  - Upgraded VCS password by using it.
  - Used VCS password some more.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4443

Differential Revision: https://secure.phabricator.com/D8272
2014-02-18 14:09:36 -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
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
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
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
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
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
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
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
b0f7e7a6af Work around hg echoing warnings to stdout under --debug
Summary:
Ref T615. Ref T4237. With `--debug`, Mercurial will echo an "ignoring untrusted configuration option" warning **to stdout** if `.hgrc` has the wrong owner.

However, we need `--debug` to make `{parents}` usable, at least until the patches I got into the upstream are widely deployed. So after getting `--debug` output, strip off any leading warnings.

These warnings should always be in English, at least, since we set `LANG` explicitly.

Test Plan: Unit tests. @asherkin, maybe you can confirm this? I can't actually get the warning, but I think my `hg` in PATH is just a bit out of date.

Reviewers: asherkin, btrahan

Reviewed By: asherkin

CC: asherkin, aran

Maniphest Tasks: T615, T4237

Differential Revision: https://secure.phabricator.com/D7784
2013-12-17 18:04:01 -08:00
epriestley
f28d3089d7 Assign PHIDs to PushLogs
Summary: Ref T4195. We need these in Herald, since HeraldTranscripts need to refer to a PHID which they acted upon.

Test Plan:
Ran migration, got PHIDs:

  mysql> select phid from repository_pushlog limit 3;
  +--------------------------------+
  | phid                           |
  +--------------------------------+
  | PHID-PSHL-25jnc6cjgzw5rwqgmr7r |
  | PHID-PSHL-2vrvmtslkrj5yv7nxsv2 |
  | PHID-PSHL-34x262zkrwoka6mplony |
  +--------------------------------+
  3 rows in set (0.00 sec)

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7780
2013-12-17 15:23:23 -08:00
epriestley
2216a5e6ef Add Subversion ref and content logs to pre-commit hooks
Summary: Ref T4195. SVN has no such thing as refs (I was thinking about writing a quasi-ref anyway like `HEAD: r23 -> r24`, but I'm not sure it would actually be useful). And content is very easy to build.

Test Plan: Pushed some stuff to SVN, got logs from it.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7766
2013-12-17 11:11:52 -08:00
epriestley
74251b3636 Support bookmark hook operations in Mercurial
Summary: Ref T4195. Turns bookmark mutations in Mercurial into log objects.

Test Plan:
Pushed a pile of bookmarks and got logs:

{F89313}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7764
2013-12-17 08:34:30 -08:00
epriestley
2725586baf Restructure HookEngine to use PushLog records for all operations
Summary:
Ref T4195. This pulls the central logic of HookEngine up one level and makes all the git stuff genrate PushLogs.

In future diffs, everything will generate PushLogs and we can hand those off to Herald.

Test Plan:
Pushed a pile of valid/invalid stuff:

{F89256}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7761
2013-12-17 08:32:33 -08:00
epriestley
d846f6508b Fix some repository URI handling issues in Git and Mercurial
Summary:
See <https://github.com/facebook/phabricator/issues/467>. @dctrwatson also ran into an issue where we were trying to `setPass()` a GitURI.

  - For Git and Mercurial, properly generate credential URIs where relevant.
  - Don't try to `setPass()` on Git-style URIs.

This isn't perfect but should clean things up a bit.

Test Plan: Added unit tests. Lots of `grep`.

Reviewers: btrahan

Reviewed By: btrahan

CC: dctrwatson, aran

Differential Revision: https://secure.phabricator.com/D7759
2013-12-12 09:45:27 -08:00
epriestley
caa6fdf56d Add a basic push log for recording repository push events
Summary:
Ref T4195. This log serves two purposes:

  - It's a log, so you can see what happened. Particularly, in Git/Hg, there is no other way to tell:
    - Who //pushed// a change (vs committed / authored)?
    - When was a change pushed?
    - What was the old value of some tag/branch before someone destroyed it?
  - We can hand these objects off to Herald to implement pre-commit rules.

This is a very basic implementation, but gets some data written and has a basic UI for it.

Test Plan: {F87339}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7705
2013-12-05 11:56:14 -08:00
epriestley
d2e9aee16d Reject dangerous changes in Git repositories by default
Summary: Ref T4189. This adds a per-repository "dangerous changes" flag, which defaults to off. This flag must be enabled to do non-appending branch mutation (delete branches / rewrite history).

Test Plan:
With flag on and off, performed various safe and dangerous pushes.

  >>> orbital ~/repos/POEMS $ git push origin :blarp
  remote: +---------------------------------------------------------------+
  remote: |      * * * PUSH REJECTED BY EVIL DRAGON BUREAUCRATS * * *     |
  remote: +---------------------------------------------------------------+
  remote:             \
  remote:              \                    ^    /^
  remote:               \                  / \  // \
  remote:                \   |\___/|      /   \//  .\
  remote:                 \  /V  V  \__  /    //  | \ \           *----*
  remote:                   /     /  \/_/    //   |  \  \          \   |
  remote:                   @___@`    \/_   //    |   \   \         \/\ \
  remote:                  0/0/|       \/_ //     |    \    \         \  \
  remote:              0/0/0/0/|        \///      |     \     \       |  |
  remote:           0/0/0/0/0/_|_ /   (  //       |      \     _\     |  /
  remote:        0/0/0/0/0/0/`/,_ _ _/  ) ; -.    |    _ _\.-~       /   /
  remote:                    ,-}        _      *-.|.-~-.           .~    ~
  remote:   \     \__/        `/\      /                 ~-. _ .-~      /
  remote:    \____(Oo)           *.   }            {                   /
  remote:    (    (--)          .----~-.\        \-`                 .~
  remote:    //__\\  \ DENIED!  ///.----..<        \             _ -~
  remote:   //    \\               ///-._ _ _ _ _ _ _{^ - - - - ~
  remote:
  remote:
  remote: DANGEROUS CHANGE: The change you're attempting to push deletes the branch 'blarp'.
  remote: Dangerous change protection is enabled for this repository.
  remote: Edit the repository configuration before making dangerous changes.
  remote:
  To ssh://dweller@localhost/diffusion/POEMS/
   ! [remote rejected] blarp (pre-receive hook declined)
  error: failed to push some refs to 'ssh://dweller@localhost/diffusion/POEMS/'

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, chad, richardvanvelzen

Maniphest Tasks: T4189

Differential Revision: https://secure.phabricator.com/D7689
2013-12-03 10:28:39 -08:00
epriestley
209861500f Use "en_US.UTF-8", not "C", as the LANG setting
Summary: `LANG=C` is smooshing UTF-8 in some cases. See IRC.

Test Plan: User confirmed this works.

Reviewers: btrahan, asherkin

Reviewed By: asherkin

CC: aran

Differential Revision: https://secure.phabricator.com/D7659
2013-11-26 08:50:36 -08:00