Skip to content

Commit

Permalink
give a more verbose message on add/add conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
novalis committed Sep 12, 2019
1 parent ea3c3aa commit 98d4a54
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 15 deletions.
43 changes: 41 additions & 2 deletions node/lib/util/cherry_pick_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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 = url !== null;
} 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;
});
Expand Down
4 changes: 4 additions & 0 deletions node/lib/util/test_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, "\\$&");
};
24 changes: 18 additions & 6 deletions node/test/util/cherry_pick_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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")}.
`,
},
};
Expand Down Expand Up @@ -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": {
Expand All @@ -755,7 +759,7 @@ a
;
`,
errorMessage: `\
Submodule ${colors.red("s")} is conflicted.
Submodule ${qm(colors.red("s"))} is conflicted.
`,
},
};
Expand All @@ -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);
});

Expand Down
14 changes: 11 additions & 3 deletions node/test/util/merge_full_open.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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": {
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions node/test/util/rebase_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Bmaster=3;Bfoo=4;Bold=3`,
onto: "4",
errorMessage:
"Conflicting entries for submodule \u001b[31mr\u001b[39m\n",
"Conflicting entries for submodule .*",
expected: `
x=E:H=4;QR 3:refs/heads/master 4: 0 3;
I s=Ss:2,
Expand Down Expand Up @@ -390,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": {
Expand Down Expand Up @@ -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`,
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);
Expand Down

0 comments on commit 98d4a54

Please sign in to comment.