From 719a321cb014db0e904cc061a7e2f7204e64f4fc Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sun, 16 Sep 2018 19:28:22 +0300 Subject: [PATCH] git: update submodules, which are on branch, as standard repositories Use `git submodule update` only for submodules which are in the detached HEAD state. This fixes IDEA-198951, IDEA-198952, IDEA-198723. --- .../push/GitRejectedPushUpdateDialog.java | 9 +- .../src/git4idea/update/GitUpdateProcess.java | 28 +++--- .../tests/git4idea/update/GitSubmoduleTest.kt | 98 +++++++++++++++++-- 3 files changed, 108 insertions(+), 27 deletions(-) diff --git a/plugins/git4idea/src/git4idea/push/GitRejectedPushUpdateDialog.java b/plugins/git4idea/src/git4idea/push/GitRejectedPushUpdateDialog.java index 1d0de77da3bc..498eaf63693f 100644 --- a/plugins/git4idea/src/git4idea/push/GitRejectedPushUpdateDialog.java +++ b/plugins/git4idea/src/git4idea/push/GitRejectedPushUpdateDialog.java @@ -38,13 +38,10 @@ import java.util.Map; import static com.intellij.xml.util.XmlStringUtil.wrapInHtml; import static git4idea.util.GitUIUtil.code; -/** - * @author Kirill Likhodedov - */ -class GitRejectedPushUpdateDialog extends DialogWrapper { +public class GitRejectedPushUpdateDialog extends DialogWrapper { - static final int MERGE_EXIT_CODE = NEXT_USER_EXIT_CODE; - static final int REBASE_EXIT_CODE = MERGE_EXIT_CODE + 1; + public static final int MERGE_EXIT_CODE = NEXT_USER_EXIT_CODE; + public static final int REBASE_EXIT_CODE = MERGE_EXIT_CODE + 1; private static final String HTML_IDENT = "    "; public static final String DESCRIPTION_START = "Push of current branch "; diff --git a/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java b/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java index 57d87bb8c01a..805e3a06dab2 100644 --- a/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java +++ b/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java @@ -60,7 +60,7 @@ public class GitUpdateProcess { @NotNull private final ChangeListManager myChangeListManager; @NotNull private final List myRepositories; - @NotNull private final Map mySubmodules; + @NotNull private final Map mySubmodulesInDetachedHead; private final boolean myCheckRebaseOverMergeProblem; private final boolean myCheckForTrackedBranchExistence; private final UpdatedFiles myUpdatedFiles; @@ -87,11 +87,17 @@ public class GitUpdateProcess { myProgressIndicator = progressIndicator == null ? new EmptyProgressIndicator() : progressIndicator; myMerger = new GitMerger(myProject); - mySubmodules = ContainerUtil.newLinkedHashMap(); for (GitRepository repository : myRepositories) { - GitSubmodule submodule = GitSubmoduleKt.asSubmodule(repository); - if (submodule != null) { - mySubmodules.put(repository, submodule); + repository.update(); + } + + mySubmodulesInDetachedHead = ContainerUtil.newLinkedHashMap(); + for (GitRepository repository : myRepositories) { + if (!repository.isOnBranch()) { + GitSubmodule submodule = GitSubmoduleKt.asSubmodule(repository); + if (submodule != null) { + mySubmodulesInDetachedHead.put(repository, submodule); + } } } } @@ -116,10 +122,6 @@ public class GitUpdateProcess { String oldText = myProgressIndicator.getText(); myProgressIndicator.setText("Updating..."); - for (GitRepository repository : myRepositories) { - repository.update(); - } - // check if update is possible if (checkRebaseInProgress() || isMergeInProgress() || areUnmergedFiles()) { return GitUpdateResult.NOT_READY; @@ -285,8 +287,8 @@ public class GitUpdateProcess { } } - for (GitRepository repository : mySubmodules.keySet()) { - GitUpdater updater = new GitSubmoduleUpdater(myProject, myGit, mySubmodules.get(repository).getParent(), repository, + for (GitRepository repository : mySubmodulesInDetachedHead.keySet()) { + GitUpdater updater = new GitSubmoduleUpdater(myProject, myGit, mySubmodulesInDetachedHead.get(repository).getParent(), repository, myProgressIndicator, myUpdatedFiles); updaters.put(repository, updater); } @@ -326,8 +328,8 @@ public class GitUpdateProcess { Map currentBranches = ContainerUtil.newLinkedHashMap(); List detachedHeads = ContainerUtil.newArrayList(); for (GitRepository repository : myRepositories) { - if (mySubmodules.containsKey(repository)) { - LOG.debug("Repository " + repository + " is a submodule, not checking its tracked branch"); + if (mySubmodulesInDetachedHead.containsKey(repository)) { + LOG.debug("Repository " + repository + " is a submodule in detached HEAD state, not checking its tracked branch"); continue; } diff --git a/plugins/git4idea/tests/git4idea/update/GitSubmoduleTest.kt b/plugins/git4idea/tests/git4idea/update/GitSubmoduleTest.kt index 2181481e1298..dc8b47ea113e 100644 --- a/plugins/git4idea/tests/git4idea/update/GitSubmoduleTest.kt +++ b/plugins/git4idea/tests/git4idea/update/GitSubmoduleTest.kt @@ -1,6 +1,7 @@ // Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package git4idea.update +import com.intellij.dvcs.DvcsUtil.getPushSupport import com.intellij.dvcs.repo.Repository import com.intellij.openapi.progress.EmptyProgressIndicator import com.intellij.openapi.util.io.FileUtil @@ -8,17 +9,31 @@ import com.intellij.openapi.vcs.Executor.cd import com.intellij.openapi.vcs.Executor.echo import com.intellij.openapi.vcs.update.UpdatedFiles import com.intellij.openapi.vfs.VfsUtil -import git4idea.config.UpdateMethod +import git4idea.config.UpdateMethod.MERGE +import git4idea.config.UpdateMethod.REBASE +import git4idea.push.GitPushOperation +import git4idea.push.GitPushRepoResult +import git4idea.push.GitPushSupport +import git4idea.push.GitRejectedPushUpdateDialog +import git4idea.push.GitRejectedPushUpdateDialog.REBASE_EXIT_CODE +import git4idea.repo.GitRepository import git4idea.test.* import java.io.File class GitSubmoduleTest : GitSubmoduleTestBase() { - fun `test submodule is updated via 'git submodule update'`() { + private lateinit var main: GitRepository + private lateinit var sub: GitRepository + private lateinit var main2: RepositoryAndParent + private lateinit var sub2: File + + override fun setUp() { + super.setUp() + // prepare second clone & parent.git - val main2 = createPlainRepo("main") + main2 = createPlainRepo("main") val sub3 = createPlainRepo("sub") - val sub2 = addSubmodule(main2.local, sub3.remote, "sub") + sub2 = addSubmodule(main2.local, sub3.remote, "sub") // clone into the project cd(testRoot) @@ -31,9 +46,11 @@ class GitSubmoduleTest : GitSubmoduleTestBase() { setupDefaultUsername() refresh() - val main = registerRepo(project, projectPath) - val sub = registerRepo(project, subFile.path) + main = registerRepo(project, projectPath) + sub = registerRepo(project, subFile.path) + } + fun `test submodule in detached HEAD state is updated via 'git submodule update'`() { // push from second clone cd(sub2) echo("a", "content\n") @@ -43,12 +60,77 @@ class GitSubmoduleTest : GitSubmoduleTestBase() { val mainHash = addCommit("Advance the submodule") git("push") - val result = GitUpdateProcess(project, EmptyProgressIndicator(), listOf(main, sub), UpdatedFiles.create(), false, true).update( - UpdateMethod.MERGE) + insertLogMarker("update process") + val result = GitUpdateProcess(project, EmptyProgressIndicator(), listOf(main, sub), UpdatedFiles.create(), false, true).update(MERGE) assertEquals("Update result is incorrect", GitUpdateResult.SUCCESS, result) assertEquals("Last commit in submodule is incorrect", submoduleHash, sub.last()) assertEquals("Last commit in main repository is incorrect", mainHash, main.last()) assertEquals("Submodule should be in detached HEAD", Repository.State.DETACHED, sub.state) } + + fun `test submodule on branch is updated as a normal repository`() { + // push from second clone + cd(sub2) + echo("a", "content\n") + val submoduleHash = addCommit("in submodule") + git("push") + + // prepare commit in first sub clone + cd(sub) + git("checkout master") + echo("b", "content\n") + addCommit("msg") + + insertLogMarker("update process") + val result = GitUpdateProcess(project, EmptyProgressIndicator(), listOf(main, sub), UpdatedFiles.create(), false, true).update(REBASE) + + assertEquals("Update result is incorrect", GitUpdateResult.SUCCESS, result) + assertEquals("Submodule should be on branch", "master", sub.currentBranchName) + assertEquals("Commit from 2nd clone not found in submodule", submoduleHash, sub.git("rev-parse HEAD^")) + } + + fun `test push rejected in submodule updates it and pushes again`() { + // push from second clone + cd(sub2) + echo("a", "content\n") + val submoduleHash = addCommit("in submodule") + git("push") + + // prepare commit in first sub clone + cd(sub) + git("checkout master") + echo("b", "content\n") + addCommit("msg") + + val updateAllRootsIfPushRejected = settings.shouldUpdateAllRootsIfPushRejected() + try { + settings.setUpdateAllRootsIfPushRejected(false) + dialogManager.registerDialogHandler(GitRejectedPushUpdateDialog::class.java, TestDialogHandler { REBASE_EXIT_CODE }) + + val pushSpecs = listOf(main, sub).associate { + it to makePushSpec(it, "master", "origin/master") + } + + insertLogMarker("push process") + val result = GitPushOperation(project, getPushSupport(vcs) as GitPushSupport, pushSpecs, null, false, false).execute() + + val mainResult = result.results[main]!! + val subResult = result.results[sub]!! + + assertEquals("Submodule push result is incorrect", GitPushRepoResult.Type.SUCCESS, subResult.type) + assertEquals("Main push result is incorrect", GitPushRepoResult.Type.UP_TO_DATE, mainResult.type) + assertEquals("Submodule should be on branch", "master", sub.currentBranchName) + assertEquals("Commit from 2nd clone not found in submodule", submoduleHash, sub.git("rev-parse HEAD^")) + } + finally { + settings.setUpdateAllRootsIfPushRejected(updateAllRootsIfPushRejected) + } + } + + private fun insertLogMarker(title: String) { + LOG.info(""); + LOG.info("--------- STARTING ${title.toUpperCase()} -----------") + LOG.info(""); + } } \ No newline at end of file