* [PATCH 0/2] CHERRY_PICK_HEAD
@ 2011-02-16 10:08 Jay Soffian
2011-02-16 10:08 ` [PATCH 1/2] Introduce CHERRY_PICK_HEAD Jay Soffian
2011-02-16 10:08 ` [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD Jay Soffian
0 siblings, 2 replies; 18+ messages in thread
From: Jay Soffian @ 2011-02-16 10:08 UTC (permalink / raw)
To: git
Cc: Jay Soffian, Ævar Arnfjörð Bjarmason,
Jonathan Nieder, Junio C Hamano
Re-roll of http://thread.gmane.org/gmane.comp.version-control.git/166878
If not ready for inclusion, hopefully very close.
Jay Soffian (2):
Introduce CHERRY_PICK_HEAD
Teach commit about CHERRY_PICK_HEAD
Documentation/git-cherry-pick.txt | 19 +++++
Documentation/git-commit.txt | 7 +-
Documentation/revisions.txt | 5 +-
branch.c | 1 +
builtin/commit.c | 136 ++++++++++++++++++++++----------
builtin/merge.c | 7 ++
builtin/revert.c | 68 ++++------------
contrib/completion/git-completion.bash | 2 +
t/t3507-cherry-pick-conflict.sh | 22 +++++-
t/t7509-commit.sh | 29 +++++++
wt-status.c | 4 +-
wt-status.h | 8 ++-
12 files changed, 204 insertions(+), 104 deletions(-)
--
1.7.4.1.28.gd46b3
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] Introduce CHERRY_PICK_HEAD
2011-02-16 10:08 [PATCH 0/2] CHERRY_PICK_HEAD Jay Soffian
@ 2011-02-16 10:08 ` Jay Soffian
2011-02-16 11:13 ` Nguyen Thai Ngoc Duy
2011-02-16 17:20 ` [PATCH v2] " Jay Soffian
2011-02-16 10:08 ` [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD Jay Soffian
1 sibling, 2 replies; 18+ messages in thread
From: Jay Soffian @ 2011-02-16 10:08 UTC (permalink / raw)
To: git
Cc: Jay Soffian, Ævar Arnfjörð Bjarmason,
Jonathan Nieder, Junio C Hamano
When a cherry-pick conflicts git advises to use:
$ git commit -c <original commit id>
to preserve the original commit message and authorship. Instead, let's
record the original commit id in CHERRY_PICK_HEAD and advise to use:
$ git commit -c CHERRY_PICK_HEAD
In the next commit, we teach git to handle the '-c CHERRY_PICK_HEAD'
part. Note that wWe record CHERRY_PICK_HEAD even in the case where there
are no conflicts so that we may use it to communicate authorship to
commit; this will then allow us to remove set_author_ident_env from
revert.c.
Contributions-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
Documentation/git-cherry-pick.txt | 19 +++++++++++++++++++
Documentation/revisions.txt | 5 ++++-
branch.c | 1 +
builtin/commit.c | 1 +
builtin/merge.c | 7 +++++++
builtin/revert.c | 20 ++++++++++++++++++--
t/t3507-cherry-pick-conflict.sh | 22 +++++++++++++++++++++-
7 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 749d68a..5d85daa 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -16,6 +16,25 @@ Given one or more existing commits, apply the change each one
introduces, recording a new commit for each. This requires your
working tree to be clean (no modifications from the HEAD commit).
+When it is not obvious how to apply a change, the following
+happens:
+
+1. The current branch and `HEAD` pointer stay at the last commit
+ successfully made.
+2. The `CHERRY_PICK_HEAD` ref is set to point at the commit that
+ introduced the change that is difficult to apply.
+3. Paths in which the change applied cleanly are updated both
+ in the index file and in your working tree.
+4. For conflicting paths, the index file records up to three
+ versions, as described in the "TRUE MERGE" section of
+ linkgit:git-merge[1]. The working tree files will include
+ a description of the conflict bracketed by the usual
+ conflict markers `<<<<<<<` and `>>>>>>>`.
+5. No other modifications are made.
+
+See linkgit:git-merge[1] for some hints on resolving such
+conflicts.
+
OPTIONS
-------
<commit>...::
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 9e92734..04fceee 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -25,7 +25,8 @@ blobs contained in a commit.
first match in the following rules:
. if `$GIT_DIR/<name>` exists, that is what you mean (this is usually
- useful only for `HEAD`, `FETCH_HEAD`, `ORIG_HEAD` and `MERGE_HEAD`);
+ useful only for `HEAD`, `FETCH_HEAD`, `ORIG_HEAD`, `MERGE_HEAD`
+ and `CHERRY_PICK_HEAD`);
. otherwise, `refs/<name>` if exists;
@@ -46,6 +47,8 @@ you can change the tip of the branch back to the state before you ran
them easily.
MERGE_HEAD records the commit(s) you are merging into your branch
when you run 'git merge'.
+CHERRY_PICK_HEAD records the commit you are cherry-picking
+when you run 'git cherry-pick'.
+
Note that any of the `refs/*` cases above may come either from
the `$GIT_DIR/refs` directory or from the `$GIT_DIR/packed-refs` file.
diff --git a/branch.c b/branch.c
index 93dc866..dc23e95 100644
--- a/branch.c
+++ b/branch.c
@@ -217,6 +217,7 @@ void create_branch(const char *head,
void remove_branch_state(void)
{
+ unlink(git_path("CHERRY_PICK_HEAD"));
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_RR"));
unlink(git_path("MERGE_MSG"));
diff --git a/builtin/commit.c b/builtin/commit.c
index 03cff5a..0def540 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1424,6 +1424,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
die("cannot update HEAD ref");
}
+ unlink(git_path("CHERRY_PICK_HEAD"));
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
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).");
+ }
resolve_undo_clear();
if (verbosity < 0)
diff --git a/builtin/revert.c b/builtin/revert.c
index dc1b702..cced2e4 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -248,6 +248,22 @@ static void set_author_ident_env(const char *message)
sha1_to_hex(commit->object.sha1));
}
+static void write_cherry_pick_head(void)
+{
+ int fd;
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
+
+ fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666);
+ if (fd < 0)
+ die_errno("Could not open '%s' for writing",
+ git_path("CHERRY_PICK_HEAD"));
+ if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
+ die_errno("Could not write to '%s'", git_path("CHERRY_PICK_HEAD"));
+ strbuf_release(&buf);
+}
+
static void advise(const char *advice, ...)
{
va_list params;
@@ -270,8 +286,7 @@ static void print_advice(void)
advise("with 'git add <paths>' or 'git rm <paths>'");
if (action == CHERRY_PICK)
- advise("and commit the result with 'git commit -c %s'",
- find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
+ advise("and commit the result with 'git commit -c CHERRY_PICK_HEAD'");
}
static void write_message(struct strbuf *msgbuf, const char *filename)
@@ -504,6 +519,7 @@ static int do_pick_commit(void)
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
}
+ write_cherry_pick_head();
}
if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 607bf25..fd569c8 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -11,6 +11,12 @@ test_description='test cherry-pick and revert with conflicts
. ./test-lib.sh
+test_cmp_rev () {
+ git rev-parse --verify "$1" >expect.rev &&
+ git rev-parse --verify "$2" >actual.rev &&
+ test_cmp expect.rev actual.rev
+}
+
test_expect_success setup '
echo unrelated >unrelated &&
@@ -51,13 +57,27 @@ test_expect_success 'advice from failed cherry-pick' "
error: could not apply \$picked... picked
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
- hint: and commit the result with 'git commit -c \$picked'
+ hint: and commit the result with 'git commit -c CHERRY_PICK_HEAD'
EOF
test_must_fail git cherry-pick picked 2>actual &&
test_cmp expected actual
"
+test_expect_success 'failed cherry-pick 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 &&
+
+ test_must_fail git cherry-pick picked &&
+
+ test_cmp_rev picked CHERRY_PICK_HEAD
+'
+
test_expect_success 'failed cherry-pick produces dirty index' '
git checkout -f initial^0 &&
--
1.7.4.1.28.gd46b3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD
2011-02-16 10:08 [PATCH 0/2] CHERRY_PICK_HEAD Jay Soffian
2011-02-16 10:08 ` [PATCH 1/2] Introduce CHERRY_PICK_HEAD Jay Soffian
@ 2011-02-16 10:08 ` Jay Soffian
2011-02-16 21:07 ` Junio C Hamano
` (2 more replies)
1 sibling, 3 replies; 18+ messages in thread
From: Jay Soffian @ 2011-02-16 10:08 UTC (permalink / raw)
To: git
Cc: Jay Soffian, Ævar Arnfjörð Bjarmason,
Jonathan Nieder, Junio C Hamano
Previously the user was advised to use commit -c CHERRY_PICK_HEAD after
a conflicting cherry-pick. While this would preserve the original
commit's authorship, it would sadly discard cherry-pick's carefully
crafted MERGE_MSG (which contains the list of conflicts as well as the
original commit-id in the case of cherry-pick -x).
On the other hand, if a bare 'commit' were performed, it would preserve
the MERGE_MSG while resetting the authorship.
In other words, there was no way to simultaneously take the authorship
from CHERRY_PICK_HEAD and the commit message from MERGE_MSG.
This change fixes that situation. A bare 'commit' will now take the
authorship from CHERRY_PICK_HEAD and the commit message from MERGE_MSG.
If the user wishes to reset authorship, that must now be done explicitly
via --reset-author.
A side-benefit of passing commit authorship along this way is that we
can eliminate redundant authorship parsing code from revert.c.
A couple minor points:
* While we're at it, we update git-completion.bash to be
CHERRY_PICK_HEAD aware.
* We remove a unused import from revert.c
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
Documentation/git-commit.txt | 7 +-
builtin/commit.c | 137 +++++++++++++++++++++----------
builtin/revert.c | 56 +-------------
contrib/completion/git-completion.bash | 2 +
t/t3507-cherry-pick-conflict.sh | 2 +-
t/t7509-commit.sh | 29 +++++++
wt-status.c | 4 +-
wt-status.h | 8 ++-
8 files changed, 139 insertions(+), 106 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index b586c0f..fd6a1f7 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -84,9 +84,10 @@ OPTIONS
linkgit:git-rebase[1] for details.
--reset-author::
- When used with -C/-c/--amend options, declare that the
- authorship of the resulting commit now belongs of the committer.
- This also renews the author timestamp.
+ When used with -C/-c/--amend options, or when committing after a
+ a conflicting cherry-pick, declare that the authorship of the
+ resulting commit now belongs of the committer. This also renews
+ the author timestamp.
--short::
When doing a dry-run, give the output in the short-format. See
diff --git a/builtin/commit.c b/builtin/commit.c
index 0def540..9d8ad8e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -56,8 +56,10 @@ static const char empty_amend_advice[] =
static unsigned char head_sha1[20];
-static char *use_message_buffer;
+static const char *use_message_buffer;
static const char commit_editmsg[] = "COMMIT_EDITMSG";
+static const char cherry_pick_head[] = "CHERRY_PICK_HEAD";
+static const char merge_head[] = "MERGE_HEAD";
static struct lock_file index_lock; /* real index */
static struct lock_file false_lock; /* used only for partial commits */
static enum {
@@ -68,6 +70,7 @@ static enum {
static const char *logfile, *force_author;
static const char *template_file;
+static const char *author_message, *author_message_buffer;
static char *edit_message, *use_message;
static char *fixup_message, *squash_message;
static int all, edit_flag, also, interactive, only, amend, signoff;
@@ -88,7 +91,8 @@ static enum {
} cleanup_mode;
static char *cleanup_arg;
-static int use_editor = 1, initial_commit, in_merge, include_status = 1;
+static enum commit_whence whence;
+static int use_editor = 1, initial_commit, include_status = 1;
static int show_ignored_in_status;
static const char *only_include_assumed;
static struct strbuf message;
@@ -163,6 +167,36 @@ static struct option builtin_commit_options[] = {
OPT_END()
};
+static void determine_whence(struct wt_status *s)
+{
+ if (file_exists(git_path(merge_head)))
+ whence = FROM_MERGE;
+ else if (file_exists(git_path(cherry_pick_head)))
+ whence = FROM_CHERRY_PICK;
+ else
+ whence = FROM_COMMIT;
+ if (s)
+ s->whence = whence;
+}
+
+static const char *whence_s(void)
+{
+ char *s = "";
+
+ switch (whence) {
+ case FROM_COMMIT:
+ break;
+ case FROM_MERGE:
+ s = "merge";
+ break;
+ case FROM_CHERRY_PICK:
+ s = "cherry-pick";
+ break;
+ }
+
+ return s;
+}
+
static void rollback_index_files(void)
{
switch (commit_style) {
@@ -378,8 +412,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
*/
commit_style = COMMIT_PARTIAL;
- if (in_merge)
- die("cannot do a partial commit during a merge.");
+ if (whence != FROM_COMMIT)
+ die("cannot do a partial commit during a %s.", whence_s());
memset(&partial, 0, sizeof(partial));
partial.strdup_strings = 1;
@@ -469,18 +503,18 @@ static void determine_author_info(struct strbuf *author_ident)
email = getenv("GIT_AUTHOR_EMAIL");
date = getenv("GIT_AUTHOR_DATE");
- if (use_message && !renew_authorship) {
+ if (author_message) {
const char *a, *lb, *rb, *eol;
- a = strstr(use_message_buffer, "\nauthor ");
+ a = strstr(author_message_buffer, "\nauthor ");
if (!a)
- die("invalid commit: %s", use_message);
+ die("invalid commit: %s", author_message);
lb = strchrnul(a + strlen("\nauthor "), '<');
rb = strchrnul(lb, '>');
eol = strchrnul(rb, '\n');
if (!*lb || !*rb || !*eol)
- die("invalid commit: %s", use_message);
+ die("invalid commit: %s", author_message);
if (lb == a + strlen("\nauthor "))
/* \nauthor <foo@example.com> */
@@ -644,7 +678,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
* This final case does not modify the template message,
* it just sets the argument to the prepare-commit-msg hook.
*/
- else if (in_merge)
+ else if (whence == FROM_MERGE)
hook_arg1 = "merge";
if (squash_message) {
@@ -694,16 +728,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
strbuf_addstr(&committer_ident, git_committer_info(0));
if (use_editor && include_status) {
char *ai_tmp, *ci_tmp;
- if (in_merge)
+ if (whence != FROM_COMMIT)
fprintf(fp,
"#\n"
- "# It looks like you may be committing a MERGE.\n"
+ "# It looks like you may be committing a %s.\n"
"# If this is not correct, please remove the file\n"
"# %s\n"
"# and try again.\n"
"#\n",
- git_path("MERGE_HEAD"));
-
+ whence_s(),
+ git_path(whence == FROM_MERGE
+ ? merge_head
+ : cherry_pick_head));
fprintf(fp,
"\n"
"# Please enter the commit message for your changes.");
@@ -766,7 +802,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
fclose(fp);
- if (!commitable && !in_merge && !allow_empty &&
+ if (!commitable && whence != FROM_MERGE && !allow_empty &&
!(amend && is_a_merge(head_sha1))) {
run_status(stdout, index_file, prefix, 0, s);
if (amend)
@@ -898,6 +934,27 @@ static void handle_untracked_files_arg(struct wt_status *s)
die("Invalid untracked files mode '%s'", untracked_files_arg);
}
+static const char *read_commit_message(const char *name) {
+ const char *out_enc, *out;
+ struct commit *commit;
+
+ commit = lookup_commit_reference_by_name(name);
+ if (!commit)
+ die("could not lookup commit %s", name);
+ out_enc = get_commit_output_encoding();
+ out = logmsg_reencode(commit, out_enc);
+
+ /*
+ * If we failed to reencode the buffer, just copy it
+ * byte for byte so the user can try to fix it up.
+ * This also handles the case where input and output
+ * encodings are identical.
+ */
+ if (out == NULL)
+ out = xstrdup(commit->buffer);
+ return out;
+}
+
static int parse_and_validate_options(int argc, const char *argv[],
const char * const usage[],
const char *prefix,
@@ -927,8 +984,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
/* Sanity check options */
if (amend && initial_commit)
die("You have nothing to amend.");
- if (amend && in_merge)
- die("You are in the middle of a merge -- cannot amend.");
+ if (amend && whence != FROM_COMMIT)
+ die("You are in the middle of a %s -- cannot amend.", whence_s());
if (fixup_message && squash_message)
die("Options --squash and --fixup cannot be used together");
if (use_message)
@@ -947,26 +1004,18 @@ static int parse_and_validate_options(int argc, const char *argv[],
use_message = edit_message;
if (amend && !use_message && !fixup_message)
use_message = "HEAD";
- if (!use_message && renew_authorship)
+ if (!use_message && whence != FROM_CHERRY_PICK && renew_authorship)
die("--reset-author can be used only with -C, -c or --amend.");
if (use_message) {
- const char *out_enc;
- struct commit *commit;
-
- commit = lookup_commit_reference_by_name(use_message);
- if (!commit)
- die("could not lookup commit %s", use_message);
- out_enc = get_commit_output_encoding();
- use_message_buffer = logmsg_reencode(commit, out_enc);
-
- /*
- * If we failed to reencode the buffer, just copy it
- * byte for byte so the user can try to fix it up.
- * This also handles the case where input and output
- * encodings are identical.
- */
- if (use_message_buffer == NULL)
- use_message_buffer = xstrdup(commit->buffer);
+ use_message_buffer = read_commit_message(use_message);
+ if (!renew_authorship) {
+ author_message = use_message;
+ author_message_buffer = use_message_buffer;
+ }
+ }
+ if (whence == FROM_CHERRY_PICK && !renew_authorship) {
+ author_message = cherry_pick_head;
+ author_message_buffer = read_commit_message(author_message);
}
if (!!also + !!only + !!all + !!interactive > 1)
@@ -1117,7 +1166,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
wt_status_prepare(&s);
gitmodules_config();
git_config(git_status_config, &s);
- in_merge = file_exists(git_path("MERGE_HEAD"));
+ determine_whence(&s);
argc = parse_options(argc, argv, prefix,
builtin_status_options,
builtin_status_usage, 0);
@@ -1140,7 +1189,6 @@ int cmd_status(int argc, const char **argv, const char *prefix)
}
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
- s.in_merge = in_merge;
s.ignore_submodule_arg = ignore_submodule_arg;
wt_status_collect(&s);
@@ -1302,8 +1350,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
wt_status_prepare(&s);
git_config(git_commit_config, &s);
- in_merge = file_exists(git_path("MERGE_HEAD"));
- s.in_merge = in_merge;
+ determine_whence(&s);
if (s.use_color == -1)
s.use_color = git_use_color_default;
@@ -1340,17 +1387,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
for (c = commit->parents; c; c = c->next)
pptr = &commit_list_insert(c->item, pptr)->next;
- } else if (in_merge) {
+ } else if (whence == FROM_MERGE) {
struct strbuf m = STRBUF_INIT;
FILE *fp;
if (!reflog_msg)
reflog_msg = "commit (merge)";
pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
- fp = fopen(git_path("MERGE_HEAD"), "r");
+ fp = fopen(git_path(merge_head), "r");
if (fp == NULL)
die_errno("could not open '%s' for reading",
- git_path("MERGE_HEAD"));
+ git_path(merge_head));
while (strbuf_getline(&m, fp, '\n') != EOF) {
unsigned char sha1[20];
if (get_sha1_hex(m.buf, sha1) < 0)
@@ -1369,7 +1416,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
parents = reduce_heads(parents);
} else {
if (!reflog_msg)
- reflog_msg = "commit";
+ reflog_msg = (whence == FROM_CHERRY_PICK)
+ ? "commit (cherry-pick)"
+ : "commit";
pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
}
@@ -1424,8 +1473,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
die("cannot update HEAD ref");
}
- unlink(git_path("CHERRY_PICK_HEAD"));
- unlink(git_path("MERGE_HEAD"));
+ unlink(git_path(cherry_pick_head));
+ unlink(git_path(merge_head));
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
diff --git a/builtin/revert.c b/builtin/revert.c
index cced2e4..28d1d70 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -3,7 +3,6 @@
#include "object.h"
#include "commit.h"
#include "tag.h"
-#include "wt-status.h"
#include "run-command.h"
#include "exec_cmd.h"
#include "utf8.h"
@@ -198,56 +197,6 @@ static void add_message_to_msg(struct strbuf *msgbuf, const char *message)
strbuf_addstr(msgbuf, p);
}
-static void set_author_ident_env(const char *message)
-{
- const char *p = message;
- if (!p)
- die ("Could not read commit message of %s",
- sha1_to_hex(commit->object.sha1));
- while (*p && *p != '\n') {
- const char *eol;
-
- for (eol = p; *eol && *eol != '\n'; eol++)
- ; /* do nothing */
- if (!prefixcmp(p, "author ")) {
- char *line, *pend, *email, *timestamp;
-
- p += 7;
- line = xmemdupz(p, eol - p);
- email = strchr(line, '<');
- if (!email)
- die ("Could not extract author email from %s",
- sha1_to_hex(commit->object.sha1));
- if (email == line)
- pend = line;
- else
- for (pend = email; pend != line + 1 &&
- isspace(pend[-1]); pend--);
- ; /* do nothing */
- *pend = '\0';
- email++;
- timestamp = strchr(email, '>');
- if (!timestamp)
- die ("Could not extract author time from %s",
- sha1_to_hex(commit->object.sha1));
- *timestamp = '\0';
- for (timestamp++; *timestamp && isspace(*timestamp);
- timestamp++)
- ; /* do nothing */
- setenv("GIT_AUTHOR_NAME", line, 1);
- setenv("GIT_AUTHOR_EMAIL", email, 1);
- setenv("GIT_AUTHOR_DATE", timestamp, 1);
- free(line);
- return;
- }
- p = eol;
- if (*p == '\n')
- p++;
- }
- die ("No author information found in %s",
- sha1_to_hex(commit->object.sha1));
-}
-
static void write_cherry_pick_head(void)
{
int fd;
@@ -284,9 +233,7 @@ static void print_advice(void)
advise("after resolving the conflicts, mark the corrected paths");
advise("with 'git add <paths>' or 'git rm <paths>'");
-
- if (action == CHERRY_PICK)
- advise("and commit the result with 'git commit -c CHERRY_PICK_HEAD'");
+ advise("and commit the result with 'git commit'");
}
static void write_message(struct strbuf *msgbuf, const char *filename)
@@ -512,7 +459,6 @@ static int do_pick_commit(void)
base_label = msg.parent_label;
next = commit;
next_label = msg.label;
- set_author_ident_env(msg.message);
add_message_to_msg(&msgbuf, msg.message);
if (no_replay) {
strbuf_addstr(&msgbuf, "(cherry picked from commit ");
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 893b771..0b0b913 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -246,6 +246,8 @@ __git_ps1 ()
fi
elif [ -f "$g/MERGE_HEAD" ]; then
r="|MERGING"
+ elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
+ r="|CHERRY-PICKING"
elif [ -f "$g/BISECT_LOG" ]; then
r="|BISECTING"
fi
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index fd569c8..365a1ba 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -57,7 +57,7 @@ test_expect_success 'advice from failed cherry-pick' "
error: could not apply \$picked... picked
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
- hint: and commit the result with 'git commit -c CHERRY_PICK_HEAD'
+ hint: and commit the result with 'git commit'
EOF
test_must_fail git cherry-pick picked 2>actual &&
diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
index 77b6920..b61fd3c 100755
--- a/t/t7509-commit.sh
+++ b/t/t7509-commit.sh
@@ -157,4 +157,33 @@ test_expect_success '--reset-author should be rejected without -c/-C/--amend' '
test_must_fail git commit -a --reset-author -m done
'
+test_expect_success 'commit respects CHERRY_PICK_HEAD and MERGE_MSG' '
+ echo "cherry-pick 1a" >>foo &&
+ test_tick &&
+ git commit -am "cherry-pick 1" --author="Cherry <cherry@pick.er>" &&
+ git tag cherry-pick-head &&
+ git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD &&
+ echo "This is a MERGE_MSG" >.git/MERGE_MSG &&
+ echo "cherry-pick 1b" >>foo &&
+ test_tick &&
+ git commit -a &&
+ author_header cherry-pick-head >expect &&
+ author_header HEAD >actual &&
+ test_cmp expect actual &&
+
+ echo "This is a MERGE_MSG" >expect &&
+ message_body HEAD >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--reset-author with CHERRY_PICK_HEAD' '
+ git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD &&
+ echo "cherry-pick 2" >>foo &&
+ test_tick &&
+ git commit -am "cherry-pick 2" --reset-author &&
+ echo "author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE" >expect &&
+ author_header HEAD >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/wt-status.c b/wt-status.c
index 123582b..fbaaf54 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -60,7 +60,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
color_fprintf_ln(s->fp, c, "# Unmerged paths:");
if (!advice_status_hints)
return;
- if (s->in_merge)
+ if (s->whence != FROM_COMMIT)
;
else if (!s->is_initial)
color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
@@ -77,7 +77,7 @@ static void wt_status_print_cached_header(struct wt_status *s)
color_fprintf_ln(s->fp, c, "# Changes to be committed:");
if (!advice_status_hints)
return;
- if (s->in_merge)
+ if (s->whence != FROM_COMMIT)
; /* NEEDSWORK: use "git reset --unresolve"??? */
else if (!s->is_initial)
color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
diff --git a/wt-status.h b/wt-status.h
index 20b17cf..bf2eb27 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -24,6 +24,12 @@ enum untracked_status_type {
SHOW_ALL_UNTRACKED_FILES
};
+enum commit_whence {
+ FROM_COMMIT,
+ FROM_MERGE,
+ FROM_CHERRY_PICK
+};
+
struct wt_status_change_data {
int worktree_status;
int index_status;
@@ -40,7 +46,7 @@ struct wt_status {
const char **pathspec;
int verbose;
int amend;
- int in_merge;
+ enum commit_whence whence;
int nowarn;
int use_color;
int relative_paths;
--
1.7.4.1.28.gd46b3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] Introduce CHERRY_PICK_HEAD
2011-02-16 10:08 ` [PATCH 1/2] Introduce CHERRY_PICK_HEAD Jay Soffian
@ 2011-02-16 11:13 ` Nguyen Thai Ngoc Duy
2011-02-16 16:50 ` Jay Soffian
2011-02-16 17:20 ` [PATCH v2] " Jay Soffian
1 sibling, 1 reply; 18+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2011-02-16 11:13 UTC (permalink / raw)
To: Jay Soffian
Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder,
Junio C Hamano
On Wed, Feb 16, 2011 at 5:08 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> When a cherry-pick conflicts git advises to use:
>
> $ git commit -c <original commit id>
>
> to preserve the original commit message and authorship. Instead, let's
> record the original commit id in CHERRY_PICK_HEAD and advise to use:
>
> $ git commit -c CHERRY_PICK_HEAD
Wouldn't it be more convenient to do "git cherry-pick --continue"
instead of "git commit -c CHERRY_PICK_HEAD"?
--
Duy
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] Introduce CHERRY_PICK_HEAD
2011-02-16 11:13 ` Nguyen Thai Ngoc Duy
@ 2011-02-16 16:50 ` Jay Soffian
0 siblings, 0 replies; 18+ messages in thread
From: Jay Soffian @ 2011-02-16 16:50 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder,
Junio C Hamano
On Wed, Feb 16, 2011 at 6:13 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Wed, Feb 16, 2011 at 5:08 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
>> When a cherry-pick conflicts git advises to use:
>>
>> $ git commit -c <original commit id>
>>
>> to preserve the original commit message and authorship. Instead, let's
>> record the original commit id in CHERRY_PICK_HEAD and advise to use:
>>
>> $ git commit -c CHERRY_PICK_HEAD
>
> Wouldn't it be more convenient to do "git cherry-pick --continue"
> instead of "git commit -c CHERRY_PICK_HEAD"?
As mentioned in the part of the commit message you trimmed away: "In
the next commit, we teach git to handle the '-c CHERRY_PICK_HEAD'
part". Now, you may ask, why use "git commit" after resolving the
conflict (a la merge) instead of "git cherry-pick --continue" (a la
rebase).
I addressed this in the previous thread, see
http://article.gmane.org/gmane.comp.version-control.git/166884
j.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] Introduce CHERRY_PICK_HEAD
2011-02-16 10:08 ` [PATCH 1/2] Introduce CHERRY_PICK_HEAD Jay Soffian
2011-02-16 11:13 ` Nguyen Thai Ngoc Duy
@ 2011-02-16 17:20 ` Jay Soffian
2011-02-16 17:25 ` Jay Soffian
2011-02-16 21:42 ` Jonathan Nieder
1 sibling, 2 replies; 18+ messages in thread
From: Jay Soffian @ 2011-02-16 17:20 UTC (permalink / raw)
To: git
Cc: Jay Soffian, Ævar Arnfjörð Bjarmason,
Jonathan Nieder, Junio C Hamano
When a cherry-pick conflicts git advises to use:
$ git commit -c <original commit id>
to preserve the original commit message and authorship. Instead, let's
record the original commit id in CHERRY_PICK_HEAD and advise to use:
$ git commit -c CHERRY_PICK_HEAD
In the next commit, we teach git to handle the '-c CHERRY_PICK_HEAD'
part. Note that we record CHERRY_PICK_HEAD even in the case where there
are no conflicts so that we may use it to communicate authorship to
commit; this will then allow us to remove set_author_ident_env from
revert.c.
Contributions-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
Compared to the original patch, this removes CHERRY_PICK_HEAD when
a conflict occurs during an interactive rebase. Also corrected a minor
typo in the commit message. There were no changes to the next patch
in the series, so I didn't resend it.
Documentation/git-cherry-pick.txt | 19 +++++++++++++++++++
Documentation/revisions.txt | 5 ++++-
branch.c | 1 +
builtin/commit.c | 1 +
builtin/merge.c | 7 +++++++
builtin/revert.c | 25 +++++++++++++++++++++++--
t/t3507-cherry-pick-conflict.sh | 22 +++++++++++++++++++++-
7 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 749d68a..5d85daa 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -16,6 +16,25 @@ Given one or more existing commits, apply the change each one
introduces, recording a new commit for each. This requires your
working tree to be clean (no modifications from the HEAD commit).
+When it is not obvious how to apply a change, the following
+happens:
+
+1. The current branch and `HEAD` pointer stay at the last commit
+ successfully made.
+2. The `CHERRY_PICK_HEAD` ref is set to point at the commit that
+ introduced the change that is difficult to apply.
+3. Paths in which the change applied cleanly are updated both
+ in the index file and in your working tree.
+4. For conflicting paths, the index file records up to three
+ versions, as described in the "TRUE MERGE" section of
+ linkgit:git-merge[1]. The working tree files will include
+ a description of the conflict bracketed by the usual
+ conflict markers `<<<<<<<` and `>>>>>>>`.
+5. No other modifications are made.
+
+See linkgit:git-merge[1] for some hints on resolving such
+conflicts.
+
OPTIONS
-------
<commit>...::
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 9e92734..04fceee 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -25,7 +25,8 @@ blobs contained in a commit.
first match in the following rules:
. if `$GIT_DIR/<name>` exists, that is what you mean (this is usually
- useful only for `HEAD`, `FETCH_HEAD`, `ORIG_HEAD` and `MERGE_HEAD`);
+ useful only for `HEAD`, `FETCH_HEAD`, `ORIG_HEAD`, `MERGE_HEAD`
+ and `CHERRY_PICK_HEAD`);
. otherwise, `refs/<name>` if exists;
@@ -46,6 +47,8 @@ you can change the tip of the branch back to the state before you ran
them easily.
MERGE_HEAD records the commit(s) you are merging into your branch
when you run 'git merge'.
+CHERRY_PICK_HEAD records the commit you are cherry-picking
+when you run 'git cherry-pick'.
+
Note that any of the `refs/*` cases above may come either from
the `$GIT_DIR/refs` directory or from the `$GIT_DIR/packed-refs` file.
diff --git a/branch.c b/branch.c
index 93dc866..dc23e95 100644
--- a/branch.c
+++ b/branch.c
@@ -217,6 +217,7 @@ void create_branch(const char *head,
void remove_branch_state(void)
{
+ unlink(git_path("CHERRY_PICK_HEAD"));
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_RR"));
unlink(git_path("MERGE_MSG"));
diff --git a/builtin/commit.c b/builtin/commit.c
index 03cff5a..0def540 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1424,6 +1424,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
die("cannot update HEAD ref");
}
+ unlink(git_path("CHERRY_PICK_HEAD"));
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
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).");
+ }
resolve_undo_clear();
if (verbosity < 0)
diff --git a/builtin/revert.c b/builtin/revert.c
index dc1b702..88e3b2a 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -248,6 +248,22 @@ static void set_author_ident_env(const char *message)
sha1_to_hex(commit->object.sha1));
}
+static void write_cherry_pick_head(void)
+{
+ int fd;
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
+
+ fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666);
+ if (fd < 0)
+ die_errno("Could not open '%s' for writing",
+ git_path("CHERRY_PICK_HEAD"));
+ if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
+ die_errno("Could not write to '%s'", git_path("CHERRY_PICK_HEAD"));
+ strbuf_release(&buf);
+}
+
static void advise(const char *advice, ...)
{
va_list params;
@@ -263,6 +279,11 @@ static void print_advice(void)
if (msg) {
fprintf(stderr, "%s\n", msg);
+ /*
+ * rebase interactive takes care of the authorship
+ * when the user invokes rebase --continue
+ */
+ unlink(git_path("CHERRY_PICK_HEAD"));
return;
}
@@ -270,8 +291,7 @@ static void print_advice(void)
advise("with 'git add <paths>' or 'git rm <paths>'");
if (action == CHERRY_PICK)
- advise("and commit the result with 'git commit -c %s'",
- find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
+ advise("and commit the result with 'git commit -c CHERRY_PICK_HEAD'");
}
static void write_message(struct strbuf *msgbuf, const char *filename)
@@ -504,6 +524,7 @@ static int do_pick_commit(void)
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
}
+ write_cherry_pick_head();
}
if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 607bf25..fd569c8 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -11,6 +11,12 @@ test_description='test cherry-pick and revert with conflicts
. ./test-lib.sh
+test_cmp_rev () {
+ git rev-parse --verify "$1" >expect.rev &&
+ git rev-parse --verify "$2" >actual.rev &&
+ test_cmp expect.rev actual.rev
+}
+
test_expect_success setup '
echo unrelated >unrelated &&
@@ -51,13 +57,27 @@ test_expect_success 'advice from failed cherry-pick' "
error: could not apply \$picked... picked
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
- hint: and commit the result with 'git commit -c \$picked'
+ hint: and commit the result with 'git commit -c CHERRY_PICK_HEAD'
EOF
test_must_fail git cherry-pick picked 2>actual &&
test_cmp expected actual
"
+test_expect_success 'failed cherry-pick 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 &&
+
+ test_must_fail git cherry-pick picked &&
+
+ test_cmp_rev picked CHERRY_PICK_HEAD
+'
+
test_expect_success 'failed cherry-pick produces dirty index' '
git checkout -f initial^0 &&
--
1.7.4.1.28.gb39462
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Introduce CHERRY_PICK_HEAD
2011-02-16 17:20 ` [PATCH v2] " Jay Soffian
@ 2011-02-16 17:25 ` Jay Soffian
2011-02-16 21:42 ` Jonathan Nieder
1 sibling, 0 replies; 18+ messages in thread
From: Jay Soffian @ 2011-02-16 17:25 UTC (permalink / raw)
To: git
Cc: Jay Soffian, Ævar Arnfjörð Bjarmason,
Jonathan Nieder, Junio C Hamano
On Wed, Feb 16, 2011 at 12:20 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> @@ -263,6 +279,11 @@ static void print_advice(void)
>
> if (msg) {
> fprintf(stderr, "%s\n", msg);
> + /*
> + * rebase interactive takes care of the authorship
> + * when the user invokes rebase --continue
> + */
> + unlink(git_path("CHERRY_PICK_HEAD"));
> return;
> }
>
>
Oops, I amended the comment and then fed the wrong commit to
format-patch. That should be:
@@ -263,6 +279,11 @@ static void print_advice(void)
if (msg) {
fprintf(stderr, "%s\n", msg);
+ /*
+ * we're in the middle of an interactive rebase, we
+ * don't want it to look like we're cherry-picking.
+ */
+ unlink(git_path("CHERRY_PICK_HEAD"));
return;
}
Let me know if you want me to resend. (Sorry.)
j.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD
2011-02-16 10:08 ` [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD Jay Soffian
@ 2011-02-16 21:07 ` Junio C Hamano
2011-02-16 21:33 ` Jay Soffian
2011-02-16 21:55 ` [PATCH 1.5/2] bash: teach __git_ps1 " Jonathan Nieder
2011-02-16 22:43 ` [PATCH 2/2] Teach commit " Jonathan Nieder
2 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2011-02-16 21:07 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder
Jay Soffian <jaysoffian@gmail.com> writes:
> If the user wishes to reset authorship, that must now be done explicitly
> via --reset-author.
This is not a new requirement, is it? Even "commit -c $that_commit"
before the previous "commit -c CHERRY_PICK_HEAD" does use the original,
no?
I think the changed code is _MUCH_ easier to follow compared to the
previous round; the only thing the reader needs to keep in mind is that
the most of the change essentially is "s/in_merge/whence != FROM_COMMIT/"
and making that work.
> * We remove a unused import from revert.c
Micronit: s/import/include/.
> @@ -766,7 +802,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>
> fclose(fp);
>
> - if (!commitable && !in_merge && !allow_empty &&
> + if (!commitable && whence != FROM_MERGE && !allow_empty &&
> !(amend && is_a_merge(head_sha1))) {
> run_status(stdout, index_file, prefix, 0, s);
> if (amend)
The original says that we reject an attempt to record a no-change commit
when the user is making his own commit without an explicit --allow-empty
request. Should cherry-pick change this?
"git cherry-pick $a_no_change_commit" does not cause conflicts but it does
fail with "nothing added to commit". If you said "whence == FROM_COMMIT"
here, you would end up allowing "git commit" following such a failure to
commit without --allow-empty, and that inconsistency is to be avoided
(note that I didn't check if you leave CHERRY_PICK_HEAD behind to trigger
this codepath when this happens).
On the other hand, when the earlier cherry-pick did fail with conflict,
but the resolution ended up to be a no-change commit, you would not
blindly want to record the result as a no-change commit, either.
So I think the above is the right thing to do, but we probably need a bit
of in-code comment to describe why we say "whence != FROM_MERGE" here.
Thanks.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD
2011-02-16 21:07 ` Junio C Hamano
@ 2011-02-16 21:33 ` Jay Soffian
0 siblings, 0 replies; 18+ messages in thread
From: Jay Soffian @ 2011-02-16 21:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ævar Arnfjörð, Jonathan Nieder
On Wed, Feb 16, 2011 at 4:07 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>
>> If the user wishes to reset authorship, that must now be done explicitly
>> via --reset-author.
>
> This is not a new requirement, is it? Even "commit -c $that_commit"
> before the previous "commit -c CHERRY_PICK_HEAD" does use the original,
> no?
It is a new requirement for when the user does a bare "commit" _and_
CHERRY_PICK_HEAD exists.
Normally a bare "commit" creates new authorship, but when
CHERRY_PICK_HEAD exists, authorship is taken from CHERRY_PICK_HEAD
commit unless --reset-author is specified.
> I think the changed code is _MUCH_ easier to follow compared to the
> previous round; the only thing the reader needs to keep in mind is that
> the most of the change essentially is "s/in_merge/whence != FROM_COMMIT/"
> and making that work.
I think I was trying to be too clever in the previous round.
>> * We remove a unused import from revert.c
>
> Micronit: s/import/include/.
Okay.
> So I think the above is the right thing to do, but we probably need a bit
> of in-code comment to describe why we say "whence != FROM_MERGE" here.
Will do. It took me several tries to understand what that code block
was trying to do, so a comment will certainly help future readers.
Thanks,
j.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Introduce CHERRY_PICK_HEAD
2011-02-16 17:20 ` [PATCH v2] " Jay Soffian
2011-02-16 17:25 ` Jay Soffian
@ 2011-02-16 21:42 ` Jonathan Nieder
2011-02-16 22:13 ` Jay Soffian
1 sibling, 1 reply; 18+ messages in thread
From: Jonathan Nieder @ 2011-02-16 21:42 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano
Jay Soffian wrote:
> When a cherry-pick conflicts git advises to use:
>
> $ git commit -c <original commit id>
>
> to preserve the original commit message and authorship. Instead, let's
> record the original commit id in CHERRY_PICK_HEAD and advise to use:
>
> $ git commit -c CHERRY_PICK_HEAD
>
> In the next commit, we teach git to handle the '-c CHERRY_PICK_HEAD'
> part. Note that we record CHERRY_PICK_HEAD even in the case where there
> are no conflicts so that we may use it to communicate authorship to
> commit; this will then allow us to remove set_author_ident_env from
> revert.c.
This "In the next commit" phrasing is dangerous, since a person can
build on top of your first commit at any time. :) I would say:
A later patch will teach "git commit" without -c to use
CHERRY_PICK_HEAD to set the authorship automatically. Note
that[...]
[...]
> +++ b/builtin/revert.c
> @@ -263,6 +279,11 @@ static void print_advice(void)
>
> if (msg) {
> fprintf(stderr, "%s\n", msg);
> + /*
> + * rebase interactive takes care of the authorship
> + * when the user invokes rebase --continue
> + */
> + unlink(git_path("CHERRY_PICK_HEAD"));
Nit: GIT_CHERRY_PICK_HELP is not just for rebase --interactive but
for arbitrary porcelain that wants to take care of the commit itself
(see v1.5.4-rc0~106^2~1, revert/cherry-pick: Allow overriding the
help text by the calling Porcelain, 2007-11-28).
The conservative thing to do is indeed to remove CHERRY_PICK_HEAD in
this case, I suppose. But I'd like to have the CHERRY_PICK_HEAD to
get the --amend safety when rebasing. I can send a separate patch
for it if you'd like.
> --- a/t/t3507-cherry-pick-conflict.sh
> +++ b/t/t3507-cherry-pick-conflict.sh
> @@ -11,6 +11,12 @@ test_description='test cherry-pick and revert with conflicts
[...]
> +test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '
Some more tests. Yes, they are repetitive. A patch on top to factor
out the setup into a function might help, but that feels out of scope
here.
With whatever subset of the below looks good,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
---
t/t3507-cherry-pick-conflict.sh | 116 +++++++++++++++++++++++++++++++++++++++
1 files changed, 116 insertions(+), 0 deletions(-)
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index fd569c8..ea52720 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -78,6 +78,122 @@ test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '
test_cmp_rev picked CHERRY_PICK_HEAD
'
+test_expect_success 'successful cherry-pick does not set 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 &&
+
+ git cherry-pick base &&
+
+ test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
+'
+
+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 &&
+
+ git cherry-pick --no-commit base &&
+
+ test_cmp_rev base CHERRY_PICK_HEAD
+'
+
+test_expect_success 'GIT_CHERRY_PICK_HELP suppresses 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 &&
+
+ (
+ GIT_CHERRY_PICK_HELP="and then do something else" &&
+ export GIT_CHERRY_PICK_HELP &&
+ test_must_fail git cherry-pick picked
+ ) &&
+
+ test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
+'
+
+test_expect_success 'git reset clears 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 &&
+
+ test_must_fail git cherry-pick picked &&
+ git reset &&
+
+ test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
+'
+
+test_expect_success 'failed commit does not clear 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 &&
+
+ test_must_fail git cherry-pick picked &&
+ test_must_fail git commit &&
+
+ test_cmp_rev picked CHERRY_PICK_HEAD
+'
+
+test_expect_success 'cancelled commit does not clear 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 &&
+
+ test_must_fail git cherry-pick picked &&
+ echo resolved >foo &&
+ git add foo &&
+ git update-index --refresh -q &&
+ test_must_fail git diff-index --exit-code HEAD &&
+ (
+ GIT_EDITOR=false &&
+ export GIT_EDITOR &&
+ test_must_fail git commit
+ ) &&
+
+ test_cmp_rev picked CHERRY_PICK_HEAD
+'
+
+test_expect_success 'successful commit clears 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 &&
+
+ test_must_fail git cherry-pick picked &&
+ echo resolved >foo &&
+ git add foo &&
+ git commit &&
+
+ test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
+'
+
test_expect_success 'failed cherry-pick produces dirty index' '
git checkout -f initial^0 &&
--
1.7.4.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1.5/2] bash: teach __git_ps1 about CHERRY_PICK_HEAD
2011-02-16 10:08 ` [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD Jay Soffian
2011-02-16 21:07 ` Junio C Hamano
@ 2011-02-16 21:55 ` Jonathan Nieder
2011-02-16 22:49 ` Junio C Hamano
2011-02-16 22:43 ` [PATCH 2/2] Teach commit " Jonathan Nieder
2 siblings, 1 reply; 18+ messages in thread
From: Jonathan Nieder @ 2011-02-16 21:55 UTC (permalink / raw)
To: Jay Soffian
Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano,
SZEDER Gábor
From: Jay Soffian <jaysoffian@gmail.com>
Make the git prompt (when enabled) show a CHERRY-PICKING indicator
when we are in the middle of a conflicted cherry-pick, analogous
to the existing MERGING and BISECTING flags.
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
> * While we're at it, we update git-completion.bash to be
> CHERRY_PICK_HEAD aware.
Hmm, it seems I don't like this "while at it". :)
contrib/completion/git-completion.bash | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 893b771..0b0b913 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -246,6 +246,8 @@ __git_ps1 ()
fi
elif [ -f "$g/MERGE_HEAD" ]; then
r="|MERGING"
+ elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
+ r="|CHERRY-PICKING"
elif [ -f "$g/BISECT_LOG" ]; then
r="|BISECTING"
fi
--
1.7.4.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Introduce CHERRY_PICK_HEAD
2011-02-16 21:42 ` Jonathan Nieder
@ 2011-02-16 22:13 ` Jay Soffian
2011-02-16 23:02 ` Jonathan Nieder
0 siblings, 1 reply; 18+ messages in thread
From: Jay Soffian @ 2011-02-16 22:13 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Ævar Arnfjörð, Junio C Hamano
On Wed, Feb 16, 2011 at 4:42 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nit: GIT_CHERRY_PICK_HELP is not just for rebase --interactive but
> for arbitrary porcelain that wants to take care of the commit itself
> (see v1.5.4-rc0~106^2~1, revert/cherry-pick: Allow overriding the
> help text by the calling Porcelain, 2007-11-28).
What is the arbitrary porcelain you have in mind? :-)
> The conservative thing to do is indeed to remove CHERRY_PICK_HEAD in
> this case, I suppose. But I'd like to have the CHERRY_PICK_HEAD to
> get the --amend safety when rebasing. I can send a separate patch
> for it if you'd like.
Please do, since I'm not really sure what you have in mind. If
CHERRY_PICK_HEAD is left-behind, it interferes with the eventually
commit done by rebase --continue. I suppose we could give commit.c
some additional logic to detect when it's being called by rebase
(setting whence = FROM_REBASE?).
In the mean time, I'll re-roll with your additional tests and Junio's
prior feedback.
Thanks,
j.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD
2011-02-16 10:08 ` [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD Jay Soffian
2011-02-16 21:07 ` Junio C Hamano
2011-02-16 21:55 ` [PATCH 1.5/2] bash: teach __git_ps1 " Jonathan Nieder
@ 2011-02-16 22:43 ` Jonathan Nieder
2011-02-17 0:05 ` Jay Soffian
2 siblings, 1 reply; 18+ messages in thread
From: Jonathan Nieder @ 2011-02-16 22:43 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano
Jay Soffian wrote:
> This change fixes that situation. A bare 'commit' will now take the
> authorship from CHERRY_PICK_HEAD and the commit message from MERGE_MSG.
> If the user wishes to reset authorship, that must now be done explicitly
> via --reset-author.
Might also be worth mentioning that it makes --amend fail in such a
situation (a change worth celebrating).
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -56,8 +56,10 @@ static const char empty_amend_advice[] =
>
> static unsigned char head_sha1[20];
>
> -static char *use_message_buffer;
> +static const char *use_message_buffer;
> static const char commit_editmsg[] = "COMMIT_EDITMSG";
> +static const char cherry_pick_head[] = "CHERRY_PICK_HEAD";
> +static const char merge_head[] = "MERGE_HEAD";
Hmm, these variables but not MERGE_MSG, MERGE_MODE, and SQUASH_MSG?
> @@ -68,6 +70,7 @@ static enum {
>
> static const char *logfile, *force_author;
> static const char *template_file;
> +static const char *author_message, *author_message_buffer;
That's not a message at all, is it? On first reading I thought it
would be a message about the author. Maybe a comment can help.
/* name and content of commit from which to copy authorship */
> @@ -88,7 +91,8 @@ static enum {
> } cleanup_mode;
> static char *cleanup_arg;
>
> -static int use_editor = 1, initial_commit, in_merge, include_status = 1;
> +static enum commit_whence whence;
> +static int use_editor = 1, initial_commit, include_status = 1;
The name "whence" is not so self-explanatory but I don't have any
better ideas (I probably would have written "merge_or_cherry_pick"; we
can be glad you came up with something better).
> @@ -163,6 +167,36 @@ static struct option builtin_commit_options[] = {
> OPT_END()
> };
>
> +static void determine_whence(struct wt_status *s)
> +{
> + if (file_exists(git_path(merge_head)))
> + whence = FROM_MERGE;
Micronit: maybe COMMITTING_A_MERGE or COMMIT_DURING_MERGE to avoid
using valuable namespace?
> @@ -644,7 +678,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
> * This final case does not modify the template message,
> * it just sets the argument to the prepare-commit-msg hook.
> */
> - else if (in_merge)
> + else if (whence == FROM_MERGE)
> hook_arg1 = "merge";
Perhaps:
else if (whence == CHERRY_PICK) {
hook_arg1 = "commit";
hook_arg2 = author_message;
}
> @@ -694,16 +728,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
[...]
> "# If this is not correct, please remove the file\n"
> "# %s\n"
> "# and try again.\n"
> "#\n",
> + whence_s(),
> + git_path(whence == FROM_MERGE
> + ? merge_head
> + : cherry_pick_head));
Ok. We probably should move away from having to suggest
"rm -f .git/whatever" in the future (maybe
git update-ref -d %s
is simpler advice? I dunno).
> @@ -898,6 +934,27 @@ static void handle_untracked_files_arg(struct wt_status *s)
> die("Invalid untracked files mode '%s'", untracked_files_arg);
> }
>
> +static const char *read_commit_message(const char *name) {
Nice. Opening '{' should be in the first column.
> +++ b/builtin/revert.c
[...]
> @@ -284,9 +233,7 @@ static void print_advice(void)
>
> advise("after resolving the conflicts, mark the corrected paths");
> advise("with 'git add <paths>' or 'git rm <paths>'");
> -
> - if (action == CHERRY_PICK)
> - advise("and commit the result with 'git commit -c CHERRY_PICK_HEAD'");
> + advise("and commit the result with 'git commit'");
Hoorah!
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -60,7 +60,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
> color_fprintf_ln(s->fp, c, "# Unmerged paths:");
> if (!advice_status_hints)
> return;
> - if (s->in_merge)
> + if (s->whence != FROM_COMMIT)
> ;
> else if (!s->is_initial)
> color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
Isn't the advice of using "git reset -- <paths>" still good in the
CHERRY_PICK case?
> @@ -77,7 +77,7 @@ static void wt_status_print_cached_header(struct wt_status *s)
> color_fprintf_ln(s->fp, c, "# Changes to be committed:");
> if (!advice_status_hints)
> return;
> - if (s->in_merge)
> + if (s->whence != FROM_COMMIT)
> ; /* NEEDSWORK: use "git reset --unresolve"??? */
Likewise here.
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -24,6 +24,12 @@ enum untracked_status_type {
> SHOW_ALL_UNTRACKED_FILES
> };
>
> +enum commit_whence {
> + FROM_COMMIT,
> + FROM_MERGE,
> + FROM_CHERRY_PICK
> +};
Style: please use tabs to indent.
> @@ -40,7 +46,7 @@ struct wt_status {
> const char **pathspec;
> int verbose;
> int amend;
> - int in_merge;
> + enum commit_whence whence;
Might benefit from a comment.
/* whether a merge or cherry-pick is in progress */
enum commit_whence whence;
Thanks, very readable.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1.5/2] bash: teach __git_ps1 about CHERRY_PICK_HEAD
2011-02-16 21:55 ` [PATCH 1.5/2] bash: teach __git_ps1 " Jonathan Nieder
@ 2011-02-16 22:49 ` Junio C Hamano
0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2011-02-16 22:49 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jay Soffian, git, Ævar Arnfjörð Bjarmason,
Junio C Hamano, SZEDER Gábor
Jonathan Nieder <jrnieder@gmail.com> writes:
> Hmm, it seems I don't like this "while at it". :)
I don't either. The series will be re-rolled so I don't have to pick this
up myself, yes?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Introduce CHERRY_PICK_HEAD
2011-02-16 22:13 ` Jay Soffian
@ 2011-02-16 23:02 ` Jonathan Nieder
2011-02-17 19:58 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: Jonathan Nieder @ 2011-02-16 23:02 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Ævar Arnfjörð, Junio C Hamano
Jay Soffian wrote:
> On Wed, Feb 16, 2011 at 4:42 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Nit: GIT_CHERRY_PICK_HELP is not just for rebase --interactive but
>> for arbitrary porcelain that wants to take care of the commit itself
>> (see v1.5.4-rc0~106^2~1, revert/cherry-pick: Allow overriding the
>> help text by the calling Porcelain, 2007-11-28).
>
> What is the arbitrary porcelain you have in mind? :-)
git sequencer, for example. Or any out-of-tree tool that is using
cherry-pick to move around commits and wants to know where they end
up.
>> The conservative thing to do is indeed to remove CHERRY_PICK_HEAD in
>> this case, I suppose. But I'd like to have the CHERRY_PICK_HEAD to
>> get the --amend safety when rebasing. I can send a separate patch
>> for it if you'd like.
>
> Please do, since I'm not really sure what you have in mind. If
> CHERRY_PICK_HEAD is left-behind, it interferes with the eventually
> commit done by rebase --continue.
Wait, does this mean that -c/-C/--amend/CHERRY_PICK_HEAD overrides
GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and GIT_AUTHOR_DATE?
*checks*
Yes, it does. The behavior is carried over from v0.99~185
(git-commit-script: get commit message from an existing one,
2005-06-25), but imho it is wrong.
Does this seem worth fixing?
Jonathan
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD
2011-02-16 22:43 ` [PATCH 2/2] Teach commit " Jonathan Nieder
@ 2011-02-17 0:05 ` Jay Soffian
0 siblings, 0 replies; 18+ messages in thread
From: Jay Soffian @ 2011-02-17 0:05 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Ævar Arnfjörð, Junio C Hamano
On Wed, Feb 16, 2011 at 5:43 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Might also be worth mentioning that it makes --amend fail in such a
> situation (a change worth celebrating).
Never made that particular mistake myself, but okay.
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -56,8 +56,10 @@ static const char empty_amend_advice[] =
>>
>> static unsigned char head_sha1[20];
>>
>> -static char *use_message_buffer;
>> +static const char *use_message_buffer;
>> static const char commit_editmsg[] = "COMMIT_EDITMSG";
>> +static const char cherry_pick_head[] = "CHERRY_PICK_HEAD";
>> +static const char merge_head[] = "MERGE_HEAD";
>
> Hmm, these variables but not MERGE_MSG, MERGE_MODE, and SQUASH_MSG?
I cleaned up the ones my patch touched. Cleaning up the rest of
commit.c was out of my purview. :-)
I'll clean them up to be consistent, but I'll do it as a separate
patch (before this one).
>> @@ -68,6 +70,7 @@ static enum {
>>
>> static const char *logfile, *force_author;
>> static const char *template_file;
>> +static const char *author_message, *author_message_buffer;
>
> That's not a message at all, is it? On first reading I thought it
> would be a message about the author. Maybe a comment can help.
>
> /* name and content of commit from which to copy authorship */
The name is consistent with the other similar purpose variables:
use_message, edit_message, squash_message, fixup_message, which all
take a committish and aren't actually messages. None of those others
have comments, but it's obvious in context how they are used.
>> @@ -88,7 +91,8 @@ static enum {
>> } cleanup_mode;
>> static char *cleanup_arg;
>>
>> -static int use_editor = 1, initial_commit, in_merge, include_status = 1;
>> +static enum commit_whence whence;
>> +static int use_editor = 1, initial_commit, include_status = 1;
>
> The name "whence" is not so self-explanatory but I don't have any
> better ideas (I probably would have written "merge_or_cherry_pick"; we
> can be glad you came up with something better).
Respectfully disagree. Whence means "from where something came" as in
"from where did this commit we're about to make originate?" and I
intentionally didn't use _ORIGIN as "origin" has another meaning
already in git context.
>> @@ -163,6 +167,36 @@ static struct option builtin_commit_options[] = {
>> OPT_END()
>> };
>>
>> +static void determine_whence(struct wt_status *s)
>> +{
>> + if (file_exists(git_path(merge_head)))
>> + whence = FROM_MERGE;
>
> Micronit: maybe COMMITTING_A_MERGE or COMMIT_DURING_MERGE to avoid
> using valuable namespace?
Respectfully disagree.
> Perhaps:
>
> else if (whence == CHERRY_PICK) {
> hook_arg1 = "commit";
> hook_arg2 = author_message;
> }
Perhaps.
> Ok. We probably should move away from having to suggest
> "rm -f .git/whatever" in the future (maybe
>
> git update-ref -d %s
>
> is simpler advice? I dunno).
Out of scope for this patch. :-)
> Nice. Opening '{' should be in the first column.
Good catch.
> Isn't the advice of using "git reset -- <paths>" still good in the
> CHERRY_PICK case?
I don't know. I couldn't make up my mind. If it's a conflicted path
you've edited in the working copy, then the advice should be "checkout
--merge". I think. Maybe. I don't find wt-status.c to be very much
fun, so I punted.
>> - if (s->in_merge)
>> + if (s->whence != FROM_COMMIT)
>> ; /* NEEDSWORK: use "git reset --unresolve"??? */
>
> Likewise here.
Checkout that NEEDSWORK. Someone should get on that. :-)
> Style: please use tabs to indent.
Who ate my tabs?
> Might benefit from a comment.
>
> /* whether a merge or cherry-pick is in progress */
> enum commit_whence whence;
Agreed.
> Thanks, very readable.
Good feedback.
j.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Introduce CHERRY_PICK_HEAD
2011-02-16 23:02 ` Jonathan Nieder
@ 2011-02-17 19:58 ` Junio C Hamano
2011-02-17 22:16 ` Jonathan Nieder
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2011-02-17 19:58 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Jay Soffian, git, Ævar Arnfjörð
Jonathan Nieder <jrnieder@gmail.com> writes:
> Wait, does this mean that -c/-C/--amend/CHERRY_PICK_HEAD overrides
> GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and GIT_AUTHOR_DATE?
>
> *checks*
>
> Yes, it does. The behavior is carried over from v0.99~185
> (git-commit-script: get commit message from an existing one,
> 2005-06-25), but imho it is wrong.
Hmph, -c/-C/--amend is like giving the identity with --author from the
command line so overriding the values from the environment variables
sounds like the right thing to do.
Where am I confused?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Introduce CHERRY_PICK_HEAD
2011-02-17 19:58 ` Junio C Hamano
@ 2011-02-17 22:16 ` Jonathan Nieder
0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Nieder @ 2011-02-17 22:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jay Soffian, git, Ævar Arnfjörð
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> Wait, does this mean that -c/-C/--amend/CHERRY_PICK_HEAD overrides
>> GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and GIT_AUTHOR_DATE?
[...]
> Hmph, -c/-C/--amend is like giving the identity with --author from the
> command line so overriding the values from the environment variables
> sounds like the right thing to do.
Good point. There are two principles at work here:
- options with more specific effect override more general ones
- options on the command line override the environment
On second thought, the latter does seem more relevant (though it need
not apply to CHERRY_PICK_HEAD).
Thanks for a sanity check.
Jonathan
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2011-02-17 22:17 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-16 10:08 [PATCH 0/2] CHERRY_PICK_HEAD Jay Soffian
2011-02-16 10:08 ` [PATCH 1/2] Introduce CHERRY_PICK_HEAD Jay Soffian
2011-02-16 11:13 ` Nguyen Thai Ngoc Duy
2011-02-16 16:50 ` Jay Soffian
2011-02-16 17:20 ` [PATCH v2] " Jay Soffian
2011-02-16 17:25 ` Jay Soffian
2011-02-16 21:42 ` Jonathan Nieder
2011-02-16 22:13 ` Jay Soffian
2011-02-16 23:02 ` Jonathan Nieder
2011-02-17 19:58 ` Junio C Hamano
2011-02-17 22:16 ` Jonathan Nieder
2011-02-16 10:08 ` [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD Jay Soffian
2011-02-16 21:07 ` Junio C Hamano
2011-02-16 21:33 ` Jay Soffian
2011-02-16 21:55 ` [PATCH 1.5/2] bash: teach __git_ps1 " Jonathan Nieder
2011-02-16 22:49 ` Junio C Hamano
2011-02-16 22:43 ` [PATCH 2/2] Teach commit " Jonathan Nieder
2011-02-17 0:05 ` Jay Soffian
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).