1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-04 20:52:43 +01:00
Commit graph

17166 commits

Author SHA1 Message Date
vrana
79218b6e47 Factor out DifferentialDiffTableOfContentsView::renderChangesetLink()
Summary:
Links from lint errors for large diffs don't work.
This diff adds TODO for it because I am not sure how to do it.
Move of changeset links rendering to a separate method would be still useful.

Test Plan:
Display ToC of large diff, verify link.
Repeat for small diff.

Reviewers: tuomaspelkonen, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1476
2012-01-24 10:42:01 -08:00
vrana
28a5f9f44d Display comment title as title instead of body with inline comments
Test Plan: Display revision containing comments with no content but with inline
comments.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1474
2012-01-24 10:41:47 -08:00
vrana
32fc92bdb9 Display title for lint and unit stars in Revision Update History
Test Plan:
Display revision with different lint and unit results.
Hover over the stars.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1475
2012-01-24 10:41:31 -08:00
epriestley
97820b2ff7 Add branch queries to differential.query
Summary: This enables some improvements in D1478. Allow revisons to be queried
by the branch which they appear on.

Test Plan: Queried revisions by branch. Ran "arc which" branch queries in SVN
and Mercurial.

Reviewers: btrahan, cpiro, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T787

Differential Revision: https://secure.phabricator.com/D1479
2012-01-24 09:44:59 -08:00
epriestley
b43eb5aa7c Add transaction-oriented editing to projects
Summary:
  - Make some editing operations transaction-oriented, like Maniphest. (This
seems to be a good model, particularly for extensibility.) I'll move the rest of
the editing operations to transactions in future diffs.
  - Make transaction-oriented operations publish feed stories.

Test Plan:
  - Created a new project.
  - Edited an existing project.
  - Created a new project via quick create flow from Maniphest.
  - Verified feed stories publish correctly.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T681

Differential Revision: https://secure.phabricator.com/D1477
2012-01-24 09:44:35 -08:00
epriestley
3c8bb8a608 Fix comment anchor jump highlighting
Summary: I accidentally broke the feature where we highlight comments which are
jumped to via anchor in D1327. We now test that the jump was sucessful by
looking for an item with the anchor ID, but we were only setting 'name'.
Instead, set 'id' as well so the highlighting code detects that the jump was
successful and adds the highlight class.

Test Plan: Clicked "Comment D1234#7" or whatever, got a nice yellow background.

Reviewers: btrahan, nh, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T796

Differential Revision: https://secure.phabricator.com/D1471
2012-01-24 09:44:20 -08:00
epriestley
7d1f62409d Move resource allocation to task queue
Summary: Run the actual resource allocation for Drydock out-of-process via the
task queue.

Test Plan: Ran "drydock_control.php", saw it insert a task and wait for task
completion. Ran "phd debug taskmaster" and saw it run the task.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1470
2012-01-24 09:44:14 -08:00
epriestley
7a9e6af008 Add buttons to delete or free tasks from the queue
Summary: See T709. I also ran into a case in Drydock where this is useful for
testing/development.

Test Plan: Freed lease of a task; deleted a task.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T709

Differential Revision: https://secure.phabricator.com/D1469
2012-01-24 09:14:06 -08:00
epriestley
51bc18e93f Improve "next steps" text in Differential
Summary:
  - Even for immutable-history Git workflows, we suggest "arc amend". Instead,
suggest "arc amend" or "arc merge" (ideally we'd know which, but we can't
currently get that information).
  - We suggest "arc amend --revision X", but this is less safe and less simple
than "arc amend", especially after D1480.
  - For Mercurial, suggest "arc merge".

Test Plan: Looked at some "Accepted" revisions.

Reviewers: btrahan, jungejason

CC: aran, epriestley

Maniphest Tasks: T662

Differential Revision: https://secure.phabricator.com/D1481
2012-01-24 09:14:04 -08:00
vrana
58f4dc0369 Unify terminology in Differential overview
Summary: You can order by Modified but the table has Updated column.

