1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-21 09:48:47 +02:00
Commit graph

306 commits

Author SHA1 Message Date
epriestley
bdbe9df65e Remove support for GitHub post-receive notifications
Summary:
  - These never actually did anything.
  - I don't even really remember why I built them, maybe the Open Source team
was pushing for more GitHub integration or something? I really have no idea.
  - Anyway, repository tailers do everything these could do (and much more).

Test Plan:
  - Ran tailers off GitHub for many months without needing post-receive hooks.
  - Grepped for relevant strings, couldn't find any references.
  - Used "Repository" edit interface for a Git repository.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T706

Differential Revision: https://secure.phabricator.com/D1273
2011-12-24 09:00:08 -08:00
epriestley
e786c44b6f Relax the same-origin check in the Git commit discovery daemon
Summary:
Git accepts either "git@x:/path" or "ssh://git@x.com/path" URIs to mean the
exact same thing, which is causing some false positives and confusion,
particularly because we sometimes mutate URIs.

Since this is just a sanity check, we don't really care about the username,
domain or credentials -- matching the paths is good enough. We're just trying to
make it hard to shoot yourself in the foot by copy-pasting the same local path
into two repositories and forgetting to change one, like I did. :P

Relax the check to only verify the paths are the same.

Test Plan:
  - Ran unit tests, which should fully cover things.
  - Ran commit discovery daemon in debug mode on incorrectly and correctly
configured repositories.

Reviewers: ajtrichards, jungejason, btrahan

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T710

Differential Revision: https://secure.phabricator.com/D1279
2011-12-24 08:54:44 -08:00
epriestley
cacdfcc8ea Remove unused PhabricatorProfileView
Summary: After D1281, this has no callsites. I don't see us wanting to go back
to it.

Test Plan: Grepped for symbol name, no hits.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: https://secure.phabricator.com/D1282
2011-12-24 08:54:31 -08:00
epriestley
b258095124 Expose symbol information over Conduit
Summary: I want to add a command like "where is ArcanistUnitTestEngine" to
phabot. I also want to add a symbol typeahead to Diffusion and generally finish
up that feature since it's useful but only half-implemented. Consolidate the
query logic and expose the data over Conduit.

Test Plan: Used /symbol/ and Conduit to lookup symbols.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T315

Differential Revision: https://secure.phabricator.com/D1260
2011-12-22 06:44:55 -08:00
epriestley
9d8b5481ae Emit full URIs to identify Differential revisions
Summary:
  - Previously, used IDs like "33" to match a commit to a Differential revision.
This has a namespacing problem because we now have an arbitrarily large number
of Phabricator installs in the world, and they may want to track commits from
other installs.
  - In Differential, parse raw IDs or full URIs. Emit only full URIs.
  - In Repositories, parse only full URIs.
  - This might cause a few commits to not be picked up in rare circumstances.
Users can fix them with "arc mark-committed". This should be exceedingly rare
because of hash matching.
  - There are some caveats for reparsing older repositories, see comments
inline. I don't think there's much broad impact here.

Test Plan:
  - Created a new revision, got a full URI.
  - Updated revision, worked correctly.
  - Ran unit tests.
  - Monkeyed with "Differential Revision" field.
  - Reviewers: btrahan, jungejason

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, epriestley, jungejason

Maniphest Tasks: T54, T692

Differential Revision: 1250
2011-12-20 19:58:22 -08:00
epriestley
43430e154d Rough cut of Project profile improvements
Summary:
  - Old page was useless and dumb.
  - New page looks a little less bad, functions a little less poorly.
  - Still lots of work to be done.

Test Plan:
  - Viewed a project.
  - Clicked all the links on the left nav.
  - Here is a screenshot:
https://secure.phabricator.com/file/view/PHID-FILE-4buzquotb3fo4dhlicrw/

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T681

Differential Revision: 1246
2011-12-20 17:19:55 -08:00
jungejason
ad6708d3f0 Fix lib map file
Summary: after PHID list controller is deleted, we need to update the map file.

