Summary: Ref T3583. Use the same approach Harbormaster does to give panels cheap forms.
Test Plan:
{F149218}
{F149219}
{F149220}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T3583
Differential Revision: https://secure.phabricator.com/D8919
Summary: Took a short pass here with the new UI, holler if something is TOO EXTREME.
Test Plan:
Tested with manual sleep builds.
{F148693}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D8901
Summary: Throwing this up for testing, swapped out all icons in timeline for their font equivelants. Used better icons where I could as well. We should feel free to use more / be fun with the icons when possible since there is no penalty anymore.
Test Plan: I browsed many, not all, timelines in my sandbox and in IE8. Some of these were just swagged, but I'm expecting we'll do more SB testing before landing.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D8827
Summary:
Fixes T4810. When a buildable completes, make an effort to update the corresponding object with a success or failure message. Commits don't support this yet, but revisions do.
{F144614}
Test Plan:
- Used `bin/harbormaster build` and `bin/harbormaster update` to run a pile of builds.
- Tried good/bad builds.
- Sent some normal mail to make sure the mail reentrancy change didn't break stuff.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4810
Differential Revision: https://secure.phabricator.com/D8803
Summary:
Ref T4809. Currently, buildables have a status field but nothing populates it. Populate it:
- When builds change state, update the Buildable state.
- Use the new Buildable state on the web UI.
- Return the new Buildable state from Conduit.
To make it easier to debug/test this:
- Provide `bin/harbormaster update Bxxx ...` to force foreground update of a Buildable.
Test Plan:
- Used `bin/harbormaster update Bxxx --force --trace` to update buildables.
- Looked at buidlable list, saw statuses reported properly.
- Used Conduit to read statuses.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4809
Differential Revision: https://secure.phabricator.com/D8799
Summary:
Ref T4809. Buildables currently have buildStatus and buildableStatus. Neither are used, and no one knows why we have two.
I'm going to use buildableStatus shortly, but buildStatus is meaningless; burn it.
Test Plan: `grep`, examined similar get/set calls, created a new buildable, ran storage upgrade.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4809
Differential Revision: https://secure.phabricator.com/D8796
Summary:
Ref T4809. This one is more straightforward. A couple of tweaks:
- Remove the WAITING status, since nothing ever sets it and I suspect nothing ever will with the modern way artifacts work (maybe). At a minimum, it's confusing with the new Target status that's also called "WAITING" but means something different.
- Consolidate 17 copies of these status names into one method.
Test Plan: Ran some queries via Conduit, got reasonable looking results.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4809
Differential Revision: https://secure.phabricator.com/D8795
Summary: Ref T4809. I need to sort out some of the "status" stuff we're doing before this is actually useful (there's no sensible "status" value to expose right now) but once that happens `arc` can query this to figure out whether it needs to warn the user about pending/failed builds.
Test Plan: Ran query with various different parameters.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4809
Differential Revision: https://secure.phabricator.com/D8794
Summary:
For Harbormaster tasks which want to poll or wait, this lets them say "try again a little later" without having to sleep and hold a queue slot.
This is basically the same as failing, except that we don't increment the failure counter. Instead, we just set the current lease to the correct length and then exit. The task will be retried after the lease expires.
Test Plan: Using both `bin/harbormaster` and `phd debug taskmaster`, ran a lot of waiting tasks through the queue, faking them to either yield or not yield in a controlled manner. The queue responded as expected, yielding tasks appropraitely and retrying them later.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8792
Summary:
Without this, build steps that have no options (like "wait for previous commits") don't actually save, since the transaction array is empty.
This also generally nice and consistent.
Test Plan: Created a new "wait" step, viewed transaction log.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8791
Summary:
This hooks up all the pieces of the build pipeline so `harbormaster.sendmessage` actually works. Particularly:
- Candidate build steps (i.e., those which interact with external systems) can now "Wait for Message". This pauses them indefinitely when they complete, until something calls `harbormaster.sendmessage`.
- After processing a target, we check if we should move it to PASSED or WAITING.
- Before updating a build, we move WAITING targets with pending messages to either PASSED or FAILED.
- I added an explicit "Building" state, which doesn't affect workflows but communicates more information to human users.
A big part of this is avoiding races. I believe we get the correct behavior no matter which order events occur in:
- We update builds after targets complete and after we receive messages, so we're guaranteed to update once both these conditions are true. This means messages can't be lost (even if they arrive before a build completes).
- The minor changes to the build engine logic mean that firing additional build updates is always safe, no matter what the current state of the build is.
- The build itself is protected by a lock in the build engine.
- The target is not covered by an explicit lock, but for all states only the engine (waiting) //or// the worker (all other states) can interact with it. All of the interactions also move the target state forward to the same destination and have no other side effects.
- Messages are only consumed inside the engine lock, so they don't need an explicit lock.
Test Plan:
- Made an HTTP request wait after completion, then ran a pile of builds through it using `bin/harbormaster build` and the web UI.
- Passed and failed message-awaiting builds with `harbormaster.sendmessage`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, zeeg
Differential Revision: https://secure.phabricator.com/D8788
Summary: Fixes T4590. Use the credentials custom field to allow Harbormaster HTTP requests to include usernames/passwords.
Test Plan: Ran a build plan with credentials, verified they were sent to the remote server.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4590
Differential Revision: https://secure.phabricator.com/D8786
Summary:
See discussion in D8773. Three small adjustments which should help prevent this kind of issue:
- When queueing followup tasks, hold them on the worker until we finish the task, then queue them only if the work was successful.
- Increase the default lease time from 60 seconds to 2 hours. Although most tasks finish in far fewer than 60 seconds, the daemons are generally stable nowadays and these short leases don't serve much of a purpose. I think they also date from an era where lease expiry and failure were less clearly distinguished.
- Increase the default wait-after-failure from 60 seconds to 5 minutes. This largely dates from the MetaMTA era, where Facebook ran services with high failure rates and it was appropriate to repeatedly hammer them until things went through. In modern infrastructure, such failures are rare.
Test Plan:
- Verified that tasks queued properly after the main task was updated.
- Verified that leases default to 7200 seconds.
- Intentionally failed a task and verified default 300 second wait before retry.
- Removed all default leases shorter than 7200 seconds (there was only one).
- Checked all the wait before retry implementations for anything much shorter than 5 minutes (they all seem reasonable).
Reviewers: btrahan, sowedance
Reviewed By: sowedance
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8774
Summary: Ref T1049. I'm fair sure this is just a case of bad data in my local install, but we probably don't want the default page for Harbormaster to break when there's invalid / missing container or buildable handles on any of the builds.
Test Plan: Loaded the page, didn't get a crash due to null reference.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: demo, epriestley, Korvin
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D8608
Summary:
Ref T1049. Currently, the "add" dialog lets you select a build step type, but then immediately creates one. If you "cancel" from the edit screen, you end up with an empty (and almost certainly invalid) build step.
Instead, don't create the step until it's valid.
Test Plan: Add Step -> Pick Type -> Add Step -> Cancel no longer creates empty step.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D8605
Summary:
Ref T1049. Allows external systems to send a message to a build target. The primary intended use case is:
- You make an HTTP request to Jenkins.
- The build goes into a "waiting" state.
- Later, Jenkins calls `harbormaster.sendmessage` to report that the target passed or failed.
- The build continues as appropriate.
This is deceptively complicated because:
- There are a lot of race concerns. We might get a message back from an external system before it even responds to the request we made. We want to make sure we process these messages no matter when we receive them.
- These messages need to be sent to a build target (vs a build or buildable) because we'll get into trouble with parallelization later on otherwise (Jenkins is told to do 3 builds; we can't tell which ones failed or what overall state is unless the message are sent to targets).
- I initially thought about implementing this as a separate "Wait for a response from an external system" build step. This gets a lot more complicated for users once we do parallelization, though. Particularly, in the case where you've told Jenkins to do 3 builds, the three "wait" steps need to know which target they're waiting for (and jenkins needs to know some unique identifier for each target). So this pretty much boils down to a more complicated, more error-prone version of using target PHIDs.
This makes the already-muddy Build UI a bit worse, but it needs a general clarity pass anyway (it's showing way too much uninteresting data, and should show a better summary of results instead).
Test Plan:
- This doesn't really do anything interesting yet.
- Used Conduit to send messages to build plans.
- Viewed the messages on the build screen.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D8604
Summary: Ref T1049. Tweaks some of the UI and code to improve / clean it up a bit.
Test Plan: Ran build plans, browsed UI.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D8603
Summary: Ref T1049. For consistency, rename these to "Harbormaster...".
Test Plan: Ran migration, ran builds, everything still works fine.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D8602
Summary: Ref T1049. D8588 already required custom code to change what it extends, so this is as good a time as we're going to get to move to more standard class name.
Test Plan: `arc liberate`; `arc lint`
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D8601
Summary:
Ref T1049. Fixes T4602. Moves all the funky field stuff to CustomField. Uses ApplicationTransactions to apply and record edits.
This makes "artifact" fields a little less nice (but still perfectly usable). With D8599, I think they're reasonable overall. We can improve this in the future.
All other field types are better (e.g., fixes weird bugs with "bool", fixes lots of weird behavior around required fields), and this gives us access to many new field types.
Test Plan:
Made a bunch of step edits. Here's an example:
{F133694}
Note that:
- "Required" fields work correctly.
- the transaction record is shown at the bottom of the page.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4602, T1049
Differential Revision: https://secure.phabricator.com/D8600
Summary:
Ref T1049. In Harbormaster, build steps may have various inputs (like a host they should run on) and outputs (like a reference to an uploaded file).
- Currently, inputs aren't defined anywhere (except implicitly at runtime).
- Instead, define inputs explicitly.
- Currently, outputs are defined in a way that loses information when misconfigured (the keys will collide).
- Instead, define inputs and outputs so they work whether a step is configured correctly or not.
- Currently, there's no simple way to see a step's inputs and outputs.
- Add some UI for this.
- Currently, reordering steps has some surprising side effects.
- Instead of invalidating steps after reordering them, validate them at display time and warn the user.
Test Plan:
{F133679}
{F133680}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, chad
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D8599
Summary: Ref T1049. This generally simplifies things. The steps which don't support variables generally don't make sense to support varaibles anyway.
Test Plan: Edited some steps.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D8588
Summary: Precedence here was mucked up.
Test Plan: Plan with no explicit "method" now defaults to POST correctly.
Reviewers: dctrwatson, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8559
Summary:
This revision adds a 'method' field to the HTTP request harbormaster build step. This allows the user to specify GET, POST, DELETE, and PUT (limited by the underlying wrapper phabricator uses for HTTP requests). I'm not sure how much sense PUT makes, but oh well.
Existing plans shouldn't break, as if this field is an empty string, we default to POST, which is the old behavior.
Fixes T4604
Test Plan: 1) Verified that the empty string does, in fact, issue a POST request. Changed the method to be GET and observed that the problem described in T4604 is resolved.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: aran, epriestley
Maniphest Tasks: T4604
Differential Revision: https://secure.phabricator.com/D8520
Summary: This can be a command, which might be arbitrarily long, but the column is VARCHAR(255).
Test Plan: `grep`
Reviewers: dctrwatson, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D8446
Summary:
Ref T1191. Test that MySQL's rules match those of `phutil_is_utf8_with_only_bmp_characters()`:
- Build a string with //every// character that we consider to be a BMP character.
- Write it into MySQL.
- Read it back out.
- Make sure MySQL didn't truncate it.
Test Plan: Ran unit test. This test runs pretty quickly (50ms), the string with every character isn't all that enormous.
Reviewers: btrahan, arice
Reviewed By: arice
CC: chad, arice, aran
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D8314
Summary:
Ref T4379. I want project subscriptions to work like this (yell if this seems whacky, since it makes subscriptions mean somethign a little different for projects than they do for other objects):
- You can only subscribe to a project if you're a project member.
- When you're added as a member, you're added as a subscriber.
- When you're removed as a member, you're removed as a subscriber.
- While you're a member, you can optionally unsubscribe.
From a UI perspective:
- We don't show the subscriber list, since it's going to be some uninteresting subset of the member list.
- We don't show CC transactions in history, since they're an uninteresting near-approximation of the membership transactions.
- You only see the subscription controls if you're a member.
To do this, I've augmented `PhabricatorSubscribableInterface` with two new methods. It would be nice if we were on PHP 5.4+ and could just use traits for this, but we should get data about version usage before we think about this. For now, copy/paste the default implementations into every implementing class.
Then, I implemented the interface in `PhabricatorProject` but with alternate defaults.
Test Plan:
- Used the normal interaction on existing objects.
- This has no actual effect on projects, verified no subscription stuff mysteriously appeared.
- Hit the new error case by fiddling with the UI.
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T4379
Differential Revision: https://secure.phabricator.com/D8165
Summary:
Hosted repositories have muddied this distinction somewhat. In some cases, we only want to use the real remote URI, and the call is only relevant for imported repositories.
In other cases, we want the URI we'd plug into `git clone`.
Move this logic into `PhabricatorRepository` and make the distinction more clear.
Test Plan: Viewed SVN, Git, and Mercurial hosted and remote repositories, all the URIs looked reasonable.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, dctrwatson
Differential Revision: https://secure.phabricator.com/D8096
Summary: Fixes T4336. This updates the build engine to delete all artifacts when targets are being deleted. This prevents conflicts when builds are restarted.
Test Plan: Restarted a build that had a lease host step and it didn't crash.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T4336
Differential Revision: https://secure.phabricator.com/D8092
Summary:
Ref T2015. Several fixes:
- `checkForCancellation()` no longer exists, and isn't relevant for resumable stops. Throw it away for now.
- Fix an issue where a build could pass even if the final step failed.
- `phlog()` exceptions so they show up in `bin/harbormaster` and the daemon logs.
- Write an exception log if a step fails.
- Add a "throw an exception" step to debug this stuff more easily.
Test Plan:
- Grepped for `checkForCancellation()`.
- Ran a failing build where the final step caused the failure.
- Observed `phlog()` in `bin/harbormaster` output.
- Observed log in web UI:
{F101168}
Reviewers: btrahan, hach-que
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D7935
Summary: Ref T2015. This workflow is a little weird (runs in a dialog, no edit-before-create step, lots of internal classnames). Make it a little more standard.
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D7908
Summary: Ref T1049. Creates convenience actions at the Buildable level to stop, resume, or restart all builds.
Test Plan:
- Stopped all builds.
- Resumed all builds.
- Restarted all builds.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7899
Summary:
Ref T1049. Improves the UI:
- Pending commands, like "stopping", are shown separately from the current status.
- Pending commands are shown on the list view.
- Builds can be restarted, stopped and resumed from the list view.
- Add a missing crumb.
Test Plan:
{F99022}
{F99023}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7898
Summary: Ref T1049. The logic in the BuildEngine is a little different from the logic on the Build itself. Make these more consistent, and make queued commands more private.
Test Plan: Restarted, stopped, and resumed a build.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7897
Summary:
Ref T1049. Currently you can cancel a build, but now that we're tracking a lot more state we can stop, resume, and restart builds.
When the user issues a command against a build, I'm writing it into an auxiliary queue (`HarbormasterBuildCommand`) and then reading them out in the worker. This is mostly to avoid race messes where we try to `save()` the object in multiple places: basically, the BuildEngine is the //only// thing that writes to Build objects, and it holds a lock while it does it.
Test Plan:
- Created a plan which runs "sleep 2" a bunch of times in a row.
- Stopped, resumed, and restarted it.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7892
Summary:
Ref T1049. Currently, the Harbormaster worker looks like this:
foreach (step) {
run_step(step);
}
This means steps can't ever be run in parallel. Instead, split it into two workers. The "Build" worker starts things off, and basically does:
update_build();
(We could theoretically do this in the original process because it should never take very long, but since there's a lock and it's a little bit complex it seemed cleaner to separate it.)
The "Target" worker runs an individual target (like a command, or an HTTP request, or whatever), then updates the build:
run_one_step(step);
update_build();
The new `update_build()` mechanism in `HarbormasterBuildEngine` does this, roughly:
figure_out_overall_status_of_all_steps();
if (build is done) { done(); }
if (build is fail) { fail(); }
foreach (step that is ready to run) {
queue_target_worker_for_step(step);
}
So, overall:
- The part of the code that updates Builds is completely separated from the part of the code that updates Targets.
- Targets can run in parallel.
Test Plan:
- Ran a bunch of builds via `bin/harbormaster build`.
- Ran a bunch of builds via web UI.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7890
Summary:
Ref T2015. Not directly related to Drydock, but I've wanted to do this for a bit.
Introduce a common base class for all the workflows in the scripts in `bin/*`. This slightly reduces code duplication by moving `isExecutable()` to the base, but also provides `getViewer()`. This is a little nicer than `PhabricatorUser::getOmnipotentUser()` and gives us a layer of indirection if we ever want to introduce more general viewer mechanisms in scripts.
Test Plan: Lint; ran some of the scripts.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D7838
Summary:
Ref T2015. Moves a bunch of raw object loads into modern policy-aware queries.
Also straightens out the Log and Lease policies a little bit: there are legitimate states where these objects are not attached to a resource (particularly, while a lease is being acquired). Handle these more gracefully.
Test Plan: Lint / browsed stuff.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D7836
Summary: See thread; fixes fatal. The actual name of this method is `getHarbormaster...`.
NOTE: This fixes a fatal in Differential which impedes review, so I'm pushing it as-is.
Test Plan: Browsed a revision.
Reviewers: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7834
Summary: Ref T2015. DrydockLease predates widespread adoption of policies. Make it -- and its query -- policy aware.
Test Plan: Browsed leases from the web UI. Grepped for callsites.
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D7826
Summary:
Ref T1049. Adds `bin/harbormaster` and `bin/harbormaster build` for applying plans from the console. Since this gets `--trace`, it's much easier to debug what's going on.
This doesn't work properly with some of the Drydock steps yet, I need to look at those. I think `setRunAllTasksInProcess` probably obsoletes some of the mechanisms. It might also not work with "Wait for Builds" but I didn't check.
Test Plan: Used `bin/harbormaster` to run a bunch of builds. Ran builds from web UI.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7825
Summary:
Ref T1049. Generally, it's useful to separate test/trial/manual runs from production/automatic runs.
For example, you don't want to email a bunch of people that the build is broken just because you messed something up when writing a new build plan. You'd rather try it first, then promote it into production once you have some good runs.
Similarly, test runs generally should not affect the outside world, etc. Finally, some build steps (like "wait for other buildables") may want to behave differently when run in production/automation than when run in a testing environment (where they should probably continue immediately).
So, formalize the distinction between automatic buildables (those created passively by the system in response to events) and manual buildables (those created explicitly by users). Add filtering, and stop the automated parts of the system from interacting with the manual parts (for example, we won't show manual results on revisions).
This also moves the "Apply Build Plan" to a third, new home: instead of the sidebar or Buildables, it's now on plans. I think this generally makes more sense given how things have developed. Broadly, this improves isolation of test environments.
Test Plan: Created some builds, browsed around, used filters, etc.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7824
Summary: Ref T1049. Adds "Repository", "Revision", "Diff" and "Commit" as searchable fields.
Test Plan: Used all the fields to filter things.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7823
Summary: Ref T4195. Ref T2783. We have an old-school implementation of this; move it into a LowLevel query and make callers all run through Conduit. I need the LowLevel query for hooks, to implement an "is merge commit" Herald rule.
Test Plan:
- Ran query via Conduit for SVN, Mercurial, Git.
- Parsed a commit which closed a revision, attach/closed worked correctly.
- Browsed Diffusion.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4195, T2783
Differential Revision: https://secure.phabricator.com/D7808
Summary: We currently have a lot of calls to `addCrumb(id(new PhabricatorCrumbView())->...)` which can be expressed much more simply with a convenience method. Nearly all crumbs are only textual.
Test Plan:
- This was mostly automated, then I cleaned up a few unusual sites manually.
- Bunch of grep / randomly clicking around.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: hach-que, aran
Differential Revision: https://secure.phabricator.com/D7787
Summary: Not every revision belongs to a repository, so we might end up here with `$repo` still equal to `null`. Don't fatal if we do.
Test Plan: iiam
Reviewers: btrahan, hach-que, zeeg
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7771
Summary: This adds a build step which will block a build from continuing if there are previous builds of the build plan still running.
Test Plan: Configured a build plan with a wait of 60 seconds and a "wait for previous builds", then started a build. While that was still building, reconfigured the plan to have a wait time of 3 seconds, started it, and saw it move into the "Waiting" status. When the 60 second build finished, both builds passed.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7745