Test Plan: /differential/filter/reviews/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1472
2012-01-23 17:39:49 -08:00
Nick Harper
76dd84e307 Replace HeraldActionConfig::getActionMap() in HeraldTranscriptController
Summary:
D1449 removed HeraldActionConfig::getActionMap(), but it was still used in
HeraldTranscriptController. This fixes the controller to use the method that
replaced getActionMap.

Test Plan: loaded a herald transcript

Reviewers: epriestley, xela

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1466
2012-01-23 13:29:44 -08:00
vrana
854ed4ea53 Display link in commit ToC for directories
Summary:
They are present in the document so there is no reason to omit the links to
them.
Similar to D1412.

Test Plan: Display commit.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1464
2012-01-23 13:23:17 -08:00
vrana
e142c7e26c Improve <title> on Diffusion pages
Test Plan:
Visit /rX123.
Visit /diffusion/X/.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1463
2012-01-21 01:33:41 -08:00
Nick Harper
d5eaef9567 Add retry loop when trying to establish db connection, log retries
Summary:
We retried if a db connection was lost when executing a query, but not when
establishing a connection. I've seen a lot of failures establishing connections
in our install (they go away when retrying), so this diff retries when
establishing connections, and logs when we retry.

Test Plan:
- Loaded phabricator in a sandbox
- Temporarily added a check in the try block to throw if there were still
  retries (to test logging, retry logic)

Reviewers: epriestley, blair

Reviewed By: epriestley

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1460
2012-01-20 13:56:36 -08:00
epriestley
27f52efd37 Minor, fix spelling issues detected by linter. 2012-01-20 07:39:55 -08:00
epriestley
d1ee08b2df Drydock Rough Cut
Summary:
Rough cut of Drydock. This is very basic and doesn't do much of use yet (it
//does// allocate EC2 machines as host resources and expose interfaces to them),
but I think the overall structure is more or less reasonable.

== Interfaces

Vision: Applications interact with Drydock resources through DrydockInterfaces,
like **command**, **filesystem** and **httpd** interfaces. Each interface allows
applications to perform some kind of operation on the resource, like executing
commands, reading/writing files, or configuring a web server. Interfaces have a
concrete, specific API:

  // Filesystem Interface
  $fs = $lease->getInterface('filesystem'); // Constants, some day?
  $fs->writeFile('index.html', 'hello world!');

  // Command Interface
  $cmd = $lease->getInterface('command');
  echo $cmd->execx('uptime');

  // HTTPD Interface
  $httpd = $lease->getInterface('httpd');
  $httpd->restart();

Interfaces are mostly just stock, although installs might add new interfaces if
they expose different ways to interact with resources (for instance, a resource
might want to expose a new 'MongoDB' interface or whatever).

Currently: We have like part of a command interface.

== Leases

Vision: Leases keep track of which resources are in use, and what they're being
used for. They allow us to know when we need to allocate more resources (too
many sandcastles on the existing hosts, e.g.) and when we can release resources
(because they are no longer being used). They also give applications something
to hold while resources are being allocated.

  // EXAMPLE: How this should work some day.
  $allocator = new DrydockAllocator();
  $allocator->setResourceType('sandcastle');
  $allocator->setAttributes(
    array(
      'diffID' => $diff->getID(),
    ));
  $lease = $allocator->allocate();
  $diff->setSandcastleLeaseID($lease->getID());

  // ...

  if ($lease->getStatus() == DrydockLeaseStatus::STATUS_ACTIVE) {
    $sandcastle_link = $lease->getInterface('httpd')->getURI('/');
  } else {
    $sandcastle_link = 'Still building your sandcastle...';
  }
  echo "Sandcastle for this diff: ".$sandcastle_link;

  // EXAMPLE: How this actually works now.
  $allocator = new DrydockAllocator();
  $allocator->setResourceType('host');
  // NOTE: Allocation is currently synchronous but will be task-driven soon.
  $lease = $allocator->allocate();

Leases are completely stock, installs will not define new lease types.

Currently: Leases exist and work but are very very basic.

== Resources