Test Plan: testEverythingImplemented passed

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1248
2011-12-20 14:45:48 -08:00
epriestley
21ba07d5bd Provide wiki pages for projects
Summary:
Provide tighter integration between Projects and Phriction. Partly, I have most
of a rewrite for the Projects homepage ready but it's not currently possible to
publish feed stories about a project so all the feeds are empty/boring. This
partly makes them more useful and partly just provides a tool integration point.

  - When you create a project, all the wiki pages in projects/<project_name>/*
are associated with it.
  - Publish updates to those pages as being related to the project so they'll
show up in project feeds.
  - Show a project link on those pages.

This is very "convention over configuration" but I think it's the right
approach. We could provide some sort of, like, "@project=derp" tag to let you
associated arbitrary pages to projects later, but just letting you move pages is
probably far better.

Test Plan:
  - Ran upgrade scripts against stupidly named projects ("der", "  der", "  der
", "der (2)", "  der (2) (2)", etc). Ended up with uniquely named projects.
  - Ran unit tests.
  - Created /projects/ wiki documents and made sure they displayed correctly.
  - Verified feed stories publish as project-related.
  - Edited projects, including perfomring a name-colliding edit.
  - Created projects, including performing a name-colliding create.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T681

Differential Revision: 1231
2011-12-20 14:03:12 -08:00
jungejason
c80d1480d5 Add Basic Auditing Functionalities
Summary:
add basic auditing functionalities. For the related commits for a
package, we detect the following conditions which might be suspicious to the
owners of the package:

* no revision specified
* revision not found
* author not match
* reviewedby not match
* owners not involved
* commit author not recognized

The owners of the package can change the status of the audit entries by
accepting it or specify concern.

The owner can turn on/off the auditing for a package.

Test Plan:
*  verified that non-owner cannot see the details of the audit and cannot modify
it
*  verified that all the audit reasons can be detected
*  tested dropdown filtering and package search
*  verified really normal change not detected
*  verified accept/concern a commit
*  tested enable/disable a package for auditing
*  verified one audit applies to all <commit, packages> to the packages the
auditor owns
*  verified that re-parsing a commit won't have effect if there exists a
 relationship for <commit, package> already

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, benmathews, btrahan, mpodobnik, prithvi, TomL, epriestley

Differential Revision: 1242
2011-12-20 13:36:53 -08:00
epriestley
ee620bde6d Publish feed stories from Maniphest
Summary: I didn't get around to this earlier; add Feed/Maniphest integration.
This is partly motivated by wanting Projects to not be terrible. Pretty
straightforward.

Test Plan:
  - Created, updated, reassigned and closed a task.
  - Verified feed stories render reasonably.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T681

Differential Revision: 1232
2011-12-20 09:55:29 -08:00
epriestley
0634009720 Share feed building code
Summary:
I pretty much copy/pasted this code; rather than do that again now that I want
to add feeds to projects, share the code.

This "Builder" is a little weird -- I don't want to call it a "View" because it
does data access. "Builder" seemed okay. We don't really have much code that
does this sort of thing right now, elsewhere.

Test Plan:
  - Viewed public feed.
  - Viewed private feed.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T681

Differential Revision: 1233
2011-12-20 08:28:31 -08:00
epriestley
e45ffda55a Move most remaining sha1() calls to HMAC
Summary:
  - For context, see T547. This is the last (maybe?) in a series of diffs that
moves us off raw sha1() calls in order to make it easier to audit the codebase
for correct use of hash functions.
  - This breaks CSRF tokens. Any open forms will generate an error when
submitted, so maybe upgrade off-peak.
  - We now generate HMAC mail keys but accept MAC or HMAC. In a few months, we
can remove the MAC version.
  - The only remaining callsite is Conduit. We can't use HMAC since Arcanist
would need to know the key. {T550} provides a better solution to this, anyway.

Test Plan:
  - Verified CSRF tokens generate properly.
  - Manually changed CSRF to an incorrect value and got an error.
  - Verified mail generates with a new mail hash.
  - Verified Phabricator accepts both old and new mail hashes.
  - Verified Phabricator rejects bad mail hashes.
  - Checked user log, things look OK.

Reviewers: btrahan, jungejason, benmathews

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T547

Differential Revision: 1237
2011-12-19 08:56:53 -08:00
epriestley
afc2f8526b Allow Phriction documents to be deleted
Summary:
  - Add a "delete" operation. Delete is just a special edit which removes the
page from indexes and shows a notice that the document has been deleted.
  - When a user deletes all the content on a page, treat it as a delete.
  - When a conduit call deletes all the content on a page, treat it as a delete.
  - Add page status to Conduit.
  - Add change type field to history.
  - Added a couple of constants to support a future 'move' change, which would
move content from one document to another.

Test Plan:
  - Verified deleted pages vanish from the document index (and restoring them
puts them back).
  - Verified deleted pages show "This page has been deleted...".
  - Created, edited and deleted a document via Conduit.
  - Deleted pages via "delete" button.
  - Deleted pages via editing content to nothing.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: skrul, aran, btrahan, epriestley

Maniphest Tasks: T680

Differential Revision: 1230
2011-12-17 11:45:25 -08:00
epriestley
81acf588e2 Take the first step on the long journey of fixing "Projects"
Summary:
  - Allow more than the 100 most recent projects to be viewed.
  - Provide some useful filters.
  - Default the view to your projects, not all projects.
  - Put query logic in a query object.
  - Put filter view logic in a view object. We can port more stuff to it later.

Test Plan: Looked at active/owned/all projects. Set page size to 5 and paged
through projects.

Reviewers: btrahan, jungejason, zeeg

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1227
2011-12-16 17:23:48 -08:00
epriestley
c97fcd12bc Share Revision/Task attaching code
Summary:
We have this code in two places; split it into an editor class so we can share
it.

This also fixes some probems with this field not //detaching// tasks properly.

Test Plan:
  - Created a revision with no attached tasks.
  - Attached it to a task.
  - Updated it.
  - Detached it.
  - Used web UI to attach/detach tasks/revisions.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan, epriestley

Differential Revision: 1225
2011-12-16 16:13:00 -08:00
Bob Trahan
128b7584da Files - kill tabs
Summary:
kill tabs for Files application.  Technique is the "filter list" on the left
hand side, with separation for "Files" versus "Image Macros".   UI quirks
include:

- the page title does not change for the 3 files filters while it does change
for each of the two image macro filters.
- standalone "file" pages do not have the filter view
- you can visit /file/upload/ standalone and it doesn't have the pretty filter
list on it

Please do give direction on these quirks if you like.  :)

This change also neuters the ?author= functionality for files.  The code is
written such that it can easily be brought back.

Test Plan: clicked around on the filters, liked what I saw.  uploaded files
fancy-like and basic-like and it worked!  made image macros and it worked!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Maniphest Tasks: T631

Differential Revision: 1219
2011-12-15 14:32:12 -08:00
jungejason
c13b7da290 Add Related Commits for Owners
Summary:
For each commit, find the affected packages, and provide a way to
search by package.

Test Plan:
create commits that touch and don't touch two packages, and verify
that they display correctly in all the UI pages.

Reviewers: epriestley, blair, nh, tuomaspelkonen

Reviewed By: epriestley

CC: benmathews, aran, epriestley, btrahan, jungejason, mpodobnik, prithvi

Maniphest Tasks: T83

Differential Revision: 1208
2011-12-14 22:48:57 -08:00
Jason Ge
16f57dce1d Fix library_map for D1198
Summary:
after a class is deleted/added, we need to run
arcanist/scripts/phutil_mapper src to update the library map.

Task ID: #

Test Plan:
run test case testEverythingImplemented and it passes.

Revert Plan:

Tags:

Reviewers: btrahan, epriestley

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1211
2011-12-14 12:46:23 -08:00
epriestley
4edfd35503 Fix qsprintf() '%nd' conversion
Summary:
I broke this a little bit in my overzealous D1174, since this block validates
both '%nd' (nullable integer) and '%d' (non-nullable integer).

Clean up the conditional checks so we catch the bad case ('%d' on a PHID
converting to 0) but let the good case ('%nd' with null) through.

Test Plan: Unit tests failed; applied patch; unit tests pass.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T670

Differential Revision: 1201
2011-12-13 17:40:24 -08:00
jhester
1fec5fd727 Add author to differential.getrevisionfeedback
Summary: Add the author PHID to the differential.getrevisionfeedback conduit api
method

Test Plan: issue differential.getrevisionfeedback query via conduit against a
valid revision and verify author phid is included in results

Reviewers: epriestley

Reviewed By: epriestley

CC: epriestley, aran, jungejason, tuomaspelkonen, jonathanhester

Differential Revision: 1190
2011-12-13 16:35:57 -08:00
epriestley
4fd81150be Remove "Updated" view from Differential
Summary:
This landed during my review drama embargo and is a generally good idea but had
some implementation issues.

@elynde reports it has been broken for some time, although it still works on
secure.phabricator.com so I'm guessing it's just taking a zillion years to run
at Facebook. It's up to more than a second for me on secure.phabricator.com:

https://secure.phabricator.com/file/view/PHID-FILE-v4ql4c66u3xnkarmrpm4/

The basic problem is that some of the data architecture around this
implementation is hard to scale. I want to pursue a similar feature eventually,
but drive it off notifications that we'll ship through real-time infrastructure
too.

I'm also trying to get rid of DifferentialRevisionListData and this simplifies
that somewhat.

Test Plan:
  - Grepped for table name, table constant, query constant, and class name; no
hits.
  - Applied SQL patch.
  - Verified that Differential no longer shows "Updated".

Reviewers: elynde, btrahan, jungejason

Reviewed By: elynde

CC: aran, elynde

Differential Revision: 1178
2011-12-07 06:55:03 -08:00
Bob Trahan
588b959c03 phabricator_format_timestamp => phabricator_datetime
Summary: make the change, kill the function.   be sure to get a good $user or
$viewer variable

Test Plan:
for each controller or view, look at it in the ui.   change timezone, refresh ui
and note change.   i did not test the OAuthSettingsPanelController; not sure how
to get to that badboy and i got a bit lazy

Maniphest Tasks: T222

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Maniphest Tasks: T222

Differential Revision: 1166
2011-12-04 08:51:34 -08:00
Nick Harper
8f5e28bf59 Add differential.query conduit method
Summary:
Created a differential.query conduit method that is built on top of
DifferentialRevisionQuery. I also added support for querying by author, ccs, and
reviewers to DifferentialRevisionQuery, so feature parity can be brought up to
match differential.find and its backing class DifferentialRevisionListData.

Test Plan:
Tried a few calls to the conduit call using the web interface, and got back
reasonable looking data.

Reviewers: epriestley, jungejason, btrahan

Reviewed By: epriestley

CC: aran, nh, epriestley

Differential Revision: 1158
2011-12-02 13:06:43 -08:00
Bob Trahan
0795cd4baa Add cycle detection to celerity mapper
Summary: create CelerityResourceGraph, which extends AbstractDirectedGraph.
since we've done a bunch of work already to load the resource graph into memory
CelerityResourceGraph simply stores a copy and makes loadEdges work off that
stored copy.

Test Plan:
made phabricator-prefab require herald-rule-editor

~/code/phabricator> ./scripts/celerity_mapper.php webroot
Finding static resources...
Processing 154
files..........................................................................................................................................................
[2011-11-22 11:28:29] EXCEPTION: (Exception) Cycle detected in resource graph:
phabricator-prefab => herald-rule-editor => phabricator-prefab at
[/Users/btrahan/Dropbox/code/phabricator/scripts/celerity_mapper.php:173]

fixed phabricator-prefab requiring herald-rule-editor.  re-ran celerity_mapper
and no errors!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1132
2011-11-29 12:09:08 -08:00
Brian Pane
5dba8abceb Add a "createcomment" method to Differential
Summary:
Added a new method differential.createcomment

Task ID: #752014

Test Plan:
I created a test diff and called this method via the conduit
from a client PHP script to add comments.  I confirmed that
1) the comment appeared on the revision, 2) URLs within the
comment were turned into hyperlinks, and 3) Phabricator
sent a notification email to the people watching the test
diff.

Reviewers: nh, jungejason, epriestley

Reviewed By: nh

CC: aran, nh

Differential Revision: 1128
2011-11-18 14:53:01 -08:00
Marek Sapota
e9693f25f8 Move event framework from Phabricator to libphutil
Summary:
Move event framework from Phabricator to libphutil so it can be used in other
phutil projects, such as Arcanist.

Test plan:
Use along with path to libphutil, events should work as expected.

Reviewers: epriestley

Differential Revision: 1098
2011-11-16 16:34:45 -08:00
Jason Ge
42383214ea Enable admin to view and delete other users' herald rules
Summary:
enable admin to delete user's herald rules. This is useful for
managing non-active users' rules. For example, ex-employees' rules. The
code change includes:

 - Added a 'All' tab which is only accessible to admin.
 - Refactor out a HeraldRuleListView which is used by both the home
   controller and the all rule controller

Test Plan:
delete an ex-employee rule as an admin; disable myself as
admin and verified that I don't have access to view other user's rules
and I'am not be able to delete them; also verified that as a non-admin,
I can still view, create and delete my own rules.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, epriestley, jungejason

Differential Revision: 1064
2011-11-15 16:21:51 -08:00
Marek Sapota
b71a55900a Allow tweaking of Differential mail by using events
Summary: Allow tweaking Differential mail before sending.

Test Plan:
Wrote a listener renaming Differential attachments and it worked without
problems.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, mareksapota, davidreuss

Differential Revision: 1091
2011-11-09 10:13:53 -08:00
adonohue
7d2a18d883 Examples using JX.View
Summary: Provide a dirt-simple working example of client-side templating and
reactive programming.

Test Plan: Load the examples

Reviewers: epriestley, mroch, tomo

Reviewed By: epriestley

CC: ide, schrockn, aran, rzadorozny, epriestley

Differential Revision: 908
2011-11-06 15:17:00 -08:00
Jason Ge
13eee1a344 Add test to check all symbols can be loaded
Summary:
make sure all symboles can be loaded to avoid issues like missing
methods in descendants of abstract base class.

Test Plan:
ran it and verified it passes; remove a method in a descendant class
and verified that the test failed.

Reviewers: epriestley, nh

Reviewed By: nh

CC: aran, nh, jungejason

Differential Revision: 1023
2011-10-20 16:43:13 -07:00
epriestley
91bf3e96c9 Provide a Differential Revision query class for affected paths
Summary:
For T262, we need to query for revisions by affected path.

We currently have a class called "DifferentialRevisionListData" but it's sort of
nasty and it would have been really cumbersome to add this query to it.

Instead, this provides a query object more in line with ManiphestTaskQuery,
which I'm pretty happy with. I'd eventually like to get rid of
DifferentialRevisionListData but it's used in a couple of places right now.

Test Plan: Used phpsh to execute queries, got back apparently-sensible result
sets.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason, epriestley

Differential Revision: 978
2011-10-06 10:27:17 -07:00
epriestley
bea4795575 Separate revision list rendering logic into a RevisionListView
Summary: I want to throw this in Diffusion as part of T262, but it's embedded in
the controller right now. Split it out.

Test Plan: Looked at various revision list views, no changes.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 977
2011-10-06 10:26:47 -07:00
epriestley
1b8562467c Add an "Event" plugin to DarkConsole for event inspection
Summary: Shows events which a page dispatched, plus all the registered
listeners.

Test Plan:
Pretty basic for now, but works OK:

https://secure.phabricator.com/file/view/PHID-FILE-49fcd23081ce55cf9369/

(I also made it dispatch some dummy events to verify they show up.)

Reviewers: aran

Reviewed By: aran

CC: aran

Differential Revision: 973
2011-10-01 08:51:54 -07:00
epriestley
522e5b4779 Build an event dispatch mechanism into Phabricator
Summary:
This is an attempt to satisfy a lot of the one-off requests a little more
generally, by providing a relatively generic piece of event architecture.

Allow the registation of event listeners which can react to various application
events (currently, task editing).

I'll doc this a bit better but I wanted to see if anyone had massive objections
to doing this or the broad approach. The specific problem I want to address is
that one client wants to do a bunch of routing for tasks via email, so it's
either build a hook, or have them override most of ManiphestReplyHandler, or
something slightly more general like this.

Test Plan: Wrote a silly listener that adds "Quack!" to a task every time it is
edited and edited some tasks. I was justly rewarded.

Reviewers: nh, jungejason, tuomaspelkonen, aran

Reviewed By: aran

CC: aran, epriestley

Differential Revision: 881
2011-09-30 12:16:40 -07:00
epriestley
b1e1b1f9bd Basic support for Mercurial in Diffusion
Summary: Change import script plus almost all the view stuff. Still some rough
edges but this seems to mostly work. Blame is currently unsupported but I think
everything else works properly.

Test Plan:
Imported the hg repository itself. It doesn't immediately seem completely
broken. Here are some screens:

https://secure.phabricator.com/file/view/PHID-FILE-1438b71cc7c4a2eb4569/
https://secure.phabricator.com/file/view/PHID-FILE-3cec4f72f39e7de2d041/
https://secure.phabricator.com/file/view/PHID-FILE-2ea4883f160e8e5098f9/
https://secure.phabricator.com/file/view/PHID-FILE-35f751a36ebf65399ade/

All the parsers were able to churn through it without errors.

Ran the new "reparse.php" script in various one-commit and repository modes.

Browsed/imported some git repos for good measure.

NOTE: The hg repository is only 15,000 commits and around 1,000 files.
Performance is okay but hg doesn't provide performant, native APIs to get some
data efficiently so we have to do some dumb stuff. If some of these interfaces
are cripplingly slow or whatever, let me know and we can start bundling some
Mercurial extensions with Arcanist.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: Makinde

CC: aran, Makinde, epriestley

Differential Revision: 960
2011-09-27 19:28:57 -07:00
epriestley
46373f2be7 Add a Mercurial message parser
Summary: See D943, this is the second parse stage. This will mark Differential revisions as "Committed" among other things.

Almost all the logic here is shared between VCSes so the implementation itself is straightforward.

Test Plan: Parsed all messages for the official Mercurial repository.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

CC:

Differential Revision: 944
2011-09-27 19:28:56 -07:00
epriestley
2fc3acc969 Improve time localization code
Summary:
  - We throw on a missing date right now, in the DateTime constructor. This can
happen in reasonable cases and this is display code, so handle it more
gracefully (see T520).
  - This stuff is a little slow and we sometimes render many hundreds of dates
per page. I've been seeing it in profiles on and off. Memoize timezones to
improve performance.
  - Some minor code duplication that would have become less-minor with the
constructor change, consolidate the logic.
  - Add some unit tests and a little documentation.

Test Plan:
  - Ran unit tests.
  - Profiled 1,000 calls to phabricator_datetime(), cost dropped from ~49ms to
~19ms with addition of memoization. This is still slower than I'd like but I
don't think there's an easy way to squeeze it down further.

Reviewers: ajtrichards, jungejason, nh, tuomaspelkonen, aran

Reviewed By: ajtrichards

CC: aran, ajtrichards, epriestley

Differential Revision: 966
2011-09-27 09:25:16 -07:00
epriestley
016b060aea Add a relation table for Revisions to local commit hashes
Summary:
This allows us to performantly query for diffs related to a given local hash.
Immediate applications are:

  - Commit detection in Mercurial and Git-Immutable workflows.
  - Some async unit test stuff @mgummelt was doing.

Test Plan:
Diffed locally under SVN/Git/hg, checked the table, got sensible output.

  mysql> select * from differential_revisionhash;
  +------------+------+------------------------------------------+
  | revisionID | type | hash                                     |
  +------------+------+------------------------------------------+
  |         40 | gtcm | 8c6fb2f95598a50f7aac64a5f4cc6c12b5db42f5 |
  |         40 | gttr | 54710e361a465f4ff39565a93b2a221b6e7dd07c |
  |         41 | hgcm | c29cb69aec14                             |
  |         41 | hgcm | e7309be4eabb                             |
  |         41 | hgcm | 4e885caeff60                             |
  |         41 | hgcm | 213ee1cd30ea                             |
  |         41 | hgcm | b4050fb3490f                             |
  |         41 | hgcm | 72a76bd7ffa2                             |
  |         41 | hgcm | 06c2687e63fb                             |
  |         41 | hgcm | 2b464bde6b48                             |
  +------------+------+------------------------------------------+
  10 rows in set (0.00 sec)

NOTE: Mercurial hashes are short-form but I'll shoot out a separate Arcanist
diff to fix this.

Reviewers: Makinde, fratrik, mgummelt, jungejason, nh, tuomaspelkonen, aran

Reviewed By: Makinde

CC: aran, Makinde

Differential Revision: 961
2011-09-26 15:02:37 -07:00
epriestley
1c1f749eba Add an "arcanist.projectinfo" Conduit call
Summary:
We currently rely on "remote_hooks_enabled" in .arcconfig to determine whether
commands like "arc amend" and "arc merge" should imply "arc mark-committed".

However, this is a historical artifact that is now bad for a bunch of reasons:

  - The option name is confusing, it really means 'repository is tracked'.
  - The option is hard to discover and generally sucks.
  - We can empirically determine the right answer since we now know if a project
is in a tracked repository.

Add a call which arcanist can make on these workflows to figure out if it is
interacting with a project in a tracked repository or not.

Also added an "isTracked()" convenience method to reduce the number of magic
strings all over the place.

Test Plan: Ran "arcanist.projectinfo" for nonexistent, untracked and tracked
projects.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: Makinde

CC: aran, epriestley, Makinde

Differential Revision: 945
2011-09-21 14:19:14 -07:00
epriestley
93b3bc8e89 Add a Mercurial message parser
Summary:
See D943, this is the second parse stage. This will mark Differential revisions
as "Committed" among other things.

Almost all the logic here is shared between VCSes so the implementation itself
is straightforward.

Test Plan: Parsed all messages for the official Mercurial repository.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: Makinde

CC: aran, Makinde

Differential Revision: 944
2011-09-16 11:09:39 -07:00
epriestley
e0b86cc81b Add a Mercurial commit discovery daemon
Summary:
Repository import has three major steps:

  - Commit discovery (serial)
  - Message parsing (parallel, mostly VCS independent)
  - Change parsing (parallel, highly VCS dependent)

This implements commit discovery for Mercurial, similar to git's parsing:

  - List the heads of all the branches.
  - If we haven't already discovered them, follow them back to their roots (or
the first commit we have discovered).
  - Import all the newly discovered commits, oldest first.

This is a little complicated but it ensures we discover commits in depth order,
so the discovery process is robust against interruption/failure. If we just
inserted commits as we went, we might read the tip, insert it, and then crash.
When we ran again, we'd think we had already discovered commits older than HEAD.

This also allows later stages to rely on being able to find Phabricator commit
IDs which correspond to parent commits.

NOTE: This importer is fairly slow because "hg" has a large startup time
(compare "hg --version" to "git --version" and "svn --version"; on my machine,
hg has 60ms of overhead for any command) and we need to run many commands (see
the whole "hg id" mess). You can expect something like 10,000 per hour, which
means you may need to run overnight to discover a large repository (IIRC, the
svn/git discovery processes are both about an order of magnitude faster). We
could improve this with batching, but I want to keep it as simple as possible
for now.

Test Plan: Discovered all the commits in the main Mercurial repository,
http://selenic.com/repo/hg.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: Makinde

CC: aran, Makinde

Differential Revision: 943
2011-09-16 11:08:52 -07:00
epriestley
b64f252f8b Fix a dirname() edge case in Diffusion
Summary:
dirname('x') returns '.', not '/'; this caused some issues for repositories with
files at the root.

There are some cases in the parsers where I should probably swap this out too
but I'll wait until I'm doing some more rigorous testing since that stuff is a
bit fragile and this fixes an immediate issue.

Test Plan: Ran unit tests. Viewed a file at root level in a test repository.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: nh

CC: aran, nh

Differential Revision: 932
2011-09-15 07:45:15 -07:00
epriestley
43a3f4d234 Build an "affected path" index when attaching diffs to revisions
Summary: See T262. This creates the index on the Differential side which we need in order to execute this query efficiently on the Diffusion side.

Also renames "DiffusionGitPathIDQuery" to "DiffusionPathIDQuery", this query object has nothing to do with git.

Test Plan: Attached top-level and sub-level diffs to revisions and verified they populated the table with sensible data.

Reviewers: bmaurer, aravindn, fmoo, jungejason, nh, tuomaspelkonen, aran

CC:

Differential Revision: 931
2011-09-15 07:45:14 -07:00
epriestley
1620bce842 Add Google as an OAuth2 provider (BETA)
Summary:
This is pretty straightforward, except:

  - We need to request read/write access to the address book to get the account
ID (which we MUST have) and real name, email and account name (which we'd like
to have). This is way more access than we should need, but there's apparently no
"get_loggedin_user_basic_information" type of call in the Google API suite (or,
at least, I couldn't find one).
  - We can't get the profile picture or profile URI since there's no Plus API
access and Google users don't have meaningful public pages otherwise.
  - Google doesn't save the fact that you've authorized the app, so every time
you want to login you need to reaffirm that you want to give us silly amounts of
access. Phabricator sessions are pretty long-duration though so this shouldn't
be a major issue.

Test Plan:
  - Registered, logged out, and logged in with Google.
  - Registered, logged out, and logged in with Facebook / Github to make sure I
didn't break anything.
  - Linked / unlinked Google accounts.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: aran, epriestley, Makinde

Differential Revision: 916
2011-09-14 07:32:04 -07:00
epriestley
4da43b31a3 Add Mercurial repository configuration and local pull support
Summary: No actual parsing/import yet, but now you can define and pull Mercurial
repositories. I merged most of the local pull code so we can share it between
hg/git.

Test Plan:
  - Created a new Mercurial repository to track Codeigniter off Bitbucket
  - Edited / saved / etc.
  - Launched the mercurial pull daemon, it pulled the repo. Killed and
relaunched, it updated the repo.
  - Launched the git fetch deamon, it still works correctly.

Reviewers: Makinde, aran, jungejason, tuomaspelkonen

Reviewed By: Makinde

CC: aran, Makinde

Differential Revision: 793
2011-09-14 07:28:22 -07:00
epriestley
888af7309a Add a simple symbol lookup interface for cross-references
Summary: This will get fancier, but here's a basic interface for doing symbol
lookups. Still all pretty tentative.

Test Plan: Looked up various things, got some sensible results.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: tuomaspelkonen

CC: aran, tuomaspelkonen

Differential Revision: 900
2011-09-13 08:49:45 -07:00
epriestley
cd05c960ff Add storage for repository symbol tracking
Summary: See T315 for an extensive description of this feature. Adds the
descibed storage table.

Test Plan: Used phpsh to read/write symbol objects.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: tuomaspelkonen

CC: aran, epriestley, tuomaspelkonen

Differential Revision: 897
2011-09-13 08:49:44 -07:00
Nick Harper
2db912e859 Add change password settings panel
Summary:
In password-based auth environments, there is now a user settings
panel to allow them to change their password.

Test Plan:
Click settings, choose password from the left:
* enter current password, new password (twice), log out, and log in with
  new password
* enter current password, non-matching passwords, and get error
* enter invalid old password, and get error
* use firebug to change csrf token and verify that it does not save with
  and invalid token
Changed config to disable password auth, loaded settings panel and saw
that password was no longer visible. Tried loading the panel anyway and
got redirected.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 890
2011-09-04 15:07:04 -07:00
epriestley
76ac8b4196 Display local commit information in Differential
Summary:
After D857, we try to attach local commit information to revisions. If this
information is available, display it on the revision.

Design on this is a little rough, I might try to combine this into the revision
update view or something like that since we're starting to take up a lot of real
estate for metadata.

Test Plan: Local diffed this and got some commit info.

Reviewers: jungejason, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, epriestley, jungejason

Differential Revision: 872
2011-08-31 13:49:50 -07:00
epriestley
5908a63dfe Add a custom lint name hook to Phabricator
Summary: Allow Conduit method so they stop raising lint warnings. See D874.

Test Plan: Ran "arc lint" on conduit files and was no longer given frivolous
warnings.

Reviewers: nh, jungejason, tuomaspelkonen, aran

Reviewed By: nh

CC: aran, nh, epriestley

Differential Revision: 875
2011-08-31 13:49:30 -07:00