* [PATCH v2 20/25] sequencer: left-trim lines read from the script
From: Johannes Schindelin @ 2016-09-11 10:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
Interactive rebase's scripts may be indented; we need to handle this
case, too, now that we prepare the sequencer to process interactive
rebases.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index 7953a05..5e5d113 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -875,6 +875,9 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
char *end_of_object_name;
int i, saved, status, padding;
+ /* left-trim */
+ bol += strspn(bol, " \t");
+
for (i = 0; i < ARRAY_SIZE(todo_command_strings); i++)
if (skip_prefix(bol, todo_command_strings[i], &bol)) {
item->command = i;
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 19/25] sequencer: remember do_recursive_merge()'s return value
From: Johannes Schindelin @ 2016-09-11 10:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
The return value of do_recursive_merge() may be positive (indicating merge
conflicts), so let's OR later error conditions so as not to overwrite them
with 0.
This is not yet a problem, but preparing for the patches to come: we will
teach the sequencer to do rebase -i's job.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 75772b8..7953a05 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -630,7 +630,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
const char *base_label, *next_label;
struct commit_message msg = { NULL, NULL, NULL, NULL };
struct strbuf msgbuf = STRBUF_INIT;
- int res, unborn = 0, allow;
+ int res = 0, unborn = 0, allow;
if (opts->no_commit) {
/*
@@ -741,7 +741,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
}
if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command == TODO_REVERT) {
- res = do_recursive_merge(base, next, base_label, next_label,
+ res |= do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
if (res < 0)
return res;
@@ -750,7 +750,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
struct commit_list *common = NULL;
struct commit_list *remotes = NULL;
- res = write_message(&msgbuf, git_path_merge_msg());
+ res |= write_message(&msgbuf, git_path_merge_msg());
commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
@@ -787,11 +787,12 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
allow = allow_empty(opts, commit);
if (allow < 0) {
- res = allow;
+ res |= allow;
goto leave;
}
if (!opts->no_commit)
- res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
+ res |= sequencer_commit(opts->edit ?
+ NULL : git_path_merge_msg(),
opts, allow, opts->edit, 0, 0);
leave:
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 18/25] sequencer: support cleaning up commit messages
From: Johannes Schindelin @ 2016-09-11 10:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
The sequencer_commit() function already knows how to amend commits, and
with this new option, it can also clean up commit messages (i.e. strip
out commented lines). This is needed to implement rebase -i's 'fixup'
and 'squash' commands as sequencer commands.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 10 +++++++---
sequencer.h | 3 ++-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 60b522e..75772b8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -485,7 +485,8 @@ static char **read_author_script(void)
* author metadata.
*/
int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty, int edit, int amend)
+ int allow_empty, int edit, int amend,
+ int cleanup_commit_message)
{
char **env = NULL;
struct argv_array array;
@@ -522,9 +523,12 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
argv_array_push(&array, "-s");
if (defmsg)
argv_array_pushl(&array, "-F", defmsg, NULL);
+ if (cleanup_commit_message)
+ argv_array_push(&array, "--cleanup=strip");
if (edit)
argv_array_push(&array, "-e");
- else if (!opts->signoff && !opts->record_origin &&
+ else if (!cleanup_commit_message &&
+ !opts->signoff && !opts->record_origin &&
git_config_get_value("commit.cleanup", &value))
argv_array_push(&array, "--cleanup=verbatim");
@@ -788,7 +792,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
}
if (!opts->no_commit)
res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
- opts, allow, opts->edit, 0);
+ opts, allow, opts->edit, 0, 0);
leave:
free_message(commit, &msg);
diff --git a/sequencer.h b/sequencer.h
index c45f5c4..688fff1 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -54,7 +54,8 @@ int sequencer_rollback(struct replay_opts *opts);
int sequencer_remove_state(struct replay_opts *opts);
int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty, int edit, int amend);
+ int allow_empty, int edit, int amend,
+ int cleanup_commit_message);
extern const char sign_off_header[];
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 17/25] sequencer: support amending commits
From: Johannes Schindelin @ 2016-09-11 10:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
This teaches the sequencer_commit() function to take an argument that
will allow us to implement "todo" commands that need to amend the commit
messages ("fixup", "squash" and "reword").
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 6 ++++--
sequencer.h | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 6e9732c..60b522e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -485,7 +485,7 @@ static char **read_author_script(void)
* author metadata.
*/
int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty, int edit)
+ int allow_empty, int edit, int amend)
{
char **env = NULL;
struct argv_array array;
@@ -514,6 +514,8 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
argv_array_push(&array, "commit");
argv_array_push(&array, "-n");
+ if (amend)
+ argv_array_push(&array, "--amend");
if (opts->gpg_sign)
argv_array_pushf(&array, "-S%s", opts->gpg_sign);
if (opts->signoff)
@@ -786,7 +788,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
}
if (!opts->no_commit)
res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
- opts, allow, opts->edit);
+ opts, allow, opts->edit, 0);
leave:
free_message(commit, &msg);
diff --git a/sequencer.h b/sequencer.h
index 7f5222f..c45f5c4 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -54,7 +54,7 @@ int sequencer_rollback(struct replay_opts *opts);
int sequencer_remove_state(struct replay_opts *opts);
int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty, int edit);
+ int allow_empty, int edit, int amend);
extern const char sign_off_header[];
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 16/25] sequencer: allow editing the commit message on a case-by-case basis
From: Johannes Schindelin @ 2016-09-11 10:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
In the upcoming commits, we will implement more and more of rebase
-i's functionality. One particular feature of the commands to come is
that some of them allow editing the commit message while others don't,
i.e. we cannot define in the replay_opts whether the commit message
should be edited or not.
Let's add a new parameter to the sequencer_commit() function. Previously,
it was the duty of the caller to ensure that the opts->edit setting
indicates whether to let the user edit the commit message or not,
indicating that it is an "all or nothing" setting, i.e. that the sequencer
wants to let the user edit *all* commit message, or none at all. In the
upcoming rebase -i mode, it will depend on the particular command that is
currently executed, though.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 6 +++---
sequencer.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index bf02565..6e9732c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -485,7 +485,7 @@ static char **read_author_script(void)
* author metadata.
*/
int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty)
+ int allow_empty, int edit)
{
char **env = NULL;
struct argv_array array;
@@ -520,7 +520,7 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
argv_array_push(&array, "-s");
if (defmsg)
argv_array_pushl(&array, "-F", defmsg, NULL);
- if (opts->edit)
+ if (edit)
argv_array_push(&array, "-e");
else if (!opts->signoff && !opts->record_origin &&
git_config_get_value("commit.cleanup", &value))
@@ -786,7 +786,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
}
if (!opts->no_commit)
res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
- opts, allow);
+ opts, allow, opts->edit);
leave:
free_message(commit, &msg);
diff --git a/sequencer.h b/sequencer.h
index 16deb6c..7f5222f 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -54,7 +54,7 @@ int sequencer_rollback(struct replay_opts *opts);
int sequencer_remove_state(struct replay_opts *opts);
int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty);
+ int allow_empty, int edit);
extern const char sign_off_header[];
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 15/25] sequencer: prepare for rebase -i's GPG settings
From: Johannes Schindelin @ 2016-09-11 10:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
The rebase command sports a `--gpg-sign` option that is heeded by the
interactive rebase.
This patch teaches the sequencer that trick, as part of the bigger
effort to make the sequencer the work horse of the interactive rebase.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 42 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 37 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 086cd0b..bf02565 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -15,6 +15,7 @@
#include "merge-recursive.h"
#include "refs.h"
#include "argv-array.h"
+#include "quote.h"
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -33,6 +34,11 @@ static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
* being rebased.
*/
static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
+/*
+ * The following files are written by git-rebase just after parsing the
+ * command-line (and are only consumed, not modified, by the sequencer).
+ */
+static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
/* We will introduce the 'interactive rebase' mode later */
static inline int is_rebase_i(const struct replay_opts *opts)
@@ -132,6 +138,16 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
return 1;
}
+static const char *gpg_sign_opt_quoted(struct replay_opts *opts)
+{
+ static struct strbuf buf = STRBUF_INIT;
+
+ strbuf_reset(&buf);
+ if (opts->gpg_sign)
+ sq_quotef(&buf, "-S%s", opts->gpg_sign);
+ return buf.buf;
+}
+
void *sequencer_entrust(struct replay_opts *opts, void *to_free)
{
ALLOC_GROW(opts->owned, opts->owned_nr + 1, opts->owned_alloc);
@@ -478,17 +494,20 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
if (is_rebase_i(opts)) {
env = read_author_script();
- if (!env)
+ if (!env) {
+ const char *gpg_opt = gpg_sign_opt_quoted(opts);
+
return error("You have staged changes in your working "
"tree. If these changes are meant to be\n"
"squashed into the previous commit, run:\n\n"
- " git commit --amend $gpg_sign_opt_quoted\n\n"
+ " git commit --amend %s\n\n"
"If they are meant to go into a new commit, "
"run:\n\n"
- " git commit $gpg_sign_opt_quoted\n\n"
+ " git commit %s\n\n"
"In both cases, once you're done, continue "
"with:\n\n"
- " git rebase --continue\n");
+ " git rebase --continue\n", gpg_opt, gpg_opt);
+ }
}
argv_array_init(&array);
@@ -980,8 +999,21 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
static int read_populate_opts(struct replay_opts *opts)
{
- if (is_rebase_i(opts))
+ if (is_rebase_i(opts)) {
+ struct strbuf buf = STRBUF_INIT;
+
+ if (read_oneliner(&buf, rebase_path_gpg_sign_opt(), 1)) {
+ if (!starts_with(buf.buf, "-S"))
+ strbuf_reset(&buf);
+ else {
+ opts->gpg_sign = buf.buf + 2;
+ sequencer_entrust(opts,
+ strbuf_detach(&buf, NULL));
+ }
+ }
+
return 0;
+ }
if (!file_exists(git_path_opts_file()))
return 0;
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 14/25] sequencer: introduce a helper to read files written by scripts
From: Johannes Schindelin @ 2016-09-11 10:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
As we are slowly teaching the sequencer to perform the hard work for
the interactive rebase, we need to read files that were written by
shell scripts.
These files typically contain a single line and are invariably ended
by a line feed (and possibly a carriage return before that). Let's use
a helper to read such files and to remove the line ending.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index 6c35fe8..086cd0b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -242,6 +242,37 @@ static int write_message(struct strbuf *msgbuf, const char *filename)
return 0;
}
+/*
+ * Reads a file that was presumably written by a shell script, i.e.
+ * with an end-of-line marker that needs to be stripped.
+ *
+ * Returns 1 if the file was read, 0 if it could not be read or does not exist.
+ */
+static int read_oneliner(struct strbuf *buf,
+ const char *path, int skip_if_empty)
+{
+ int orig_len = buf->len;
+
+ if (!file_exists(path))
+ return 0;
+
+ if (strbuf_read_file(buf, path, 0) < 0) {
+ warning_errno("could not read '%s'", path);
+ return 0;
+ }
+
+ if (buf->len > orig_len && buf->buf[buf->len - 1] == '\n') {
+ if (--buf->len > orig_len && buf->buf[buf->len - 1] == '\r')
+ --buf->len;
+ buf->buf[buf->len] = '\0';
+ }
+
+ if (skip_if_empty && buf->len == orig_len)
+ return 0;
+
+ return 1;
+}
+
static struct tree *empty_tree(void)
{
return lookup_tree(EMPTY_TREE_SHA1_BIN);
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 13/25] sequencer: prepare for rebase -i's commit functionality
From: Johannes Schindelin @ 2016-09-11 10:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
In interactive rebases, we commit a little bit differently than the
sequencer did so far: we heed the "author-script", the "message" and
the "amend" files in the .git/rebase-merge/ subdirectory.
Likewise, we may want to edit the commit message *even* when providing
a file containing the suggested commit message. Therefore we change the
code to not even provide a default message when we do not want any, and
to call the editor explicitly.
As interactive rebase's GPG settings are configured differently from
how cherry-pick (and therefore sequencer) handles them, we will leave
support for that to the next commit.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
sequencer.h | 3 ++
2 files changed, 89 insertions(+), 11 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index ca1961c..6c35fe8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -27,6 +27,19 @@ static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
+/*
+ * A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
+ * GIT_AUTHOR_DATE that will be used for the commit that is currently
+ * being rebased.
+ */
+static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
+
+/* We will introduce the 'interactive rebase' mode later */
+static inline int is_rebase_i(const struct replay_opts *opts)
+{
+ return 0;
+}
+
static const char *get_dir(const struct replay_opts *opts)
{
return git_path_seq_dir();
@@ -377,20 +390,76 @@ static int is_index_unchanged(void)
return !hashcmp(active_cache_tree->sha1, head_commit->tree->object.oid.hash);
}
+static char **read_author_script(void)
+{
+ struct strbuf script = STRBUF_INIT;
+ int i, count = 0;
+ char *p, *p2, **env;
+ size_t env_size;
+
+ if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0)
+ return NULL;
+
+ for (p = script.buf; *p; p++)
+ if (skip_prefix(p, "'\\\\''", (const char **)&p2))
+ strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
+ else if (*p == '\'')
+ strbuf_splice(&script, p-- - script.buf, 1, "", 0);
+ else if (*p == '\n') {
+ *p = '\0';
+ count++;
+ }
+
+ env_size = (count + 1) * sizeof(*env);
+ strbuf_grow(&script, env_size);
+ memmove(script.buf + env_size, script.buf, script.len);
+ p = script.buf + env_size;
+ env = (char **)strbuf_detach(&script, NULL);
+
+ for (i = 0; i < count; i++) {
+ env[i] = p;
+ p += strlen(p) + 1;
+ }
+ env[count] = NULL;
+
+ return env;
+}
+
/*
* If we are cherry-pick, and if the merge did not result in
* hand-editing, we will hit this commit and inherit the original
* author date and name.
+ *
* If we are revert, or if our cherry-pick results in a hand merge,
* we had better say that the current user is responsible for that.
+ *
+ * An exception is when sequencer_commit() is called during an
+ * interactive rebase: in that case, we will want to retain the
+ * author metadata.
*/
-static int run_git_commit(const char *defmsg, struct replay_opts *opts,
+int sequencer_commit(const char *defmsg, struct replay_opts *opts,
int allow_empty)
{
+ char **env = NULL;
struct argv_array array;
int rc;
const char *value;
+ if (is_rebase_i(opts)) {
+ env = read_author_script();
+ if (!env)
+ return error("You have staged changes in your working "
+ "tree. If these changes are meant to be\n"
+ "squashed into the previous commit, run:\n\n"
+ " git commit --amend $gpg_sign_opt_quoted\n\n"
+ "If they are meant to go into a new commit, "
+ "run:\n\n"
+ " git commit $gpg_sign_opt_quoted\n\n"
+ "In both cases, once you're done, continue "
+ "with:\n\n"
+ " git rebase --continue\n");
+ }
+
argv_array_init(&array);
argv_array_push(&array, "commit");
argv_array_push(&array, "-n");
@@ -399,14 +468,13 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
argv_array_pushf(&array, "-S%s", opts->gpg_sign);
if (opts->signoff)
argv_array_push(&array, "-s");
- if (!opts->edit) {
- argv_array_push(&array, "-F");
- argv_array_push(&array, defmsg);
- if (!opts->signoff &&
- !opts->record_origin &&
- git_config_get_value("commit.cleanup", &value))
- argv_array_push(&array, "--cleanup=verbatim");
- }
+ if (defmsg)
+ argv_array_pushl(&array, "-F", defmsg, NULL);
+ if (opts->edit)
+ argv_array_push(&array, "-e");
+ else if (!opts->signoff && !opts->record_origin &&
+ git_config_get_value("commit.cleanup", &value))
+ argv_array_push(&array, "--cleanup=verbatim");
if (allow_empty)
argv_array_push(&array, "--allow-empty");
@@ -414,8 +482,11 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
if (opts->allow_empty_message)
argv_array_push(&array, "--allow-empty-message");
- rc = run_command_v_opt(array.argv, RUN_GIT_CMD);
+ rc = run_command_v_opt_cd_env(array.argv, RUN_GIT_CMD, NULL,
+ (const char *const *)env);
argv_array_clear(&array);
+ free(env);
+
return rc;
}
@@ -664,7 +735,8 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
goto leave;
}
if (!opts->no_commit)
- res = run_git_commit(git_path_merge_msg(), opts, allow);
+ res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
+ opts, allow);
leave:
free_message(commit, &msg);
@@ -877,6 +949,9 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
static int read_populate_opts(struct replay_opts *opts)
{
+ if (is_rebase_i(opts))
+ return 0;
+
if (!file_exists(git_path_opts_file()))
return 0;
/*
diff --git a/sequencer.h b/sequencer.h
index 0b3950d..16deb6c 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -53,6 +53,9 @@ int sequencer_continue(struct replay_opts *opts);
int sequencer_rollback(struct replay_opts *opts);
int sequencer_remove_state(struct replay_opts *opts);
+int sequencer_commit(const char *defmsg, struct replay_opts *opts,
+ int allow_empty);
+
extern const char sign_off_header[];
void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 12/25] sequencer: remember the onelines when parsing the todo file
From: Johannes Schindelin @ 2016-09-11 10:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
The `git-rebase-todo` file contains a list of commands. Most of those
commands have the form
<verb> <sha1> <oneline>
The <oneline> is displayed primarily for the user's convenience, as
rebase -i really interprets only the <verb> <sha1> part. However, there
are *some* places in interactive rebase where the <oneline> is used to
display messages, e.g. for reporting at which commit we stopped.
So let's just remember it when parsing the todo file; we keep a copy of
the entire todo file anyway (to write out the new `done` and
`git-rebase-todo` file just before processing each command), so all we
need to do is remember the begin offsets and lengths.
As we will have to parse and remember the command-line for `exec` commands
later, we do not call the field "oneline" but rather "arg" (and will reuse
that for exec's command-line).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index 0c8dec4..ca1961c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -713,6 +713,8 @@ static int read_and_refresh_cache(struct replay_opts *opts)
struct todo_item {
enum todo_command command;
struct commit *commit;
+ const char *arg;
+ int arg_len;
size_t offset_in_buf;
};
@@ -764,6 +766,9 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
status = get_sha1(bol, commit_sha1);
*end_of_object_name = saved;
+ item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
+ item->arg_len = (int)(eol - item->arg);
+
if (status < 0)
return -1;
@@ -905,6 +910,8 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
item->command = command;
item->commit = commit;
+ item->arg = NULL;
+ item->arg_len = 0;
item->offset_in_buf = todo_list->buf.len;
subject_len = find_commit_subject(commit_buffer, &subject);
strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 11/25] sequencer: refactor the code to obtain a short commit name
From: Johannes Schindelin @ 2016-09-11 10:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
Not only does this DRY up the code (providing a better documentation what
the code is about, as well as allowing to change the behavior in a single
place), it also makes it substantially shorter to use the same
functionality in functions to be introduced when we teach the sequencer to
process interactive-rebase's git-rebase-todo file.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 053e78c..0c8dec4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -157,13 +157,18 @@ struct commit_message {
const char *message;
};
+static const char *short_commit_name(struct commit *commit)
+{
+ return find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV);
+}
+
static int get_message(struct commit *commit, struct commit_message *out)
{
const char *abbrev, *subject;
int subject_len;
out->message = logmsg_reencode(commit, NULL, get_commit_output_encoding());
- abbrev = find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV);
+ abbrev = short_commit_name(commit);
subject_len = find_commit_subject(out->message, &subject);
@@ -647,8 +652,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
error(command == TODO_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
- find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV),
- msg.subject);
+ short_commit_name(commit), msg.subject);
print_advice(res == 1, opts);
rerere(opts->allow_rerere_auto);
goto leave;
@@ -904,9 +908,7 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
item->offset_in_buf = todo_list->buf.len;
subject_len = find_commit_subject(commit_buffer, &subject);
strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
- find_unique_abbrev(commit->object.oid.hash,
- DEFAULT_ABBREV),
- subject_len, subject);
+ short_commit_name(commit), subject_len, subject);
unuse_commit_buffer(commit, commit_buffer);
}
return 0;
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 10/25] sequencer: get rid of the subcommand field
From: Johannes Schindelin @ 2016-09-11 10:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
The subcommands are used exactly once, at the very beginning of
sequencer_pick_revisions(), to determine what to do. This is an
unnecessary level of indirection: we can simply call the correct
function to begin with. So let's do that.
While at it, ensure that the subcommands return an error code so that
they do not have to die() all over the place (bad practice for library
functions...).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/revert.c | 36 ++++++++++++++++--------------------
sequencer.c | 35 +++++++++++------------------------
sequencer.h | 13 ++++---------
3 files changed, 31 insertions(+), 53 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 7365559..c9ae4dc 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
}
-static void parse_args(int argc, const char **argv, struct replay_opts *opts)
+static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
{
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
const char *me = action_name(opts);
@@ -115,25 +115,15 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
if (opts->keep_redundant_commits)
opts->allow_empty = 1;
- /* Set the subcommand */
- if (cmd == 'q')
- opts->subcommand = REPLAY_REMOVE_STATE;
- else if (cmd == 'c')
- opts->subcommand = REPLAY_CONTINUE;
- else if (cmd == 'a')
- opts->subcommand = REPLAY_ROLLBACK;
- else
- opts->subcommand = REPLAY_NONE;
-
/* Check for incompatible command line arguments */
- if (opts->subcommand != REPLAY_NONE) {
+ if (cmd) {
char *this_operation;
- if (opts->subcommand == REPLAY_REMOVE_STATE)
+ if (cmd == 'q')
this_operation = "--quit";
- else if (opts->subcommand == REPLAY_CONTINUE)
+ else if (cmd == 'c')
this_operation = "--continue";
else {
- assert(opts->subcommand == REPLAY_ROLLBACK);
+ assert(cmd == 'a');
this_operation = "--abort";
}
@@ -156,7 +146,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
"--edit", opts->edit,
NULL);
- if (opts->subcommand != REPLAY_NONE) {
+ if (cmd) {
opts->revs = NULL;
} else {
struct setup_revision_opt s_r_opt;
@@ -174,6 +164,14 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
if (argc > 1)
usage_with_options(usage_str, options);
+
+ if (cmd == 'q')
+ return sequencer_remove_state(opts);
+ if (cmd == 'c')
+ return sequencer_continue(opts);
+ if (cmd == 'a')
+ return sequencer_rollback(opts);
+ return sequencer_pick_revisions(opts);
}
int cmd_revert(int argc, const char **argv, const char *prefix)
@@ -185,8 +183,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
opts.edit = 1;
opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
- parse_args(argc, argv, &opts);
- res = sequencer_pick_revisions(&opts);
+ res = run_sequencer(argc, argv, &opts);
if (res < 0)
die(_("revert failed"));
return res;
@@ -199,8 +196,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
- parse_args(argc, argv, &opts);
- res = sequencer_pick_revisions(&opts);
+ res = run_sequencer(argc, argv, &opts);
if (res < 0)
die(_("cherry-pick failed"));
return res;
diff --git a/sequencer.c b/sequencer.c
index 7ba932b..053e78c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -127,7 +127,7 @@ void *sequencer_entrust(struct replay_opts *opts, void *to_free)
return to_free;
}
-static void remove_sequencer_state(const struct replay_opts *opts)
+int sequencer_remove_state(struct replay_opts *opts)
{
struct strbuf dir = STRBUF_INIT;
int i;
@@ -141,6 +141,8 @@ static void remove_sequencer_state(const struct replay_opts *opts)
strbuf_addf(&dir, "%s", get_dir(opts));
remove_dir_recursively(&dir, 0);
strbuf_release(&dir);
+
+ return 0;
}
static const char *action_name(const struct replay_opts *opts)
@@ -971,7 +973,7 @@ static int rollback_single_pick(void)
return reset_for_rollback(head_sha1);
}
-static int sequencer_rollback(struct replay_opts *opts)
+int sequencer_rollback(struct replay_opts *opts)
{
FILE *f;
unsigned char sha1[20];
@@ -1006,9 +1008,8 @@ static int sequencer_rollback(struct replay_opts *opts)
}
if (reset_for_rollback(sha1))
goto fail;
- remove_sequencer_state(opts);
strbuf_release(&buf);
- return 0;
+ return sequencer_remove_state(opts);
fail:
strbuf_release(&buf);
return -1;
@@ -1093,8 +1094,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
* Sequence of picks finished successfully; cleanup by
* removing the .git/sequencer directory
*/
- remove_sequencer_state(opts);
- return 0;
+ return sequencer_remove_state(opts);
}
static int continue_single_pick(void)
@@ -1107,11 +1107,14 @@ static int continue_single_pick(void)
return run_command_v_opt(argv, RUN_GIT_CMD);
}
-static int sequencer_continue(struct replay_opts *opts)
+int sequencer_continue(struct replay_opts *opts)
{
struct todo_list todo_list = TODO_LIST_INIT;
int res;
+ if (read_and_refresh_cache(opts))
+ return -1;
+
if (!file_exists(get_todo_path(opts)))
return continue_single_pick();
if (read_populate_opts(opts))
@@ -1150,26 +1153,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
unsigned char sha1[20];
int i, res;
- if (opts->subcommand == REPLAY_NONE)
- assert(opts->revs);
-
+ assert(opts->revs);
if (read_and_refresh_cache(opts))
return -1;
- /*
- * Decide what to do depending on the arguments; a fresh
- * cherry-pick should be handled differently from an existing
- * one that is being continued
- */
- if (opts->subcommand == REPLAY_REMOVE_STATE) {
- remove_sequencer_state(opts);
- return 0;
- }
- if (opts->subcommand == REPLAY_ROLLBACK)
- return sequencer_rollback(opts);
- if (opts->subcommand == REPLAY_CONTINUE)
- return sequencer_continue(opts);
-
for (i = 0; i < opts->revs->pending.nr; i++) {
unsigned char sha1[20];
const char *name = opts->revs->pending.objects[i].name;
diff --git a/sequencer.h b/sequencer.h
index 04892a9..0b3950d 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -10,16 +10,8 @@ enum replay_action {
REPLAY_PICK
};
-enum replay_subcommand {
- REPLAY_NONE,
- REPLAY_REMOVE_STATE,
- REPLAY_CONTINUE,
- REPLAY_ROLLBACK
-};
-
struct replay_opts {
enum replay_action action;
- enum replay_subcommand subcommand;
/* Boolean options */
int edit;
@@ -48,7 +40,7 @@ struct replay_opts {
void **owned;
int owned_nr, owned_alloc;
};
-#define REPLAY_OPTS_INIT { -1, -1 }
+#define REPLAY_OPTS_INIT { -1 }
/*
* Make it the duty of sequencer_remove_state() to release the memory;
@@ -57,6 +49,9 @@ struct replay_opts {
void *sequencer_entrust(struct replay_opts *opts, void *to_free);
int sequencer_pick_revisions(struct replay_opts *opts);
+int sequencer_continue(struct replay_opts *opts);
+int sequencer_rollback(struct replay_opts *opts);
+int sequencer_remove_state(struct replay_opts *opts);
extern const char sign_off_header[];
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 09/25] sequencer: avoid completely different messages for different actions
From: Johannes Schindelin @ 2016-09-11 10:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index b971ad0..7ba932b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -232,11 +232,8 @@ static int error_dirty_index(struct replay_opts *opts)
if (read_cache_unmerged())
return error_resolve_conflict(action_name(opts));
- /* Different translation strings for cherry-pick and revert */
- if (opts->action == REPLAY_PICK)
- error(_("Your local changes would be overwritten by cherry-pick."));
- else
- error(_("Your local changes would be overwritten by revert."));
+ error(_("Your local changes would be overwritten by %s."),
+ action_name(opts));
if (advice_commit_before_merge)
advise(_("Commit your changes or stash them to proceed."));
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 08/25] sequencer: completely revamp the "todo" script parsing
From: Johannes Schindelin @ 2016-09-11 10:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
When we came up with the "sequencer" idea, we really wanted to have
kind of a plumbing equivalent of the interactive rebase. Hence the
choice of words: the "todo" script, a "pick", etc.
However, when it came time to implement the entire shebang, somehow this
idea got lost and the sequencer was used as working horse for
cherry-pick and revert instead. So as not to interfere with the
interactive rebase, it even uses a separate directory to store its
state.
Furthermore, it also is stupidly strict about the "todo" script it
accepts: while it parses commands in a way that was *designed* to be
similar to the interactive rebase, it then goes on to *error out* if the
commands disagree with the overall action (cherry-pick or revert).
Finally, the sequencer code chose to deviate from the interactive rebase
code insofar that it *reformats* the "todo" script instead of just
writing the part of the parsed script that were not yet processed. This
is not only unnecessary churn, but might well lose information that is
valuable to the user (i.e. comments after the commands).
Let's just bite the bullet and rewrite the entire parser; the code now
becomes not only more elegant: it allows us to go on and teach the
sequencer how to parse *true* "todo" scripts as used by the interactive
rebase itself. In a way, the sequencer is about to grow up to do its
older brother's job. Better.
In particular, we choose to maintain the list of commands in an array
instead of a linked list: this is flexible enough to allow us later on to
even implement rebase -i's reordering of fixup!/squash! commits very
easily (and with a very nice speed bonus, at least on Windows).
While at it, do not stop at the first problem, but list *all* of the
problems. This will help the user when the sequencer will do `rebase
-i`'s work by allowing to address all issues in one go rather than going
back and forth until the todo list is valid.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 275 +++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 159 insertions(+), 116 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index da6de22..b971ad0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -473,7 +473,26 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
return 1;
}
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+enum todo_command {
+ TODO_PICK = 0,
+ TODO_REVERT
+};
+
+static const char *todo_command_strings[] = {
+ "pick",
+ "revert"
+};
+
+static const char *command_to_string(const enum todo_command command)
+{
+ if (command < ARRAY_SIZE(todo_command_strings))
+ return todo_command_strings[command];
+ die("Unknown command: %d", command);
+}
+
+
+static int do_pick_commit(enum todo_command command, struct commit *commit,
+ struct replay_opts *opts)
{
unsigned char head[20];
struct commit *base, *next, *parent;
@@ -535,7 +554,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
/* TRANSLATORS: The first %s will be "revert" or
"cherry-pick", the second %s a SHA1 */
return error(_("%s: cannot parse parent commit %s"),
- action_name(opts), oid_to_hex(&parent->object.oid));
+ command_to_string(command),
+ oid_to_hex(&parent->object.oid));
if (get_message(commit, &msg) != 0)
return error(_("Cannot get commit message for %s"),
@@ -548,7 +568,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
* reverse of it if we are revert.
*/
- if (opts->action == REPLAY_REVERT) {
+ if (command == TODO_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -589,7 +609,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
}
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REPLAY_REVERT) {
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command == TODO_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
if (res < 0)
@@ -615,17 +635,17 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
* However, if the merge did not even start, then we don't want to
* write it at all.
*/
- if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1) &&
+ if (command == TODO_PICK && !opts->no_commit && (res == 0 || res == 1) &&
update_ref(NULL, "CHERRY_PICK_HEAD", commit->object.oid.hash, NULL,
REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
res = -1;
- if (opts->action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1) &&
+ if (command == TODO_REVERT && ((opts->no_commit && res == 0) || res == 1) &&
update_ref(NULL, "REVERT_HEAD", commit->object.oid.hash, NULL,
REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
res = -1;
if (res) {
- error(opts->action == REPLAY_REVERT
+ error(command == TODO_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV),
@@ -687,116 +707,121 @@ static int read_and_refresh_cache(struct replay_opts *opts)
return 0;
}
-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
- struct replay_opts *opts)
+struct todo_item {
+ enum todo_command command;
+ struct commit *commit;
+ size_t offset_in_buf;
+};
+
+struct todo_list {
+ struct strbuf buf;
+ struct todo_item *items;
+ int nr, alloc, current;
+};
+
+#define TODO_LIST_INIT { STRBUF_INIT }
+
+static void todo_list_release(struct todo_list *todo_list)
{
- struct commit_list *cur = NULL;
- const char *sha1_abbrev = NULL;
- const char *action_str = opts->action == REPLAY_REVERT ? "revert" : "pick";
- const char *subject;
- int subject_len;
+ strbuf_release(&todo_list->buf);
+ free(todo_list->items);
+ todo_list->items = NULL;
+ todo_list->nr = todo_list->alloc = 0;
+}
- for (cur = todo_list; cur; cur = cur->next) {
- const char *commit_buffer = get_commit_buffer(cur->item, NULL);
- sha1_abbrev = find_unique_abbrev(cur->item->object.oid.hash, DEFAULT_ABBREV);
- subject_len = find_commit_subject(commit_buffer, &subject);
- strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
- subject_len, subject);
- unuse_commit_buffer(cur->item, commit_buffer);
- }
- return 0;
+struct todo_item *append_new_todo(struct todo_list *todo_list)
+{
+ ALLOC_GROW(todo_list->items, todo_list->nr + 1, todo_list->alloc);
+ return todo_list->items + todo_list->nr++;
}
-static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
+static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
{
unsigned char commit_sha1[20];
- enum replay_action action;
char *end_of_object_name;
- int saved, status, padding;
-
- if (starts_with(bol, "pick")) {
- action = REPLAY_PICK;
- bol += strlen("pick");
- } else if (starts_with(bol, "revert")) {
- action = REPLAY_REVERT;
- bol += strlen("revert");
- } else
- return NULL;
+ int i, saved, status, padding;
+
+ for (i = 0; i < ARRAY_SIZE(todo_command_strings); i++)
+ if (skip_prefix(bol, todo_command_strings[i], &bol)) {
+ item->command = i;
+ break;
+ }
+ if (i >= ARRAY_SIZE(todo_command_strings))
+ return -1;
/* Eat up extra spaces/ tabs before object name */
padding = strspn(bol, " \t");
if (!padding)
- return NULL;
+ return -1;
bol += padding;
- end_of_object_name = bol + strcspn(bol, " \t\n");
+ end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
saved = *end_of_object_name;
*end_of_object_name = '\0';
status = get_sha1(bol, commit_sha1);
*end_of_object_name = saved;
- /*
- * Verify that the action matches up with the one in
- * opts; we don't support arbitrary instructions
- */
- if (action != opts->action) {
- if (action == REPLAY_REVERT)
- error((opts->action == REPLAY_REVERT)
- ? _("Cannot revert during another revert.")
- : _("Cannot revert during a cherry-pick."));
- else
- error((opts->action == REPLAY_REVERT)
- ? _("Cannot cherry-pick during a revert.")
- : _("Cannot cherry-pick during another cherry-pick."));
- return NULL;
- }
-
if (status < 0)
- return NULL;
+ return -1;
- return lookup_commit_reference(commit_sha1);
+ item->commit = lookup_commit_reference(commit_sha1);
+ return !item->commit;
}
-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
- struct replay_opts *opts)
+static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
{
- struct commit_list **next = todo_list;
- struct commit *commit;
+ struct todo_item *item;
char *p = buf;
- int i;
+ int i, res = 0;
for (i = 1; *p; i++) {
char *eol = strchrnul(p, '\n');
- commit = parse_insn_line(p, eol, opts);
- if (!commit)
- return error(_("Could not parse line %d."), i);
- next = commit_list_append(commit, next);
+
+ item = append_new_todo(todo_list);
+ item->offset_in_buf = p - todo_list->buf.buf;
+ if (parse_insn_line(item, p, eol)) {
+ res |= error(_("Invalid line %d: %.*s"),
+ i, (int)(eol - p), p);
+ item->command = -1;
+ }
p = *eol ? eol + 1 : eol;
}
- if (!*todo_list)
+ if (!todo_list->nr)
return error(_("No commits parsed."));
- return 0;
+ return res;
}
-static int read_populate_todo(struct commit_list **todo_list,
+static int read_populate_todo(struct todo_list *todo_list,
struct replay_opts *opts)
{
const char *todo_file = get_todo_path(opts);
- struct strbuf buf = STRBUF_INIT;
int fd, res;
+ strbuf_reset(&todo_list->buf);
fd = open(todo_file, O_RDONLY);
if (fd < 0)
return error_errno(_("Could not open %s"), todo_file);
- if (strbuf_read(&buf, fd, 0) < 0) {
+ if (strbuf_read(&todo_list->buf, fd, 0) < 0) {
close(fd);
- strbuf_release(&buf);
return error(_("Could not read %s."), todo_file);
}
close(fd);
- res = parse_insn_buffer(buf.buf, todo_list, opts);
- strbuf_release(&buf);
+ res = parse_insn_buffer(todo_list->buf.buf, todo_list);
+ if (!res) {
+ enum todo_command valid =
+ opts->action == REPLAY_PICK ? TODO_PICK : TODO_REVERT;
+ int i;
+
+ for (i = 0; i < todo_list->nr; i++)
+ if (valid == todo_list->items[i].command)
+ continue;
+ else if (valid == TODO_PICK)
+ return error(_("Cannot cherry-pick during a revert."));
+ else
+ return error(_("Cannot revert during a cherry-pick."));
+ }
+
if (res)
return error(_("Unusable instruction sheet: %s"), todo_file);
return 0;
@@ -858,18 +883,33 @@ static int read_populate_opts(struct replay_opts *opts)
return 0;
}
-static int walk_revs_populate_todo(struct commit_list **todo_list,
+static int walk_revs_populate_todo(struct todo_list *todo_list,
struct replay_opts *opts)
{
+ enum todo_command command = opts->action == REPLAY_PICK ?
+ TODO_PICK : TODO_REVERT;
+ const char *command_string = todo_command_strings[command];
struct commit *commit;
- struct commit_list **next;
if (prepare_revs(opts))
return -1;
- next = todo_list;
- while ((commit = get_revision(opts->revs)))
- next = commit_list_append(commit, next);
+ while ((commit = get_revision(opts->revs))) {
+ struct todo_item *item = append_new_todo(todo_list);
+ const char *commit_buffer = get_commit_buffer(commit, NULL);
+ const char *subject;
+ int subject_len;
+
+ item->command = command;
+ item->commit = commit;
+ item->offset_in_buf = todo_list->buf.len;
+ subject_len = find_commit_subject(commit_buffer, &subject);
+ strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
+ find_unique_abbrev(commit->object.oid.hash,
+ DEFAULT_ABBREV),
+ subject_len, subject);
+ unuse_commit_buffer(commit, commit_buffer);
+ }
return 0;
}
@@ -977,30 +1017,22 @@ static int sequencer_rollback(struct replay_opts *opts)
return -1;
}
-static int save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
{
static struct lock_file todo_lock;
- struct strbuf buf = STRBUF_INIT;
- int fd;
+ const char *todo_path = get_todo_path(opts);
+ int next = todo_list->current, offset, fd;
- fd = hold_lock_file_for_update(&todo_lock, git_path_todo_file(), 0);
+ fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
if (fd < 0)
- return error_errno(_("Could not lock '%s'"),
- git_path_todo_file());
- if (format_todo(&buf, todo_list, opts) < 0) {
- strbuf_release(&buf);
- return error(_("Could not format %s."), git_path_todo_file());
- }
- if (write_in_full(fd, buf.buf, buf.len) < 0) {
- strbuf_release(&buf);
- return error_errno(_("Could not write to %s"),
- git_path_todo_file());
- }
- if (commit_lock_file(&todo_lock) < 0) {
- strbuf_release(&buf);
- return error(_("Error wrapping up %s."), git_path_todo_file());
- }
- strbuf_release(&buf);
+ return error_errno(_("Could not lock '%s'"), todo_path);
+ offset = next < todo_list->nr ?
+ todo_list->items[next].offset_in_buf : todo_list->buf.len;
+ if (write_in_full(fd, todo_list->buf.buf + offset,
+ todo_list->buf.len - offset) < 0)
+ return error_errno(_("Could not write to '%s'"), todo_path);
+ if (commit_lock_file(&todo_lock) < 0)
+ return error(_("Error wrapping up %s."), todo_path);
return 0;
}
@@ -1039,9 +1071,8 @@ static int save_opts(struct replay_opts *opts)
return res;
}
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
+static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
{
- struct commit_list *cur;
int res;
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
@@ -1051,10 +1082,12 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
if (read_and_refresh_cache(opts))
return -1;
- for (cur = todo_list; cur; cur = cur->next) {
- if (save_todo(cur, opts))
+ while (todo_list->current < todo_list->nr) {
+ struct todo_item *item = todo_list->items + todo_list->current;
+ if (save_todo(todo_list, opts))
return -1;
- res = do_pick_commit(cur->item, opts);
+ res = do_pick_commit(item->command, item->commit, opts);
+ todo_list->current++;
if (res)
return res;
}
@@ -1079,38 +1112,46 @@ static int continue_single_pick(void)
static int sequencer_continue(struct replay_opts *opts)
{
- struct commit_list *todo_list = NULL;
+ struct todo_list todo_list = TODO_LIST_INIT;
+ int res;
if (!file_exists(get_todo_path(opts)))
return continue_single_pick();
- if (read_populate_opts(opts) ||
- read_populate_todo(&todo_list, opts))
+ if (read_populate_opts(opts))
return -1;
+ if ((res = read_populate_todo(&todo_list, opts)))
+ goto release_todo_list;
/* Verify that the conflict has been resolved */
if (file_exists(git_path_cherry_pick_head()) ||
file_exists(git_path_revert_head())) {
- int ret = continue_single_pick();
- if (ret)
- return ret;
+ res = continue_single_pick();
+ if (res)
+ goto release_todo_list;
}
- if (index_differs_from("HEAD", 0))
- return error_dirty_index(opts);
- todo_list = todo_list->next;
- return pick_commits(todo_list, opts);
+ if (index_differs_from("HEAD", 0)) {
+ res = error_dirty_index(opts);
+ goto release_todo_list;
+ }
+ todo_list.current++;
+ res = pick_commits(&todo_list, opts);
+release_todo_list:
+ todo_list_release(&todo_list);
+ return res;
}
static int single_pick(struct commit *cmit, struct replay_opts *opts)
{
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
- return do_pick_commit(cmit, opts);
+ return do_pick_commit(opts->action == REPLAY_PICK ?
+ TODO_PICK : TODO_REVERT, cmit, opts);
}
int sequencer_pick_revisions(struct replay_opts *opts)
{
- struct commit_list *todo_list = NULL;
+ struct todo_list todo_list = TODO_LIST_INIT;
unsigned char sha1[20];
- int i;
+ int i, res;
if (opts->subcommand == REPLAY_NONE)
assert(opts->revs);
@@ -1185,7 +1226,9 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return -1;
if (save_opts(opts))
return -1;
- return pick_commits(todo_list, opts);
+ res = pick_commits(&todo_list, opts);
+ todo_list_release(&todo_list);
+ return res;
}
void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 07/25] sequencer: future-proof read_populate_todo()
From: Johannes Schindelin @ 2016-09-11 10:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
Over the next commits, we will work on improving the sequencer to the
point where it can process the edit script of an interactive rebase. To
that end, we will need to teach the sequencer to read interactive
rebase's todo file. In preparation, we consolidate all places where
that todo file is needed to call a function that we will later extend.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 3ca231f..da6de22 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -32,6 +32,11 @@ static const char *get_dir(const struct replay_opts *opts)
return git_path_seq_dir();
}
+static const char *get_todo_path(const struct replay_opts *opts)
+{
+ return git_path_todo_file();
+}
+
static int is_rfc2822_line(const char *buf, int len)
{
int i;
@@ -776,25 +781,24 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
static int read_populate_todo(struct commit_list **todo_list,
struct replay_opts *opts)
{
+ const char *todo_file = get_todo_path(opts);
struct strbuf buf = STRBUF_INIT;
int fd, res;
- fd = open(git_path_todo_file(), O_RDONLY);
+ fd = open(todo_file, O_RDONLY);
if (fd < 0)
- return error_errno(_("Could not open %s"),
- git_path_todo_file());
+ return error_errno(_("Could not open %s"), todo_file);
if (strbuf_read(&buf, fd, 0) < 0) {
close(fd);
strbuf_release(&buf);
- return error(_("Could not read %s."), git_path_todo_file());
+ return error(_("Could not read %s."), todo_file);
}
close(fd);
res = parse_insn_buffer(buf.buf, todo_list, opts);
strbuf_release(&buf);
if (res)
- return error(_("Unusable instruction sheet: %s"),
- git_path_todo_file());
+ return error(_("Unusable instruction sheet: %s"), todo_file);
return 0;
}
@@ -1077,7 +1081,7 @@ static int sequencer_continue(struct replay_opts *opts)
{
struct commit_list *todo_list = NULL;
- if (!file_exists(git_path_todo_file()))
+ if (!file_exists(get_todo_path(opts)))
return continue_single_pick();
if (read_populate_opts(opts) ||
read_populate_todo(&todo_list, opts))
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 06/25] sequencer: release memory that was allocated when reading options
From: Johannes Schindelin @ 2016-09-11 10:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
The sequencer reads options from disk and stores them in its struct
for use during sequencer's operations.
With this patch, the memory is released afterwards, plugging a
memory leak.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 8d56a05..3ca231f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -131,6 +131,8 @@ static void remove_sequencer_state(const struct replay_opts *opts)
free(opts->owned[i]);
free(opts->owned);
+ free(opts->xopts);
+
strbuf_addf(&dir, "%s", get_dir(opts));
remove_dir_recursively(&dir, 0);
strbuf_release(&dir);
@@ -815,13 +817,18 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
else if (!strcmp(key, "options.mainline"))
opts->mainline = git_config_int(key, value);
- else if (!strcmp(key, "options.strategy"))
+ else if (!strcmp(key, "options.strategy")) {
git_config_string(&opts->strategy, key, value);
- else if (!strcmp(key, "options.gpg-sign"))
+ sequencer_entrust(opts, (char *) opts->strategy);
+ }
+ else if (!strcmp(key, "options.gpg-sign")) {
git_config_string(&opts->gpg_sign, key, value);
+ sequencer_entrust(opts, (char *) opts->gpg_sign);
+ }
else if (!strcmp(key, "options.strategy-option")) {
ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
- opts->xopts[opts->xopts_nr++] = xstrdup(value);
+ opts->xopts[opts->xopts_nr++] =
+ sequencer_entrust(opts, xstrdup(value));
} else
return error(_("Invalid key: %s"), key);
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 04/25] sequencer: future-proof remove_sequencer_state()
From: Johannes Schindelin @ 2016-09-11 10:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
In a couple of commits, we will teach the sequencer to handle the
nitty gritty of the interactive rebase, which keeps its state in a
different directory.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index c2fbf6f..8d272fb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -27,6 +27,11 @@ static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
+static const char *get_dir(const struct replay_opts *opts)
+{
+ return git_path_seq_dir();
+}
+
static int is_rfc2822_line(const char *buf, int len)
{
int i;
@@ -109,13 +114,13 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
return 1;
}
-static void remove_sequencer_state(void)
+static void remove_sequencer_state(const struct replay_opts *opts)
{
- struct strbuf seq_dir = STRBUF_INIT;
+ struct strbuf dir = STRBUF_INIT;
- strbuf_addstr(&seq_dir, git_path_seq_dir());
- remove_dir_recursively(&seq_dir, 0);
- strbuf_release(&seq_dir);
+ strbuf_addf(&dir, "%s", get_dir(opts));
+ remove_dir_recursively(&dir, 0);
+ strbuf_release(&dir);
}
static const char *action_name(const struct replay_opts *opts)
@@ -940,7 +945,7 @@ static int sequencer_rollback(struct replay_opts *opts)
}
if (reset_for_rollback(sha1))
goto fail;
- remove_sequencer_state();
+ remove_sequencer_state(opts);
strbuf_release(&buf);
return 0;
fail:
@@ -1034,7 +1039,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
* Sequence of picks finished successfully; cleanup by
* removing the .git/sequencer directory
*/
- remove_sequencer_state();
+ remove_sequencer_state(opts);
return 0;
}
@@ -1095,7 +1100,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
* one that is being continued
*/
if (opts->subcommand == REPLAY_REMOVE_STATE) {
- remove_sequencer_state();
+ remove_sequencer_state(opts);
return 0;
}
if (opts->subcommand == REPLAY_ROLLBACK)
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 05/25] sequencer: allow the sequencer to take custody of malloc()ed data
From: Johannes Schindelin @ 2016-09-11 10:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
The sequencer is our attempt to lib-ify cherry-pick. Yet it behaves
like a one-shot command when it reads its configuration: memory is
allocated and released only when the command exits.
This is kind of okay for git-cherry-pick, which *is* a one-shot
command. All the work to make the sequencer its work horse was
done to allow using the functionality as a library function, though,
including proper clean-up after use.
This patch introduces an API to pass the responsibility of releasing
certain memory to the sequencer. Example:
const char *label =
sequencer_entrust(opts, xstrfmt("From: %s", email));
The entrusted memory will remain valid until sequencer_remove_state() is
called, or the program exits, whichever comes first.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 13 +++++++++++++
sequencer.h | 10 ++++++++++
2 files changed, 23 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index 8d272fb..8d56a05 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -114,9 +114,22 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
return 1;
}
+void *sequencer_entrust(struct replay_opts *opts, void *to_free)
+{
+ ALLOC_GROW(opts->owned, opts->owned_nr + 1, opts->owned_alloc);
+ opts->owned[opts->owned_nr++] = to_free;
+
+ return to_free;
+}
+
static void remove_sequencer_state(const struct replay_opts *opts)
{
struct strbuf dir = STRBUF_INIT;
+ int i;
+
+ for (i = 0; i < opts->owned_nr; i++)
+ free(opts->owned[i]);
+ free(opts->owned);
strbuf_addf(&dir, "%s", get_dir(opts));
remove_dir_recursively(&dir, 0);
diff --git a/sequencer.h b/sequencer.h
index dd4d33a..04892a9 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -43,9 +43,19 @@ struct replay_opts {
/* Only used by REPLAY_NONE */
struct rev_info *revs;
+
+ /* malloc()ed data entrusted to the sequencer */
+ void **owned;
+ int owned_nr, owned_alloc;
};
#define REPLAY_OPTS_INIT { -1, -1 }
+/*
+ * Make it the duty of sequencer_remove_state() to release the memory;
+ * For ease of use, return the same pointer.
+ */
+void *sequencer_entrust(struct replay_opts *opts, void *to_free);
+
int sequencer_pick_revisions(struct replay_opts *opts);
extern const char sign_off_header[];
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 03/25] sequencer: avoid unnecessary indirection
From: Johannes Schindelin @ 2016-09-11 10:52 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
We really do not need the *pointer to a* pointer to the options in
the read_populate_opts() function.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index cb16cbd..c2fbf6f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -813,7 +813,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
return 0;
}
-static int read_populate_opts(struct replay_opts **opts)
+static int read_populate_opts(struct replay_opts *opts)
{
if (!file_exists(git_path_opts_file()))
return 0;
@@ -823,7 +823,7 @@ static int read_populate_opts(struct replay_opts **opts)
* about this case, though, because we wrote that file ourselves, so we
* are pretty certain that it is syntactically correct.
*/
- if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) < 0)
+ if (git_config_from_file(populate_opts_cb, git_path_opts_file(), opts) < 0)
return error(_("Malformed options sheet: %s"),
git_path_opts_file());
return 0;
@@ -1054,7 +1054,7 @@ static int sequencer_continue(struct replay_opts *opts)
if (!file_exists(git_path_todo_file()))
return continue_single_pick();
- if (read_populate_opts(&opts) ||
+ if (read_populate_opts(opts) ||
read_populate_todo(&todo_list, opts))
return -1;
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 02/25] sequencer: use memoized sequencer directory path
From: Johannes Schindelin @ 2016-09-11 10:52 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/commit.c | 2 +-
sequencer.c | 11 ++++++-----
sequencer.h | 5 +----
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index bb9f79b..e79af9d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -183,7 +183,7 @@ static void determine_whence(struct wt_status *s)
whence = FROM_MERGE;
else if (file_exists(git_path_cherry_pick_head())) {
whence = FROM_CHERRY_PICK;
- if (file_exists(git_path(SEQ_DIR)))
+ if (file_exists(git_path_seq_dir()))
sequencer_in_use = 1;
}
else
diff --git a/sequencer.c b/sequencer.c
index eec8a60..cb16cbd 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -21,10 +21,11 @@
const char sign_off_header[] = "Signed-off-by: ";
static const char cherry_picked_prefix[] = "(cherry picked from commit ";
-static GIT_PATH_FUNC(git_path_todo_file, SEQ_TODO_FILE)
-static GIT_PATH_FUNC(git_path_opts_file, SEQ_OPTS_FILE)
-static GIT_PATH_FUNC(git_path_seq_dir, SEQ_DIR)
-static GIT_PATH_FUNC(git_path_head_file, SEQ_HEAD_FILE)
+GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
+
+static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
+static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
+static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
static int is_rfc2822_line(const char *buf, int len)
{
@@ -112,7 +113,7 @@ static void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
- strbuf_addstr(&seq_dir, git_path(SEQ_DIR));
+ strbuf_addstr(&seq_dir, git_path_seq_dir());
remove_dir_recursively(&seq_dir, 0);
strbuf_release(&seq_dir);
}
diff --git a/sequencer.h b/sequencer.h
index db425ad..dd4d33a 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -1,10 +1,7 @@
#ifndef SEQUENCER_H
#define SEQUENCER_H
-#define SEQ_DIR "sequencer"
-#define SEQ_HEAD_FILE "sequencer/head"
-#define SEQ_TODO_FILE "sequencer/todo"
-#define SEQ_OPTS_FILE "sequencer/opts"
+const char *git_path_seq_dir(void);
#define APPEND_SIGNOFF_DEDUP (1u << 0)
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 00/25] Prepare the sequencer for the upcoming rebase -i patches
From: Johannes Schindelin @ 2016-09-11 10:52 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1472457609.git.johannes.schindelin@gmx.de>
This patch series marks the '4' in the countdown to speed up rebase -i
by implementing large parts in C. It is based on the `libify-sequencer`
patch series that I submitted last week.
The patches in this series merely prepare the sequencer code for the
next patch series that actually teaches the sequencer to run an
interactive rebase.
The reason to split these two patch series is simple: to keep them at a
sensible size.
The two patch series after that are much smaller: a two-patch "series"
that switches rebase -i to use the sequencer (except with --root or
--preserve-merges), and a couple of patches to move several pretty
expensive script processing steps to C (think: autosquash).
The end game of this patch series is a git-rebase--helper that makes
rebase -i 5x faster on Windows (according to t/perf/p3404). Travis says
that even MacOSX and Linux benefit (4x and 3x, respectively).
I have been working on this since early February, whenever time allowed,
and it is time to put it into the users' hands. To that end, I will most
likely submit the remaining three patch series in the next two days, and
integrate the whole shebang into Git for Windows 2.10.0.
Therefore I would be most grateful for every in-depth review.
Changes vs v1:
- clarified why the code refactoring into the short_commit_name()
function is desirable.
- fixed a typo in a commit message (s/se/so/).
- removed a bogus call to read_and_refresh_cache(opts) that was added
to sequencer_rollback() by mistake.
- clarified in the commit message why the TODO_LIST_INIT macro does not
assign all-zeroes.
- converted IS_REBASE_I() into an inline function.
- clarified the comment about retaining author metadata when calling
sequencer_commit() in rebase -i mode.
- simplified TODO_LIST_INIT and REPLAY_OPTS_INIT.
- added an example how to use sequencer_entrust() to the commit message
introducing it.
- the gpg_sign option's value is now also entrusted to the sequencer
(i.e. it is released in sequencer_remove_state()).
- renamed the "set_me_free_after_use" to "to_free". Since that is still
not a self-explanatory name, add a comment to the function
declaration.
- clarified in the "completely revamped todo parsing" commit message
that the main benefit of the early parsing is for rebase -i.
- start the `enum todo_command` with TODO_PICK that is explicitly
assigned the value 0.
- an error was marked as translatable.
- converted one forgotten git_path_todo_file() to use
get_todo_path(opts) instead.
- fixed numbering of "malformed instruction sheet"s after removing one.
- reintroduced the overzealous assumption that todo scripts can only
perform revert commands during `git revert` and pick commands during
`git cherry-pick`. This overzealous assumption is *still* disabled in
rebase -i mode, of course.
- clarified in the commit message why we call the field "arg", not
"oneline", and fixed the description that claimed that we store the
end offset (we store the length instead).
- clarified in the commit message of "allow editing the commit message
on a case-by-case basis" that we are talking about the
sequencer_commit() function, and how things were done previously.
- some grammar touch-ups.
- marked a couple of error messages for translation.
- explicitly assign the first todo_command the value 0, so that we can
be certain that the array of command strings matches up.
- removed code to skip CR/LF at the end of line after reading with
read_oneliner(): That function already ensures that.
- ensured that the todo_list is released even when reading/parsing fails.
- replaced an error(..., strerror()) call with an error_errno(...) one.
- clarified in the commit message why the new todo_list maintains the
script as an array instead of a linked list.
- renamed the append_todo() function into append_new_todo(), to explain
better what the function does and why it does not take the values as
parameters that should populate the new todo_item.
- marked action_name() for translation.
- surrounded all file names in error messages in sequencer.c with single
quotes, for consistency.
- removed a bogus hint for translators that asked them to translate
commands of the git-rebase-todo file (or cherry-pick's equivalent).
- turned capitals after semicolons to lower-case.
Johannes Schindelin (25):
sequencer: use static initializers for replay_opts
sequencer: use memoized sequencer directory path
sequencer: avoid unnecessary indirection
sequencer: future-proof remove_sequencer_state()
sequencer: allow the sequencer to take custody of malloc()ed data
sequencer: release memory that was allocated when reading options
sequencer: future-proof read_populate_todo()
sequencer: completely revamp the "todo" script parsing
sequencer: avoid completely different messages for different actions
sequencer: get rid of the subcommand field
sequencer: refactor the code to obtain a short commit name
sequencer: remember the onelines when parsing the todo file
sequencer: prepare for rebase -i's commit functionality
sequencer: introduce a helper to read files written by scripts
sequencer: prepare for rebase -i's GPG settings
sequencer: allow editing the commit message on a case-by-case basis
sequencer: support amending commits
sequencer: support cleaning up commit messages
sequencer: remember do_recursive_merge()'s return value
sequencer: left-trim lines read from the script
sequencer: refactor write_message()
sequencer: remove overzealous assumption in rebase -i mode
sequencer: mark action_name() for translation
sequencer: quote filenames in error messages
sequencer: remove bogus hint for translators
builtin/commit.c | 2 +-
builtin/revert.c | 42 ++--
sequencer.c | 632 +++++++++++++++++++++++++++++++++++++------------------
sequencer.h | 31 +--
4 files changed, 468 insertions(+), 239 deletions(-)
Based-On: libify-sequencer at https://github.com/dscho/git
Fetch-Base-Via: git fetch https://github.com/dscho/git libify-sequencer
Published-As: https://github.com/dscho/git/releases/tag/prepare-sequencer-v2
Fetch-It-Via: git fetch https://github.com/dscho/git prepare-sequencer-v2
Interdiff vs v1:
diff --git a/sequencer.c b/sequencer.c
index 7d56676..cdff0f1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -41,7 +41,10 @@ static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
/* We will introduce the 'interactive rebase' mode later */
-#define IS_REBASE_I() 0
+static inline int is_rebase_i(const struct replay_opts *opts)
+{
+ return 0;
+}
static const char *get_dir(const struct replay_opts *opts)
{
@@ -145,12 +148,12 @@ static const char *gpg_sign_opt_quoted(struct replay_opts *opts)
return buf.buf;
}
-void *sequencer_entrust(struct replay_opts *opts, void *set_me_free_after_use)
+void *sequencer_entrust(struct replay_opts *opts, void *to_free)
{
ALLOC_GROW(opts->owned, opts->owned_nr + 1, opts->owned_alloc);
- opts->owned[opts->owned_nr++] = set_me_free_after_use;
+ opts->owned[opts->owned_nr++] = to_free;
- return set_me_free_after_use;
+ return to_free;
}
int sequencer_remove_state(struct replay_opts *opts)
@@ -173,7 +176,7 @@ int sequencer_remove_state(struct replay_opts *opts)
static const char *action_name(const struct replay_opts *opts)
{
- return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
+ return opts->action == REPLAY_REVERT ? N_("revert") : N_("cherry-pick");
}
struct commit_message {
@@ -248,11 +251,11 @@ static int write_with_lock_file(const char *filename,
if (msg_fd < 0)
return error_errno(_("Could not lock '%s'"), filename);
if (write_in_full(msg_fd, buf, len) < 0)
- return error_errno(_("Could not write to %s"), filename);
+ return error_errno(_("Could not write to '%s'"), filename);
if (append_eol && write(msg_fd, "\n", 1) < 0)
- return error_errno(_("Could not write eol to %s"), filename);
+ return error_errno(_("Could not write eol to '%s"), filename);
if (commit_lock_file(&msg_file) < 0)
- return error(_("Error wrapping up %s."), filename);
+ return error(_("Error wrapping up '%s'."), filename);
return 0;
}
@@ -309,10 +312,10 @@ static struct tree *empty_tree(void)
static int error_dirty_index(struct replay_opts *opts)
{
if (read_cache_unmerged())
- return error_resolve_conflict(action_name(opts));
+ return error_resolve_conflict(_(action_name(opts)));
error(_("Your local changes would be overwritten by %s."),
- action_name(opts));
+ _(action_name(opts)));
if (advice_commit_before_merge)
advise(_("Commit your changes or stash them to proceed."));
@@ -330,7 +333,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
if (checkout_fast_forward(from, to, 1))
return -1; /* the callee should have complained already */
- strbuf_addf(&sb, _("%s: fast-forward"), action_name(opts));
+ strbuf_addf(&sb, _("%s: fast-forward"), _(action_name(opts)));
transaction = ref_transaction_begin(&err);
if (!transaction ||
@@ -406,7 +409,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
/* TRANSLATORS: %s will be "revert" or "cherry-pick" */
return error(_("%s: Unable to write new index file"),
- action_name(opts));
+ _(action_name(opts)));
rollback_lock_file(&index_lock);
if (opts->signoff)
@@ -488,9 +491,13 @@ static char **read_author_script(void)
* If we are cherry-pick, and if the merge did not result in
* hand-editing, we will hit this commit and inherit the original
* author date and name.
+ *
* If we are revert, or if our cherry-pick results in a hand merge,
- * we had better say that the current user is responsible for that
- * (except, of course, while running an interactive rebase).
+ * we had better say that the current user is responsible for that.
+ *
+ * An exception is when sequencer_commit() is called during an
+ * interactive rebase: in that case, we will want to retain the
+ * author metadata.
*/
int sequencer_commit(const char *defmsg, struct replay_opts *opts,
int allow_empty, int edit, int amend,
@@ -501,7 +508,7 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
int rc;
const char *value;
- if (IS_REBASE_I()) {
+ if (is_rebase_i(opts)) {
env = read_author_script();
if (!env) {
const char *gpg_opt = gpg_sign_opt_quoted(opts);
@@ -513,7 +520,7 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
"If they are meant to go into a new commit, "
"run:\n\n"
" git commit %s\n\n"
- "In both case, once you're done, continue "
+ "In both cases, once you're done, continue "
"with:\n\n"
" git rebase --continue\n", gpg_opt, gpg_opt);
}
@@ -613,7 +620,7 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
}
enum todo_command {
- TODO_PICK,
+ TODO_PICK = 0,
TODO_REVERT
};
@@ -690,8 +697,6 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
return fast_forward_to(commit->object.oid.hash, head, unborn, opts);
if (parent && parse_commit(parent) < 0)
- /* TRANSLATORS: The first %s will be "revert" or
- "cherry-pick", the second %s a SHA1 */
return error(_("%s: cannot parse parent commit %s"),
command_to_string(command),
oid_to_hex(&parent->object.oid));
@@ -833,14 +838,14 @@ static int read_and_refresh_cache(struct replay_opts *opts)
if (read_index_preload(&the_index, NULL) < 0) {
rollback_lock_file(&index_lock);
return error(_("git %s: failed to read the index"),
- action_name(opts));
+ _(action_name(opts)));
}
refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
if (the_index.cache_changed && index_fd >= 0) {
if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) {
rollback_lock_file(&index_lock);
return error(_("git %s: failed to refresh the index"),
- action_name(opts));
+ _(action_name(opts)));
}
}
rollback_lock_file(&index_lock);
@@ -861,7 +866,7 @@ struct todo_list {
int nr, alloc, current;
};
-#define TODO_LIST_INIT { STRBUF_INIT, NULL, 0, 0, 0 }
+#define TODO_LIST_INIT { STRBUF_INIT }
static void todo_list_release(struct todo_list *todo_list)
{
@@ -871,7 +876,7 @@ static void todo_list_release(struct todo_list *todo_list)
todo_list->nr = todo_list->alloc = 0;
}
-struct todo_item *append_todo(struct todo_list *todo_list)
+struct todo_item *append_new_todo(struct todo_list *todo_list)
{
ALLOC_GROW(todo_list->items, todo_list->nr + 1, todo_list->alloc);
return todo_list->items + todo_list->nr++;
@@ -925,11 +930,11 @@ static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
for (i = 1; *p; i++) {
char *eol = strchrnul(p, '\n');
- item = append_todo(todo_list);
+ item = append_new_todo(todo_list);
item->offset_in_buf = p - todo_list->buf.buf;
if (parse_insn_line(item, p, eol)) {
- error("Invalid line: %.*s", (int)(eol - p), p);
- res |= error(_("Could not parse line %d."), i);
+ res |= error(_("Invalid line %d: %.*s"),
+ i, (int)(eol - p), p);
item->command = -1;
}
p = *eol ? eol + 1 : eol;
@@ -948,16 +953,31 @@ static int read_populate_todo(struct todo_list *todo_list,
strbuf_reset(&todo_list->buf);
fd = open(todo_file, O_RDONLY);
if (fd < 0)
- return error_errno(_("Could not open %s"), todo_file);
+ return error_errno(_("Could not open '%s'"), todo_file);
if (strbuf_read(&todo_list->buf, fd, 0) < 0) {
close(fd);
- return error(_("Could not read %s."), todo_file);
+ return error(_("Could not read '%s'."), todo_file);
}
close(fd);
res = parse_insn_buffer(todo_list->buf.buf, todo_list);
if (res)
- return error(_("Unusable instruction sheet: %s"), todo_file);
+ return error(_("Unusable instruction sheet: '%s'"), todo_file);
+
+ if (!is_rebase_i(opts)) {
+ enum todo_command valid =
+ opts->action == REPLAY_PICK ? TODO_PICK : TODO_REVERT;
+ int i;
+
+ for (i = 0; i < todo_list->nr; i++)
+ if (valid == todo_list->items[i].command)
+ continue;
+ else if (valid == TODO_PICK)
+ return error(_("Cannot cherry-pick during a revert."));
+ else
+ return error(_("Cannot revert during a cherry-pick."));
+ }
+
return 0;
}
@@ -1003,22 +1023,16 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
static int read_populate_opts(struct replay_opts *opts)
{
- if (IS_REBASE_I()) {
+ if (is_rebase_i(opts)) {
struct strbuf buf = STRBUF_INIT;
if (read_oneliner(&buf, rebase_path_gpg_sign_opt(), 1)) {
- if (buf.len && buf.buf[buf.len - 1] == '\n') {
- if (--buf.len &&
- buf.buf[buf.len - 1] == '\r')
- buf.len--;
- buf.buf[buf.len] = '\0';
- }
-
if (!starts_with(buf.buf, "-S"))
strbuf_reset(&buf);
else {
opts->gpg_sign = buf.buf + 2;
- strbuf_detach(&buf, NULL);
+ sequencer_entrust(opts,
+ strbuf_detach(&buf, NULL));
}
}
@@ -1034,7 +1048,7 @@ static int read_populate_opts(struct replay_opts *opts)
* are pretty certain that it is syntactically correct.
*/
if (git_config_from_file(populate_opts_cb, git_path_opts_file(), opts) < 0)
- return error(_("Malformed options sheet: %s"),
+ return error(_("Malformed options sheet: '%s'"),
git_path_opts_file());
return 0;
}
@@ -1044,13 +1058,14 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
{
enum todo_command command = opts->action == REPLAY_PICK ?
TODO_PICK : TODO_REVERT;
+ const char *command_string = todo_command_strings[command];
struct commit *commit;
if (prepare_revs(opts))
return -1;
while ((commit = get_revision(opts->revs))) {
- struct todo_item *item = append_todo(todo_list);
+ struct todo_item *item = append_new_todo(todo_list);
const char *commit_buffer = get_commit_buffer(commit, NULL);
const char *subject;
int subject_len;
@@ -1061,8 +1076,7 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
item->arg_len = 0;
item->offset_in_buf = todo_list->buf.len;
subject_len = find_commit_subject(commit_buffer, &subject);
- strbuf_addf(&todo_list->buf, "%s %s %.*s\n",
- opts->action == REPLAY_PICK ? "pick" : "revert",
+ strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
short_commit_name(commit), subject_len, subject);
unuse_commit_buffer(commit, commit_buffer);
}
@@ -1077,7 +1091,7 @@ static int create_seq_dir(void)
return -1;
}
else if (mkdir(git_path_seq_dir(), 0777) < 0)
- return error_errno(_("Could not create sequencer directory %s"),
+ return error_errno(_("Could not create sequencer directory '%s'"),
git_path_seq_dir());
return 0;
}
@@ -1096,12 +1110,12 @@ static int save_head(const char *head)
strbuf_addf(&buf, "%s\n", head);
if (write_in_full(fd, buf.buf, buf.len) < 0) {
rollback_lock_file(&head_lock);
- return error_errno(_("Could not write to %s"),
+ return error_errno(_("Could not write to '%s'"),
git_path_head_file());
}
if (commit_lock_file(&head_lock) < 0) {
rollback_lock_file(&head_lock);
- return error(_("Error wrapping up %s."), git_path_head_file());
+ return error(_("Error wrapping up '%s'."), git_path_head_file());
}
return 0;
}
@@ -1136,9 +1150,6 @@ int sequencer_rollback(struct replay_opts *opts)
unsigned char sha1[20];
struct strbuf buf = STRBUF_INIT;
- if (read_and_refresh_cache(opts))
- return -1;
-
f = fopen(git_path_head_file(), "r");
if (!f && errno == ENOENT) {
/*
@@ -1149,9 +1160,9 @@ int sequencer_rollback(struct replay_opts *opts)
return rollback_single_pick();
}
if (!f)
- return error_errno(_("cannot open %s"), git_path_head_file());
+ return error_errno(_("cannot open '%s'"), git_path_head_file());
if (strbuf_getline_lf(&buf, f)) {
- error(_("cannot read %s: %s"), git_path_head_file(),
+ error(_("cannot read '%s': %s"), git_path_head_file(),
ferror(f) ? strerror(errno) : _("unexpected end of file"));
fclose(f);
goto fail;
@@ -1183,16 +1194,14 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
if (fd < 0)
- return error_errno(_("Could not lock '%s'"),
- git_path_todo_file());
+ return error_errno(_("Could not lock '%s'"), todo_path);
offset = next < todo_list->nr ?
todo_list->items[next].offset_in_buf : todo_list->buf.len;
if (write_in_full(fd, todo_list->buf.buf + offset,
todo_list->buf.len - offset) < 0)
- return error(_("Could not write to %s (%s)"),
- todo_path, strerror(errno));
+ return error_errno(_("Could not write to '%s'"), todo_path);
if (commit_lock_file(&todo_lock) < 0)
- return error(_("Error wrapping up %s."), todo_path);
+ return error(_("Error wrapping up '%s'."), todo_path);
return 0;
}
@@ -1279,21 +1288,25 @@ int sequencer_continue(struct replay_opts *opts)
if (!file_exists(get_todo_path(opts)))
return continue_single_pick();
- if (read_populate_opts(opts) ||
- read_populate_todo(&todo_list, opts))
+ if (read_populate_opts(opts))
return -1;
+ if ((res = read_populate_todo(&todo_list, opts)))
+ goto release_todo_list;
/* Verify that the conflict has been resolved */
if (file_exists(git_path_cherry_pick_head()) ||
file_exists(git_path_revert_head())) {
- int ret = continue_single_pick();
- if (ret)
- return ret;
+ res = continue_single_pick();
+ if (res)
+ goto release_todo_list;
+ }
+ if (index_differs_from("HEAD", 0)) {
+ res = error_dirty_index(opts);
+ goto release_todo_list;
}
- if (index_differs_from("HEAD", 0))
- return error_dirty_index(opts);
todo_list.current++;
res = pick_commits(&todo_list, opts);
+release_todo_list:
todo_list_release(&todo_list);
return res;
}
diff --git a/sequencer.h b/sequencer.h
index e272549..688fff1 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -40,9 +40,13 @@ struct replay_opts {
void **owned;
int owned_nr, owned_alloc;
};
-#define REPLAY_OPTS_INIT { -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, 0, 0, NULL, NULL, 0, 0 }
+#define REPLAY_OPTS_INIT { -1 }
-void *sequencer_entrust(struct replay_opts *opts, void *set_me_free_after_use);
+/*
+ * Make it the duty of sequencer_remove_state() to release the memory;
+ * For ease of use, return the same pointer.
+ */
+void *sequencer_entrust(struct replay_opts *opts, void *to_free);
int sequencer_pick_revisions(struct replay_opts *opts);
int sequencer_continue(struct replay_opts *opts);
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 6465edf..7b7a89d 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -459,6 +459,17 @@ test_expect_success 'malformed instruction sheet 1' '
test_expect_code 128 git cherry-pick --continue
'
+test_expect_success 'malformed instruction sheet 2' '
+ pristine_detach initial &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_expect_code 128 git cherry-pick --continue
+'
+
test_expect_success 'empty commit set' '
pristine_detach initial &&
test_expect_code 128 git cherry-pick base..base
--
2.10.0.windows.1.10.g803177d
base-commit: 8a04d01a511ecffba8a397d26ee7fa6ca56b6e7e
^ permalink raw reply
* [PATCH v2 01/25] sequencer: use static initializers for replay_opts
From: Johannes Schindelin @ 2016-09-11 10:52 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
This change is not completely faithful: instead of initializing all fields
to 0, we choose to initialize command and subcommand to -1 (instead of
defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
it makes no difference at all, but future-proofs the code to require
explicit assignments for both fields.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/revert.c | 6 ++----
sequencer.h | 1 +
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 4e69380..7365559 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -178,10 +178,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
int cmd_revert(int argc, const char **argv, const char *prefix)
{
- struct replay_opts opts;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
- memset(&opts, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
@@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
{
- struct replay_opts opts;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
- memset(&opts, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
diff --git a/sequencer.h b/sequencer.h
index 5ed5cb1..db425ad 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -47,6 +47,7 @@ struct replay_opts {
/* Only used by REPLAY_NONE */
struct rev_info *revs;
};
+#define REPLAY_OPTS_INIT { -1, -1 }
int sequencer_pick_revisions(struct replay_opts *opts);
--
2.10.0.windows.1.10.g803177d
^ permalink raw reply related
* [PATCH v2 4/4] add: modify already added files when --chmod is given
From: Thomas Gummerer @ 2016-09-11 10:30 UTC (permalink / raw)
To: git
Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160911103028.5492-1-t.gummerer@gmail.com>
When the chmod option was added to git add, it was hooked up to the diff
machinery, meaning that it only works when the version in the index
differs from the version on disk.
As the option was supposed to mirror the chmod option in update-index,
which always changes the mode in the index, regardless of the status of
the file, make sure the option behaves the same way in git add.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
builtin/add.c | 36 +++++++++++++++++++++++++-----------
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
cache.h | 10 +++++-----
read-cache.c | 14 ++++++--------
t/t3700-add.sh | 21 +++++++++++++++++++++
6 files changed, 59 insertions(+), 26 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index b1dddb4..892198a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -26,10 +26,25 @@ static int patch_interactive, add_interactive, edit_interactive;
static int take_worktree_changes;
struct update_callback_data {
- int flags, force_mode;
+ int flags;
int add_errors;
};
+static void chmod_pathspec(struct pathspec *pathspec, int force_mode)
+{
+ int i;
+
+ for (i = 0; i < active_nr; i++) {
+ struct cache_entry *ce = active_cache[i];
+
+ if (pathspec && !ce_path_match(ce, pathspec, NULL))
+ continue;
+
+ if (chmod_cache_entry(ce, force_mode) < 0)
+ fprintf(stderr, "cannot chmod '%s'", ce->name);
+ }
+}
+
static int fix_unmerged_status(struct diff_filepair *p,
struct update_callback_data *data)
{
@@ -65,8 +80,7 @@ static void update_callback(struct diff_queue_struct *q,
die(_("unexpected diff status %c"), p->status);
case DIFF_STATUS_MODIFIED:
case DIFF_STATUS_TYPE_CHANGED:
- if (add_file_to_index(&the_index, path,
- data->flags, data->force_mode)) {
+ if (add_file_to_index(&the_index, path, data->flags)) {
if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
die(_("updating files failed"));
data->add_errors++;
@@ -84,15 +98,14 @@ static void update_callback(struct diff_queue_struct *q,
}
}
-int add_files_to_cache(const char *prefix, const struct pathspec *pathspec,
- int flags, int force_mode)
+int add_files_to_cache(const char *prefix,
+ const struct pathspec *pathspec, int flags)
{
struct update_callback_data data;
struct rev_info rev;
memset(&data, 0, sizeof(data));
data.flags = flags;
- data.force_mode = force_mode;
init_revisions(&rev, prefix);
setup_revisions(0, NULL, &rev, NULL);
@@ -281,7 +294,7 @@ static int add_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
-static int add_files(struct dir_struct *dir, int flags, int force_mode)
+static int add_files(struct dir_struct *dir, int flags)
{
int i, exit_status = 0;
@@ -294,8 +307,7 @@ static int add_files(struct dir_struct *dir, int flags, int force_mode)
}
for (i = 0; i < dir->nr; i++)
- if (add_file_to_index(&the_index, dir->entries[i]->name,
- flags, force_mode)) {
+ if (add_file_to_index(&the_index, dir->entries[i]->name, flags)) {
if (!ignore_add_errors)
die(_("adding files failed"));
exit_status = 1;
@@ -441,11 +453,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)
plug_bulk_checkin();
- exit_status |= add_files_to_cache(prefix, &pathspec, flags, force_mode);
+ exit_status |= add_files_to_cache(prefix, &pathspec, flags);
if (add_new_files)
- exit_status |= add_files(&dir, flags, force_mode);
+ exit_status |= add_files(&dir, flags);
+ if (force_mode)
+ chmod_pathspec(&pathspec, force_mode);
unplug_bulk_checkin();
finish:
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8672d07..a83c78f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -548,7 +548,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
* entries in the index.
*/
- add_files_to_cache(NULL, NULL, 0, 0);
+ add_files_to_cache(NULL, NULL, 0);
/*
* NEEDSWORK: carrying over local changes
* when branches have different end-of-line
diff --git a/builtin/commit.c b/builtin/commit.c
index 77e3dc8..7a1ade0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -387,7 +387,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
*/
if (all || (also && pathspec.nr)) {
hold_locked_index(&index_lock, 1);
- add_files_to_cache(also ? prefix : NULL, &pathspec, 0, 0);
+ add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
diff --git a/cache.h b/cache.h
index 44a4f76..eb6d6b8 100644
--- a/cache.h
+++ b/cache.h
@@ -367,8 +367,8 @@ extern void free_name_hash(struct index_state *istate);
#define rename_cache_entry_at(pos, new_name) rename_index_entry_at(&the_index, (pos), (new_name))
#define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
#define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
-#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags), 0)
-#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags), 0)
+#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))
+#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))
#define chmod_cache_entry(ce, force_mode) chmod_index_entry(&the_index, (ce), (force_mode))
#define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL, NULL)
#define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
@@ -582,8 +582,8 @@ extern int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_IGNORE_ERRORS 4
#define ADD_CACHE_IGNORE_REMOVAL 8
#define ADD_CACHE_INTENT 16
-extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags, int force_mode);
-extern int add_file_to_index(struct index_state *, const char *path, int flags, int force_mode);
+extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
+extern int add_file_to_index(struct index_state *, const char *path, int flags);
extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, int force_mode);
extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
@@ -1820,7 +1820,7 @@ void packet_trace_identity(const char *prog);
* return 0 if success, 1 - if addition of a file failed and
* ADD_FILES_IGNORE_ERRORS was specified in flags
*/
-int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags, int force_mode);
+int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags);
/* diff.c */
extern int diff_auto_refresh_index;
diff --git a/read-cache.c b/read-cache.c
index 367be57..b92d72e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -627,7 +627,7 @@ void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)
hashcpy(ce->sha1, sha1);
}
-int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags, int force_mode)
+int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
{
int size, namelen, was_same;
mode_t st_mode = st->st_mode;
@@ -656,11 +656,10 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
else
ce->ce_flags |= CE_INTENT_TO_ADD;
- if (S_ISREG(st_mode) && force_mode)
- ce->ce_mode = create_ce_mode(force_mode);
- else if (trust_executable_bit && has_symlinks)
+
+ if (trust_executable_bit && has_symlinks) {
ce->ce_mode = create_ce_mode(st_mode);
- else {
+ } else {
/* If there is an existing entry, pick the mode bits and type
* from it, otherwise assume unexecutable regular file.
*/
@@ -719,13 +718,12 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
return 0;
}
-int add_file_to_index(struct index_state *istate, const char *path,
- int flags, int force_mode)
+int add_file_to_index(struct index_state *istate, const char *path, int flags)
{
struct stat st;
if (lstat(path, &st))
die_errno("unable to stat '%s'", path);
- return add_to_index(istate, path, &st, flags, force_mode);
+ return add_to_index(istate, path, &st, flags);
}
struct cache_entry *make_cache_entry(unsigned int mode,
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 2978cb9..62c7f71 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -349,4 +349,25 @@ test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x with symlinks' '
test_mode_in_index 100755 foo2
'
+test_expect_success 'git add --chmod=[+-]x changes index with already added file' '
+ echo foo >foo3 &&
+ git add foo3 &&
+ git add --chmod=+x foo3 &&
+ test_mode_in_index 100755 foo3 &&
+ echo foo >xfoo3 &&
+ chmod 755 xfoo3 &&
+ git add xfoo3 &&
+ git add --chmod=-x xfoo3 &&
+ test_mode_in_index 100644 xfoo3
+'
+
+test_expect_success 'file status is changed after git add --chmod=+x' '
+ echo "AM foo4" >expected &&
+ echo foo >foo4 &&
+ git add foo4 &&
+ git add --chmod=+x foo4 &&
+ git status -s foo4 >actual &&
+ test_cmp expected actual
+'
+
test_done
--
2.10.0.304.gf2ff484
^ permalink raw reply related
* [PATCH v2 3/4] read-cache: introduce chmod_index_entry
From: Thomas Gummerer @ 2016-09-11 10:30 UTC (permalink / raw)
To: git
Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160911103028.5492-1-t.gummerer@gmail.com>
As there are chmod options for both add and update-index, introduce a
new chmod_index_entry function to do the work. Use it in update-index,
while it will be used in add in the next patch.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
builtin/update-index.c | 8 +-------
cache.h | 2 ++
read-cache.c | 19 +++++++++++++++++++
3 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 85a57db..1569c81 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -423,20 +423,14 @@ static void chmod_path(int force_mode, const char *path)
{
int pos;
struct cache_entry *ce;
- unsigned int mode;
char flip = force_mode == 0777 ? '+' : '-';
pos = cache_name_pos(path, strlen(path));
if (pos < 0)
goto fail;
ce = active_cache[pos];
- mode = ce->ce_mode;
- if (!S_ISREG(mode))
+ if (chmod_cache_entry(ce, force_mode) < 0)
goto fail;
- ce->ce_mode = create_ce_mode(force_mode);
- cache_tree_invalidate_path(&the_index, path);
- ce->ce_flags |= CE_UPDATE_IN_BASE;
- active_cache_changed |= CE_ENTRY_CHANGED;
report("chmod %cx '%s'", flip, path);
return;
diff --git a/cache.h b/cache.h
index b780a91..44a4f76 100644
--- a/cache.h
+++ b/cache.h
@@ -369,6 +369,7 @@ extern void free_name_hash(struct index_state *istate);
#define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags), 0)
#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags), 0)
+#define chmod_cache_entry(ce, force_mode) chmod_index_entry(&the_index, (ce), (force_mode))
#define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL, NULL)
#define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
#define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
@@ -584,6 +585,7 @@ extern int remove_file_from_index(struct index_state *, const char *path);
extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags, int force_mode);
extern int add_file_to_index(struct index_state *, const char *path, int flags, int force_mode);
extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
+extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, int force_mode);
extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
extern void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
extern int index_name_is_other(const struct index_state *, const char *, int);
diff --git a/read-cache.c b/read-cache.c
index 491e52d..367be57 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -756,6 +756,25 @@ struct cache_entry *make_cache_entry(unsigned int mode,
return ret;
}
+/*
+ * Change the mode of an index entry to force_mode, where force_mode can
+ * either be 0777 or 0666.
+ * Returns -1 if the chmod for the particular cache entry failed (if it's
+ * not a regular file), 0 otherwise.
+ */
+int chmod_index_entry(struct index_state *istate, struct cache_entry *ce,
+ int force_mode)
+{
+ if (!S_ISREG(ce->ce_mode))
+ return -1;
+ ce->ce_mode = create_ce_mode(force_mode);
+ cache_tree_invalidate_path(istate, ce->name);
+ ce->ce_flags |= CE_UPDATE_IN_BASE;
+ istate->cache_changed |= CE_ENTRY_CHANGED;
+
+ return 0;
+}
+
int ce_same_name(const struct cache_entry *a, const struct cache_entry *b)
{
int len = ce_namelen(a);
--
2.10.0.304.gf2ff484
^ permalink raw reply related
* [PATCH v2 2/4] update-index: use the same structure for chmod as add
From: Thomas Gummerer @ 2016-09-11 10:30 UTC (permalink / raw)
To: git
Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160911103028.5492-1-t.gummerer@gmail.com>
While the chmod options for update-index and the add have the same
functionality, they are using different ways to parse and handle the
option internally. Unify these modes in order to make further
refactoring simpler.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
builtin/update-index.c | 49 +++++++++++++++++++++----------------------------
1 file changed, 21 insertions(+), 28 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index ba04b19..85a57db 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -419,11 +419,12 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
return 0;
}
-static void chmod_path(int flip, const char *path)
+static void chmod_path(int force_mode, const char *path)
{
int pos;
struct cache_entry *ce;
unsigned int mode;
+ char flip = force_mode == 0777 ? '+' : '-';
pos = cache_name_pos(path, strlen(path));
if (pos < 0)
@@ -432,17 +433,11 @@ static void chmod_path(int flip, const char *path)
mode = ce->ce_mode;
if (!S_ISREG(mode))
goto fail;
- switch (flip) {
- case '+':
- ce->ce_mode |= 0111; break;
- case '-':
- ce->ce_mode &= ~0111; break;
- default:
- goto fail;
- }
+ ce->ce_mode = create_ce_mode(force_mode);
cache_tree_invalidate_path(&the_index, path);
ce->ce_flags |= CE_UPDATE_IN_BASE;
active_cache_changed |= CE_ENTRY_CHANGED;
+
report("chmod %cx '%s'", flip, path);
return;
fail:
@@ -788,16 +783,6 @@ static int really_refresh_callback(const struct option *opt,
return refresh(opt->value, REFRESH_REALLY);
}
-static int chmod_callback(const struct option *opt,
- const char *arg, int unset)
-{
- char *flip = opt->value;
- if ((arg[0] != '-' && arg[0] != '+') || arg[1] != 'x' || arg[2])
- return error("option 'chmod' expects \"+x\" or \"-x\"");
- *flip = arg[0];
- return 0;
-}
-
static int resolve_undo_clear_callback(const struct option *opt,
const char *arg, int unset)
{
@@ -917,7 +902,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
- char set_executable_bit = 0;
+ char *chmod_arg = 0;
+ int force_mode = 0;
struct refresh_params refresh_args = {0, &has_errors};
int lock_error = 0;
int split_index = -1;
@@ -955,10 +941,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
PARSE_OPT_NOARG | /* disallow --cacheinfo=<mode> form */
PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
(parse_opt_cb *) cacheinfo_callback},
- {OPTION_CALLBACK, 0, "chmod", &set_executable_bit, N_("(+/-)x"),
- N_("override the executable bit of the listed files"),
- PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
- chmod_callback},
+ OPT_STRING( 0, "chmod", &chmod_arg, N_("(+/-)x"),
+ N_("override the executable bit of the listed files")),
{OPTION_SET_INT, 0, "assume-unchanged", &mark_valid_only, NULL,
N_("mark files as \"not changing\""),
PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, MARK_FLAG},
@@ -1018,6 +1002,15 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(update_index_usage, options);
+ if (!chmod_arg)
+ force_mode = 0;
+ else if (!strcmp(chmod_arg, "-x"))
+ force_mode = 0666;
+ else if (!strcmp(chmod_arg, "+x"))
+ force_mode = 0777;
+ else
+ die(_("option 'chmod' expects \"+x\" or \"-x\""));
+
git_config(git_default_config, NULL);
/* We can't free this memory, it becomes part of a linked list parsed atexit() */
@@ -1055,8 +1048,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
setup_work_tree();
p = prefix_path(prefix, prefix_length, path);
update_one(p);
- if (set_executable_bit)
- chmod_path(set_executable_bit, p);
+ if (force_mode)
+ chmod_path(force_mode, p);
free(p);
ctx.argc--;
ctx.argv++;
@@ -1100,8 +1093,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
}
p = prefix_path(prefix, prefix_length, buf.buf);
update_one(p);
- if (set_executable_bit)
- chmod_path(set_executable_bit, p);
+ if (force_mode)
+ chmod_path(force_mode, p);
free(p);
}
strbuf_release(&unquoted);
--
2.10.0.304.gf2ff484
^ permalink raw reply related
* [PATCH v2 1/4] add: document the chmod option
From: Thomas Gummerer @ 2016-09-11 10:30 UTC (permalink / raw)
To: git
Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160911103028.5492-1-t.gummerer@gmail.com>
The git add --chmod option was introduced in 4e55ed3 ("add: add
--chmod=+x / --chmod=-x options", 2016-05-31), but was never
documented. Document the feature.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-add.txt | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 6a96a66..7ed63dc 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p]
[--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
[--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing]
- [--] [<pathspec>...]
+ [--chmod=(+|-)x] [--] [<pathspec>...]
DESCRIPTION
-----------
@@ -165,6 +165,11 @@ for "git add --no-all <pathspec>...", i.e. ignored removed files.
be ignored, no matter if they are already present in the work
tree or not.
+--chmod=(+|-)x::
+ Override the executable bit of the added files. The executable
+ bit is only changed in the index, the files on disk are left
+ unchanged.
+
\--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
--
2.10.0.304.gf2ff484
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox