Page MenuHomePhabricator
Files F18453407

D20253.diff
No OneTemporary
Actions

D20253.diff
View Options

diff --git a/src/workflow/ArcanistPatchWorkflow.php b/src/workflow/ArcanistPatchWorkflow.php
--- a/src/workflow/ArcanistPatchWorkflow.php
+++ b/src/workflow/ArcanistPatchWorkflow.php
@@ -307,6 +307,14 @@
$repository_api->execxLocal('checkout -b %s', $branch_name);
}
+ // Synchronize submodule state, since the checkout may have modified
+ // submodule references. See PHI1083.
+
+ // Note that newer versions of "git checkout" include a
+ // "--recurse-submodules" flag which accomplishes this goal a little
+ // more simply. For now, use the more compatible form.
+ $repository_api->execPassthru('submodule update --init --recursive');
+
echo phutil_console_format(
"%s\n",
pht(
@@ -714,6 +722,24 @@
throw new ArcanistUsageException(pht('Unable to apply patch!'));
}
+ // See PHI1083 and PHI648. If the patch applied changes to submodules,
+ // it only updates the submodule pointer, not the actual submodule. We're
+ // left with the pointer update staged in the index, and the unmodified
+ // submodule on disk.
+
+ // If we then "git commit --all" or "git add --all", the unmodified
+ // submodule on disk is added to the index as a change, which effectively
+ // undoes the patch we just applied and reverts the submodule back to
+ // the previous state.
+
+ // To avoid this, do a submodule update before we continue.
+
+ // We could also possibly skip the "--all" flag so we don't have to do
+ // this submodule update, but we want to leave the working copy in a
+ // clean state anyway, so we're going to have to do an update at some
+ // point. This usually doesn't cost us anything.
+ $repository_api->execPassthru('submodule update --init --recursive');
+
if ($this->shouldCommit()) {
if ($bundle->getFullAuthor()) {
$author_cmd = csprintf('--author=%s', $bundle->getFullAuthor());
@@ -735,14 +761,23 @@
if ($this->canBranch() &&
!$this->shouldBranch() &&
$this->shouldCommit() && $has_base_revision) {
+
+ // See PHI1083 and PHI648. Synchronize submodule state after mutating
+ // the working copy.
+
$repository_api->execxLocal('checkout %s', $original_branch);
+ $repository_api->execPassthru('submodule update --init --recursive');
+
$ex = null;
try {
$repository_api->execxLocal('cherry-pick %s', $new_branch);
+ $repository_api->execPassthru('submodule update --init --recursive');
} catch (Exception $ex) {
// do nothing
}
+
$repository_api->execxLocal('branch -D %s', $new_branch);
+
if ($ex) {
echo phutil_console_format(
"\n<bg:red>** %s**</bg>\n",
@@ -751,11 +786,6 @@
}
}
- // 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(
"<bg:green>** %s **</bg> %s\n",
pht('OKAY'),

File Metadata

Mime Type
text/plain
Expires
Tue, Sep 2, 4:13 AM (2 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
9040183
Default Alt Text
D20253.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility