Re: [PATCH v4 20/45] cherry-pick: copy notes and run hooks
On Sun, Jun 9, 2013 at 12:22 PM, Thomas Rast wrote:
> Felipe Contreras writes:
>
>> +static void finish(struct replay_opts *opts)
>> +{
>> + if (opts->action != REPLAY_PICK)
>> + return;
>> +
>> + run_rewrite_hook(&rewritten, "cherry-pick");
>> + copy_rewrite_notes(&rewritten, "cherry-pick");
>> +}
>> +
>
> Ok, so I see that with the previous two commits, you automatically get
> handling of the notes.rewrite.cherry-pick variable and friends. This is
> good.
>
> However, there are some open points:
>
> * The docs in git-config(1) "notes.rewrite.cherry-pick" and githooks(5)
> "post-rewrite" and are now stale in so far as they contain a list of
> commands doing rewriting.
Fine.
--- a/builtin/sequencer.c
+++ b/builtin/sequencer.c
@@ -28,9 +28,9 @@ static void finish(struct replay_opts *opts)
if (opts->action != REPLAY_PICK)
return;
- name = opts->action_name ? opts->action_name : "cherry-pick";
+ name = opts->action_name
- if (!*name)
+ if (!name || !*name)
return;
run_rewrite_hook(&rewritten, name);
Now, we won't run when 'git cherry-pick' is called, only when an
action-name is specified; when called from 'git rebase'.
> * This pretends to be cherry-pick even when the hook is called from
> rebase.
No.
http://mid.gmane.org/1370796057-25312-31-git-send-email-felipe.contre...@gmail.com
> * githooks(5) documents explicitly that by the time post-rewrite is
> called, the notes have been rewritten. Your change does it in the
> opposite order.
OK.
But it doesn't matter, because the patch won't be applied.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 20/45] cherry-pick: copy notes and run hooks
Felipe Contreras writes:
> +static void finish(struct replay_opts *opts)
> +{
> + if (opts->action != REPLAY_PICK)
> + return;
> +
> + run_rewrite_hook(&rewritten, "cherry-pick");
> + copy_rewrite_notes(&rewritten, "cherry-pick");
> +}
> +
Ok, so I see that with the previous two commits, you automatically get
handling of the notes.rewrite.cherry-pick variable and friends. This is
good.
However, there are some open points:
* The docs in git-config(1) "notes.rewrite.cherry-pick" and githooks(5)
"post-rewrite" and are now stale in so far as they contain a list of
commands doing rewriting.
* This pretends to be cherry-pick even when the hook is called from
rebase.
We could claim (and document) that git-rebase with certain options
shall be the same as running cherry-pick with some other options.
However, git-am already goes out of its way to ensure that it only
does the rewriting/post-rewrite if called from rebase (so that the
user-facing git-am command is not affected). So it's more consistent
to ensure that git-cherry-pick, when called from rebase, also pretends
to be rebase.
* githooks(5) documents explicitly that by the time post-rewrite is
called, the notes have been rewritten. Your change does it in the
opposite order.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 20/45] cherry-pick: copy notes and run hooks
Signed-off-by: Felipe Contreras
---
builtin/rewrite.c | 1 +
builtin/sequencer.c | 18 +-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/builtin/rewrite.c b/builtin/rewrite.c
index 02e0ea9..acbad44 100644
--- a/builtin/rewrite.c
+++ b/builtin/rewrite.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "rewrite.h"
#include "run-command.h"
+#include "builtin.h"
void add_rewritten(struct rewritten *list, unsigned char *from, unsigned char
*to)
{
diff --git a/builtin/sequencer.c b/builtin/sequencer.c
index d40fda9..fe96f1f 100644
--- a/builtin/sequencer.c
+++ b/builtin/sequencer.c
@@ -21,6 +21,15 @@
static struct rewritten rewritten;
+static void finish(struct replay_opts *opts)
+{
+ if (opts->action != REPLAY_PICK)
+ return;
+
+ run_rewrite_hook(&rewritten, "cherry-pick");
+ copy_rewrite_notes(&rewritten, "cherry-pick");
+}
+
static void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
@@ -935,6 +944,8 @@ static int pick_commits(struct commit_list *todo_list,
struct replay_opts *opts)
}
}
+ finish(opts);
+
/*
* Sequence of picks finished successfully; cleanup by
* removing the .git/sequencer directory
@@ -1006,8 +1017,13 @@ static int sequencer_skip(struct replay_opts *opts)
static int single_pick(struct commit *cmit, struct replay_opts *opts)
{
+ int ret;
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
- return do_pick_commit(cmit, opts);
+ ret = do_pick_commit(cmit, opts);
+ if (ret)
+ return ret;
+ finish(opts);
+ return 0;
}
int sequencer_pick_revisions(struct replay_opts *opts)
--
1.8.3.698.g079b096
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
