Re: [PATCH v3 1/4] Introduce CHERRY_PICK_HEAD
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:35
Jay Soffian [off-list ref] writes:
quoted hunk ↗ jump to hunk
diff --git a/builtin/merge.c b/builtin/merge.c index 9403747..454dad2 100644 --- a/builtin/merge.c +++ b/builtin/merge.c@@ -971,6 +971,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix) else die("You have not concluded your merge (MERGE_HEAD exists)."); } + if (file_exists(git_path("CHERRY_PICK_HEAD"))) { + if (advice_resolve_conflict) + die("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).\n" + "Please, commit your changes before you can merge."); + else + die("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists)."); + }
Micronit: "Please, commit your changes before you can merge." - We are not merging in this codepath to begin with; - I'd suggest rephrasing this (together with "MERGE_HEAD" codepath) to something like: "Commit your changes first before retrying."
+test_expect_success 'cherry-pick --no-commit sets CHERRY_PICK_HEAD' ' + + git checkout -f initial^0 && + git read-tree -u --reset HEAD && + git clean -d -f -f -q -x && + + git update-index --refresh && + git diff-index --exit-code HEAD &&
Getting tired of seeing these five lines repeated over and over. Perhaps
it is time to introduce:
pristine_detach () {
git checkout -f "$1^0" &&
git read-tree -u --reset HEAD &&
git clean -d -f -f -q -x
}
I don't think "diff-index --exit-code HEAD" is necessary as the point of
this testsuite is not about testing "read-tree --reset", and the index
refresh is just a prerequisite for diff-index that can go together with
it.
+ git cherry-pick --no-commit base && + + test_cmp_rev base CHERRY_PICK_HEAD
If the next "git commit" would notice and use this information, that would
introduce an unpleasant regression to one use case in my workflow, which
is to pick and consolidate one or more small pieces made on a private
"misc" branch, possibly with a bit of further work, into a new commit with
a readable explanation that is unrelated to any of the original commits:
git cherry-pick --no-commit $some_commit
git cherry-pick --no-commit $another_commit ;# optional
edit ;# optional
git commit
I'd prefer to see a way to tell cherry-pick not to leave CHERRY_PICK_HEAD
behind when "cherry-pick --no-commit" results in a successful cherry-pick
to avoid a backward incompatibility surprise. Otherwise people need to
retrain their fingers to say --reset-author when they run "git commit".
Other then the above three points, this patch looks good. Thanks.