diff --git a/node/lib/util/cherry_pick_util.js b/node/lib/util/cherry_pick_util.js index c7a64c216..9586a5525 100644 --- a/node/lib/util/cherry_pick_util.js +++ b/node/lib/util/cherry_pick_util.js @@ -515,6 +515,9 @@ exports.computeChanges = co.wrap(function *(repo, index, targetCommit) { * `repo`. If `conflicts` is non-empty, return a non-empty string desribing * them. Otherwise, return the empty string. * + * Half-open the conflicting repos and fetch all commits that are in the + * conflict; this will make it easier for users to resolve the conflict. + * * @param {NodeGit.Repository} repo * @param {NodeGit.Index} index * @param {Object} conflicts from sub name to `Conflict` @@ -523,11 +526,47 @@ exports.computeChanges = co.wrap(function *(repo, index, targetCommit) { exports.writeConflicts = co.wrap(function *(repo, index, conflicts) { let errorMessage = ""; const names = Object.keys(conflicts).sort(); + const opener = new Open.Opener(repo, null); + const fetcher = yield opener.fetcher(); for (let name of names) { - yield ConflictUtil.addConflict(index, name, conflicts[name]); + let conflict = conflicts[name]; + let configured = false; + try { + // We just want to see if it's configured + const url = yield fetcher.getSubmoduleUrl(name); + configured = true; + } catch (e) { + // nope, so we cannot fetch + } + if (configured) { + const bare = Open.SUB_OPEN_OPTION.FORCE_BARE; + const subRepo = yield opener.getSubrepo(name, bare); + + for (const stage of [conflict.ancestor, conflict.our, + conflict.their]) { + if (stage !== null) { + yield fetcher.fetchSha(subRepo, name, stage.id); + } + } + } + yield ConflictUtil.addConflict(index, name, conflict); errorMessage += `\ -Conflicting entries for submodule ${colors.red(name)} +Conflicting entries for submodule ${colors.red(name)}. `; + if (conflict.our !== null && conflict.their !== null) { + // add-add + const root = repo.workdir(); + const our = conflict.our.id; + const their = conflict.their.id; + errorMessage += ` +To choose your version of the ${name}, use the following magic: + git -C ${root} update-index --cache-info 160000,${our},${name} +To choose their version of the ${name}, use the following magic: + git -C ${root} update-index --cache-info 160000,${their},${name} +To compare, try: + git -C ${root}${name} diff ${their} ${our} +`; + } } return errorMessage; }); diff --git a/node/lib/util/repo_ast_util.js b/node/lib/util/repo_ast_util.js index a330aca2f..d2d6deda3 100644 --- a/node/lib/util/repo_ast_util.js +++ b/node/lib/util/repo_ast_util.js @@ -96,6 +96,9 @@ function diffObjects(first, * @return {String} */ function colorExp(text) { + if (text === null) { + text = "(null)"; + } return colors.green(text); } @@ -108,6 +111,9 @@ function colorExp(text) { * @return {String} */ function colorAct(text) { + if (text === null) { + text = "(null)"; + } return colors.yellow(text); } diff --git a/node/lib/util/test_util.js b/node/lib/util/test_util.js index 4d47c5ba5..1f208d03e 100644 --- a/node/lib/util/test_util.js +++ b/node/lib/util/test_util.js @@ -281,3 +281,7 @@ exports.makeBareCopy = co.wrap(function *(repo, path) { yield NodeGit.Remote.delete(bare, "origin"); return bare; }); + +exports.quotemeta = function(str) { + return String(str).replace(/\W/g, "\\$&"); +}; diff --git a/node/test/util/cherry_pick_util.js b/node/test/util/cherry_pick_util.js index 14cab55cc..bffec7bbe 100644 --- a/node/test/util/cherry_pick_util.js +++ b/node/test/util/cherry_pick_util.js @@ -41,8 +41,11 @@ const Open = require("../../lib/util/open"); const RepoASTTestUtil = require("../../lib/util/repo_ast_test_util"); const Submodule = require("../../lib/util/submodule"); const SubmoduleChange = require("../../lib/util/submodule_change"); +const TestUtil = require("../../lib/util/test_util"); const UserError = require("../../lib/util/user_error"); +const qm = TestUtil.quotemeta; + /** * Return a commit map as expected from a manipulator for `RepoASTTestUtil` * from a result having the `newMetaCommit` and `submoduleCommits` properties @@ -558,11 +561,12 @@ describe("writeConflicts", function () { }, expected: "x=E:I *README.md=~*~*S:1;W README.md=hello world", result: `\ -Conflicting entries for submodule ${colors.red("README.md")} +Conflicting entries for submodule ${colors.red("README.md")}. `, }, "two conflicts": { state: "x=S", + conflicts: { z: new Conflict(null, null, @@ -573,8 +577,8 @@ Conflicting entries for submodule ${colors.red("README.md")} }, expected: "x=E:I *z=~*~*S:1,*a=~*~*S:1", result: `\ -Conflicting entries for submodule ${colors.red("a")} -Conflicting entries for submodule ${colors.red("z")} +Conflicting entries for submodule ${colors.red("a")}. +Conflicting entries for submodule ${colors.red("z")}. `, }, }; @@ -737,7 +741,7 @@ a ; `, errorMessage: `\ -Submodule ${colors.red("s")} is conflicted. +Submodule ${qm(colors.red("s"))} is conflicted.* `, }, "conflict in a sub pick, success in another": { @@ -755,7 +759,7 @@ a ; `, errorMessage: `\ -Submodule ${colors.red("s")} is conflicted. +Submodule ${qm(colors.red("s"))} is conflicted. `, }, }; @@ -768,7 +772,15 @@ Submodule ${colors.red("s")} is conflicted. const eightCommitSha = reverseCommitMap["8"]; const eightCommit = yield x.getCommit(eightCommitSha); const result = yield CherryPickUtil.rewriteCommit(x, eightCommit); - assert.equal(result.errorMessage, c.errorMessage || null); + const errorMessage = c.errorMessage || null; + + + if (null === result.errorMessage) { + assert(null === errorMessage); + } else { + const re = new RegExp(c.errorMessage); + assert.match(result.errorMessage, re); + } return mapCommits(maps, result); }); diff --git a/node/test/util/merge_full_open.js b/node/test/util/merge_full_open.js index 35ff79a83..6235ffb10 100644 --- a/node/test/util/merge_full_open.js +++ b/node/test/util/merge_full_open.js @@ -38,6 +38,7 @@ const MergeUtil = require("../../lib//util/merge_util"); const MergeCommon = require("../../lib//util/merge_common"); const RepoASTTestUtil = require("../../lib/util/repo_ast_test_util"); const Open = require("../../lib/util/open"); +const TestUtil = require("../../lib/util/test_util"); /** * Return the commit map required by 'RepoASTTestUtil.testMultiRepoManipulator' @@ -315,7 +316,7 @@ x=S:C2-1 s=Sa:a;C3-1 s=Sa:b;Bmaster=2;Bfoo=3`, fails: true, expected: `x=E:Qmessage\n#M 2: 3: 0 3;I *s=~*S:a*S:b`, errorMessage: `\ -Conflicting entries for submodule ${colors.red("s")} +Conflicting entries for submodule ${TestUtil.quotemeta(colors.red("s"))}.* `, }, "conflict in submodule": { @@ -325,7 +326,7 @@ x=U:C3-2 s=Sa:a;C4-2 s=Sa:b;Bmaster=3;Bfoo=4`, fromCommit: "4", fails: true, errorMessage: `\ -Submodule ${colors.red("s")} is conflicted. +Submodule ${TestUtil.quotemeta(colors.red("s"))} is conflicted. `, expected: ` x=E:Qmessage\n#M 3: 4: 0 4; @@ -410,7 +411,14 @@ x=S:C2-1 r=Sa:1,s=Sa:1,t=Sa:1; message, editMessage); const errorMessage = c.errorMessage || null; - assert.equal(result.errorMessage, errorMessage); + + if (null === result.errorMessage) { + assert(null === errorMessage); + } else { + const re = new RegExp(c.errorMessage); + assert.match(result.errorMessage, re); + } + if (upToDate) { assert.isNull(result.metaCommit); return; // RETURN diff --git a/node/test/util/rebase_util.js b/node/test/util/rebase_util.js index 568f07f81..bee60fb4b 100644 --- a/node/test/util/rebase_util.js +++ b/node/test/util/rebase_util.js @@ -38,6 +38,7 @@ const RebaseUtil = require("../../lib/util/rebase_util"); const RepoASTTestUtil = require("../../lib/util/repo_ast_test_util"); const SequencerState = require("../../lib/util/sequencer_state"); const SequencerStateUtil = require("../../lib/util/sequencer_state_util"); +const TestUtil = require("../../lib/util/test_util"); const CommitAndRef = SequencerState.CommitAndRef; const REBASE = SequencerState.TYPE.REBASE; @@ -338,13 +339,35 @@ x=S:C2-1 s=Sa:1,t=Sb:1;C3-2 s=Sa:j;C4-2 t=Sb:k;Bmaster=3;Bfoo=4;Bold=3`, expected: ` x=E:C3M-4 s=Sa:j;Bmaster=3M`, }, - "adding subs on both": { + "adding unrelated subs on both": { initial: ` q=B|r=B|s=B|x=S:C2-1 s=Ss:1;C3-2 q=Sq:1;C4-2 r=Sr:1;Bmaster=3;Bfoo=4;Bold=3`, onto: "4", expected: ` x=E:C3M-4 q=Sq:1;Bmaster=3M`, }, + "adding same sub on both": { + initial: ` +q=B|r=B|s=B|x=S:C2-1 s=Ss:1;C3-2 r=Sr:1,s=Ss:2;C4-2 r=Sr:1,q=Sq:1;Bmaster=3; +Bfoo=4;Bold=3`, + onto: "4", + expected: ` +x=E:C3M-4 s=Ss:2;Bmaster=3M`, + }, + "add/add conflict": { + initial: ` +q=B|r=B:C5-1;C6-5;Bother=6| +s=B| +x=S:C2-1 s=Ss:1;C3-2 r=Sr:6,s=Ss:2;C4-2 r=Sr:5,q=Sq:1; +Bmaster=3;Bfoo=4;Bold=3`, + onto: "4", + errorMessage: + "Conflicting entries for submodule .*", + expected: ` + x=E:H=4;QR 3:refs/heads/master 4: 0 3; + I s=Ss:2, + *r=~*S:5*S:6`, + }, "open sub ffwd'd": { initial: ` a=B:CX-1;Bmaster=X| @@ -368,7 +391,7 @@ t >>>>>>> message ;`, errorMessage: `\ -Submodule ${colors.red("s")} is conflicted. +Submodule ${TestUtil.quotemeta(colors.red("s"))} is conflicted.* `, }, "does not close open submodules when rewinding": { @@ -399,10 +422,14 @@ a=B|x=S:C2-1 s=Sa:1;Bmaster=2;Os;C3-1 t=Sa:1;Bfoo=3;Bold=2`, const onto = yield repo.getCommit(reverseCommitMap[c.onto]); const errorMessage = c.errorMessage || null; const result = yield RebaseUtil.rebase(repo, onto); - if (null !== result.errorMessage) { + + if (null === result.errorMessage) { + assert(errorMessage === null); + } else { assert.isString(result.errorMessage); + const re = new RegExp(errorMessage); + assert.match(result.errorMessage, re); } - assert.equal(result.errorMessage, errorMessage); return result; }); const rebase = makeRebaser(rebaseOp);