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
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
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
Summary:
I locked this down a little bit recently, but make
double-extra-super-sure that we aren't sending the user anywhere suspicious or
open-redirecty. This also locks down protocol-relative URIs (//evil.com/path)
although I don't think any browsers do bad stuff with them in this context, and
header injection URIs (although I don't think any of the modern PHP runtimes are
vulnerable).
Test Plan:
- Ran tests.
- Hit redirect page with valid and invalid next URIs; was punted to / for
invalid ones and to the right place for valid ones.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: arice, aran, epriestley, btrahan
Differential Revision: https://secure.phabricator.com/D1369
Summary:
- PHP uses a SAPI ("server API") to determine how it interacts with the caller
(e.g., how to read the environment, how to read flags, what code to execute).
- There are several different SAPIs: cli, cgi, cgi-fcgi, apache, etc.
- Each SAPI has different behavior -- for instance, the "cgi" SAPI emits some
CGI headers unless told not to, so a script like 'echo "x"' actually echoes some
headers and then 'x' as an HTTP body.
- In some setups, "php" may be php-cgi.
- If you run php-cgi as "php scriptname.php" and your ENV has an existing CGI
request in it, it runs that CGI request instead of the script. This causes an
infinite loop.
- Add checks to verify that "php" is the "cli" SAPI binary, not some other
SAPI.
- In particular, cPanel uses suphp and is affected by this configuration
issue. See this thread:
https://lists.marsching.com/pipermail/suphp/2008-September/002036.html
Test Plan:
- On a cPanel + suphp machine, ran setup and was stopped for having the
"cgi-fcgi" SAPI instead of throw into an infinite loop.
- Applied the suggested remedy, setup now runs fine.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan, epriestley
Differential Revision: https://secure.phabricator.com/D1390
Summary:
With T764, http://localhost doesn't work anymore. So add instructions
about how to support it by modifying the hosts file.
Test Plan:
- turned on setup mode and the error message did show up
- turned off the setup mode and the error message also showed up
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, epriestley
Maniphest Tasks: T764
Differential Revision: https://secure.phabricator.com/D1370
Summary:
Chrome/Chromium won't set cookies on these domains, at least under
Ubuntu. See T754. Detect brokenness and explode.
Test Plan:
Logged into phabricator as "http://derps/" (failed) and
"http://derps.com/" (worked) in Chromium. Set config to "http://derps/" (config
exploded) and "http://local.aphront.com/" (config OK).
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan
Maniphest Tasks: T754
Differential Revision: https://secure.phabricator.com/D1355
Summary:
we used to need this function for security purposes, but no longer need
it. remove it so that some call sites can be optimized via smarter data
fetching, and so the whole codebase can have one less thing in it.
Test Plan:
verified the images displayed properly for each of the following
- viewed a diff with added images.
- viewed a user feed
- viewed a user profile
- viewed all image macros
- viewed a paste and clicked through "raw link"
weakness in testing around proxy files and transformed files. not sure what
these are. changes here are very programmatic however.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, btrahan, epriestley
Maniphest Tasks: T672
Differential Revision: https://secure.phabricator.com/D1354
Summary: When private messaged, the bot responds via private message to the
sender, instead of sending a private message to itself.
Test Plan: Mentioned tasks in public channels and private messages.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan
Maniphest Tasks: T274
Differential Revision: https://secure.phabricator.com/D1350
Summary: Simple notificaiton handler that reads the difx event timeline and
posts notifications to IRC.
Test Plan: Ran it in #phabricator.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan
Differential Revision: https://secure.phabricator.com/D1337
Summary: Preview of Add Reviewers looks silly without actually showing them
Test Plan:
Go to any diff
Leap into action: Add Reviewers
Add some reviewers
Write some comment
Preview including Added reviewers should be displayed
Change action to Comment
Added reviewers should disappear
Repeat with Add CCs
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley, vrana
Differential Revision: https://secure.phabricator.com/D1276
Summary:
It used to be more useful for daemons to spew random debugging information, but
features like "phd debug" and some fixes to error reporting like D1101 provide
better ways to debug, test, develop and diagnose daemons.
- Stop writing "." every time MetaMTA sends a message.
- Stop spewing the entire IRC protocol from the IRC bot unless in debug mode.
- Stop writing GC daemon log entries about collecting daemon logs (DURRR)
unless in debug mode.
Test Plan: Ran daemons in debug and non-debug modes, got expected level of
noisiness.
Reviewers: jungejason, nh, btrahan
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: https://secure.phabricator.com/D1268
Summary: Allow the bot to answer the question "where is X?", where X is a
symbol.
Test Plan:
phabotlocal joined the chat room.
epriestley: phabotlocal: where is DarkConsole?
phabotlocal left the chat room. (Remote host closed the connection)
phabotlocal joined the chat room.
epriestley: phabotlocal: where is DarkConsole?
phabotlocal left the chat room. (Remote host closed the connection)
phabotlocal joined the chat room.
epriestley: phabotlocal: where is DarkConsole?
phabotlocal: class DarkConsole (php):
http://local.aphront.com/diffusion/SUBC/browse/src/aphront/console/api/DarkConsole.php$22
epriestley: thanks phabotlocal that is vastly more useful
phabotlocal left the chat room. (Remote host closed the connection)
Reviewers: btrahan, jungejason
Reviewed By: jungejason
CC: aran, jungejason
Maniphest Tasks: T315
Differential Revision: https://secure.phabricator.com/D1261
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
Test Plan:
Created a listener that adds some patterns to $matches array, reloaded
Differential, some changesets were not shown as generated.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley, mareksapota
Differential Revision: 1200
Summary: See T632. When we miss a @mention, preserve the original case. This
approach is slightly unwieldy, but preserves backward compatibility (remarkup is
cached in Differential and Maniphest).
Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-u7z5j73dxrr4vuwkdcy3/
Reviewers: aran, btrahan
Reviewed By: aran
CC: aran, epriestley
Differential Revision: 1141
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
Summary: a well-titled diff this be. i feel 'meh' about the change; doesn't
seem to help too much imo.
Test Plan:
edited my custom conf file to have errors -
127.0.0.1 => 127.0.0.2
mysql_user => mysql_users
and for phabricator to be in setup mode. for each error i verified that i liked
the display.
Reviewers: epriestley, jungejason
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: 1129
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
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
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
Summary: See T548 and D996. Makes Phabricator configure the remarkup engine so
http:// and https:// get linked. Also make the "named link" syntax respect the
whitelist.
Test Plan:
- Whitelisted URIs (they get linked).
- Other URIs (not linked).
- Whitelisted, named URIs (linked).
- Other, named URIs (treated as phriction links).
- Actual phriction links (work correctly).
Reviewers: jungejason, nh, tuomaspelkonen, aran, benmathews
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: 997
Summary: There are currently two files, but all scripts require both of them,
which is clearly silly. In the longer term I want to rewrite all of this init
stuff to be more structured (e.g., merge webroot/index.php and __init_script__
better) but this reduces the surface area of the ad-hoc "include files" API we
have now, at least.
Test Plan:
- Grepped for __init_env__.php (no hits)
- Ran a unit test (to test unit changes)
- Ran a daemon (to test daemon changes)
Reviewers: jungejason, nh, tuomaspelkonen, aran
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: 976
Summary:
This is a pretty bad, but working implmentation of a way to kill one particular PID that
is controlled by Phabricator. Also remove the second 'stop' from the ##phd help## list.
Test Plan:
[ricky@rhelpad01 phabricator] (phd-stop-twice)$ ./bin/phd status
PID Started Daemon
30154 Oct 1 2011, 2:38:08 AM PhabricatorMetaMTADaemon
30172 Oct 1 2011, 2:38:09 AM PhabricatorMetaMTADaemon
30190 Oct 1 2011, 2:38:09 AM PhabricatorMetaMTADaemon
30210 Oct 1 2011, 2:38:09 AM PhabricatorMetaMTADaemon
[ricky@rhelpad01 phabricator] (phd-stop-twice)$ ./bin/phd stop 30190
Stopping daemon 'PhabricatorMetaMTADaemon' (30190)...
Daemon 30190 exited normally.
[ricky@rhelpad01 phabricator] (phd-stop-twice)$ ./bin/phd stop 123456
123456 is not controlled by Phabricator. Not killing.
[ricky@rhelpad01 phabricator] (phd-stop-twice)$ ./bin/phd stop
Stopping daemon 'PhabricatorMetaMTADaemon' (30154)...
Stopping daemon 'PhabricatorMetaMTADaemon' (30172)...
Stopping daemon 'PhabricatorMetaMTADaemon' (30210)...
Daemon 30210 exited normally.
Daemon 30154 exited normally.
Daemon 30172 exited normally.
Reviewers: epriestley
CC:
Differential Revision: 975
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
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
setup
Summary: See T481. We'll fail the pcntl test if we don't have this, in a
potentially confusing way. Test and detect missing 'php' explicitly before we
try the pcntl test, so we can give the user a better error message.
Test Plan: In setup mode, did a good run and then faked it to execute 'phpx'
instead to get a failure.
Reviewers: johnduhart, jungejason, tuomaspelkonen, aran
Reviewed By: tuomaspelkonen
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 878
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
Summary: If the link text is a URI, just treat it as a nameable (and possibly
relative) URI link. See tasks.
Test Plan: Copy/pasted the doc example into Phriction, links worked.
Reviewers: skrul, hunterbridges, jungejason, tuomaspelkonen, aran
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: 882
Summary: This may make it easier to debug problems with CLI + pcntl
Test Plan: Added a syntax error to the script and got more useful output
Reviewers: johnduhart, jungejason, tuomaspelkonen, aran
Reviewed By: johnduhart
CC: aran, johnduhart
Differential Revision: 869
Summary: Do JOIN in the protocol handler, after we receive 376 ("end of motd").
Test Plan: Ran bot, it joined a channel after receieving a 376 command.
Reviewers: moos3, codeblock, aran, jungejason, tuomaspelkonen
Reviewed By: moos3
CC: aran, moos3
Differential Revision: 855
"user":"authenticationusername",
"pass":"thisuserspassowrd",
This will allow people with internal irc servers to use this if they control access from ldap for irc.
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
Summary:
We don't currently validate CSRF tokens on this workflow. This allows an
attacker to upload arbitrary files on the user's behalf. Although I believe the
tight list of servable mime-types means that's more or less the end of the
attack, this is still a vulnerability.
In the long term, the right solution is probably to pass CSRF tokens on all Ajax
requests in an HTTP header (or just a GET param) or something like that.
However, this endpoint is unique and this is the quickest and most direct way to
close the hole.
Test Plan:
- Drop-uploaded files to Files, Maniphest, Phriction and Differential.
- Modified CSRF vaidator to use __csrf__.'x' and verified uploads and form
submissions don't work.
Reviewers: andrewjcg, aran, jungejason, tuomaspelkonen, erling
Commenters: andrewjcg, pedram
CC: aran, epriestley, andrewjcg, pedram
Differential Revision: 758
Summary:
Provide a {T123} syntax which pulls in the entire name of an object, not just a
link to it. A major use for this is organizing projects using wiki pages. Since
handle links show object status now, this lets you organize stuff in an ad-hoc
way and get a reasonable overview of it. We can make handles richer in the
future, too.
The performance on this isn't perfect (it adds some potential single gets) but I
think it's okay for now and I don't want to make remarkup engine even more
complex until the preprocess/postprocess stuff has had a chance to settle and
I'm more confident it works.
In Differential and Maniphest we'll also incorrectly cache the object
state/name, but that'll fix itself once I move the cache code to use
preprocess/postprocess correctly.
Test Plan:
- See https://secure.phabricator.com/file/view/PHID-FILE-5f9ca32407bec20899b9/
for an example.
- Generated and looked over the documentation.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, hunterbridges
CC: skrul, aran, jungejason, epriestley
Differential Revision: 784
Summary:
This accomplishes two goals:
- Data fetching is now grouped across blocks.
- Demonstrates that D737 actually works.
Test Plan: Used @mentions in Phriction preview, they rendered properly. Verified
only one service call was being made across blocks.
Reviewed By: jungejason
Reviewers: hunterbridges, jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 738
query
Summary:
- Provide an example unit test, and document it.
- Document database isolation better.
- When we issue an unsimulated query to the isolated connection, throw a
helpful message.
- Pygments is complaining about my madeup "lang=demo", change it to
"lang=text".
Test Plan:
- Ran the unit test (sanity check).
- Ran all other unit tests (verify I didn't break isolation).
- Added a queryfx(..., 'SELECT 1') to a test and verified it throws.
- Read the documentation.
Reviewed By: edward
Reviewers: edward, jungejason, tuomaspelkonen, aran
CC: aran, edward
Differential Revision: 773
Summary:
This gets all the major pieces working. Allows you to drag-and-drop files in
Differential and Phriction, and embed files in remarkup with {Fxxx} references.
See also task.
I'm explicitly not documenting this yet since it's still pretty rough.
Test Plan: Dragged and dropped stuff into Differential and Phriction.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, tomo
Commenters: tomo
CC: aran, tomo, jungejason
Differential Revision: 674
Summary: This has to table scan a ginormous table right now, give it a fighting
chance with a more usable key.
Test Plan:
- Launched GC daemon, no errors.
- Used test console to create a new transcript.
- Viewed some old transcripts.
- Ran EXPLAIN on the SELECT and verified it was utilizing the garbageCollected
key.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 735
Summary: This didn't end up getting used but I neglected to delete it.
Test Plan: git grep
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, jungejason
Differential Revision: 721
problems
Summary: Detect more PHP misconfigurations in setup.
Test Plan: Broke my configuration, ran setup, it seemed to detect all the
problems and issue meaningful error messages.
Reviewed By: jungejason
Reviewers: hunterbridges, 10098, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 717
Summary:
See T344. Currently, there's a hard-coded 12MB filesize limit and some awkward
interactions with MySQL's max_allowed_packet. Make this system generally more
robust:
- Move the upload limit to configuration.
- Add setup steps which reconcile max_allowed_packet vs MySQL file storage
limits.
- Add a layer of indirection between uploading files and storage engines.
- Allow the definition of new storage engines.
- Define a local disk storage engine.
- Add a "storage engine selector" class which manages choosing which storage
engines to put files in.
- Document storage engines.
- Document file storage classes.
Test Plan:
Setup mode:
- Disabled MySQL storage engine, misconfigured it, configured it correctly.
- Disabled file storage engine, set it to something invalid, set it to
something valid.
- Verified max_allowed_packet is read correctly.
Application mode:
- Configured local file storage.
- Uploaded large and small files.
- Verified larger files were written to local storage.
- Verified smaller files were written to MySQL blob storage.
Documentation:
- Read documentation.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 695
Summary:
Share code with the new PhabricatorDifferenceEngine, which handles diffs with no
changes correctly.
(This isn't the same issue as file moves, but I ran into it while generating a
repro case.)
Test Plan: Previously, changes which didn't change file content (e.g., property
changes) would throw. Now they work.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 698
Summary:
Single brackets are getting some troublesome false positives in Facebook's
install. Particularly, there's a weird convention at Facebook of tagging diffs
by putting stuff like "[perf]" or "[chat]" in the title, although this isn't
turned into structured data at any stage. When commits appear in Diffusion, we
currently link such ad-hoc tags to Phriction.
Wikipedia uses double-bracket sytnax, as do many other wikis, so this seems like
a reasonable burden to place on the lightweightness of the markup. The
alternative is selectively disabling Phriction markup in some interfaces, but
I'd rather allow integration in commit messages and just guard the syntax more
closely.
(I'm not providing any sort of migration plan since this landed less than a week
ago and I'm pretty confident no one has built a huge wiki yet, but I added a
CHANGELOG note.)
Test Plan: Edited a wiki document and added some links. Verified single brackets
were unlinked and double brackets were linked.
Reviewed By: jungejason
Reviewers: hsb, aran, jungejason, tuomaspelkonen
CC: aran, jungejason, epriestley
Differential Revision: 689
Summary:
This is really rough and needs work (particularly, there's some diff code I
really need to refactor since I sort-of-copy-pasted it) but basically
functional.
Show text changes between diffs and allow users to revert to earlier versions.
Differential's line-oriented diff style isn't ideal for large blocks of text but
I'm betting this is probably good enough in most cases. We can see how bad it is
in practice and then fix it if needbe.
I added a bunch of support for "description" but didn't add the feature in this
diff, I'll either follow up or task it out since it should be a pretty
straightforward change.
Test Plan: Looked at history for several Phriction documents, clicked "previous
change" / "next change", clicked revert buttons.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen
CC: aran, hsb, epriestley
Differential Revision: 687
Summary:
- Exceptions on the rendering pathway currently go uncaught and result in a
blank page. Commonly, this is a bad require_celerity_resource() call. Although
we can't safely render a page if the rendering pathway is broken, we can show a
useful message.
- When PHP exits because of a fatal error, there is an opportunity to run code
in the shutdown handler. This allows us to show messages at least some of the
time, e.g. "call to unknown function derp() in somefile.php at line 99"
- flip dem tables
Test Plan: Added fatals ("derp();") and rendering exceptions
("require_celerity_resource('does-not-exist')") to a controller and verified
that the error handling behavior is now more useful.
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley
Differential Revision: 680
Summary: Document linking and some general layout improvements. I'd like to
eventually do more meta-dataey things with links (like store them separately and
check them for 404s) but this is a decent start.
Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-d756b94a06b69c273fce/
Reviewed By: jungejason
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 650
Summary:
A bunch of the .sql patch files don't explicitly specify the table engine, but
we should always use InnoDB with the exception of one table which needs MyISAM
for FULLTEXT.
MySQL doesn't no-op an ALTER TABLE statment that changes the engine back to
itself and converting large tables can be time consuming, so convert only the
required tables.
Test Plan: Ran on secure.phabricator.com and my local box, it fixed all the
issues in about 3 seconds on secure.phabricator.com and <<1 second on my local.
Reviewed By: codeblock
Reviewers: codeblock, tuomaspelkonen, jungejason, aran
CC: aran, epriestley, codeblock
Differential Revision: 641
Summary:
If we're going to hardcode a path, at least let's do it in a way that works on RHEL too.
Test Plan:
Successfully ran the setup script on RHEL.
Reviewers:
epriestley
CC:
Differential Revision: 623
Summary:
Add the differential parse cache to the GC. This is the largest object in the
system by a wide margin, I think.
This table is potentially gigantic which is why the script truncates it before
doing a schema change.
Test Plan: Ran the GC daemon, it cleaned up some parse caches.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
Commenters: tuomaspelkonen
CC: aran, jungejason, tuomaspelkonen, epriestley
Differential Revision: 620
Summary:
Now that we store language with pastes, we can include this in Conduit.
Test Plan:
Tried it on a paste with a blank language, and one with a specified language.
16:14:50 <@CodeBlock> P1
16:14:51 <@codeblock-phabot> P1: http://phabricator.local/P1 - test.php
16:15:05 <@CodeBlock> P43
16:15:06 <@codeblock-phabot> P43: http://phabricator.local/P43 - sadoijfoisaf (php)
Reviewers:
epriestley, Ttech
CC:
Differential Revision: 616
Summary:
Phabricator generates a bunch of data that we don't need to keep around forever,
add a GC daemon to get rid of it with some basic configuration options.
This needs a couple more diffs to get some of the details but I think this is a
reasonable start.
I also fixed a couple of UI things related to this, e.g. the daemon logs page
going crazy when a daemon gets stuck in a loop and dumps tons of data to stdout.
Test Plan:
- Ran gc daemon in 'phd debug' mode and saw it delete stuff, then sleep once
it had cleaned everything up.
- Mucked around with TTLs and verified they work correctly.
- Viewed gc'd transcripts in the web interface and made sure they displayed
okay.
- Viewed daemon logs before/after garbage collection.
- Running some run-at / run-for tests now, I'll update if the daemon doesn't
shut off in ~10-15 minutes. :P
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 583
Summary: When the remote closes the connection, phabot goes into a busy loop
because of PHP's "nothing should ever be an error" semantics. Instead, detect
connection termination.
Test Plan: Disabled the "PONG" response in the protocol handler and let freenode
disconnect phabot. It spun out of control before, now it detects the issue and
exits to await automatic restart.
Reviewed By: jungejason
Reviewers: codeblock, jungejason, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 582
Summary:
Summarize (or at least link to) pastes when P<id> is given in a channel. This depends on D552!
Test Plan:
04:34:03 <@codeblock-phabot> P15 - http://phabricator.local/P15
04:35:22 <@CodeBlock> P1 asdf foo Bar Baz Diet Sprite P4
04:35:24 <@codeblock-phabot> P1 - http://phabricator.local/P1
04:35:24 <@codeblock-phabot> P4 - http://phabricator.local/P4
Reviewers:
epriestley
CC:
Differential Revision: 553
Summary:
Maniphest tasks can now be summarized by referencing their respective T<id> in irc.
Test Plan:
03:29:28 <@CodeBlock> T218
03:29:29 <@codeblock-phabot> T218: NYANCAT (Priority: Wishlist) - https://secure.phabricator.com/T218
03:30:14 <@CodeBlock> T58 asdf foo bar baz mcFoo T163
03:30:15 <@codeblock-phabot> T58: Allow AphrontFormView to render as a <div /> instead of a form (Priority: Wishlist) - https://secure.phabricator.com/T58
03:30:15 <@codeblock-phabot> T163: arc amend is slow (Priority: Needs Triage) - https://secure.phabricator.com/T163
...
03:28:23 <@CodeBlock> Oh. Hah, I'm testing against production Conduit. Ah well.
03:28:40 <@CodeBlock> I don't always test my code...but when I do, I do it in prod! :D
Reviewers:
epriestley
CC:
Differential Revision: 551
Summary:
When a user gets @mentioned in Differential, add them as a CC.
No Maniphest hookup yet since I want to make that one a little more formal.
Depends on D518.
Test Plan:
@mentioned a user and they were added as a CC.
Reviewed By: jungejason
Reviewers: tomo, mroch, jsp, jungejason, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 519
Summary:Make a new directory, src/infrastructure/markup/remarkup/markuprule/paste/
Make a new class called PhabricatorRemarkupRulePaste in that directory.
Add the rule to DifferentialMarkupEngineFactory.
Test Plan: Created a task in maniphest. Put P1 and P2 in the content.
Created P1 and P2 in Paste. Verified P1 and P2 were highlighted and
linked correctly.
Reviewers:epriestley, codeblock
CC:jungejason
Differential Revision: 539
Summary:
The web console already correctly checks for processes which are running but
can't be signaled. Share this check in the CLI.
Test Plan:
Looked at the web console. Poked at the CLI.
Reviewed By: fratrik
Reviewers: fratrik
CC: aran, fratrik
Differential Revision: 544
Summary:
Currently we send logs to the logfile in 'phd debug', but we should send them to
the console instead.
Also fixed some %C stuff which could theoretically cause problems if a user had
percentage symbols in their paths (heaven forbid).
fratrik, this or D535 might have been involved in frustrating your efforts to
debug the "sudo" stuff.
Test Plan:
Ran "phd debug irc derpderp" and "phd launch irc derpderp". In the former case,
the exception appeared in the console. In the latter, it appeared in the log.
Reviewed By: codeblock
Reviewers: codeblock, jungejason, tuomaspelkonen, aran, fratrik
CC: aran, codeblock
Differential Revision: 536
Summary:
Provides basic Remarkup support for @mentions. No application integration yet so
these aren't terribly useful until that happens.
Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-83d68e7af6085ae928df/
Reviewers: tomo, mroch, jsp
Commenters: tomo
CC: aran, tomo, epriestley
Differential Revision: 517
Summary:
The MySQL MyISAM fulltext backend defaults to a minimum length of 4, but many
3-letter words are useful search terms. Provide instructions for configuring
MySQL to reduce the auto-stopword length.
Test Plan:
Followed instructions, searched for a 3-letter term and got a hit.
Reviewed By: jungejason
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran, jungejason
Differential Revision: 471
Summary:
Tweaks to the paste app:
- I realized that unlike all the other apps, it makes more sense for the
default view of this one to be "create paste" instead of "list pastes" since
when you access the application directly you are most often wanting to share
something. Swap list out of the default slot and make edit the default.
- Make the textarea bigger (usability).
- Allow you to copy an existing paste.
- Implement 'raw view'.
- Tweak/adjust list view (usability, formatting).
- Tweak page titles.
Test Plan:
Created, copied, and listed pastes. Viewed raw paste. Created an invalid paste.
Tried to create a copy of a nonexistant paste.
Reviewed By: codeblock
Reviewers: codeblock, jungejason, aran, tuomaspelkonen
CC: aran, epriestley, codeblock
Differential Revision: 456
Summary:
Nicks can contain more characters than were allowed. The new regex is ugly,
but should include most of them.
Test Plan:
Joined the bot as phabot-c0d`eb1^ock successfully.
Reviewers:
epriestley
CC:
Differential Revision: 453
Summary:
remove accessing the db config info directly. Use
DatabaseConfigurationProvider instead. Also fixed a minor issue where
different number of newlines are output in PhabricatorSetup.php's output.
Test Plan:
executed upgrade_schema.php; executed PhabricatorSetup.php by
setting 'phabricator.setup' to true.
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, jungejason, epriestley
Differential Revision: 443
Summary:
Make 'gd' an explicit optional dependency, test for it in setup, and make the
software behave correctly if it is not available.
When generating file thumnails, provide reasonable defaults and behavior for
non-image files.
Test Plan:
Uploaded text files, pdf files, etc., and got real thumbnails instead of a
broken image.
Simulated setup and gd failures and walked through setup process and image
fallback for thumbnails.
Reviewed By: aran
Reviewers: toulouse, jungejason, tuomaspelkonen, aran
CC: aran, epriestley
Differential Revision: 446
Summary:
Make it easier to find obvious problems in daemons by letting them run
undaemonized in the console without requiring the user to know the magical
incantations for loading libraries, etc.
Test Plan:
Ran "phd debug nice", simulated some failures (e.g., bringing down Phabricator,
daemon fatal) and got useful error messages.
Reviewed By: jungejason
Reviewers: toulouse, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 448
Summary:
- Make the instructional text generally more useful.
- Show the current configured adapter.
- When the configuration prevents outbound email from being delivered, show a
warning.
- Detect 'curl' extension during setup since it's more-or-less required
- Add curl extension to the install scripts
codeblock: can you verify the rhel-derivs changes are correct?
Test Plan:
Set adapter to test, verified warning; entered setup mode and verified curl. Ran
apt-get on an ubuntu box. Ran yum on an amazon linux box.
Reviewed By: toulouse
Reviewers: toulouse, codeblock
Commenters: codeblock
CC: aran, jungejason, tuomaspelkonen, codeblock, epriestley, toulouse
Differential Revision: 438
error message if that doesn't work
Summary:
This workflow is needlessly bad right now, make it not terrible.
Also removed some related, unused code.
Test Plan:
Ran 'phd launch nice' with no directory and with a failing mkdir command.
Reviewed By: toulouse
Reviewers: hsb, toulouse, codeblock
CC: aran, toulouse
Differential Revision: 440
Summary:
There was a last-minute edit to this to fix a typo before rP089d8327 landed
which accidentally made it impossible to pass the check. :)
Test Plan:
Put install into setup mode, changed protocol to 'http', 'ftp'.
Reviewed By: cadamo
Reviewers: toulouse, codeblock, cadamo
Commenters: toulouse, codeblock
CC: aran, cadamo, toulouse, codeblock
Differential Revision: 434
Summary:
When a user hits 'cancel' on a 'new', 'edit', or 'reply' operation, add a little
"Changes discarded. __Undo__" insert so they can get their change back. No undo
for delete since there's an explicit prompt. Once this lands we can make
'escape' work again to close dialogs.
This change started feeling really good when I was merging all the duplicate
code and making things more consistent, but by the time I started writing client
rendering it felt gross. I'm not really thrilled with it but I guess it's a step
forward? The feature seems pretty OK in practice. Let me know how much barfing
this causes and I can try to remedy the most acute concerns.
This also fixes a bug where replies always (?) appear on the 'new' side of the
diff (I think?).
Test Plan:
Applied 'new', 'edit', 'delete' and 'reply' operations, pressed 'cancel' and
'okay' in each case, with and without changing text where relevant. All
behaviors seem to conform with expectations, except that canceling out of 'edit'
without changing the text gives you an option to undo when it shouldn't really.
There's no super easy way to get at the original text right now.
Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: simpkins, aran, epriestley
Differential Revision: 406
Summary:
Well, since I couldn't regenerate my arcanist cert I figured out that this wass because "workflows" are unavailable there now. I really can not figure out why but it was.
I added in the setup script, the ability to check if is present the protocol of the host and if it has a trailing slash a the end of the line, since both are needed to generate the cert.
Users now only be able to upload valid image files with mimetype of jpg, jpeg,
png and gif.
Test Plan:
FIRST: DO NOT apply those changes! then
1- go to settings->arcanist certificate and the click on regenerate ... humm
2- On your config file, delete the trailing slash at the end and the protocol on "phabricator.base-uri", then go to setting->arcanist certificate. Here you
will see something like this "phabricator.example.comapi\/" instead of
"http:\/\/phabricator.example.com\/api\/".
SECOND: Now apply this changes:
1- Go to settings->arcanist certificate and the click on regenerate.
2- On your config file, delete the trailing slash at the end and the protocol
on "phabricator.base-uri", and setup "phabricator.setup" to true.
3- Then go to setting->arcanist certificate and you could see that this was successfully generated.
THIRD:
Go to settings->account and try to upload an invalid image file, and do the same on "youruserna"->edit profile.
Reviewed By: epriestley
Reviewers: epriestley jungejason
CC: epriestley jugesason cadamo aran
Differential Revision: 391
Summary:
Allows you to link to comments with "D123#3" or "T123#3", then adds a pile of JS
to try to make it not terrible. :/
The thing I'm trying to avoid here is when someone says "look at this!
http://blog.com/#comment-239291" and you click and your browser jumps somewhere
random and you have no idea which comment they meant. Since I really hate this,
I've tried to avoid it by making sure the comment is always highlighted.
Test Plan:
Put T1#1 and D1#1 in remarkup and verified they linked properly.
Clicked anchors on individual comments.
Faked all comments hidden in Differential and verified they expanded on anchor
or anchor change.
Reviewed By: aran
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 383
Summary:
In RHEL6 at the least, pcntl installs from distro package management to the CLI
but not to Apache. Since we don't need it in apache and it's a pain to build
manually, just verify it exists on the CLI.
Test Plan:
Simulated script failures and verified setup output.
Reviewed By: codeblock
Reviewers: codeblock, aran, jungejason, tuomaspelkonen
CC: aran, epriestley, kevinwallace, codeblock
Differential Revision: 380
Summary:
Sendmail is seriously difficult to configure; SendGrid is extremely easy. It's
also pretty expensive ($80/mo) but there are a bunch of startups that already
have plans so it's effectively free for them.
Test Plan:
Configured SendGrid and sent reply email through it.
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley
Differential Revision: 376
