Page MenuHomePhabricator

Pass conduit credentials down to children workflow
ClosedPublic

Authored by vm on Aug 28 2014, 7:16 PM.
Tags
None
Referenced Files
F14075639: D10381.diff
Thu, Nov 21, 12:39 PM
Unknown Object (File)
Sat, Nov 16, 9:52 PM
Unknown Object (File)
Oct 22 2024, 12:07 PM
Unknown Object (File)
Oct 14 2024, 6:35 AM
Unknown Object (File)
Oct 11 2024, 1:23 AM
Unknown Object (File)
Oct 11 2024, 1:23 AM
Unknown Object (File)
Oct 11 2024, 1:23 AM
Unknown Object (File)
Oct 11 2024, 1:23 AM

Details

Summary

Calling arc patch on a diff that's dependent on a different diff tries to patch the parent first.
To patch the parent a child workflow is created, and a conduit is passed down, but the credentials are not and it is not marked as authenticated.

Then when the child tries to get the commit message for the dependency, it checks isConduitAuthenticated() https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/ArcanistPatchWorkflow.php;2c3268f03ed70d3221eb1642bIc99ebb39b12902e$800 and on failure pops up an interactive editor for the commit message.

Instead we just pass down the credentials to the childred and mark them as authenticated, so this is not a problem.

Test Plan

With two diffs where DA2 depends on DA1, run arc patch --force --nobranch DA2 ... this no longer pops an interactive editor for the commit message for the dependency.

Diff Detail

Repository
rARC Arcanist
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

vm retitled this revision from to Pass conduit credentials down to children workflow.
vm updated this object.
vm edited the test plan for this revision. (Show Details)
vm added reviewers: epriestley, chad.
hach-que added inline comments.
src/workflow/ArcanistPatchWorkflow.php
800–802 ↗(On Diff #24987)

Remove debugging code?

src/workflow/ArcanistPatchWorkflow.php
800–802 ↗(On Diff #24987)

Oops. Thought I did.

vm edited edge metadata.

Remove debugging code

Ping, can we get this in soon? We'd like to support dependent diffs in our automated testing workflow (which relies on arc patch and hence needs diff).

chad removed a reviewer: chad.
epriestley edited edge metadata.

Possibly conduitURI and forcedConduitVersion should be getting passed down too, although there is probably no effect in practice.

T2247 discusses a flavor of this case in more detail.

We also plan to simplify the login/credentials dance a bit (T5955).

Anyway, this seems fine for the moment.

This revision is now accepted and ready to land.Sep 8 2014, 2:42 PM
epriestley updated this revision to Diff 25112.

Closed by commit rARC1b8ce98304f5 (authored by @vm, committed by @epriestley).