From b199ca80866038e3e7b164ed53f23646b842cc5c Mon Sep 17 00:00:00 2001 From: Tim McJilton Date: Tue, 5 Jun 2018 20:58:47 +0000 Subject: [PATCH 1/2] [ARCUNIT] Set the ConfigurationManager of ConfigurationDrivenUnitTestEngines Summary: The Configuration Manager is supported by ArcanistUnitTestEngine but not support by the ArcanistConfigurationDrivenUnitTestEngine. Added the configuration manager as one of the initially set properties of an ArcUnitTestEngine created by the ArcanistConfigurationDrivenTestEngine Test Plan: Ran arc unit against a project without the change, verified the Configuration was none. Added this change and ran again and verified it was set Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D19465 --- src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php index 6cc659af..5576d357 100644 --- a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php +++ b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php @@ -133,6 +133,7 @@ final class ArcanistConfigurationDrivenUnitTestEngine $engine ->setWorkingCopy($this->getWorkingCopy()) ->setEnableCoverage($this->getEnableCoverage()) + ->setConfigurationManager($this->getConfigurationManager()) ->setRenderer($renderer); // TODO: At some point, maybe we should emit a warning here if an engine From df7313bdf2a34d03667fe20c850a10ad3878e27d Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 7 Jun 2018 09:49:45 -0700 Subject: [PATCH 2/2] In "arc patch", update submodules slightly later Summary: Ref T13151. See PHI648. With `arc patch --nobranch`, we update submodules a little too early. I //believe// it is safe to just update them a little later, after the intermediate branch management logic runs. Test Plan: Ran `arc patch --nobranch`, saw submodule update run later. Not 100% sure this doesn't cause weird issues, but I can't anticipate any. Reviewers: amckinley, jmeador Reviewed By: jmeador Maniphest Tasks: T13151 Differential Revision: https://secure.phabricator.com/D19475 --- src/workflow/ArcanistPatchWorkflow.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/workflow/ArcanistPatchWorkflow.php b/src/workflow/ArcanistPatchWorkflow.php index a2b3fe40..dba98fb0 100644 --- a/src/workflow/ArcanistPatchWorkflow.php +++ b/src/workflow/ArcanistPatchWorkflow.php @@ -714,9 +714,6 @@ EOTEXT throw new ArcanistUsageException(pht('Unable to apply patch!')); } - // in case there were any submodule changes involved - $repository_api->execPassthru('submodule update --init --recursive'); - if ($this->shouldCommit()) { if ($bundle->getFullAuthor()) { $author_cmd = csprintf('--author=%s', $bundle->getFullAuthor()); @@ -754,6 +751,11 @@ EOTEXT } } + // Synchronize submodule state, since the patch may have made changes + // to ".gitmodules". We do this after we finish managing branches so + // the behavior is correct under "--nobranch"; see PHI648. + $repository_api->execPassthru('submodule update --init --recursive'); + echo phutil_console_format( "** %s ** %s\n", pht('OKAY'),