Page MenuHomePhabricator

arc patch with dependent diffs is strange
Open, Needs TriagePublic

Description

We use jenkins for our build runs and jenkins usually does a arc patch --nobranch <diff_id> to get the changes and to run the tests.

However with dependent diffs, say D2 depending on D1, running arc patch --nobranch D2 tries to first patch D1 as expected. However the workflow to patch DA1 seems to be not authenticated with conduit and fails to get the commit message. https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/ArcanistPatchWorkflow.php;9fc8a2f61b197ca2de8297ece559513683001922$800 It then opens up an interactive editor and Jenkins fails.

If I comment out the authentication check and bypass it, then the commit message for D1 is found and it is applied successfully. However it gets applied on arcpatch-D1 and then applying D2 causes arc to prompt
This diff is against commit <D1_hash>, but the commit is nowhere in the working copy. Try to apply it against the current working copy state? <HEAD_hash> [Y/n]

On saying yes, you end up with two commits on HEAD with no extra branches lying around. However, I feel like some of this workflow needs to be cleaned up to avoid these unnecessary prompts and make arc patch more friendly with dependent diffs.

Event Timeline

vm created this task.Aug 28 2014, 9:21 AM
vm raised the priority of this task from to Needs Triage.
vm updated the task description. (Show Details)
vm added a project: Arcanist.
vm added subscribers: vm, seshness.
vm added a comment.Sep 2 2014, 6:01 PM

Another problem I notices recently, say you have D2 depending on D1, and D1 gets reviewed and landed into HEAD.
arc patch D2 still tries to cherry-pick D1 and then fails since you are now trying to cherry pick an empty commit.

BYK added a subscriber: BYK.Jun 3 2015, 9:39 PM

@vm: I've opened a separate bug for the "already-landed dependency" case: T8419.

epriestley moved this task from Backlog to arc patch on the Arcanist board.Feb 11 2016, 6:34 PM
srijan added a subscriber: srijan.May 16 2016, 2:16 PM