Skip to content

Commit 34ed09b

Browse files
novalisnovalis
committed
give a more verbose message on add/add conflicts
1 parent ea3c3aa commit 34ed09b

File tree

5 files changed

+83
-15
lines changed

5 files changed

+83
-15
lines changed

‎node/lib/util/cherry_pick_util.js‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,9 @@ exports.computeChanges = co.wrap(function *(repo, index, targetCommit) {
515515
* `repo`. If `conflicts` is non-empty, return a non-empty string desribing
516516
* them. Otherwise, return the empty string.
517517
*
518+
* Half-open the conflicting repos and fetch all commits that are in the
519+
* conflict; this will make it easier for users to resolve the conflict.
520+
*
518521
* @param {NodeGit.Repository} repo
519522
* @param {NodeGit.Index} index
520523
* @param {Object} conflicts from sub name to `Conflict`
@@ -523,11 +526,47 @@ exports.computeChanges = co.wrap(function *(repo, index, targetCommit) {
523526
exports.writeConflicts = co.wrap(function *(repo, index, conflicts) {
524527
let errorMessage = "";
525528
const names = Object.keys(conflicts).sort();
529+
const opener = new Open.Opener(repo, null);
530+
const fetcher = yield opener.fetcher();
526531
for (let name of names) {
527-
yield ConflictUtil.addConflict(index, name, conflicts[name]);
532+
let conflict = conflicts[name];
533+
let configured = false;
534+
try {
535+
// We just want to see if it's configured
536+
const url = yield fetcher.getSubmoduleUrl(name);
537+
configured = true;
538+
} catch (e) {
539+
// nope, so we cannot fetch
540+
}
541+
if (configured) {
542+
const bare = Open.SUB_OPEN_OPTION.FORCE_BARE;
543+
const subRepo = yield opener.getSubrepo(name, bare);
544+
545+
for (const stage of [conflict.ancestor, conflict.our,
546+
conflict.their]) {
547+
if (stage !== null) {
548+
yield fetcher.fetchSha(subRepo, name, stage.id);
549+
}
550+
}
551+
}
552+
yield ConflictUtil.addConflict(index, name, conflict);
528553
errorMessage += `\
529-
Conflicting entries for submodule ${colors.red(name)}
554+
Conflicting entries for submodule ${colors.red(name)}.
530555
`;
556+
if (conflict.our !== null && conflict.their !== null) {
557+
// add-add
558+
const root = repo.workdir();
559+
const our = conflict.our.id;
560+
const their = conflict.their.id;
561+
errorMessage += `
562+
To choose your version of the ${name}, use the following magic:
563+
git -C ${root} update-index --cache-info 160000,${our},${name}
564+
To choose their version of the ${name}, use the following magic:
565+
git -C ${root} update-index --cache-info 160000,${their},${name}
566+
To compare, try:
567+
git -C ${root}${name} diff ${their} ${our}
568+
`;
569+
}
531570
}
532571
return errorMessage;
533572
});

‎node/lib/util/test_util.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,3 +281,7 @@ exports.makeBareCopy = co.wrap(function *(repo, path) {
281281
yield NodeGit.Remote.delete(bare, "origin");
282282
return bare;
283283
});
284+
285+
exports.quotemeta = function(str) {
286+
return String(str).replace(/\W/g, "\\$&");
287+
};

‎node/test/util/cherry_pick_util.js‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,11 @@ const Open = require("../../lib/util/open");
4141
const RepoASTTestUtil = require("../../lib/util/repo_ast_test_util");
4242
const Submodule = require("../../lib/util/submodule");
4343
const SubmoduleChange = require("../../lib/util/submodule_change");
44+
const TestUtil = require("../../lib/util/test_util");
4445
const UserError = require("../../lib/util/user_error");
4546

47+
const qm = TestUtil.quotemeta;
48+
4649
/**
4750
* Return a commit map as expected from a manipulator for `RepoASTTestUtil`
4851
* from a result having the `newMetaCommit` and `submoduleCommits` properties
@@ -558,11 +561,12 @@ describe("writeConflicts", function () {
558561
},
559562
expected: "x=E:I *README.md=~*~*S:1;W README.md=hello world",
560563
result: `\
561-
Conflicting entries for submodule ${colors.red("README.md")}
564+
Conflicting entries for submodule ${colors.red("README.md")}.
562565
`,
563566
},
564567
"two conflicts": {
565568
state: "x=S",
569+
566570
conflicts: {
567571
z: new Conflict(null,
568572
null,
@@ -573,8 +577,8 @@ Conflicting entries for submodule ${colors.red("README.md")}
573577
},
574578
expected: "x=E:I *z=~*~*S:1,*a=~*~*S:1",
575579
result: `\
576-
Conflicting entries for submodule ${colors.red("a")}
577-
Conflicting entries for submodule ${colors.red("z")}
580+
Conflicting entries for submodule ${colors.red("a")}.
581+
Conflicting entries for submodule ${colors.red("z")}.
578582
`,
579583
},
580584
};
@@ -737,7 +741,7 @@ a
737741
;
738742
`,
739743
errorMessage: `\
740-
Submodule ${colors.red("s")} is conflicted.
744+
Submodule ${qm(colors.red("s"))} is conflicted.*
741745
`,
742746
},
743747
"conflict in a sub pick, success in another": {
@@ -755,7 +759,7 @@ a
755759
;
756760
`,
757761
errorMessage: `\
758-
Submodule ${colors.red("s")} is conflicted.
762+
Submodule ${qm(colors.red("s"))} is conflicted.
759763
`,
760764
},
761765
};
@@ -768,7 +772,15 @@ Submodule ${colors.red("s")} is conflicted.
768772
const eightCommitSha = reverseCommitMap["8"];
769773
const eightCommit = yield x.getCommit(eightCommitSha);
770774
const result = yield CherryPickUtil.rewriteCommit(x, eightCommit);
771-
assert.equal(result.errorMessage, c.errorMessage || null);
775+
const errorMessage = c.errorMessage || null;
776+
777+
778+
if (null === result.errorMessage) {
779+
assert(null === errorMessage);
780+
} else {
781+
const re = new RegExp(c.errorMessage);
782+
assert.match(result.errorMessage, re);
783+
}
772784
return mapCommits(maps, result);
773785
});
774786

‎node/test/util/merge_full_open.js‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ const MergeUtil = require("../../lib//util/merge_util");
3838
const MergeCommon = require("../../lib//util/merge_common");
3939
const RepoASTTestUtil = require("../../lib/util/repo_ast_test_util");
4040
const Open = require("../../lib/util/open");
41+
const TestUtil = require("../../lib/util/test_util");
4142

4243
/**
4344
* 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`,
315316
fails: true,
316317
expected: `x=E:Qmessage\n#M 2: 3: 0 3;I *s=~*S:a*S:b`,
317318
errorMessage: `\
318-
Conflicting entries for submodule ${colors.red("s")}
319+
Conflicting entries for submodule ${TestUtil.quotemeta(colors.red("s"))}.*
319320
`,
320321
},
321322
"conflict in submodule": {
@@ -325,7 +326,7 @@ x=U:C3-2 s=Sa:a;C4-2 s=Sa:b;Bmaster=3;Bfoo=4`,
325326
fromCommit: "4",
326327
fails: true,
327328
errorMessage: `\
328-
Submodule ${colors.red("s")} is conflicted.
329+
Submodule ${TestUtil.quotemeta(colors.red("s"))} is conflicted.
329330
`,
330331
expected: `
331332
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;
410411
message,
411412
editMessage);
412413
const errorMessage = c.errorMessage || null;
413-
assert.equal(result.errorMessage, errorMessage);
414+
415+
if (null === result.errorMessage) {
416+
assert(null === errorMessage);
417+
} else {
418+
const re = new RegExp(c.errorMessage);
419+
assert.match(result.errorMessage, re);
420+
}
421+
414422
if (upToDate) {
415423
assert.isNull(result.metaCommit);
416424
return; // RETURN

‎node/test/util/rebase_util.js‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ const RebaseUtil = require("../../lib/util/rebase_util");
3838
const RepoASTTestUtil = require("../../lib/util/repo_ast_test_util");
3939
const SequencerState = require("../../lib/util/sequencer_state");
4040
const SequencerStateUtil = require("../../lib/util/sequencer_state_util");
41+
const TestUtil = require("../../lib/util/test_util");
4142

4243
const CommitAndRef = SequencerState.CommitAndRef;
4344
const REBASE = SequencerState.TYPE.REBASE;
@@ -361,7 +362,7 @@ x=S:C2-1 s=Ss:1;C3-2 r=Sr:6,s=Ss:2;C4-2 r=Sr:5,q=Sq:1;
361362
Bmaster=3;Bfoo=4;Bold=3`,
362363
onto: "4",
363364
errorMessage:
364-
"Conflicting entries for submodule \u001b[31mr\u001b[39m\n",
365+
"Conflicting entries for submodule .*",
365366
expected: `
366367
x=E:H=4;QR 3:refs/heads/master 4: 0 3;
367368
I s=Ss:2,
@@ -390,7 +391,7 @@ t
390391
>>>>>>> message
391392
;`,
392393
errorMessage: `\
393-
Submodule ${colors.red("s")} is conflicted.
394+
Submodule ${TestUtil.quotemeta(colors.red("s"))} is conflicted.*
394395
`,
395396
},
396397
"does not close open submodules when rewinding": {
@@ -421,10 +422,14 @@ a=B|x=S:C2-1 s=Sa:1;Bmaster=2;Os;C3-1 t=Sa:1;Bfoo=3;Bold=2`,
421422
const onto = yield repo.getCommit(reverseCommitMap[c.onto]);
422423
const errorMessage = c.errorMessage || null;
423424
const result = yield RebaseUtil.rebase(repo, onto);
424-
if (null !== result.errorMessage) {
425+
426+
if (null === result.errorMessage) {
427+
assert(errorMessage === null);
428+
} else {
425429
assert.isString(result.errorMessage);
430+
const re = new RegExp(errorMessage);
431+
assert.match(result.errorMessage, re);
426432
}
427-
assert.equal(result.errorMessage, errorMessage);
428433
return result;
429434
});
430435
const rebase = makeRebaser(rebaseOp);

0 commit comments

Comments
 (0)