* [PATCH v15 09/27] bisect--helper: `bisect_write` shell function in C
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
Reimplement the `bisect_write` shell function in C and add a
`bisect-write` subcommand to `git bisect--helper` to call it from
git-bisect.sh
Using `--bisect-write` subcommand is a temporary measure to port shell
function in C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired but its implementation will
be called by some other methods.
Note: bisect_write() uses two variables namely TERM_GOOD and TERM_BAD
from the global shell script thus we need to pass it to the subcommand
using the arguments. We then store them in a struct bisect_terms and
pass the memory address around functions.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
builtin/bisect--helper.c | 94 ++++++++++++++++++++++++++++++++++++++++++++----
git-bisect.sh | 25 +++----------
2 files changed, 91 insertions(+), 28 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index c542e8b..3f19b68 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -19,9 +19,15 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --write-terms <bad_term> <good_term>"),
N_("git bisect--helper --bisect-clean-state"),
N_("git bisect--helper --bisect-reset [<commit>]"),
+ N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
NULL
};
+struct bisect_terms {
+ const char *term_good;
+ const char *term_bad;
+};
+
/*
* Check whether the string `term` belongs to the set of strings
* included in the variable arguments.
@@ -149,6 +155,63 @@ static int check_expected_revs(const char **revs, int rev_nr)
return 0;
}
+static int bisect_write(const char *state, const char *rev,
+ const struct bisect_terms *terms, int nolog)
+{
+ struct strbuf tag = STRBUF_INIT;
+ struct strbuf commit_name = STRBUF_INIT;
+ struct object_id oid;
+ struct commit *commit;
+ struct pretty_print_context pp = {0};
+ FILE *fp = NULL;
+ int retval = 0;
+
+ if (!strcmp(state, terms->term_bad))
+ strbuf_addf(&tag, "refs/bisect/%s", state);
+ else if (one_of(state, terms->term_good, "skip", NULL))
+ strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
+ else {
+ error(_("Bad bisect_write argument: %s"), state);
+ retval = -1;
+ goto finish;
+ }
+
+ if (get_oid(rev, &oid)) {
+ error(_("couldn't get the oid of the rev '%s'"), rev);
+ retval = -1;
+ goto finish;
+ }
+
+ if (update_ref(NULL, tag.buf, oid.hash, NULL, 0,
+ UPDATE_REFS_MSG_ON_ERR)) {
+ retval = -1;
+ goto finish;
+ }
+
+ fp = fopen(git_path_bisect_log(), "a");
+ if (!fp) {
+ error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
+ retval = -1;
+ goto finish;
+ }
+
+ commit = lookup_commit_reference(oid.hash);
+ format_commit_message(commit, "%s", &commit_name, &pp);
+ fprintf(fp, "# %s: [%s] %s\n", state, sha1_to_hex(oid.hash),
+ commit_name.buf);
+
+ if (!nolog)
+ fprintf(fp, "git bisect %s %s\n", state, rev);
+
+ goto finish;
+finish:
+ if (fp)
+ fclose(fp);
+ strbuf_release(&tag);
+ strbuf_release(&commit_name);
+ return retval;
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -156,9 +219,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
WRITE_TERMS,
BISECT_CLEAN_STATE,
BISECT_RESET,
- CHECK_EXPECTED_REVS
+ CHECK_EXPECTED_REVS,
+ BISECT_WRITE
} cmdmode = 0;
- int no_checkout = 0;
+ int no_checkout = 0, res = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", &cmdmode,
N_("perform 'git bisect next'"), NEXT_ALL),
@@ -170,10 +234,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("reset the bisection state"), BISECT_RESET),
OPT_CMDMODE(0, "check-expected-revs", &cmdmode,
N_("check for expected revs"), CHECK_EXPECTED_REVS),
+ OPT_CMDMODE(0, "bisect-write", &cmdmode,
+ N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
OPT_END()
};
+ struct bisect_terms terms;
argc = parse_options(argc, argv, prefix, options,
git_bisect_helper_usage, 0);
@@ -182,24 +249,37 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
usage_with_options(git_bisect_helper_usage, options);
switch (cmdmode) {
+ int nolog;
case NEXT_ALL:
return bisect_next_all(prefix, no_checkout);
case WRITE_TERMS:
if (argc != 2)
die(_("--write-terms requires two arguments"));
- return write_terms(argv[0], argv[1]);
+ res = write_terms(argv[0], argv[1]);
+ break;
case BISECT_CLEAN_STATE:
if (argc != 0)
die(_("--bisect-clean-state requires no arguments"));
- return bisect_clean_state();
+ res = bisect_clean_state();
+ break;
case BISECT_RESET:
if (argc > 1)
die(_("--bisect-reset requires either zero or one arguments"));
- return bisect_reset(argc ? argv[0] : NULL);
+ res = bisect_reset(argc ? argv[0] : NULL);
+ break;
case CHECK_EXPECTED_REVS:
- return check_expected_revs(argv, argc);
+ res = check_expected_revs(argv, argc);
+ break;
+ case BISECT_WRITE:
+ if (argc != 4 && argc != 5)
+ die(_("--bisect-write requires either 4 or 5 arguments"));
+ nolog = (argc == 5) && !strcmp(argv[4], "nolog");
+ terms.term_good = xstrdup(argv[2]);
+ terms.term_bad = xstrdup(argv[3]);
+ res = bisect_write(argv[0], argv[1], &terms, nolog);
+ break;
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
- return 0;
+ return res;
}
diff --git a/git-bisect.sh b/git-bisect.sh
index c3e43248..dfdec33 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -144,7 +144,7 @@ bisect_start() {
0) state=$TERM_BAD ; bad_seen=1 ;;
*) state=$TERM_GOOD ;;
esac
- eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
+ eval="$eval git bisect--helper --bisect-write '$state' '$rev' '$TERM_GOOD' '$TERM_BAD' 'nolog' &&"
done
#
# Verify HEAD.
@@ -220,23 +220,6 @@ bisect_start() {
trap '-' 0
}
-bisect_write() {
- state="$1"
- rev="$2"
- nolog="$3"
- case "$state" in
- "$TERM_BAD")
- tag="$state" ;;
- "$TERM_GOOD"|skip)
- tag="$state"-"$rev" ;;
- *)
- die "$(eval_gettext "Bad bisect_write argument: \$state")" ;;
- esac
- git update-ref "refs/bisect/$tag" "$rev" || exit
- echo "# $state: $(git show-branch $rev)" >>"$GIT_DIR/BISECT_LOG"
- test -n "$nolog" || echo "git bisect $state $rev" >>"$GIT_DIR/BISECT_LOG"
-}
-
bisect_skip() {
all=''
for arg in "$@"
@@ -263,7 +246,7 @@ bisect_state() {
bisected_head=$(bisect_head)
rev=$(git rev-parse --verify "$bisected_head") ||
die "$(eval_gettext "Bad rev input: \$bisected_head")"
- bisect_write "$state" "$rev"
+ git bisect--helper --bisect-write "$state" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit
git bisect--helper --check-expected-revs "$rev" ;;
2,"$TERM_BAD"|*,"$TERM_GOOD"|*,skip)
shift
@@ -276,7 +259,7 @@ bisect_state() {
done
for rev in $hash_list
do
- bisect_write "$state" "$rev"
+ git bisect--helper --bisect-write "$state" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit
done
git bisect--helper --check-expected-revs $hash_list ;;
*,"$TERM_BAD")
@@ -413,7 +396,7 @@ bisect_replay () {
cmd="bisect_start $rev"
eval "$cmd" ;;
"$TERM_GOOD"|"$TERM_BAD"|skip)
- bisect_write "$command" "$rev" ;;
+ git bisect--helper --bisect-write "$command" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit;;
terms)
bisect_terms $rev ;;
*)
--
https://github.com/git/git/pull/287
^ permalink raw reply related
* [PATCH v15 04/27] bisect--helper: `bisect_clean_state` shell function in C
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
Reimplement `bisect_clean_state` shell function in C and add a
`bisect-clean-state` subcommand to `git bisect--helper` to call it from
git-bisect.sh .
Using `--bisect-clean-state` subcommand is a measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired but its implementation will
be called by bisect_reset() and bisect_start().
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
bisect.c | 43 +++++++++++++++++++++++++++++++++++++++++++
bisect.h | 2 ++
builtin/bisect--helper.c | 14 +++++++++++++-
git-bisect.sh | 26 +++-----------------------
4 files changed, 61 insertions(+), 24 deletions(-)
diff --git a/bisect.c b/bisect.c
index 6f512c2..45d598d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -430,6 +430,12 @@ static int read_bisect_refs(void)
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
+static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
+static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
+static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
+static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
+static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
+static GIT_PATH_FUNC(git_path_head_name, "head-name")
static void read_bisect_paths(struct argv_array *array)
{
@@ -1040,3 +1046,40 @@ int estimate_bisect_steps(int all)
return (e < 3 * x) ? n : n - 1;
}
+
+static int mark_for_removal(const char *refname, const struct object_id *oid,
+ int flag, void *cb_data)
+{
+ struct string_list *refs = cb_data;
+ char *ref = xstrfmt("refs/bisect%s", refname);
+ string_list_append(refs, ref);
+ return 0;
+}
+
+int bisect_clean_state(void)
+{
+ int result = 0;
+
+ /* There may be some refs packed during bisection */
+ struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
+ for_each_ref_in("refs/bisect", mark_for_removal, (void *) &refs_for_removal);
+ string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
+ result = delete_refs(&refs_for_removal, REF_NODEREF);
+ refs_for_removal.strdup_strings = 1;
+ string_list_clear(&refs_for_removal, 0);
+ unlink_or_warn(git_path_bisect_expected_rev());
+ unlink_or_warn(git_path_bisect_ancestors_ok());
+ unlink_or_warn(git_path_bisect_log());
+ unlink_or_warn(git_path_bisect_names());
+ unlink_or_warn(git_path_bisect_run());
+ unlink_or_warn(git_path_bisect_terms());
+ /* Cleanup head-name if it got left by an old version of git-bisect */
+ unlink_or_warn(git_path_head_name());
+ /*
+ * Cleanup BISECT_START last to support the --no-checkout option
+ * introduced in the commit 4796e823a.
+ */
+ unlink_or_warn(git_path_bisect_start());
+
+ return result;
+}
diff --git a/bisect.h b/bisect.h
index acd12ef..0ae63d4 100644
--- a/bisect.h
+++ b/bisect.h
@@ -28,4 +28,6 @@ extern int estimate_bisect_steps(int all);
extern void read_bisect_terms(const char **bad, const char **good);
+extern int bisect_clean_state(void);
+
#endif
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 65cf519..4254d61 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -5,10 +5,15 @@
#include "refs.h"
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
+static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
+static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
+static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
+static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
N_("git bisect--helper --write-terms <bad_term> <good_term>"),
+ N_("git bisect--helper --bisect-clean-state"),
NULL
};
@@ -83,7 +88,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
NEXT_ALL = 1,
- WRITE_TERMS
+ WRITE_TERMS,
+ BISECT_CLEAN_STATE
} cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
@@ -91,6 +97,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("perform 'git bisect next'"), NEXT_ALL),
OPT_CMDMODE(0, "write-terms", &cmdmode,
N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS),
+ OPT_CMDMODE(0, "bisect-clean-state", &cmdmode,
+ N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
OPT_END()
@@ -109,6 +117,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
if (argc != 2)
die(_("--write-terms requires two arguments"));
return write_terms(argv[0], argv[1]);
+ case BISECT_CLEAN_STATE:
+ if (argc != 0)
+ die(_("--bisect-clean-state requires no arguments"));
+ return bisect_clean_state();
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 9ef6cb8..f1202df 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -186,7 +186,7 @@ bisect_start() {
#
# Get rid of any old bisect state.
#
- bisect_clean_state || exit
+ git bisect--helper --bisect-clean-state || exit
#
# Change state.
@@ -195,7 +195,7 @@ bisect_start() {
# We have to trap this to be able to clean up using
# "bisect_clean_state".
#
- trap 'bisect_clean_state' 0
+ trap 'git bisect--helper --bisect-clean-state' 0
trap 'exit 255' 1 2 3 15
#
@@ -430,27 +430,7 @@ bisect_reset() {
die "$(eval_gettext "Could not check out original HEAD '\$branch'.
Try 'git bisect reset <commit>'.")"
fi
- bisect_clean_state
-}
-
-bisect_clean_state() {
- # There may be some refs packed during bisection.
- git for-each-ref --format='%(refname) %(objectname)' refs/bisect/\* |
- while read ref hash
- do
- git update-ref -d $ref $hash || exit
- done
- rm -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
- rm -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
- rm -f "$GIT_DIR/BISECT_LOG" &&
- rm -f "$GIT_DIR/BISECT_NAMES" &&
- rm -f "$GIT_DIR/BISECT_RUN" &&
- rm -f "$GIT_DIR/BISECT_TERMS" &&
- # Cleanup head-name if it got left by an old version of git-bisect
- rm -f "$GIT_DIR/head-name" &&
- git update-ref -d --no-deref BISECT_HEAD &&
- # clean up BISECT_START last
- rm -f "$GIT_DIR/BISECT_START"
+ git bisect--helper --bisect-clean-state || exit
}
bisect_replay () {
--
https://github.com/git/git/pull/287
^ permalink raw reply related
* [PATCH v15 23/27] bisect--helper: `bisect_replay` shell function in C
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
Reimplement the `bisect_replay` shell function in C and also add
`--bisect-replay` subcommand to `git bisect--helper` to call it from
git-bisect.sh
Using `--bisect-replay` subcommand is a temporary measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other method.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
builtin/bisect--helper.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 32 +-----------
2 files changed, 124 insertions(+), 33 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index c18ca07..b367d8d 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -32,6 +32,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-autostart"),
N_("git bisect--helper --bisect-state (bad|new) [<rev>]"),
N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
+ N_("git bisect--helper --bisect-replay <filename>"),
NULL
};
@@ -601,7 +602,6 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
terms->term_good = arg;
} else if (!strcmp(arg, "--term-bad") ||
!strcmp(arg, "--term-new")) {
- const char *next_arg;
if (starts_with(argv[++i], "'"))
terms->term_bad = sq_dequote(xstrdup(argv[i]));
else
@@ -875,6 +875,117 @@ static int bisect_log(void)
return status;
}
+static int get_next_word(const char *line, int pos, struct strbuf *word)
+{
+ int i, len = strlen(line), begin = 0;
+ strbuf_reset(word);
+ for (i = pos; i < len; i++) {
+ if (line[i] == ' ' && begin)
+ return i + 1;
+
+ if (!begin)
+ begin = 1;
+ strbuf_addch(word, line[i]);
+ }
+
+ return i;
+}
+
+static int bisect_replay(struct bisect_terms *terms, const char *filename)
+{
+ struct strbuf line = STRBUF_INIT;
+ struct strbuf word = STRBUF_INIT;
+ FILE *fp = NULL;
+ int res = 0;
+
+ if (is_empty_or_missing_file(filename)) {
+ error(_("no such file with name '%s' exists"), filename);
+ res = -1;
+ goto finish;
+ }
+
+ if (bisect_reset(NULL)) {
+ res = -1;
+ goto finish;
+ }
+
+ fp = fopen(filename, "r");
+ if (!fp) {
+ res = -1;
+ goto finish;
+ }
+
+ while (strbuf_getline(&line, fp) != EOF) {
+ int pos = 0;
+ while (pos < line.len) {
+ pos = get_next_word(line.buf, pos, &word);
+
+ if (!strcmp(word.buf, "git")) {
+ continue;
+ } else if (!strcmp(word.buf, "git-bisect")) {
+ continue;
+ } else if (!strcmp(word.buf, "bisect")) {
+ continue;
+ } else if (!strcmp(word.buf, "#")) {
+ break;
+ }
+
+ get_terms(terms);
+ if (check_and_set_terms(terms, word.buf)) {
+ res = -1;
+ goto finish;
+ }
+
+ if (!strcmp(word.buf, "start")) {
+ struct argv_array argv = ARGV_ARRAY_INIT;
+ sq_dequote_to_argv_array(line.buf+pos, &argv);
+ if (bisect_start(terms, 0, argv.argv, argv.argc)) {
+ argv_array_clear(&argv);
+ res = -1;
+ goto finish;
+ }
+ argv_array_clear(&argv);
+ break;
+ }
+
+ if (one_of(word.buf, terms->term_good,
+ terms->term_bad, "skip", NULL)) {
+ if (bisect_write(word.buf, line.buf+pos, terms, 0)) {
+ res = -1;
+ goto finish;
+ }
+ break;
+ }
+
+ if (!strcmp(word.buf, "terms")) {
+ struct argv_array argv = ARGV_ARRAY_INIT;
+ sq_dequote_to_argv_array(line.buf+pos, &argv);
+ if (bisect_terms(terms, argv.argv, argv.argc)) {
+ argv_array_clear(&argv);
+ res = -1;
+ goto finish;
+ }
+ argv_array_clear(&argv);
+ break;
+ }
+
+ error(_("?? what are you talking about?"));
+ res = -1;
+ goto finish;
+ }
+ }
+ goto finish;
+finish:
+ if (fp)
+ fclose(fp);
+ strbuf_release(&line);
+ strbuf_release(&word);
+ if (res)
+ return -1;
+
+ return bisect_auto_next(terms, NULL);
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -888,7 +999,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_AUTO_NEXT,
BISECT_AUTOSTART,
BISECT_STATE,
- BISECT_LOG
+ BISECT_LOG,
+ BISECT_REPLAY
} cmdmode = 0;
int no_checkout = 0, res = 0;
struct option options[] = {
@@ -914,6 +1026,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("mark the state of ref (or refs)"), BISECT_STATE),
OPT_CMDMODE(0, "bisect-log", &cmdmode,
N_("output the contents of BISECT_LOG"), BISECT_LOG),
+ OPT_CMDMODE(0, "bisect-replay", &cmdmode,
+ N_("replay the bisection process from the given file"), BISECT_REPLAY),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
OPT_END()
@@ -998,6 +1112,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
die(_("--bisect-log requires 0 arguments"));
res = bisect_log();
break;
+ case BISECT_REPLAY:
+ if (argc != 1)
+ die(_("--bisect-replay requires 1 argument"));
+ terms.term_good = "good";
+ terms.term_bad = "bad";
+ res = bisect_replay(&terms, argv[0]);
+ break;
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index a47e3b5..bf66ee2 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,36 +77,6 @@ bisect_visualize() {
eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
}
-bisect_replay () {
- file="$1"
- test "$#" -eq 1 || die "$(gettext "No logfile given")"
- test -r "$file" || die "$(eval_gettext "cannot read \$file for replaying")"
- git bisect--helper --bisect-reset || exit
- while read git bisect command rev
- do
- test "$git $bisect" = "git bisect" || test "$git" = "git-bisect" || continue
- if test "$git" = "git-bisect"
- then
- rev="$command"
- command="$bisect"
- fi
- get_terms
- git bisect--helper --check-and-set-terms "$command" "$TERM_GOOD" "$TERM_BAD" || exit
- get_terms
- case "$command" in
- start)
- eval "git bisect--helper --bisect-start $rev" ;;
- "$TERM_GOOD"|"$TERM_BAD"|skip)
- git bisect--helper --bisect-write "$command" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit;;
- terms)
- git bisect--helper --bisect-terms $rev || exit;;
- *)
- die "$(gettext "?? what are you talking about?")" ;;
- esac
- done <"$file"
- git bisect--helper --bisect-auto-next
-}
-
bisect_run () {
git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
@@ -201,7 +171,7 @@ case "$#" in
reset)
git bisect--helper --bisect-reset "$@" ;;
replay)
- bisect_replay "$@" ;;
+ git bisect--helper --bisect-replay "$@" ;;
log)
git bisect--helper --bisect-log ;;
run)
--
https://github.com/git/git/pull/287
^ permalink raw reply related
* [PATCH v15 10/27] bisect--helper: `check_and_set_terms` shell function in C
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
Reimplement the `check_and_set_terms` shell function in C and add
`check-and-set-terms` subcommand to `git bisect--helper` to call it from
git-bisect.sh
Using `--check-and-set-terms` subcommand is a temporary measure to port
shell function in C so as to use the existing test suite. As more
functions are ported, this subcommand will be retired but its
implementation will be called by some other methods.
check_and_set_terms() sets and receives two global variables namely
TERM_GOOD and TERM_BAD in the shell script. Luckily the file BISECT_TERMS
also contains the value of those variables so its appropriate to evoke the
method get_terms() after calling the subcommand so that it retrieves the
value of TERM_GOOD and TERM_BAD from the file BISECT_TERMS. The two
global variables are passed as arguments to the subcommand.
Also introduce set_terms() to copy the `term_good` and `term_bad` into
`struct bisect_terms` and write it out to the file BISECT_TERMS.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
builtin/bisect--helper.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 36 ++++--------------------------------
2 files changed, 48 insertions(+), 33 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3f19b68..c6c11e3 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -20,6 +20,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-clean-state"),
N_("git bisect--helper --bisect-reset [<commit>]"),
N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
+ N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
NULL
};
@@ -212,6 +213,38 @@ static int bisect_write(const char *state, const char *rev,
return retval;
}
+static int set_terms(struct bisect_terms *terms, const char *bad,
+ const char *good)
+{
+ terms->term_good = xstrdup(good);
+ terms->term_bad = xstrdup(bad);
+ return write_terms(terms->term_bad, terms->term_good);
+}
+
+static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)
+{
+ int has_term_file = !is_empty_or_missing_file(git_path_bisect_terms());
+
+ if (one_of(cmd, "skip", "start", "terms", NULL))
+ return 0;
+
+ if (has_term_file &&
+ strcmp(cmd, terms->term_bad) &&
+ strcmp(cmd, terms->term_good))
+ return error(_("Invalid command: you're currently in a "
+ "%s/%s bisect"), terms->term_bad,
+ terms->term_good);
+
+ if (!has_term_file) {
+ if (one_of(cmd, "bad", "good", NULL))
+ return set_terms(terms, "bad", "good");
+ if (one_of(cmd, "new", "old", NULL))
+ return set_terms(terms, "new", "old");
+ }
+
+ return 0;
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -220,7 +253,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_CLEAN_STATE,
BISECT_RESET,
CHECK_EXPECTED_REVS,
- BISECT_WRITE
+ BISECT_WRITE,
+ CHECK_AND_SET_TERMS
} cmdmode = 0;
int no_checkout = 0, res = 0;
struct option options[] = {
@@ -236,6 +270,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("check for expected revs"), CHECK_EXPECTED_REVS),
OPT_CMDMODE(0, "bisect-write", &cmdmode,
N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE),
+ OPT_CMDMODE(0, "check-and-set-terms", &cmdmode,
+ N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
OPT_END()
@@ -278,6 +314,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
terms.term_bad = xstrdup(argv[3]);
res = bisect_write(argv[0], argv[1], &terms, nolog);
break;
+ case CHECK_AND_SET_TERMS:
+ if (argc != 3)
+ die(_("--check-and-set-terms requires 3 arguments"));
+ terms.term_good = xstrdup(argv[1]);
+ terms.term_bad = xstrdup(argv[2]);
+ res = check_and_set_terms(&terms, argv[0]);
+ break;
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index dfdec33..bdf2227 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -238,7 +238,8 @@ bisect_skip() {
bisect_state() {
bisect_autostart
state=$1
- check_and_set_terms $state
+ git bisect--helper --check-and-set-terms $state $TERM_GOOD $TERM_BAD || exit
+ get_terms
case "$#,$state" in
0,*)
die "Please call 'bisect_state' with at least one argument." ;;
@@ -390,7 +391,8 @@ bisect_replay () {
command="$bisect"
fi
get_terms
- check_and_set_terms "$command"
+ git bisect--helper --check-and-set-terms "$command" "$TERM_GOOD" "$TERM_BAD" || exit
+ get_terms
case "$command" in
start)
cmd="bisect_start $rev"
@@ -480,36 +482,6 @@ get_terms () {
fi
}
-check_and_set_terms () {
- cmd="$1"
- case "$cmd" in
- skip|start|terms) ;;
- *)
- if test -s "$GIT_DIR/BISECT_TERMS" && test "$cmd" != "$TERM_BAD" && test "$cmd" != "$TERM_GOOD"
- then
- die "$(eval_gettext "Invalid command: you're currently in a \$TERM_BAD/\$TERM_GOOD bisect.")"
- fi
- case "$cmd" in
- bad|good)
- if ! test -s "$GIT_DIR/BISECT_TERMS"
- then
- TERM_BAD=bad
- TERM_GOOD=good
- git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD" || exit
- fi
- ;;
- new|old)
- if ! test -s "$GIT_DIR/BISECT_TERMS"
- then
- TERM_BAD=new
- TERM_GOOD=old
- git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD" || exit
- fi
- ;;
- esac ;;
- esac
-}
-
bisect_voc () {
case "$1" in
bad) echo "bad|new" ;;
--
https://github.com/git/git/pull/287
^ permalink raw reply related
* [PATCH v15 22/27] bisect--helper: `bisect_log` shell function in C
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
Reimplement the `bisect_log` shell function in C and also add
`--bisect-log` subcommand to `git bisect--helper` to call it from
git-bisect.sh .
Using `--bisect-log` subcommand is a temporary measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other method.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
builtin/bisect--helper.c | 27 ++++++++++++++++++++++++++-
git-bisect.sh | 7 +------
2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 493034c..c18ca07 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -858,6 +858,23 @@ static int bisect_state(struct bisect_terms *terms, const char **argv,
return -1;
}
+static int bisect_log(void)
+{
+ int fd, status;
+ fd = open(git_path_bisect_log(), O_RDONLY);
+ if (fd < 0)
+ return -1;
+
+ status = copy_fd(fd, 1);
+ if (status) {
+ close(fd);
+ return -1;
+ }
+
+ close(fd);
+ return status;
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -870,7 +887,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_NEXT,
BISECT_AUTO_NEXT,
BISECT_AUTOSTART,
- BISECT_STATE
+ BISECT_STATE,
+ BISECT_LOG
} cmdmode = 0;
int no_checkout = 0, res = 0;
struct option options[] = {
@@ -894,6 +912,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("start the bisection if BISECT_START empty or missing"), BISECT_AUTOSTART),
OPT_CMDMODE(0, "bisect-state", &cmdmode,
N_("mark the state of ref (or refs)"), BISECT_STATE),
+ OPT_CMDMODE(0, "bisect-log", &cmdmode,
+ N_("output the contents of BISECT_LOG"), BISECT_LOG),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
OPT_END()
@@ -973,6 +993,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
get_terms(&terms);
res = bisect_state(&terms, argv, argc);
break;
+ case BISECT_LOG:
+ if (argc > 1)
+ die(_("--bisect-log requires 0 arguments"));
+ res = bisect_log();
+ break;
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index a9eebbb..a47e3b5 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -166,11 +166,6 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
done
}
-bisect_log () {
- test -s "$GIT_DIR/BISECT_LOG" || die "$(gettext "We are not bisecting.")"
- cat "$GIT_DIR/BISECT_LOG"
-}
-
get_terms () {
if test -s "$GIT_DIR/BISECT_TERMS"
then
@@ -208,7 +203,7 @@ case "$#" in
replay)
bisect_replay "$@" ;;
log)
- bisect_log ;;
+ git bisect--helper --bisect-log ;;
run)
bisect_run "$@" ;;
terms)
--
https://github.com/git/git/pull/287
^ permalink raw reply related
* [PATCH v15 05/27] t6030: explicitly test for bisection cleanup
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
Add test to explicitly check that 'git bisect reset' is working as
expected. This is already covered implicitly by the test suite.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
I faced this problem while converting `bisect_clean_state` and the tests
where showing breakages but it wasn't clear as to where exactly are they
breaking. This will patch will help in that. Also I tested the test
coverage of the test suite before this patch and it covers this (I did
this by purposely changing names of files in git-bisect.sh and running
the test suite).
---
t/t6030-bisect-porcelain.sh | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 5e5370f..18e7998 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -894,4 +894,21 @@ test_expect_success 'bisect start takes options and revs in any order' '
test_cmp expected actual
'
+test_expect_success 'git bisect reset cleans bisection state properly' '
+ git bisect reset &&
+ git bisect start &&
+ git bisect good $HASH1 &&
+ git bisect bad $HASH4 &&
+ git bisect reset &&
+ test -z "$(git for-each-ref "refs/bisect/*")" &&
+ test_path_is_missing "$GIT_DIR/BISECT_EXPECTED_REV" &&
+ test_path_is_missing "$GIT_DIR/BISECT_ANCESTORS_OK" &&
+ test_path_is_missing "$GIT_DIR/BISECT_LOG" &&
+ test_path_is_missing "$GIT_DIR/BISECT_RUN" &&
+ test_path_is_missing "$GIT_DIR/BISECT_TERMS" &&
+ test_path_is_missing "$GIT_DIR/head-name" &&
+ test_path_is_missing "$GIT_DIR/BISECT_HEAD" &&
+ test_path_is_missing "$GIT_DIR/BISECT_START"
+'
+
test_done
--
https://github.com/git/git/pull/287
^ permalink raw reply related
* [PATCH v15 06/27] wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
is_empty_file() can help to refactor a lot of code. This will be very
helpful in porting "git bisect" to C.
Suggested-by: Torsten Bögershausen <tboegi@web.de>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
builtin/am.c | 20 ++------------------
cache.h | 3 +++
wrapper.c | 13 +++++++++++++
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 739b34d..9e1e9d6 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -30,22 +30,6 @@
#include "mailinfo.h"
/**
- * Returns 1 if the file is empty or does not exist, 0 otherwise.
- */
-static int is_empty_file(const char *filename)
-{
- struct stat st;
-
- if (stat(filename, &st) < 0) {
- if (errno == ENOENT)
- return 1;
- die_errno(_("could not stat %s"), filename);
- }
-
- return !st.st_size;
-}
-
-/**
* Returns the length of the first line of msg.
*/
static int linelen(const char *msg)
@@ -1324,7 +1308,7 @@ static int parse_mail(struct am_state *state, const char *mail)
goto finish;
}
- if (is_empty_file(am_path(state, "patch"))) {
+ if (is_empty_or_missing_file(am_path(state, "patch"))) {
printf_ln(_("Patch is empty. Was it split wrong?"));
die_user_resolve(state);
}
@@ -1896,7 +1880,7 @@ static void am_run(struct am_state *state, int resume)
resume = 0;
}
- if (!is_empty_file(am_path(state, "rewritten"))) {
+ if (!is_empty_or_missing_file(am_path(state, "rewritten"))) {
assert(state->rebasing);
copy_notes_for_rebase(state);
run_post_rewrite_hook(state);
diff --git a/cache.h b/cache.h
index b780a91..49f214b 100644
--- a/cache.h
+++ b/cache.h
@@ -1916,4 +1916,7 @@ void sleep_millisec(int millisec);
*/
void safe_create_dir(const char *dir, int share);
+/* Return 1 if the file is empty or does not exists, 0 otherwise. */
+extern int is_empty_or_missing_file(const char *filename);
+
#endif /* CACHE_H */
diff --git a/wrapper.c b/wrapper.c
index e7f1979..78f6431 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -679,3 +679,16 @@ void sleep_millisec(int millisec)
{
poll(NULL, 0, millisec);
}
+
+int is_empty_or_missing_file(const char *filename)
+{
+ struct stat st;
+
+ if (stat(filename, &st) < 0) {
+ if (errno == ENOENT)
+ return 1;
+ die_errno(_("could not stat %s"), filename);
+ }
+
+ return !st.st_size;
+}
--
https://github.com/git/git/pull/287
^ permalink raw reply related
* Re: Can we make interactive add easier to use?
From: Matthieu Moy @ 2016-10-14 13:46 UTC (permalink / raw)
To: Robert Dailey; +Cc: Git
In-Reply-To: <CAHd499AnuVximRgM0MKdq5JC-hwkrhox6bK_KA+XGrawoz2W+g@mail.gmail.com>
Robert Dailey <rcdailey.lists@gmail.com> writes:
> Normally when I use interactive add, I just want to add files to the
> index via simple numbers, instead of typing paths. So I'll do this as
> quick as I can:
>
> 1. Type `git add -i`
> 2. Press `u` after prompt appears
> 3. Press numbers for the files I want to add, ENTER key
> 4. ENTER key again to go back to main add -i menu
> 5. Press `q` to exit interactive add
> 6. Type `git commit`
>
> This feels very tedious. Is there a simplified workflow for this?
My workflow is to ... not use "git add -i" ;-).
To add patch hunks individually, "git add -p" jumps directly to the
"patch" inner loop of "git add -i".
To add whole individual files, a plain "git add" using zsh's smart
completion (autocompletes only files for which "git add" is not a
no-op), or globs.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 0/2] infinite loop in "git ls-tree" for broken symlink
From: Jeff King @ 2016-10-14 13:42 UTC (permalink / raw)
To: Petr Stodulka; +Cc: git
In-Reply-To: <1476451012-9925-1-git-send-email-pstodulk@redhat.com>
On Fri, Oct 14, 2016 at 03:16:50PM +0200, Petr Stodulka wrote:
> I have realized that this wasn't fixed after all when refs.c
> was "rewritten". Issue is caused by broken symlink under refs/heads,
> which causes infinite loop for "git ls-tree" command. It was replied
> earlier [0] and Michael previously fixed that in better way probably,
> then my proposed patch 2/2, but it contains more changes and I have not
> enough time to study changes in refs* code, so I propose now just my
> little patch, which was previously modified by Michael.
>
> If you prefer different solution, I am ok with this. It is really
> just quick proposal. Patch 1/2 contains test for this issue. If you
> will prefer different solution with different exit code, test should
> be corrected. Basic idea is, that timeout should'n expire with
> exit code 124.
>
> [0] http://marc.info/?l=git&m=142712669103790&w=2
> [1] https://github.com/mhagger/git, branch "ref-broken-symlinks"
I think I fixed this semi-independently last week; see the thread at:
http://public-inbox.org/git/20161006164723.ocg2nbgtulpjcksp@sigill.intra.peff.net/
I say semi-independently because I didn't actually know about your
previous report, but saw it in the wild myself. But I did talk with
Michael off-list, and he suggested the belt-and-suspenders retry counter
in my second patch.
The fix is at e8c42cb in Junio's tree, and it's currently merged to
'next'.
-Peff
^ permalink raw reply
* [PATCH v4 00/25] Prepare the sequencer for the upcoming rebase -i patches
From: Johannes Schindelin @ 2016-10-14 13:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476120229.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 (read: there will be three more patch
series after that before the full benefit hits git.git: sequencer-i,
rebase--helper and rebase-i-extra). It is based on the `libify-sequencer`
patch series I submitted earlier.
The patches in this series merely prepare the sequencer code for the
next patch series that actually teaches the sequencer to run rebase -i's
commands.
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 already
integrated the whole shebang into Git for Windows 2.10.0 and 2.10.1
where it has been running without complaints (and some quite positive
feedback).
Changes vs v3:
- fixed TRANSLATORS: comment to help the tool extracting those comments.
- reordered the patch introducing the short_commit_name() function so it
can be used in the patch revamping the todo parsing right away, as
opposed to fixing up the find_unique_abbrev() ugliness in a later
patch.
- backed out the write_message_gently() function of this patch series:
it is not used by the end of this patch series and would therefore let
the build fail with DEVELOPER=1. That function is now introduced as
part of the patch in the sequencer-i series that adds support for the
'edit' command.
- edited "skip CR/skip LF" to say "strip" instead.
- used xstrdup_or_null() where appropriate.
- abstracted out git_config_string_dup() instead of adding
near-duplicate code.
- marked the append_new_todo() function as file-local, pointed out by
Ramsay.
- made the "you have staged changes" advice prettier by moving it out of
the run_git_commit() function, based on a suggestion by Hannes Sixt.
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: eventually release memory allocated for the option values
sequencer: future-proof read_populate_todo()
sequencer: refactor the code to obtain a short commit name
sequencer: completely revamp the "todo" script parsing
sequencer: strip CR from the todo script
sequencer: avoid completely different messages for different actions
sequencer: get rid of the subcommand field
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: allow editing the commit message on a case-by-case basis
sequencer: support amending commits
sequencer: support cleaning up commit messages
sequencer: do not try to commit when there were merge conflicts
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: start error messages consistently with lower case
sequencer: mark all error messages for translation
builtin/commit.c | 2 +-
builtin/revert.c | 46 ++-
sequencer.c | 679 ++++++++++++++++++++++++++++--------------
sequencer.h | 23 +-
t/t3501-revert-cherry-pick.sh | 2 +-
5 files changed, 492 insertions(+), 260 deletions(-)
base-commit: 3cdd5d19178a54d2e51b5098d43b57571241d0ab
Published-As: https://github.com/dscho/git/releases/tag/prepare-sequencer-v4
Fetch-It-Via: git fetch https://github.com/dscho/git prepare-sequencer-v4
Interdiff vs v3:
diff --git a/builtin/revert.c b/builtin/revert.c
index 0a7b5f4..4ca5b51 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -166,10 +166,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
usage_with_options(usage_str, options);
/* These option values will be free()d */
- if (opts->gpg_sign)
- opts->gpg_sign = xstrdup(opts->gpg_sign);
- if (opts->strategy)
- opts->strategy = xstrdup(opts->strategy);
+ opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
+ opts->strategy = xstrdup_or_null(opts->strategy);
if (cmd == 'q')
return sequencer_remove_state(opts);
diff --git a/sequencer.c b/sequencer.c
index 86d86ce..1cf70f7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -265,12 +265,6 @@ static int write_message(struct strbuf *msgbuf, const char *filename)
return res;
}
-static int write_file_gently(const char *filename,
- const char *text, int append_eol)
-{
- return write_with_lock_file(filename, text, strlen(text), append_eol);
-}
-
/*
* Reads a file that was presumably written by a shell script, i.e.
* with an end-of-line marker that needs to be stripped.
@@ -489,6 +483,20 @@ static char **read_author_script(void)
return env;
}
+static const char staged_changes_advice[] =
+N_("you have staged changes in your working tree\n"
+"If these changes are meant to be squashed into the previous commit, run:\n"
+"\n"
+" git commit --amend %s\n"
+"\n"
+"If they are meant to go into a new commit, run:\n"
+"\n"
+" git commit %s\n"
+"\n"
+"In both cases, once you're done, continue with:\n"
+"\n"
+" git rebase --continue\n");
+
/*
* If we are cherry-pick, and if the merge did not result in
* hand-editing, we will hit this commit and inherit the original
@@ -515,18 +523,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
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 %s\n\n"
- "If they are meant to go into a new "
- "commit, run:\n\n"
- " git commit %s\n\n"
- "In both cases, once you're done, "
- "continue with:\n\n"
- " git rebase --continue\n"),
+ return error(_(staged_changes_advice),
gpg_opt, gpg_opt);
}
}
@@ -702,10 +699,8 @@ 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 a "todo" command like
- * "revert" or "pick", the second %s a SHA1.
- */
+ /* TRANSLATORS: The first %s will be a "todo" command like
+ "revert" or "pick", the second %s a SHA1. */
return error(_("%s: cannot parse parent commit %s"),
command_to_string(command),
oid_to_hex(&parent->object.oid));
@@ -885,7 +880,7 @@ static void todo_list_release(struct todo_list *todo_list)
todo_list->nr = todo_list->alloc = 0;
}
-struct todo_item *append_new_todo(struct todo_list *todo_list)
+static 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++;
@@ -939,10 +934,10 @@ static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
for (i = 1; *p; i++, p = next_p) {
char *eol = strchrnul(p, '\n');
- next_p = *eol ? eol + 1 /* skip LF */ : eol;
+ next_p = *eol ? eol + 1 /* strip LF */ : eol;
if (p != eol && eol[-1] == '\r')
- eol--; /* skip Carriage Return */
+ eol--; /* strip Carriage Return */
item = append_new_todo(todo_list);
item->offset_in_buf = p - todo_list->buf.buf;
@@ -994,6 +989,16 @@ static int read_populate_todo(struct todo_list *todo_list,
return 0;
}
+static int git_config_string_dup(char **dest,
+ const char *var, const char *value)
+{
+ if (!value)
+ return config_error_nonbool(var);
+ free(*dest);
+ *dest = xstrdup(value);
+ return 0;
+}
+
static int populate_opts_cb(const char *key, const char *value, void *data)
{
struct replay_opts *opts = data;
@@ -1013,22 +1018,10 @@ 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")) {
- if (!value)
- config_error_nonbool(key);
- else {
- free(opts->strategy);
- opts->strategy = xstrdup(value);
- }
- }
- else if (!strcmp(key, "options.gpg-sign")) {
- if (!value)
- config_error_nonbool(key);
- else {
- free(opts->gpg_sign);
- opts->gpg_sign = xstrdup(value);
- }
- }
+ else if (!strcmp(key, "options.strategy"))
+ git_config_string_dup(&opts->strategy, key, value);
+ else if (!strcmp(key, "options.gpg-sign"))
+ git_config_string_dup(&opts->gpg_sign, key, value);
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);
--
2.10.1.513.g00ef6dd
^ permalink raw reply
* [PATCH v4 01/25] sequencer: use static initializers for replay_opts
From: Johannes Schindelin @ 2016-10-14 13:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.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.1.513.g00ef6dd
^ permalink raw reply related
* Can we make interactive add easier to use?
From: Robert Dailey @ 2016-10-14 13:20 UTC (permalink / raw)
To: Git
Normally when I use interactive add, I just want to add files to the
index via simple numbers, instead of typing paths. So I'll do this as
quick as I can:
1. Type `git add -i`
2. Press `u` after prompt appears
3. Press numbers for the files I want to add, ENTER key
4. ENTER key again to go back to main add -i menu
5. Press `q` to exit interactive add
6. Type `git commit`
This feels very tedious. Is there a simplified workflow for this? I
remember using a "git index" unofficial extension to git that let you
do a `git status` that showed numbers next to each item (including
untracked files!) and you could do `git add 1, 2, 3-5`, etc.
Thoughts? Even though this feels like I'm complaining, honestly I am
just consulting the experts here to see if I'm missing out on some
usability features. Thanks in advance!!
^ permalink raw reply
* Re: [PATCH 2/2] resolve_ref_unsafe(): limit the number of "stat_ref" retries
From: Petr Stodulka @ 2016-10-14 13:20 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty
In-Reply-To: <1476451012-9925-3-git-send-email-pstodulk@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 2571 bytes --]
FYI,
I modified the patch slightly.
On 14.10.2016 15:16, Petr Stodulka wrote:
> From: Michael Haggerty <mhagger@alum.mit.edu>
>
> If there is a broken symlink where a loose reference file is expected,
> then the attempt to open() it fails with ENOENT. This error is
> misinterpreted to mean that the loose reference file itself has
> disappeared due to a race, causing the lookup to be retried. But in
> this scenario, the retries all suffer from the same problem, causing
> an infinite loop.
>
> So put a limit (of 5) on the number of times that the stat_ref step
> can be retried.
>
> Based-on-patch-by: Petr Stodulka <pstodulk@redhat.com>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs/files-backend.c | 6 ++++--
> refs/refs-internal.h | 6 ++++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index d16feb1..245a0b5 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1353,6 +1353,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
> int fd;
> int ret = -1;
> int save_errno;
> + int retries = 0;
>
> *type = 0;
> strbuf_reset(&sb_path);
> @@ -1390,7 +1391,8 @@ static int files_read_raw_ref(struct ref_store *ref_store,
> if (S_ISLNK(st.st_mode)) {
> strbuf_reset(&sb_contents);
> if (strbuf_readlink(&sb_contents, path, 0) < 0) {
> - if (errno == ENOENT || errno == EINVAL)
> + if ((errno == ENOENT || errno == EINVAL) &&
> + retries++ < MAXRETRIES)
> /* inconsistent with lstat; retry */
> goto stat_ref;
> else
> @@ -1426,7 +1428,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
> */
> fd = open(path, O_RDONLY);
> if (fd < 0) {
> - if (errno == ENOENT)
> + if (errno == ENOENT && retries++ < MAXRETRIES)
> /* inconsistent with lstat; retry */
> goto stat_ref;
> else
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 708b260..37e6b99 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -255,6 +255,12 @@ int rename_ref_available(const char *old_refname, const char *new_refname);
> /* We allow "recursive" symbolic refs. Only within reason, though */
> #define SYMREF_MAXDEPTH 5
>
> +/*
> + * We allow only MAXRETRIES tries to jump on stat_ref, because of possible
> + * infinite loop
> + */
> +#define MAXRETRIES 5
> +
> /* Include broken references in a do_for_each_ref*() iteration: */
> #define DO_FOR_EACH_INCLUDE_BROKEN 0x01
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH v4 06/25] sequencer: future-proof read_populate_todo()
From: Johannes Schindelin @ 2016-10-14 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.git.johannes.schindelin@gmx.de>
Over the next commits, we will work on improving the sequencer to the
point where it can process the todo 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 04c55f2..fb0b94b 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;
@@ -769,25 +774,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;
}
@@ -1075,7 +1079,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.1.513.g00ef6dd
^ permalink raw reply related
* [PATCH v4 10/25] sequencer: avoid completely different messages for different actions
From: Johannes Schindelin @ 2016-10-14 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.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 eac531b..9bca056 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -229,11 +229,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.1.513.g00ef6dd
^ permalink raw reply related
* [PATCH 2/2] resolve_ref_unsafe(): limit the number of "stat_ref" retries
From: Petr Stodulka @ 2016-10-14 13:16 UTC (permalink / raw)
To: git; +Cc: pstodulk, Michael Haggerty
In-Reply-To: <1476451012-9925-1-git-send-email-pstodulk@redhat.com>
From: Michael Haggerty <mhagger@alum.mit.edu>
If there is a broken symlink where a loose reference file is expected,
then the attempt to open() it fails with ENOENT. This error is
misinterpreted to mean that the loose reference file itself has
disappeared due to a race, causing the lookup to be retried. But in
this scenario, the retries all suffer from the same problem, causing
an infinite loop.
So put a limit (of 5) on the number of times that the stat_ref step
can be retried.
Based-on-patch-by: Petr Stodulka <pstodulk@redhat.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs/files-backend.c | 6 ++++--
refs/refs-internal.h | 6 ++++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index d16feb1..245a0b5 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1353,6 +1353,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
int fd;
int ret = -1;
int save_errno;
+ int retries = 0;
*type = 0;
strbuf_reset(&sb_path);
@@ -1390,7 +1391,8 @@ static int files_read_raw_ref(struct ref_store *ref_store,
if (S_ISLNK(st.st_mode)) {
strbuf_reset(&sb_contents);
if (strbuf_readlink(&sb_contents, path, 0) < 0) {
- if (errno == ENOENT || errno == EINVAL)
+ if ((errno == ENOENT || errno == EINVAL) &&
+ retries++ < MAXRETRIES)
/* inconsistent with lstat; retry */
goto stat_ref;
else
@@ -1426,7 +1428,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
*/
fd = open(path, O_RDONLY);
if (fd < 0) {
- if (errno == ENOENT)
+ if (errno == ENOENT && retries++ < MAXRETRIES)
/* inconsistent with lstat; retry */
goto stat_ref;
else
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 708b260..37e6b99 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -255,6 +255,12 @@ int rename_ref_available(const char *old_refname, const char *new_refname);
/* We allow "recursive" symbolic refs. Only within reason, though */
#define SYMREF_MAXDEPTH 5
+/*
+ * We allow only MAXRETRIES tries to jump on stat_ref, because of possible
+ * infinite loop
+ */
+#define MAXRETRIES 5
+
/* Include broken references in a do_for_each_ref*() iteration: */
#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
--
2.5.5
^ permalink raw reply related
* [PATCH 0/2] infinite loop in "git ls-tree" for broken symlink
From: Petr Stodulka @ 2016-10-14 13:16 UTC (permalink / raw)
To: git; +Cc: pstodulk
Hi,
I have realized that this wasn't fixed after all when refs.c
was "rewritten". Issue is caused by broken symlink under refs/heads,
which causes infinite loop for "git ls-tree" command. It was replied
earlier [0] and Michael previously fixed that in better way probably,
then my proposed patch 2/2, but it contains more changes and I have not
enough time to study changes in refs* code, so I propose now just my
little patch, which was previously modified by Michael.
If you prefer different solution, I am ok with this. It is really
just quick proposal. Patch 1/2 contains test for this issue. If you
will prefer different solution with different exit code, test should
be corrected. Basic idea is, that timeout should'n expire with
exit code 124.
[0] http://marc.info/?l=git&m=142712669103790&w=2
[1] https://github.com/mhagger/git, branch "ref-broken-symlinks"
Michael Haggerty (1):
resolve_ref_unsafe(): limit the number of "stat_ref" retries
Petr Stodulka (1):
Add test for ls-tree with broken symlink under refs/heads
refs/files-backend.c | 6 ++++--
refs/refs-internal.h | 6 ++++++
t/t3103-ls-tree-misc.sh | 9 +++++++++
3 files changed, 19 insertions(+), 2 deletions(-)
--
2.5.5
^ permalink raw reply
* [PATCH 1/2] Add test for ls-tree with broken symlink under refs/heads
From: Petr Stodulka @ 2016-10-14 13:16 UTC (permalink / raw)
To: git; +Cc: pstodulk
In-Reply-To: <1476451012-9925-1-git-send-email-pstodulk@redhat.com>
git ls-tree goes into an infinite loop while serving pretty vanilla requests,
if the refs/heads/ directory contains a symlink that's broken. Added test
which check if git ends with expected exit code or timeout expires.
---
t/t3103-ls-tree-misc.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh
index 09dcf04..faf79c4 100755
--- a/t/t3103-ls-tree-misc.sh
+++ b/t/t3103-ls-tree-misc.sh
@@ -21,4 +21,13 @@ test_expect_success 'ls-tree fails with non-zero exit code on broken tree' '
test_must_fail git ls-tree -r HEAD
'
+test_expect_success 'ls-tree fails due to broken symlink instead of infinite loop' '
+ mkdir foo_infinit &&
+ cd foo_infinit &&
+ git init testrepo &&
+ cd testrepo &&
+ mkdir -p .git/refs/remotes &&
+ ln -s ../remotes/foo .git/refs/heads/bar &&
+ test_expect_code 128 timeout 2 git ls-tree bar
+'
test_done
--
2.5.5
^ permalink raw reply related
* [PATCH v4 02/25] sequencer: use memoized sequencer directory path
From: Johannes Schindelin @ 2016-10-14 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.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 1cba3b7..9fddb19 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.1.513.g00ef6dd
^ permalink raw reply related
* [PATCH v4 07/25] sequencer: refactor the code to obtain a short commit name
From: Johannes Schindelin @ 2016-10-14 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.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 | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index fb0b94b..499f5ee 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -147,13 +147,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);
@@ -621,8 +626,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
error(opts->action == REPLAY_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;
--
2.10.1.513.g00ef6dd
^ permalink raw reply related
* [PATCH v4 05/25] sequencer: eventually release memory allocated for the option values
From: Johannes Schindelin @ 2016-10-14 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.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.
To remedy that, we now take custody of the option values in question,
requiring those values to be malloc()ed or strdup()ed (an alternative
approach, to add a list of pointers to malloc()ed data and to ask the
sequencer to release all of them in the end, was proposed earlier but
rejected during review).
Sadly, the current approach makes the code uglier, as we now have to
take care to strdup() the values passed via the command-line.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/revert.c | 4 ++++
sequencer.c | 26 ++++++++++++++++++++++----
sequencer.h | 6 +++---
3 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 7365559..ba5a88c 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -174,6 +174,10 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
if (argc > 1)
usage_with_options(usage_str, options);
+
+ /* These option values will be free()d */
+ opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
+ opts->strategy = xstrdup_or_null(opts->strategy);
}
int cmd_revert(int argc, const char **argv, const char *prefix)
diff --git a/sequencer.c b/sequencer.c
index 8d272fb..04c55f2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -117,6 +117,13 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
static void remove_sequencer_state(const struct replay_opts *opts)
{
struct strbuf dir = STRBUF_INIT;
+ int i;
+
+ free(opts->gpg_sign);
+ free(opts->strategy);
+ for (i = 0; i < opts->xopts_nr; i++)
+ free(opts->xopts[i]);
+ free(opts->xopts);
strbuf_addf(&dir, "%s", get_dir(opts));
remove_dir_recursively(&dir, 0);
@@ -280,7 +287,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
struct merge_options o;
struct tree *result, *next_tree, *base_tree, *head_tree;
int clean;
- const char **xopt;
+ char **xopt;
static struct lock_file index_lock;
hold_locked_index(&index_lock, 1);
@@ -583,7 +590,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
- res |= try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
+ res |= try_merge_command(opts->strategy,
+ opts->xopts_nr, (const char **)opts->xopts,
common, sha1_to_hex(head), remotes);
free_commit_list(common);
free_commit_list(remotes);
@@ -783,6 +791,16 @@ static int read_populate_todo(struct commit_list **todo_list,
return 0;
}
+static int git_config_string_dup(char **dest,
+ const char *var, const char *value)
+{
+ if (!value)
+ return config_error_nonbool(var);
+ free(*dest);
+ *dest = xstrdup(value);
+ return 0;
+}
+
static int populate_opts_cb(const char *key, const char *value, void *data)
{
struct replay_opts *opts = data;
@@ -803,9 +821,9 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
else if (!strcmp(key, "options.mainline"))
opts->mainline = git_config_int(key, value);
else if (!strcmp(key, "options.strategy"))
- git_config_string(&opts->strategy, key, value);
+ git_config_string_dup(&opts->strategy, key, value);
else if (!strcmp(key, "options.gpg-sign"))
- git_config_string(&opts->gpg_sign, key, value);
+ git_config_string_dup(&opts->gpg_sign, key, value);
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);
diff --git a/sequencer.h b/sequencer.h
index dd4d33a..8453669 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -34,11 +34,11 @@ struct replay_opts {
int mainline;
- const char *gpg_sign;
+ char *gpg_sign;
/* Merge strategy */
- const char *strategy;
- const char **xopts;
+ char *strategy;
+ char **xopts;
size_t xopts_nr, xopts_alloc;
/* Only used by REPLAY_NONE */
--
2.10.1.513.g00ef6dd
^ permalink raw reply related
* [PATCH v4 04/25] sequencer: future-proof remove_sequencer_state()
From: Johannes Schindelin @ 2016-10-14 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.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.1.513.g00ef6dd
^ permalink raw reply related
* [PATCH v4 03/25] sequencer: avoid unnecessary indirection
From: Johannes Schindelin @ 2016-10-14 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.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.1.513.g00ef6dd
^ permalink raw reply related
* [PATCH v4 09/25] sequencer: strip CR from the todo script
From: Johannes Schindelin @ 2016-10-14 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.git.johannes.schindelin@gmx.de>
It is not unheard of that editors on Windows write CR/LF even if the
file originally had only LF. This is particularly awkward for exec lines
of a rebase -i todo sheet. Take for example the insn "exec echo": The
shell script parser splits at the LF and leaves the CR attached to
"echo", which leads to the unknown command "echo\r".
Work around that by stripping CR when reading the todo commands, as we
already do for LF.
This happens to fix t9903.14 and .15 in MSYS1 environments (with the
rebase--helper patches based on this patch series): the todo script
constructed in such a setup contains CR/LF thanks to MSYS1 runtime's
cleverness.
Based on a report and a patch by Johannes Sixt.
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 f797e8a..eac531b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -776,6 +776,9 @@ static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
next_p = *eol ? eol + 1 /* strip LF */ : eol;
+ if (p != eol && eol[-1] == '\r')
+ eol--; /* strip Carriage Return */
+
item = append_new_todo(todo_list);
item->offset_in_buf = p - todo_list->buf.buf;
if (parse_insn_line(item, p, eol)) {
--
2.10.1.513.g00ef6dd
^ permalink raw reply related
* [PATCH v4 08/25] sequencer: completely revamp the "todo" script parsing
From: Johannes Schindelin @ 2016-10-14 13:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1476450940.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 when it comes to writing the file with the remaining
commands, 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 | 284 ++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 163 insertions(+), 121 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 499f5ee..f797e8a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -470,7 +470,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;
@@ -529,10 +548,11 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
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 */
+ /* TRANSLATORS: The first %s will be a "todo" command like
+ "revert" or "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"),
@@ -545,7 +565,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;
@@ -586,7 +606,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)
@@ -613,17 +633,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"),
short_commit_name(commit), msg.subject);
@@ -684,116 +704,122 @@ 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;
+static 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;
- char *p = buf;
- int i;
+ struct todo_item *item;
+ char *p = buf, *next_p;
+ int i, res = 0;
- for (i = 1; *p; i++) {
+ for (i = 1; *p; i++, p = next_p) {
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);
- p = *eol ? eol + 1 : eol;
+
+ next_p = *eol ? eol + 1 /* strip LF */ : eol;
+
+ 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;
+ }
}
- 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;
@@ -860,18 +886,31 @@ 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,
+ short_commit_name(commit), subject_len, subject);
+ unuse_commit_buffer(commit, commit_buffer);
+ }
return 0;
}
@@ -979,30 +1018,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;
}
@@ -1041,9 +1072,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);
@@ -1053,10 +1083,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;
}
@@ -1081,38 +1113,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);
@@ -1187,7 +1227,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.1.513.g00ef6dd
^ 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