Vision: Resources represent some actual thing we've put somewhere, whether it's
a host, a block of storage, a webroot, or whatever else. Applications interact
through resources by acquiring leases to them, and then getting interfaces
through these leases. The lease acquisition process has a side effect of
allocating new resources if a lease can't be acquired on existing resources
(e.g., the application wants storage but all storage resources are full) and
things are configured to autoscale.

Resources may themselves acquire leases in order to allocate. For instance, a
storage resource might first acquire a lease to a host resource. A 'test
scaffold' resource might lease a storage resource and a mysql resource.

Not all resources are auto-allocate: the entry-level version of Drydock is that
you manually allocate a couple boxes and configure them through the web console.
Then, e.g.,  'storage' / 'webroot' resources allocate on top of them, but the
host pool itself does not autoscale.

Resources are completely stock, they are abstract shells representing any
arbitrary thing.

Currently: Resource exist ('host' only) but are very very basic.

== Blueprints

Vision: Blueprints contain instructions for building interfaces to, (possibly)
allocating, updating, managing, and destroying a specific type of resource in a
specific location. One way to think of them is that they are scripts for
creating and deleting resources. For example, the LocalHost, RemoteHost and
EC2Host blueprints can all manage 'host' resources.

Eventually, we will support more types of resources (storage, webroot,
sandcastle, test scaffold, phacility deployment) and more providers for resource
types, some of which will be in the Phabricator mainline and some of which will
be custom.

Blueprints are very custom and specific to application types, so installs will
define new blueprints if they are making significant use of Drydock.

Currently: They exist but have few capabilities. The stock blueprints do nearly
nothing useful. There is a technically functional blueprint for host allocation
in EC2.

== Allocator

This is just the actual code to execute the lease acquisition process.

Test Plan: Ran "drydock_control.php" script, it allocated a machine in EC2,
acquired a lease on it, interfaced with it, and then released the lease. Ran it
again, got a fresh lease on the existing resource.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D1454
2012-01-19 21:12:57 -08:00
epriestley
ff339e152e Improve error message for "phd stop" with bad PID
Summary: "phd launch ircbot" works but "phd stop ircbot" gives you a potentially
confusing message. Improve messaging.

Test Plan: Ran "phd stop ircbot", "phd stop 87888" (actual PID)

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T791

Differential Revision: https://secure.phabricator.com/D1457
2012-01-19 21:12:50 -08:00
epriestley
8b4b614d15 Show all branches a commit appears on
Summary:
We show the contextual branch (always the repository default branch) when
viewing a commit. Instead, show all branches the commit appears on.

Also pull some of the duplicated DiffusionXQuery stuff into a DiffusionQuery
base class, I'll do a followup to reduce more duplication.

Test Plan: Looked at a commit in Git. My HG and SVN setups are a little borked
so I kind of faked tests in them -- I'm fixing them now.

Reviewers: btrahan, jungejason, fratrik

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T768

Differential Revision: https://secure.phabricator.com/D1458
2012-01-19 21:12:44 -08:00
epriestley
008461a395 Use getBestURI() to set file handle URIs
Summary: getBestURI() = best URI

Test Plan:
It says "best" in the name so it must be the best!

Also in Maniphest emails we'll link you to /view/ even for binaries and other
non-viewable content.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: anjali, aran

Differential Revision: https://secure.phabricator.com/D1461
2012-01-19 17:16:30 -08:00
awyler
6080d74112 Created personal vs. global herald rule distingtion
Summary:
A personal rule only has actions targeting the owner.  Likewise, only they can
edit the rule. OTOH, a global may affect any target and is editable by anyone.

There are no new action types.  Instead, type of the rule modifies the available
targets and the messaging in the ui.  This is beneficial because herald rule
adapters don't need to be aware of the difference between emailing the owner of
a personal rule and emailing an arbitrary user.

This diff sets up the logic and ui for creating personal/global rules.  All
existing rules have been defaulted to global.

TODO: Filter all existing rules into personal/global
TODO: Create a UI for surfacing (relevant?) global rules.

Test Plan:
1. Created a personal rule to email myself.  Created a dumby revision satisfying
the conditions of that rule.  Verified that I recieved a herald email.
2. Removed my adminship, change the owner of a personal rule. verified that I
couldn't edit the rule.
3.Changed rule type to global. verified that I could edit the rule.
4. Verified that admins can edit both global and personal rules.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, zizzy

Differential Revision: https://secure.phabricator.com/D1449
2012-01-19 11:21:49 -08:00
Evan Priestley
c4804aa019 Merge pull request #89 from Koolvin/f6a78452f36454101ab5c8add297657680557091
Added Fn directive to IRCbot
2012-01-19 10:56:12 -08:00
root
f6a78452f3 Added Fn directive to IRCbot D1456 2012-01-19 10:50:53 -08:00
Evan Priestley
e367bfd804 Merge pull request #88 from dingram/mark-paths-generated
Add support for marking files as "generated" by regexp against path
2012-01-19 10:36:50 -08:00
Dave Ingram
3edf60627d Add support for marking files as "generated" by regexp against path
Summary:
Not all auto-generated files can include the magical
"generated" annotation for one reason or another, but they may follow
path rules. This patch allows files to be marked as automatically
generated by matching the path with a regular expression.

Test Plan:
Alter 'differential.generated-paths' setting in config.
Create a new diff that affects a file matching one of those regular
expressions. Verify that Differential marks it as automatically
generated and therefore probably not worth reviewing (in the same way as
the magical "generated" annotation.

Reviewers: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1455
2012-01-19 18:30:19 +00:00
tuomaspelkonen
7f9fb3597d Make updating postponed tests work again for Facebook.
Summary: It was broken by D!352

Test Plan: Praying that it works.

Reviewers: nh, epriestley, andrewjcg

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1453
2012-01-18 20:46:16 -08:00
epriestley
ad36865e50 Add optional "Re:" prefix to all threaded mail and allow disabling mail about
your own actions

Summary:
  - Mail.app on Lion has cumbersome threading rules, see T782. Add an option to
stick "Re: " in front of all threaded mail so it behaves. This is horrible, but
apparently the least-horrible option.
  - While I was in there, I added an option for T228.

Test Plan:
  - Sent a bunch of threaded and unthreaded mail with varous "Re:" settings,
seemed to get "Re:" in the right places.
  - Disabled email about my stuff, created a task with just me, got voided mail,
added a CC, got mail to just the CC.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, mkjones

Maniphest Tasks: T228, T782

Differential Revision: https://secure.phabricator.com/D1448
2012-01-18 15:20:50 -08:00
epriestley
0402e4e06e Make "purge_cache.php --differential" also purge the inline comment cache
Summary: We purge the cache for primary comments only right now, not inline
comments.

Test Plan: Ran "purge_cache.php --differential".

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T683

Differential Revision: https://secure.phabricator.com/D1451
2012-01-18 15:20:33 -08:00
awyler
14d16eab17 Enable Phabricator admin to change the owner of a herald rule
Summary:
Added a typeahead in the edit herald rule page that allows an admin or
owner to change the current owner of a rule.  If the typeahead is emptied, the
current owner will remain owner.

Test Plan:
Created a test rule.  Changed the owner.  Deleted the owner in the
typahead. Verified expected behavior.

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran, jungejason, epriestley, xela

Differential Revision: https://secure.phabricator.com/D1322
2012-01-18 11:59:35 -08:00
epriestley
42ddad1bc8 Add project.query to Conduit
Summary: Add a conduit method to query project information.

Test Plan: Ran method from API test console.

Reviewers: bill, btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T681

Differential Revision: https://secure.phabricator.com/D1444
2012-01-18 09:57:26 -08:00
vrana
6472dbe168 Change fileName to filename
Summary: There are lots of callsites to $changeset->getFilename() so it seemed
easier to rename getFileName() to getFilename() even if it includes database
change. Plus I think that getFilename() is better.

Test Plan:
Alter database.
Open revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1437
2012-01-17 10:50:14 -08:00
vrana
ae0d9770a5 Fill <a href> in PhabricatorMenuItem
Summary: The <a href> attribute is useful because user knows where the link goes
before opening it plus he can copy it to the clipboard plus he can add it to the
bookmarks.

Test Plan:
Display revision.
View Options.
Click.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1436
2012-01-17 10:49:39 -08:00
epriestley
ef768f9694 Use phutil_utf8_hard_wrap() in Phabricator
Summary: See D1433.

Test Plan: Created a new diff with a line >80chars, observed it wrapping
correctly.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D1438
2012-01-17 09:48:58 -08:00
epriestley
e2c75d5dc2 Improve Differential handling of disabled users
Summary:
We currently allow you to assign code review to disabled users, but
should not.

Test Plan:
  - Created revisions with no reviewers and only disabled reviewers, was
appropriately warned.
  - Looked at a disabled user handle link, was clearly informed.
  - Tried to create a new revision with a disabled reviewer, was rebuffed.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D1429
2012-01-17 09:27:19 -08:00
jungejason
4faab06c3c Enable herald rule for commits that need auditing
Summary:
enable herald commit rules to have access to auditing info.

Note that the new herald condition I added contains info for the
packages. I thought about using a simpler herald condition like
"Requires audit is true or false" and let it work together with the
existing "Affected package contains any of the package". It doesn't work
because we need the info about the package to decide if the commit
requires audit, but the herald conditions work separately.

Test Plan:
- A commit requiring auditing was detected by a herald rule that checks
  the auditing status
- A commit not requiring auditing was not detected by a herald rule
  which checks auditing status, but was detected by a rule which doesn't
  check the auditing status

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1399
2012-01-17 09:13:07 -08:00
epriestley
05ee317555 Remove "parsedHunk" property
Summary: This is never read anywhere and clearly has no effect.

Test Plan: grep

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D1434
2012-01-17 08:09:56 -08:00
epriestley
8f1c7dc663 Remove some unused nonsense
Summary: These blocks do nothing. end() produces a side effect on the internal
array pointer, but the code does not depend on it.

Test Plan: Reasoned about the code? Also viewed some diffs.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D1432
2012-01-17 08:09:45 -08:00
epriestley
2d8b35db93 Remove getDisplayLine()
Summary: No callsites anywhere. Unclear what this method is even supposed to do.

Test Plan: grep

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D1435
2012-01-17 08:09:38 -08:00
epriestley
96c08a4f37 Put file name in view URI, so downloading it with option-return creates a
usefully-named file

Summary:
If you Command-L + Option-Return to download stuff off, e.g., Paste,
you get "PHID-FILE-ad98abg9bsd9ashbs.txt" in your download folder. Put the file
name in the URI instead, so you get a reasonably named file.

Test Plan: Downloaded some files, got reasonable results.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D1427
2012-01-16 17:38:10 -08:00
vrana
f94c05a5bb Declare property
Test Plan: Display revision

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1430
2012-01-16 17:36:11 -08:00
vrana
8bed2f4387 Utilize phutil_render_tag()
Test Plan: Display diff with lint error.

Reviewers: pad, epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1428
2012-01-16 17:08:21 -08:00
epriestley
5f1438354b Document nginx, s3 storage in Phabricator
Summary: Add nginx documentation and s3 documentation and some other doc tweaks.

Test Plan: Generated, read documentation.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, wnemay

Maniphest Tasks: T638

Differential Revision: https://secure.phabricator.com/D1426
2012-01-16 16:15:27 -08:00
epriestley
b35ea500cc Allow files to be deleted
Summary:
A couple of people mentioned that they've had users accidentally upload
sensitive files. Allow files to be deleted.

(At some point it might be nice to keep the file handle around and log who
deleted it, but this addresses the immediate problem without needing too much
work.)

Test Plan: Deleted some files.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T780

Differential Revision: https://secure.phabricator.com/D1423
2012-01-16 16:15:18 -08:00
vrana
c0592b55d7 Use generic URIs
Summary:
/diffusion/X/history/?copies=0 is same as /diffusion/X/history/
/countdown/1/?chrome=1 is same as /countdown/1/

Test Plan:
Visit /diffusion/X/history/, click on Show/Hide Copies/Branches twice.
Visit /countdown/1/, click on Disable/Enable Chrome twice.

Reviewers: epriestley, tuomaspelkonen

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1424
2012-01-16 14:53:05 -08:00
epriestley
1651be91ec Remove daemon PID files for missing daemons when running "phd stop"
Summary: When we try to kill a daemon but discover it isn't running, we should
remove the PID file. We can also simplify the logic here.

Test Plan: Ran "phd stop" a couple of times, subsequent runs did not try to stop
a legion of dead daemons.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T781

Differential Revision: https://secure.phabricator.com/D1421
2012-01-16 12:59:41 -08:00
epriestley
56447ed2cc Add more options to Remarkup
Summary:
See D1416. Add options to file-embed syntax, and document new code and
embed options.

Test Plan: Used new options in markup blocks.

Reviewers: davidreuss, btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T336

Differential Revision: https://secure.phabricator.com/D1417
2012-01-16 11:53:16 -08:00
epriestley
82c0795e54 Unify logic for username validation
Summary: Revisit of D1254. Don't require lowercase, just standardize the logic.
The current implementation has nonuniform logic -- PeopleEditController forbids
uppercase.

Test Plan: Ran unit tests, see also D1254.

Reviewers: btrahan, jungejason, aran

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D1415
2012-01-16 11:52:59 -08:00
epriestley
5fd46dce66 Improve order of operations in upgrade script
Summary:
  - Run "phd stop" before stopping apache. This is essentially a smoke test for
PHABRICATOR_ENV being set.
  - Run documentation generation after everything else. Between the pull and the
restart we have some minor exposure to APC issues with deleted files and
out-of-date module definitions, and this limits that.
  - Pull commands out of (x && y) stuff, this prevents "set -e" from working
correctly.

Test Plan: Ran upgrade script locally.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D1419
2012-01-16 11:52:01 -08:00
epriestley
5333b16d7f Clarify header block documentation
Summary: Make it more explicit that headers are block formatters, see T778.

Test Plan: Read docs.

Reviewers: davidreuss, btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T778

Differential Revision: https://secure.phabricator.com/D1420
2012-01-16 11:51:54 -08:00
epriestley
80643d63a8 Detect empty $PATH environmental var
Summary:
By default, PHP-FMP (an alternate PHP FCGI SAPI) cleans the entire environment
for child processes. This means we have no $PATH.

This causes some confusing failures for reasons I don't fully understand. If you
do these things:

  exec_manual('env');
  exec_manual('export');

...they show no $PATH, as expected. If you do this:

  exec_manual('echo $PATH');

...it shows a path. And this works (i.e., it finds the executable):

  exec_manual('ls');

...but this fails (it says "no ls in ((null))"):

  exec_manual('which ls');

So, basically, the sh -c process itself gets a default PATH somehow, but its
children don't. I don't realllly get why this happens, but clearly an empty
$PATH is a misconfiguration, and can easily be remedied.

See discussion here: https://github.com/facebook/libphutil/issues/7

Test Plan: Applied patch to Centos6 + nginx + PHP-FPM machine, ran setup, the
configuration issue was detected and I was given information on resolving it.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D1413
2012-01-16 11:49:19 -08:00
epriestley
f81021fa7f Improve error message for Conduit path problems
Summary:
A few people in IRC have been having issues here recently. If you misconfigure
the IRC bot, e.g., you get a 200 response back with a bunch of login HTML in it.
This is unhelpful.

Try to detect that a conduit request is going to the wrong path and raise a
concise, explicit error which is comprehensible from the CLI.

Also created a "PlainText" response and moved the IE nosniff header to the base
response object.

Test Plan: As a logged-out user, hit various nonsense with "?__conduit__=true"
in the URI. Got good error messages. Hit nonsense without it, got login screens.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T775

Differential Revision: https://secure.phabricator.com/D1407
2012-01-16 11:48:21 -08:00