* [PATCH v17 3/7] wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
From: Pranit Bauva via GitGitGadget @ 2019-01-02 15:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pranit Bauva
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.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 8f27f3375b..310eefe9e8 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -34,22 +34,6 @@
#include "packfile.h"
#include "repository.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.
*/
@@ -1220,7 +1204,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."));
die_user_resolve(state);
}
@@ -1803,7 +1787,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 ca36b44ee0..9ff7e56b74 100644
--- a/cache.h
+++ b/cache.h
@@ -1788,4 +1788,7 @@ void safe_create_dir(const char *dir, int share);
*/
extern int print_sha1_ellipsis(void);
+/* 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 e4fa9d84cd..ea3cf64d4c 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -690,3 +690,16 @@ int xgethostname(char *buf, size_t len)
buf[len - 1] = 0;
return ret;
}
+
+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;
+}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v17 7/7] bisect--helper: `bisect_start` shell function partially in C
From: Pranit Bauva via GitGitGadget @ 2019-01-02 15:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pranit Bauva
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
Reimplement the `bisect_start` shell function partially in C and add
`bisect-start` subcommand to `git bisect--helper` to call it from
git-bisect.sh .
The last part is not converted because it calls another shell function.
`bisect_start` shell function will be completed after the `bisect_next`
shell function is ported in C.
Using `--bisect-start` 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 and will be called by some
other methods.
Also introduce a method `bisect_append_log_quoted` to keep things short
and crisp.
Note that we are a bit lax about command-line parsing because the helper
is not supposed to be called by the user directly (but only from the git
bisect script).
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Stephan Beyer <s-beyer@gmx.net>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
builtin/bisect--helper.c | 230 ++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 132 +---------------------
2 files changed, 230 insertions(+), 132 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d61edd3c91..22e669e3b1 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -7,6 +7,7 @@
#include "argv-array.h"
#include "run-command.h"
#include "prompt.h"
+#include "quote.h"
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -14,6 +15,8 @@ static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
+static GIT_PATH_FUNC(git_path_head_name, "head-name")
+static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
@@ -24,6 +27,8 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
+ N_("git bisect--helper --bisect-start [--term-{old,good}=<term> --term-{new,bad}=<term>]"
+ "[--no-checkout] [<bad> [<good>...]] [--] [<paths>...]"),
NULL
};
@@ -389,6 +394,219 @@ static int bisect_terms(struct bisect_terms *terms, const char *option)
return 0;
}
+static int bisect_append_log_quoted(const char **argv)
+{
+ int retval = 0;
+ FILE *fp = fopen(git_path_bisect_log(), "a");
+ struct strbuf orig_args = STRBUF_INIT;
+
+ if (!fp)
+ return -1;
+
+ if (fprintf(fp, "git bisect start") < 1) {
+ retval = -1;
+ goto finish;
+ }
+
+ sq_quote_argv(&orig_args, argv);
+ if (fprintf(fp, "%s\n", orig_args.buf) < 1)
+ retval = -1;
+
+finish:
+ fclose(fp);
+ strbuf_release(&orig_args);
+ return retval;
+}
+
+static int bisect_start(struct bisect_terms *terms, int no_checkout,
+ const char **argv, int argc)
+{
+ int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
+ int flags, pathspec_pos, retval = 0;
+ struct string_list revs = STRING_LIST_INIT_DUP;
+ struct string_list states = STRING_LIST_INIT_DUP;
+ struct strbuf start_head = STRBUF_INIT;
+ struct strbuf bisect_names = STRBUF_INIT;
+ struct object_id head_oid;
+ struct object_id oid;
+ const char *head;
+
+ if (is_bare_repository())
+ no_checkout = 1;
+
+ /*
+ * Check for one bad and then some good revisions
+ */
+ for (i = 0; i < argc; i++) {
+ if (!strcmp(argv[i], "--")) {
+ has_double_dash = 1;
+ break;
+ }
+ }
+
+ for (i = 0; i < argc; i++) {
+ const char *arg = argv[i];
+ if (!strcmp(argv[i], "--")) {
+ break;
+ } else if (!strcmp(arg, "--no-checkout")) {
+ no_checkout = 1;
+ } else if (!strcmp(arg, "--term-good") ||
+ !strcmp(arg, "--term-old")) {
+ must_write_terms = 1;
+ free((void *) terms->term_good);
+ terms->term_good = xstrdup(argv[++i]);
+ } else if (skip_prefix(arg, "--term-good=", &arg) ||
+ skip_prefix(arg, "--term-old=", &arg)) {
+ must_write_terms = 1;
+ free((void *) terms->term_good);
+ terms->term_good = xstrdup(arg);
+ } else if (!strcmp(arg, "--term-bad") ||
+ !strcmp(arg, "--term-new")) {
+ must_write_terms = 1;
+ free((void *) terms->term_bad);
+ terms->term_bad = xstrdup(argv[++i]);
+ } else if (skip_prefix(arg, "--term-bad=", &arg) ||
+ skip_prefix(arg, "--term-new=", &arg)) {
+ must_write_terms = 1;
+ free((void *) terms->term_bad);
+ terms->term_bad = xstrdup(arg);
+ } else if (starts_with(arg, "--") &&
+ !one_of(arg, "--term-good", "--term-bad", NULL)) {
+ return error(_("unrecognized option: '%s'"), arg);
+ } else {
+ char *commit_id = xstrfmt("%s^{commit}", arg);
+ if (get_oid(commit_id, &oid) && has_double_dash)
+ die(_("'%s' does not appear to be a valid "
+ "revision"), arg);
+
+ string_list_append(&revs, oid_to_hex(&oid));
+ free(commit_id);
+ }
+ }
+ pathspec_pos = i;
+
+ /*
+ * The user ran "git bisect start <sha1> <sha1>", hence did not
+ * explicitly specify the terms, but we are already starting to
+ * set references named with the default terms, and won't be able
+ * to change afterwards.
+ */
+ if (revs.nr)
+ must_write_terms = 1;
+ for (i = 0; i < revs.nr; i++) {
+ if (bad_seen) {
+ string_list_append(&states, terms->term_good);
+ } else {
+ bad_seen = 1;
+ string_list_append(&states, terms->term_bad);
+ }
+ }
+
+ /*
+ * Verify HEAD
+ */
+ head = resolve_ref_unsafe("HEAD", 0, &head_oid, &flags);
+ if (!head)
+ if (get_oid("HEAD", &head_oid))
+ return error(_("bad HEAD - I need a HEAD"));
+
+ /*
+ * Check if we are bisecting
+ */
+ if (!is_empty_or_missing_file(git_path_bisect_start())) {
+ /* Reset to the rev from where we started */
+ strbuf_read_file(&start_head, git_path_bisect_start(), 0);
+ strbuf_trim(&start_head);
+ if (!no_checkout) {
+ struct argv_array argv = ARGV_ARRAY_INIT;
+
+ argv_array_pushl(&argv, "checkout", start_head.buf,
+ "--", NULL);
+ if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
+ retval = error(_("checking out '%s' failed."
+ " Try 'git bisect start "
+ "<valid-branch>'."),
+ start_head.buf);
+ goto finish;
+ }
+ }
+ } else {
+ /* Get the rev from where we start. */
+ if (!get_oid(head, &head_oid) &&
+ !starts_with(head, "refs/heads/")) {
+ strbuf_reset(&start_head);
+ strbuf_addstr(&start_head, oid_to_hex(&head_oid));
+ } else if (!get_oid(head, &head_oid) &&
+ skip_prefix(head, "refs/heads/", &head)) {
+ /*
+ * This error message should only be triggered by
+ * cogito usage, and cogito users should understand
+ * it relates to cg-seek.
+ */
+ if (!is_empty_or_missing_file(git_path_head_name()))
+ return error(_("won't bisect on cg-seek'ed tree"));
+ strbuf_addstr(&start_head, head);
+ } else {
+ return error(_("bad HEAD - strange symbolic ref"));
+ }
+ }
+
+ /*
+ * Get rid of any old bisect state.
+ */
+ if (bisect_clean_state())
+ return -1;
+
+ /*
+ * In case of mistaken revs or checkout error, or signals received,
+ * "bisect_auto_next" below may exit or misbehave.
+ * We have to trap this to be able to clean up using
+ * "bisect_clean_state".
+ */
+
+ /*
+ * Write new start state
+ */
+ write_file(git_path_bisect_start(), "%s\n", start_head.buf);
+
+ if (no_checkout) {
+ get_oid(start_head.buf, &oid);
+ if (update_ref(NULL, "BISECT_HEAD", &oid, NULL, 0,
+ UPDATE_REFS_MSG_ON_ERR)) {
+ retval = -1;
+ goto finish;
+ }
+ }
+
+ if (pathspec_pos < argc - 1)
+ sq_quote_argv(&bisect_names, argv + pathspec_pos);
+ write_file(git_path_bisect_names(), "%s\n", bisect_names.buf);
+
+ for (i = 0; i < states.nr; i++)
+ if (bisect_write(states.items[i].string,
+ revs.items[i].string, terms, 1)) {
+ retval = -1;
+ goto finish;
+ }
+
+ if (must_write_terms && write_terms(terms->term_bad,
+ terms->term_good)) {
+ retval = -1;
+ goto finish;
+ }
+
+ retval = bisect_append_log_quoted(argv);
+ if (retval)
+ retval = -1;
+
+finish:
+ string_list_clear(&revs, 0);
+ string_list_clear(&states, 0);
+ strbuf_release(&start_head);
+ strbuf_release(&bisect_names);
+ return retval;
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -400,7 +618,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_WRITE,
CHECK_AND_SET_TERMS,
BISECT_NEXT_CHECK,
- BISECT_TERMS
+ BISECT_TERMS,
+ BISECT_START
} cmdmode = 0;
int no_checkout = 0, res = 0, nolog = 0;
struct option options[] = {
@@ -422,6 +641,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
OPT_CMDMODE(0, "bisect-terms", &cmdmode,
N_("print out the bisect terms"), BISECT_TERMS),
+ OPT_CMDMODE(0, "bisect-start", &cmdmode,
+ N_("start the bisect session"), BISECT_START),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
OPT_BOOL(0, "no-log", &nolog,
@@ -431,7 +652,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
argc = parse_options(argc, argv, prefix, options,
- git_bisect_helper_usage, PARSE_OPT_KEEP_UNKNOWN);
+ git_bisect_helper_usage,
+ PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_UNKNOWN);
if (!cmdmode)
usage_with_options(git_bisect_helper_usage, options);
@@ -477,6 +699,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
return error(_("--bisect-terms requires 0 or 1 argument"));
res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL);
break;
+ case BISECT_START:
+ set_terms(&terms, "bad", "good");
+ res = bisect_start(&terms, no_checkout, argv, argc);
+ break;
default:
return error("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index bdb614e3c2..efee12b8b1 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -71,122 +71,7 @@ bisect_autostart() {
}
bisect_start() {
- #
- # Check for one bad and then some good revisions.
- #
- has_double_dash=0
- for arg; do
- case "$arg" in --) has_double_dash=1; break ;; esac
- done
- orig_args=$(git rev-parse --sq-quote "$@")
- bad_seen=0
- eval=''
- must_write_terms=0
- revs=''
- if test "z$(git rev-parse --is-bare-repository)" != zfalse
- then
- mode=--no-checkout
- else
- mode=''
- fi
- while [ $# -gt 0 ]; do
- arg="$1"
- case "$arg" in
- --)
- shift
- break
- ;;
- --no-checkout)
- mode=--no-checkout
- shift ;;
- --term-good|--term-old)
- shift
- must_write_terms=1
- TERM_GOOD=$1
- shift ;;
- --term-good=*|--term-old=*)
- must_write_terms=1
- TERM_GOOD=${1#*=}
- shift ;;
- --term-bad|--term-new)
- shift
- must_write_terms=1
- TERM_BAD=$1
- shift ;;
- --term-bad=*|--term-new=*)
- must_write_terms=1
- TERM_BAD=${1#*=}
- shift ;;
- --*)
- die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
- *)
- rev=$(git rev-parse -q --verify "$arg^{commit}") || {
- test $has_double_dash -eq 1 &&
- die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
- break
- }
- revs="$revs $rev"
- shift
- ;;
- esac
- done
-
- for rev in $revs
- do
- # The user ran "git bisect start <sha1>
- # <sha1>", hence did not explicitly specify
- # the terms, but we are already starting to
- # set references named with the default terms,
- # and won't be able to change afterwards.
- must_write_terms=1
-
- case $bad_seen in
- 0) state=$TERM_BAD ; bad_seen=1 ;;
- *) state=$TERM_GOOD ;;
- esac
- eval="$eval git bisect--helper --bisect-write '$state' '$rev' '$TERM_GOOD' '$TERM_BAD' 'nolog' &&"
- done
- #
- # Verify HEAD.
- #
- head=$(GIT_DIR="$GIT_DIR" git symbolic-ref -q HEAD) ||
- head=$(GIT_DIR="$GIT_DIR" git rev-parse --verify HEAD) ||
- die "$(gettext "Bad HEAD - I need a HEAD")"
-
- #
- # Check if we are bisecting.
- #
- start_head=''
- if test -s "$GIT_DIR/BISECT_START"
- then
- # Reset to the rev from where we started.
- start_head=$(cat "$GIT_DIR/BISECT_START")
- if test "z$mode" != "z--no-checkout"
- then
- git checkout "$start_head" -- ||
- die "$(eval_gettext "Checking out '\$start_head' failed. Try 'git bisect reset <valid-branch>'.")"
- fi
- else
- # Get rev from where we start.
- case "$head" in
- refs/heads/*|$_x40)
- # This error message should only be triggered by
- # cogito usage, and cogito users should understand
- # it relates to cg-seek.
- [ -s "$GIT_DIR/head-name" ] &&
- die "$(gettext "won't bisect on cg-seek'ed tree")"
- start_head="${head#refs/heads/}"
- ;;
- *)
- die "$(gettext "Bad HEAD - strange symbolic ref")"
- ;;
- esac
- fi
-
- #
- # Get rid of any old bisect state.
- #
- git bisect--helper --bisect-clean-state || exit
+ git bisect--helper --bisect-start $@ || exit
#
# Change state.
@@ -198,23 +83,10 @@ bisect_start() {
trap 'git bisect--helper --bisect-clean-state' 0
trap 'exit 255' 1 2 3 15
- #
- # Write new start state.
- #
- echo "$start_head" >"$GIT_DIR/BISECT_START" && {
- test "z$mode" != "z--no-checkout" ||
- git update-ref --no-deref BISECT_HEAD "$start_head"
- } &&
- git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
- eval "$eval true" &&
- if test $must_write_terms -eq 1
- then
- git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD" || exit
- fi &&
- echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
#
# Check if we can proceed to the next bisect state.
#
+ get_terms
bisect_auto_next
trap '-' 0
--
gitgitgadget
^ permalink raw reply related
* [PATCH v17 5/7] bisect--helper: `bisect_next_check` shell function in C
From: Pranit Bauva via GitGitGadget @ 2019-01-02 15:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pranit Bauva
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
Reimplement `bisect_next_check` shell function in C and add
`bisect-next-check` subcommand to `git bisect--helper` to call it from
git-bisect.sh .
`bisect_voc` shell function is no longer useful now and is replaced by
using a char *[] of "new|bad" and "good|old" values.
Using `--bisect-next-check` 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 but its implementation will
be called by some other methods.
Helped-by: Stephan Beyer <s-beyer@gmx.net>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
builtin/bisect--helper.c | 88 +++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 60 ++-------------------------
2 files changed, 91 insertions(+), 57 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 02ea5a9611..38ae825b9a 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -6,6 +6,7 @@
#include "dir.h"
#include "argv-array.h"
#include "run-command.h"
+#include "prompt.h"
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -21,6 +22,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-reset [<commit>]"),
N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
+ N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
NULL
};
@@ -44,6 +46,9 @@ static void set_terms(struct bisect_terms *terms, const char *bad,
terms->term_bad = xstrdup(bad);
}
+static const char *vocab_bad = "bad|new";
+static const char *vocab_good = "good|old";
+
/*
* Check whether the string `term` belongs to the set of strings
* included in the variable arguments.
@@ -262,6 +267,78 @@ static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)
return 0;
}
+static int mark_good(const char *refname, const struct object_id *oid,
+ int flag, void *cb_data)
+{
+ int *m_good = (int *)cb_data;
+ *m_good = 0;
+ return 1;
+}
+
+static const char *need_bad_and_good_revision_warning =
+ N_("You need to give me at least one %s and %s revision.\n"
+ "You can use \"git bisect %s\" and \"git bisect %s\" for that.");
+
+static const char *need_bisect_start_warning =
+ N_("You need to start by \"git bisect start\".\n"
+ "You then need to give me at least one %s and %s revision.\n"
+ "You can use \"git bisect %s\" and \"git bisect %s\" for that.");
+
+static int bisect_next_check(const struct bisect_terms *terms,
+ const char *current_term)
+{
+ int missing_good = 1, missing_bad = 1, retval = 0;
+ const char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
+ const char *good_glob = xstrfmt("%s-*", terms->term_good);
+
+ if (ref_exists(bad_ref))
+ missing_bad = 0;
+
+ for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
+ (void *) &missing_good);
+
+ if (!missing_good && !missing_bad)
+ goto finish;
+
+ if (!current_term) {
+ retval = -1;
+ goto finish;
+ }
+
+ if (missing_good && !missing_bad &&
+ !strcmp(current_term, terms->term_good)) {
+ char *yesno;
+ /*
+ * have bad (or new) but not good (or old). We could bisect
+ * although this is less optimum.
+ */
+ warning(_("bisecting only with a %s commit"), terms->term_bad);
+ if (!isatty(0))
+ goto finish;
+ /*
+ * TRANSLATORS: Make sure to include [Y] and [n] in your
+ * translation. The program will only accept English input
+ * at this point.
+ */
+ yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
+ if (starts_with(yesno, "N") || starts_with(yesno, "n"))
+ retval = -1;
+ goto finish;
+ }
+ if (!is_empty_or_missing_file(git_path_bisect_start())) {
+ retval = error(_(need_bad_and_good_revision_warning),
+ vocab_bad, vocab_good, vocab_bad, vocab_good);
+ } else {
+ retval = error(_(need_bisect_start_warning),
+ vocab_good, vocab_bad, vocab_good, vocab_bad);
+ }
+
+finish:
+ free((void *) good_glob);
+ free((void *) bad_ref);
+ return retval;
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -271,7 +348,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
CHECK_EXPECTED_REVS,
BISECT_RESET,
BISECT_WRITE,
- CHECK_AND_SET_TERMS
+ CHECK_AND_SET_TERMS,
+ BISECT_NEXT_CHECK
} cmdmode = 0;
int no_checkout = 0, res = 0, nolog = 0;
struct option options[] = {
@@ -289,6 +367,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
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_CMDMODE(0, "bisect-next-check", &cmdmode,
+ N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
OPT_BOOL(0, "no-log", &nolog,
@@ -333,6 +413,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
set_terms(&terms, argv[2], argv[1]);
res = check_and_set_terms(&terms, argv[0]);
break;
+ case BISECT_NEXT_CHECK:
+ if (argc != 2 && argc != 3)
+ return error(_("--bisect-next-check requires 2 or 3 arguments"));
+ set_terms(&terms, argv[1], argv[0]);
+ res = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);
+ break;
default:
return error("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 9e993a8187..5ef3e25621 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -271,59 +271,14 @@ bisect_state() {
bisect_auto_next
}
-bisect_next_check() {
- missing_good= missing_bad=
- git show-ref -q --verify refs/bisect/$TERM_BAD || missing_bad=t
- test -n "$(git for-each-ref "refs/bisect/$TERM_GOOD-*")" || missing_good=t
-
- case "$missing_good,$missing_bad,$1" in
- ,,*)
- : have both $TERM_GOOD and $TERM_BAD - ok
- ;;
- *,)
- # do not have both but not asked to fail - just report.
- false
- ;;
- t,,"$TERM_GOOD")
- # have bad (or new) but not good (or old). we could bisect although
- # this is less optimum.
- eval_gettextln "Warning: bisecting only with a \$TERM_BAD commit." >&2
- if test -t 0
- then
- # TRANSLATORS: Make sure to include [Y] and [n] in your
- # translation. The program will only accept English input
- # at this point.
- gettext "Are you sure [Y/n]? " >&2
- read yesno
- case "$yesno" in [Nn]*) exit 1 ;; esac
- fi
- : bisect without $TERM_GOOD...
- ;;
- *)
- bad_syn=$(bisect_voc bad)
- good_syn=$(bisect_voc good)
- if test -s "$GIT_DIR/BISECT_START"
- then
-
- eval_gettextln "You need to give me at least one \$bad_syn and one \$good_syn revision.
-(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
- else
- eval_gettextln "You need to start by \"git bisect start\".
-You then need to give me at least one \$good_syn and one \$bad_syn revision.
-(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
- fi
- exit 1 ;;
- esac
-}
-
bisect_auto_next() {
- bisect_next_check && bisect_next || :
+ git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD && bisect_next || :
}
bisect_next() {
case "$#" in 0) ;; *) usage ;; esac
bisect_autostart
- bisect_next_check $TERM_GOOD
+ git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD $TERM_GOOD|| exit
# Perform all bisection computation, display and checkout
git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo --no-checkout)
@@ -355,7 +310,7 @@ bisect_next() {
}
bisect_visualize() {
- bisect_next_check fail
+ git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
if test $# = 0
then
@@ -409,7 +364,7 @@ bisect_replay () {
}
bisect_run () {
- bisect_next_check fail
+ git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
test -n "$*" || die "$(gettext "bisect run failed: no command provided.")"
@@ -484,13 +439,6 @@ get_terms () {
fi
}
-bisect_voc () {
- case "$1" in
- bad) echo "bad|new" ;;
- good) echo "good|old" ;;
- esac
-}
-
bisect_terms () {
get_terms
if ! test -s "$GIT_DIR/BISECT_TERMS"
--
gitgitgadget
^ permalink raw reply related
* [PATCH v17 4/7] bisect--helper: `check_and_set_terms` shell function in C
From: Pranit Bauva via GitGitGadget @ 2019-01-02 15:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pranit Bauva
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.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.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
builtin/bisect--helper.c | 39 ++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 36 ++++--------------------------------
2 files changed, 42 insertions(+), 33 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index df821be4b2..02ea5a9611 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 [--no-log] <state> <revision> <good_term> <bad_term>"),
+ N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
NULL
};
@@ -234,6 +235,33 @@ static int bisect_write(const char *state, const char *rev,
return retval;
}
+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)) {
+ set_terms(terms, "bad", "good");
+ return write_terms(terms->term_bad, terms->term_good);
+ }
+ if (one_of(cmd, "new", "old", NULL)) {
+ set_terms(terms, "new", "old");
+ return write_terms(terms->term_bad, terms->term_good);
+ }
+ }
+
+ return 0;
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -242,7 +270,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_CLEAN_STATE,
CHECK_EXPECTED_REVS,
BISECT_RESET,
- BISECT_WRITE
+ BISECT_WRITE,
+ CHECK_AND_SET_TERMS
} cmdmode = 0;
int no_checkout = 0, res = 0, nolog = 0;
struct option options[] = {
@@ -258,6 +287,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("reset the bisection state"), BISECT_RESET),
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_BOOL(0, "no-log", &nolog,
@@ -296,6 +327,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
set_terms(&terms, argv[3], argv[2]);
res = bisect_write(argv[0], argv[1], &terms, nolog);
break;
+ case CHECK_AND_SET_TERMS:
+ if (argc != 3)
+ return error(_("--check-and-set-terms requires 3 arguments"));
+ set_terms(&terms, argv[2], argv[1]);
+ res = check_and_set_terms(&terms, argv[0]);
+ break;
default:
return error("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index ebf445223c..9e993a8187 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"
@@ -482,36 +484,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" ;;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v17 2/7] bisect--helper: `bisect_write` shell function in C
From: Pranit Bauva via GitGitGadget @ 2019-01-02 15:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pranit Bauva
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.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.
Add a log_commit() helper function to write the contents of the commit message
header to a file which will be re-used in future parts of the code as
well.
Also introduce a function free_terms() to free the memory of `struct
bisect_terms` and set_terms() to set the values of members in `struct
bisect_terms`.
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
builtin/bisect--helper.c | 105 +++++++++++++++++++++++++++++++++++++--
git-bisect.sh | 25 ++--------
2 files changed, 106 insertions(+), 24 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index aa6495dc84..df821be4b2 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -12,15 +12,37 @@ 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_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
+static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
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"),
N_("git bisect--helper --bisect-reset [<commit>]"),
+ N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
NULL
};
+struct bisect_terms {
+ char *term_good;
+ char *term_bad;
+};
+
+static void free_terms(struct bisect_terms *terms)
+{
+ FREE_AND_NULL(terms->term_good);
+ FREE_AND_NULL(terms->term_bad);
+}
+
+static void set_terms(struct bisect_terms *terms, const char *bad,
+ const char *good)
+{
+ free((void *)terms->term_good);
+ terms->term_good = xstrdup(good);
+ free((void *)terms->term_bad);
+ terms->term_bad = xstrdup(bad);
+}
+
/*
* Check whether the string `term` belongs to the set of strings
* included in the variable arguments.
@@ -148,6 +170,70 @@ static int bisect_reset(const char *commit)
return bisect_clean_state();
}
+static void log_commit(FILE *fp, char *fmt, const char *state,
+ struct commit *commit)
+{
+ struct pretty_print_context pp = {0};
+ struct strbuf commit_msg = STRBUF_INIT;
+ char *label = xstrfmt(fmt, state);
+
+ format_commit_message(commit, "%s", &commit_msg, &pp);
+
+ fprintf(fp, "# %s: [%s] %s\n", label, oid_to_hex(&commit->object.oid),
+ commit_msg.buf);
+
+ strbuf_release(&commit_msg);
+ free(label);
+}
+
+static int bisect_write(const char *state, const char *rev,
+ const struct bisect_terms *terms, int nolog)
+{
+ struct strbuf tag = STRBUF_INIT;
+ struct object_id oid;
+ struct commit *commit;
+ 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 {
+ retval = error(_("Bad bisect_write argument: %s"), state);
+ goto finish;
+ }
+
+ if (get_oid(rev, &oid)) {
+ retval = error(_("couldn't get the oid of the rev '%s'"), rev);
+ goto finish;
+ }
+
+ if (update_ref(NULL, tag.buf, &oid, NULL, 0,
+ UPDATE_REFS_MSG_ON_ERR)) {
+ retval = -1;
+ goto finish;
+ }
+
+ fp = fopen(git_path_bisect_log(), "a");
+ if (!fp) {
+ retval = error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
+ goto finish;
+ }
+
+ commit = lookup_commit_reference(the_repository, &oid);
+ log_commit(fp, "%s", state, commit);
+
+ if (!nolog)
+ fprintf(fp, "git bisect %s %s\n", state, rev);
+
+finish:
+ if (fp)
+ fclose(fp);
+ strbuf_release(&tag);
+ return retval;
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -155,9 +241,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
WRITE_TERMS,
BISECT_CLEAN_STATE,
CHECK_EXPECTED_REVS,
- BISECT_RESET
+ BISECT_RESET,
+ BISECT_WRITE
} cmdmode = 0;
- int no_checkout = 0;
+ int no_checkout = 0, res = 0, nolog = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", &cmdmode,
N_("perform 'git bisect next'"), NEXT_ALL),
@@ -169,10 +256,15 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("check for expected revs"), CHECK_EXPECTED_REVS),
OPT_CMDMODE(0, "bisect-reset", &cmdmode,
N_("reset the bisection state"), BISECT_RESET),
+ 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_BOOL(0, "no-log", &nolog,
+ N_("no log for BISECT_WRITE ")),
OPT_END()
};
+ struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
argc = parse_options(argc, argv, prefix, options,
git_bisect_helper_usage, 0);
@@ -198,8 +290,15 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
if (argc > 1)
return error(_("--bisect-reset requires either no argument or a commit"));
return !!bisect_reset(argc ? argv[0] : NULL);
+ case BISECT_WRITE:
+ if (argc != 4 && argc != 5)
+ return error(_("--bisect-write requires either 4 or 5 arguments"));
+ set_terms(&terms, argv[3], argv[2]);
+ res = bisect_write(argv[0], argv[1], &terms, nolog);
+ break;
default:
return error("BUG: unknown subcommand '%d'", cmdmode);
}
- return 0;
+ free_terms(&terms);
+ return !!res;
}
diff --git a/git-bisect.sh b/git-bisect.sh
index afbfbc1f8e..ebf445223c 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 ;;
*)
--
gitgitgadget
^ permalink raw reply related
* [PATCH v17 0/7] git bisect: convert from shell to C
From: Tanushree Tumane via GitGitGadget @ 2019-01-02 15:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>
Changes since Pranit's v16:
===========================
bisect--helper: bisect_reset shell function in C
================================================
* The return !printf(...); construct was considered unsafe and disentangled
into printf(...); return 0;
* It is more elegant to release branch and argv first, and then return
error(...) than to error(...); release branch; clear argv; return -1;
* Prefix !! to bisect_reset(...) function call in cmd_bisect__helper(...)
because its return value will be given to exit()
bisect--helper: bisect_write shell function in C
================================================
* The option [--no-log] is passed before <state>.
* Remove const from struct bisect_terms variables because const pointers
refers to memory owned by another code path.
* Use FREE_AND_NULL(...) in free_terms(...).
* It makes more sense to free terms before set in set_terms(...) to avoid
memory leak.
* Remove fail: retval = -1; for cleaner code flow, replace goto fail; with
retval = -1; goto finish; or retval = error(...); goto finish;
* It is more consistent to introduce --no-log as option like --no-checkout.
* Initialize struct bisect_terms terms in cmd_bisect__helper(...).
* The return res; in cmd_bisect__helper(...) may return -1, so prefixed !!
to handle such cases.
bisect--helper: check_and_set_terms shell function in C
=======================================================
* free_terms() before set_terms() is obsolete as set_terms() frees terms
before set.
bisect--helper: bisect_next_check shell function in C
=====================================================
* [<term>] is first argument of bisect-next-check.
* vocab_good and vocab_bad for easier readability.
* Declare *_warning char pointers.
* Remove fail: label, its only job was to set retval = -1;.
bisect--helper: get_terms & bisect_terms shell function in C
============================================================
* Remove fail: label, its only job was to set res = -1;.
* Parse the argv in cmd_bisect__helper() and pass option to bisect_terms().
bisect--helper: bisect_start shell function partially in C
==========================================================
* Remove fail: label, its only job was to set retval = -1;.
* Spell correction unrecognised to unrecognized.
* if (revs.nr) must_write_terms = 1; is more readable than must_write_terms
|= !!revs.nr;.
* error() messages starts with small letters.
* sha1_to_hex() is old practice.
* sq_quote_argv() now requires only two args.
* Instead of nesting ifs, use &&.
Pranit Bauva (7):
bisect--helper: `bisect_reset` shell function in C
bisect--helper: `bisect_write` shell function in C
wrapper: move is_empty_file() and rename it as
is_empty_or_missing_file()
bisect--helper: `check_and_set_terms` shell function in C
bisect--helper: `bisect_next_check` shell function in C
bisect--helper: `get_terms` & `bisect_terms` shell function in C
bisect--helper: `bisect_start` shell function partially in C
builtin/am.c | 20 +-
builtin/bisect--helper.c | 563 +++++++++++++++++++++++++++++++++++-
cache.h | 3 +
git-bisect.sh | 314 ++------------------
t/t6030-bisect-porcelain.sh | 2 +-
wrapper.c | 13 +
6 files changed, 595 insertions(+), 320 deletions(-)
base-commit: 7f4e64169352e03476b0ea64e7e2973669e491a2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-101%2Ftanushree27%2Fgit-bisect_part2_fixup-v17
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-101/tanushree27/git-bisect_part2_fixup-v17
Pull-Request: https://github.com/gitgitgadget/git/pull/101
Range-diff vs v16:
1: f1e89ba517 ! 1: 338ebdc97a bisect--helper: `bisect_reset` shell function in C
@@ -16,8 +16,9 @@
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
+ Mentored by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
+ Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
--- a/builtin/bisect--helper.c
@@ -53,8 +54,10 @@
+ struct strbuf branch = STRBUF_INIT;
+
+ if (!commit) {
-+ if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1)
-+ return !printf(_("We are not bisecting.\n"));
++ if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1) {
++ printf(_("We are not bisecting.\n"));
++ return 0;
++ }
+ strbuf_rtrim(&branch);
+ } else {
+ struct object_id oid;
@@ -69,11 +72,11 @@
+
+ argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL);
+ if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
-+ error(_("Could not check out original HEAD '%s'. Try "
-+ "'git bisect reset <commit>'."), branch.buf);
+ strbuf_release(&branch);
+ argv_array_clear(&argv);
-+ return -1;
++ return error(_("could not check out original"
++ " HEAD '%s'. Try 'git bisect"
++ "reset <commit>'."), branch.buf);
+ }
+ argv_array_clear(&argv);
+ }
@@ -110,7 +113,7 @@
+ case BISECT_RESET:
+ if (argc > 1)
+ return error(_("--bisect-reset requires either no argument or a commit"));
-+ return bisect_reset(argc ? argv[0] : NULL);
++ return !!bisect_reset(argc ? argv[0] : NULL);
default:
return error("BUG: unknown subcommand '%d'", cmdmode);
}
2: da4ac60ad4 ! 2: 7d6291d9dd bisect--helper: `bisect_write` shell function in C
@@ -27,8 +27,9 @@
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
+ Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
+ Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
--- a/builtin/bisect--helper.c
@@ -44,27 +45,27 @@
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> <good_term> <bad_term> [<nolog>]"),
++ N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
NULL
};
+struct bisect_terms {
-+ const char *term_good;
-+ const char *term_bad;
++ char *term_good;
++ char *term_bad;
+};
+
+static void free_terms(struct bisect_terms *terms)
+{
-+ if (!terms->term_good)
-+ free((void *) terms->term_good);
-+ if (!terms->term_bad)
-+ free((void *) terms->term_bad);
++ FREE_AND_NULL(terms->term_good);
++ FREE_AND_NULL(terms->term_bad);
+}
+
+static void set_terms(struct bisect_terms *terms, const char *bad,
+ const char *good)
+{
++ free((void *)terms->term_good);
+ terms->term_good = xstrdup(good);
++ free((void *)terms->term_bad);
+ terms->term_bad = xstrdup(bad);
+}
+
@@ -105,35 +106,33 @@
+ } 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);
-+ goto fail;
++ retval = error(_("Bad bisect_write argument: %s"), state);
++ goto finish;
+ }
+
+ if (get_oid(rev, &oid)) {
-+ error(_("couldn't get the oid of the rev '%s'"), rev);
-+ goto fail;
++ retval = error(_("couldn't get the oid of the rev '%s'"), rev);
++ goto finish;
+ }
+
+ if (update_ref(NULL, tag.buf, &oid, NULL, 0,
-+ UPDATE_REFS_MSG_ON_ERR))
-+ goto fail;
++ 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());
-+ goto fail;
++ retval = error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
++ goto finish;
+ }
+
-+ commit = lookup_commit_reference(&oid);
++ commit = lookup_commit_reference(the_repository, &oid);
+ log_commit(fp, "%s", state, commit);
+
+ if (!nolog)
+ fprintf(fp, "git bisect %s %s\n", state, rev);
+
-+ goto finish;
-+
-+fail:
-+ retval = -1;
+finish:
+ if (fp)
+ fclose(fp);
@@ -153,7 +152,7 @@
+ BISECT_WRITE
} cmdmode = 0;
- int no_checkout = 0;
-+ int no_checkout = 0, res = 0;
++ int no_checkout = 0, res = 0, nolog = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", &cmdmode,
N_("perform 'git bisect next'"), NEXT_ALL),
@@ -162,31 +161,24 @@
OPT_CMDMODE(0, "bisect-reset", &cmdmode,
N_("reset the bisection state"), BISECT_RESET),
+ OPT_CMDMODE(0, "bisect-write", &cmdmode,
-+ N_("update the refs according to the bisection state and may write it to BISECT_LOG"), BISECT_WRITE),
++ 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_BOOL(0, "no-log", &nolog,
++ N_("no log for BISECT_WRITE ")),
OPT_END()
};
-+ struct bisect_terms terms;
++ struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
argc = parse_options(argc, argv, prefix, options,
git_bisect_helper_usage, 0);
-@@
- 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 > 1)
return error(_("--bisect-reset requires either no argument or a commit"));
- return bisect_reset(argc ? argv[0] : NULL);
+ return !!bisect_reset(argc ? argv[0] : NULL);
+ case BISECT_WRITE:
+ if (argc != 4 && argc != 5)
+ return error(_("--bisect-write requires either 4 or 5 arguments"));
-+ nolog = (argc == 5) && !strcmp(argv[4], "nolog");
+ set_terms(&terms, argv[3], argv[2]);
+ res = bisect_write(argv[0], argv[1], &terms, nolog);
+ break;
@@ -195,7 +187,7 @@
}
- return 0;
+ free_terms(&terms);
-+ return res;
++ return !!res;
}
diff --git a/git-bisect.sh b/git-bisect.sh
3: 8874ae1a3a ! 3: 5f24e7cef2 wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
@@ -9,14 +9,13 @@
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/builtin/am.c b/builtin/am.c
--- a/builtin/am.c
+++ b/builtin/am.c
@@
- #include "string-list.h"
#include "packfile.h"
+ #include "repository.h"
-/**
- * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -61,7 +60,7 @@
+++ b/cache.h
@@
*/
- void safe_create_dir(const char *dir, int share);
+ extern int print_sha1_ellipsis(void);
+/* Return 1 if the file is empty or does not exists, 0 otherwise. */
+extern int is_empty_or_missing_file(const char *filename);
4: 7e9fdedc45 ! 4: 4e97a08a10 bisect--helper: `check_and_set_terms` shell function in C
@@ -20,8 +20,9 @@
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
+ Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
+ Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
--- a/builtin/bisect--helper.c
@@ -29,7 +30,7 @@
@@
N_("git bisect--helper --bisect-clean-state"),
N_("git bisect--helper --bisect-reset [<commit>]"),
- N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
+ N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
+ N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
NULL
};
@@ -53,12 +54,10 @@
+
+ if (!has_term_file) {
+ if (one_of(cmd, "bad", "good", NULL)) {
-+ free_terms(terms);
+ set_terms(terms, "bad", "good");
+ return write_terms(terms->term_bad, terms->term_good);
+ }
-+ else if (one_of(cmd, "new", "old", NULL)) {
-+ free_terms(terms);
++ if (one_of(cmd, "new", "old", NULL)) {
+ set_terms(terms, "new", "old");
+ return write_terms(terms->term_bad, terms->term_good);
+ }
@@ -78,17 +77,17 @@
+ BISECT_WRITE,
+ CHECK_AND_SET_TERMS
} cmdmode = 0;
- int no_checkout = 0, res = 0;
+ int no_checkout = 0, res = 0, nolog = 0;
struct option options[] = {
@@
N_("reset the bisection state"), BISECT_RESET),
OPT_CMDMODE(0, "bisect-write", &cmdmode,
- N_("update the refs according to the bisection state and may write it to BISECT_LOG"), BISECT_WRITE),
+ 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()
+ OPT_BOOL(0, "no-log", &nolog,
@@
set_terms(&terms, argv[3], argv[2]);
res = bisect_write(argv[0], argv[1], &terms, nolog);
5: ca12543868 ! 5: e9f400b1d6 bisect--helper: `bisect_next_check` shell function in C
@@ -17,8 +17,9 @@
Helped-by: Stephan Beyer <s-beyer@gmx.net>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
+ Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
+ Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
--- a/builtin/bisect--helper.c
@@ -33,9 +34,9 @@
static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@
N_("git bisect--helper --bisect-reset [<commit>]"),
- N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
+ N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
-+ N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
++ N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
NULL
};
@@ -43,10 +44,8 @@
terms->term_bad = xstrdup(bad);
}
-+static const char *voc[] = {
-+ "bad|new",
-+ "good|old"
-+};
++static const char *vocab_bad = "bad|new";
++static const char *vocab_good = "good|old";
+
/*
* Check whether the string `term` belongs to the set of strings
@@ -63,6 +62,15 @@
+ return 1;
+}
+
++static const char *need_bad_and_good_revision_warning =
++ N_("You need to give me at least one %s and %s revision.\n"
++ "You can use \"git bisect %s\" and \"git bisect %s\" for that.");
++
++static const char *need_bisect_start_warning =
++ N_("You need to start by \"git bisect start\".\n"
++ "You then need to give me at least one %s and %s revision.\n"
++ "You can use \"git bisect %s\" and \"git bisect %s\" for that.");
++
+static int bisect_next_check(const struct bisect_terms *terms,
+ const char *current_term)
+{
@@ -79,18 +87,19 @@
+ if (!missing_good && !missing_bad)
+ goto finish;
+
-+ if (!current_term)
-+ goto fail;
++ if (!current_term) {
++ retval = -1;
++ goto finish;
++ }
+
-+ if (missing_good && !missing_bad && current_term &&
++ if (missing_good && !missing_bad &&
+ !strcmp(current_term, terms->term_good)) {
+ char *yesno;
+ /*
+ * have bad (or new) but not good (or old). We could bisect
+ * although this is less optimum.
+ */
-+ fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
-+ terms->term_bad);
++ warning(_("bisecting only with a %s commit"), terms->term_bad);
+ if (!isatty(0))
+ goto finish;
+ /*
@@ -100,28 +109,17 @@
+ */
+ yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
+ if (starts_with(yesno, "N") || starts_with(yesno, "n"))
-+ goto fail;
-+
++ retval = -1;
+ goto finish;
+ }
+ if (!is_empty_or_missing_file(git_path_bisect_start())) {
-+ error(_("You need to give me at least one %s and "
-+ "%s revision. You can use \"git bisect %s\" "
-+ "and \"git bisect %s\" for that.\n"),
-+ voc[0], voc[1], voc[0], voc[1]);
-+ goto fail;
++ retval = error(_(need_bad_and_good_revision_warning),
++ vocab_bad, vocab_good, vocab_bad, vocab_good);
+ } else {
-+ error(_("You need to start by \"git bisect start\". You "
-+ "then need to give me at least one %s and %s "
-+ "revision. You can use \"git bisect %s\" and "
-+ "\"git bisect %s\" for that.\n"),
-+ voc[1], voc[0], voc[1], voc[0]);
-+ goto fail;
++ retval = error(_(need_bisect_start_warning),
++ vocab_good, vocab_bad, vocab_good, vocab_bad);
+ }
-+ goto finish;
+
-+fail:
-+ retval = -1;
+finish:
+ free((void *) good_glob);
+ free((void *) bad_ref);
@@ -139,17 +137,17 @@
+ CHECK_AND_SET_TERMS,
+ BISECT_NEXT_CHECK
} cmdmode = 0;
- int no_checkout = 0, res = 0;
+ int no_checkout = 0, res = 0, nolog = 0;
struct option options[] = {
@@
- N_("update the refs according to the bisection state and may write it to BISECT_LOG"), BISECT_WRITE),
+ 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_CMDMODE(0, "bisect-next-check", &cmdmode,
+ N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
- OPT_END()
+ OPT_BOOL(0, "no-log", &nolog,
@@
set_terms(&terms, argv[2], argv[1]);
res = check_and_set_terms(&terms, argv[0]);
@@ -245,8 +243,8 @@
- bisect_next_check fail
+ git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
- while true
- do
+ test -n "$*" || die "$(gettext "bisect run failed: no command provided.")"
+
@@
fi
}
6: db7b281c8c ! 6: f2b1706e5b bisect--helper: `get_terms` & `bisect_terms` shell function in C
@@ -14,18 +14,24 @@
Also use error() to report "no terms defined" and accordingly change the
test in t6030.
+ We need to use PARSE_OPT_KEEP_UNKNOWN here to allow for parameters that
+ look like options (e.g --term-good) but should not be parsed by
+ cmd_bisect__helper(). This change is safe because all other cmdmodes have
+ strict argc checks already.
+
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
+ Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
+ Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@
- N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"),
+ N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
- N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
+ N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
+ N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
NULL
};
@@ -41,18 +47,17 @@
+ int res = 0;
+
+ fp = fopen(git_path_bisect_terms(), "r");
-+ if (!fp)
-+ goto fail;
++ if (!fp) {
++ res = -1;
++ goto finish;
++ }
+
+ free_terms(terms);
+ strbuf_getline_lf(&str, fp);
+ terms->term_bad = strbuf_detach(&str, NULL);
+ strbuf_getline_lf(&str, fp);
+ terms->term_good = strbuf_detach(&str, NULL);
-+ goto finish;
+
-+fail:
-+ res = -1;
+finish:
+ if (fp)
+ fclose(fp);
@@ -60,32 +65,26 @@
+ return res;
+}
+
-+static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
++static int bisect_terms(struct bisect_terms *terms, const char *option)
+{
-+ int i;
-+
+ if (get_terms(terms))
+ return error(_("no terms defined"));
+
-+ if (argc > 1)
-+ return error(_("--bisect-term requires exactly one argument"));
-+
-+ if (argc == 0)
-+ return !printf(_("Your current terms are %s for the old state\n"
-+ "and %s for the new state.\n"),
-+ terms->term_good, terms->term_bad);
-+
-+ for (i = 0; i < argc; i++) {
-+ if (!strcmp(argv[i], "--term-good"))
-+ printf(_("%s\n"), terms->term_good);
-+ else if (!strcmp(argv[i], "--term-bad"))
-+ printf(_("%s\n"), terms->term_bad);
-+ else
-+ error(_("BUG: invalid argument %s for 'git bisect terms'.\n"
-+ "Supported options are: "
-+ "--term-good|--term-old and "
-+ "--term-bad|--term-new."), argv[i]);
++ if (option == NULL) {
++ printf(_("Your current terms are %s for the old state\n"
++ "and %s for the new state.\n"),
++ terms->term_good, terms->term_bad);
++ return 0;
+ }
++ if (one_of(option, "--term-good", "--term-old", NULL))
++ printf("%s\n", terms->term_good);
++ else if (one_of(option, "--term-bad", "--term-new", NULL))
++ printf("%s\n", terms->term_bad);
++ else
++ return error(_("invalid argument %s for 'git bisect terms'.\n"
++ "Supported options are: "
++ "--term-good|--term-old and "
++ "--term-bad|--term-new."), option);
+
+ return 0;
+}
@@ -101,7 +100,7 @@
+ BISECT_NEXT_CHECK,
+ BISECT_TERMS
} cmdmode = 0;
- int no_checkout = 0, res = 0;
+ int no_checkout = 0, res = 0, nolog = 0;
struct option options[] = {
@@
N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
@@ -111,9 +110,9 @@
+ N_("print out the bisect terms"), BISECT_TERMS),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
- OPT_END()
+ OPT_BOOL(0, "no-log", &nolog,
@@
- struct bisect_terms terms;
+ struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
argc = parse_options(argc, argv, prefix, options,
- git_bisect_helper_usage, 0);
@@ -128,7 +127,7 @@
+ case BISECT_TERMS:
+ if (argc > 1)
+ return error(_("--bisect-terms requires 0 or 1 argument"));
-+ res = bisect_terms(&terms, argv, argc);
++ res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL);
+ break;
default:
return error("BUG: unknown subcommand '%d'", cmdmode);
@@ -142,7 +141,7 @@
git bisect--helper --bisect-write "$command" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit;;
terms)
- bisect_terms $rev ;;
-+ git bisect--helper --bisect-terms $rev || exit;;
++ git bisect--helper --bisect-terms $rev || exit;;
*)
die "$(gettext "?? what are you talking about?")" ;;
esac
7: e14e913797 ! 7: 87a033b858 bisect--helper: `bisect_start` shell function partially in C
@@ -6,7 +6,7 @@
`bisect-start` subcommand to `git bisect--helper` to call it from
git-bisect.sh .
- The last part is not converted because it calls another shell function
+ The last part is not converted because it calls another shell function.
`bisect_start` shell function will be completed after the `bisect_next`
shell function is ported in C.
@@ -18,12 +18,17 @@
Also introduce a method `bisect_append_log_quoted` to keep things short
and crisp.
+ Note that we are a bit lax about command-line parsing because the helper
+ is not supposed to be called by the user directly (but only from the git
+ bisect script).
+
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Stephan Beyer <s-beyer@gmx.net>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
+ Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
+ Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
--- a/builtin/bisect--helper.c
@@ -47,10 +52,10 @@
N_("git bisect--helper --next-all [--no-checkout]"),
@@
N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
- N_("git bisect--helper --bisect-next-check [<term>] <good_term> <bad_term>"),
+ N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
+ N_("git bisect--helper --bisect-start [--term-{old,good}=<term> --term-{new,bad}=<term>]"
-+ "[--no-checkout] [<bad> [<good>...]] [--] [<paths>...]"),
++ "[--no-checkout] [<bad> [<good>...]] [--] [<paths>...]"),
NULL
};
@@ -67,17 +72,15 @@
+ if (!fp)
+ return -1;
+
-+ if (fprintf(fp, "git bisect start") < 1)
-+ goto fail;
++ if (fprintf(fp, "git bisect start") < 1) {
++ retval = -1;
++ goto finish;
++ }
+
-+ sq_quote_argv(&orig_args, argv, 0);
++ sq_quote_argv(&orig_args, argv);
+ if (fprintf(fp, "%s\n", orig_args.buf) < 1)
-+ goto fail;
++ retval = -1;
+
-+ goto finish;
-+
-+fail:
-+ retval = -1;
+finish:
+ fclose(fp);
+ strbuf_release(&orig_args);
@@ -138,7 +141,7 @@
+ terms->term_bad = xstrdup(arg);
+ } else if (starts_with(arg, "--") &&
+ !one_of(arg, "--term-good", "--term-bad", NULL)) {
-+ return error(_("unrecognised option: '%s'"), arg);
++ return error(_("unrecognized option: '%s'"), arg);
+ } else {
+ char *commit_id = xstrfmt("%s^{commit}", arg);
+ if (get_oid(commit_id, &oid) && has_double_dash)
@@ -157,7 +160,8 @@
+ * set references named with the default terms, and won't be able
+ * to change afterwards.
+ */
-+ must_write_terms |= !!revs.nr;
++ if (revs.nr)
++ must_write_terms = 1;
+ for (i = 0; i < revs.nr; i++) {
+ if (bad_seen) {
+ string_list_append(&states, terms->term_good);
@@ -173,7 +177,7 @@
+ head = resolve_ref_unsafe("HEAD", 0, &head_oid, &flags);
+ if (!head)
+ if (get_oid("HEAD", &head_oid))
-+ return error(_("Bad HEAD - I need a HEAD"));
++ return error(_("bad HEAD - I need a HEAD"));
+
+ /*
+ * Check if we are bisecting
@@ -188,10 +192,11 @@
+ argv_array_pushl(&argv, "checkout", start_head.buf,
+ "--", NULL);
+ if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
-+ error(_("checking out '%s' failed. Try 'git "
-+ "bisect start <valid-branch>'."),
-+ start_head.buf);
-+ goto fail;
++ retval = error(_("checking out '%s' failed."
++ " Try 'git bisect start "
++ "<valid-branch>'."),
++ start_head.buf);
++ goto finish;
+ }
+ }
+ } else {
@@ -199,7 +204,7 @@
+ if (!get_oid(head, &head_oid) &&
+ !starts_with(head, "refs/heads/")) {
+ strbuf_reset(&start_head);
-+ strbuf_addstr(&start_head, sha1_to_hex(head_oid.hash));
++ strbuf_addstr(&start_head, oid_to_hex(&head_oid));
+ } else if (!get_oid(head, &head_oid) &&
+ skip_prefix(head, "refs/heads/", &head)) {
+ /*
@@ -211,7 +216,7 @@
+ return error(_("won't bisect on cg-seek'ed tree"));
+ strbuf_addstr(&start_head, head);
+ } else {
-+ return error(_("Bad HEAD - strange symbolic ref"));
++ return error(_("bad HEAD - strange symbolic ref"));
+ }
+ }
+
@@ -236,31 +241,33 @@
+ if (no_checkout) {
+ get_oid(start_head.buf, &oid);
+ if (update_ref(NULL, "BISECT_HEAD", &oid, NULL, 0,
-+ UPDATE_REFS_MSG_ON_ERR))
-+ goto fail;
++ UPDATE_REFS_MSG_ON_ERR)) {
++ retval = -1;
++ goto finish;
++ }
+ }
+
+ if (pathspec_pos < argc - 1)
-+ sq_quote_argv(&bisect_names, argv + pathspec_pos, 0);
++ sq_quote_argv(&bisect_names, argv + pathspec_pos);
+ write_file(git_path_bisect_names(), "%s\n", bisect_names.buf);
+
+ for (i = 0; i < states.nr; i++)
+ if (bisect_write(states.items[i].string,
-+ revs.items[i].string, terms, 1))
-+ goto fail;
++ revs.items[i].string, terms, 1)) {
++ retval = -1;
++ goto finish;
++ }
+
-+ if (must_write_terms)
-+ if (write_terms(terms->term_bad, terms->term_good))
-+ goto fail;
++ if (must_write_terms && write_terms(terms->term_bad,
++ terms->term_good)) {
++ retval = -1;
++ goto finish;
++ }
+
+ retval = bisect_append_log_quoted(argv);
+ if (retval)
-+ goto fail;
-+
-+ goto finish;
++ retval = -1;
+
-+fail:
-+ retval = -1;
+finish:
+ string_list_clear(&revs, 0);
+ string_list_clear(&states, 0);
@@ -280,7 +287,7 @@
+ BISECT_TERMS,
+ BISECT_START
} cmdmode = 0;
- int no_checkout = 0, res = 0;
+ int no_checkout = 0, res = 0, nolog = 0;
struct option options[] = {
@@
N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
@@ -290,9 +297,9 @@
+ N_("start the bisect session"), BISECT_START),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
- OPT_END()
+ OPT_BOOL(0, "no-log", &nolog,
@@
- struct bisect_terms terms;
+ struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
argc = parse_options(argc, argv, prefix, options,
- git_bisect_helper_usage, PARSE_OPT_KEEP_UNKNOWN);
@@ -303,7 +310,7 @@
usage_with_options(git_bisect_helper_usage, options);
@@
return error(_("--bisect-terms requires 0 or 1 argument"));
- res = bisect_terms(&terms, argv, argc);
+ res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL);
break;
+ case BISECT_START:
+ set_terms(&terms, "bad", "good");
8: 161bbaced1 < -: ---------- t6030: make various test to pass GETTEXT_POISON tests
--
gitgitgadget
^ permalink raw reply
* [PATCH v17 1/7] bisect--helper: `bisect_reset` shell function in C
From: Pranit Bauva via GitGitGadget @ 2019-01-02 15:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pranit Bauva
In-Reply-To: <pull.101.v17.git.gitgitgadget@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
Reimplement `bisect_reset` shell function in C and add a `--bisect-reset`
subcommand to `git bisect--helper` to call it from git-bisect.sh .
Using `bisect_reset` subcommand is a temporary measure to port shell
functions to C so as to use the existing test suite. As more functions
are ported, this subcommand would be retired but its implementation will
be called by some other method.
Note: --bisect-clean-state subcommand has not been retired as there are
still a function namely `bisect_start()` which still uses this
subcommand.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
---
builtin/bisect--helper.c | 51 +++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 28 ++--------------------
2 files changed, 52 insertions(+), 27 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 4b5fadcbe1..aa6495dc84 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -3,15 +3,21 @@
#include "parse-options.h"
#include "bisect.h"
#include "refs.h"
+#include "dir.h"
+#include "argv-array.h"
+#include "run-command.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_start, "BISECT_START")
+static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
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"),
+ N_("git bisect--helper --bisect-reset [<commit>]"),
NULL
};
@@ -106,13 +112,50 @@ static void check_expected_revs(const char **revs, int rev_nr)
}
}
+static int bisect_reset(const char *commit)
+{
+ struct strbuf branch = STRBUF_INIT;
+
+ if (!commit) {
+ if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1) {
+ printf(_("We are not bisecting.\n"));
+ return 0;
+ }
+ strbuf_rtrim(&branch);
+ } else {
+ struct object_id oid;
+
+ if (get_oid_commit(commit, &oid))
+ return error(_("'%s' is not a valid commit"), commit);
+ strbuf_addstr(&branch, commit);
+ }
+
+ if (!file_exists(git_path_bisect_head())) {
+ struct argv_array argv = ARGV_ARRAY_INIT;
+
+ argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL);
+ if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
+ strbuf_release(&branch);
+ argv_array_clear(&argv);
+ return error(_("could not check out original"
+ " HEAD '%s'. Try 'git bisect"
+ "reset <commit>'."), branch.buf);
+ }
+ argv_array_clear(&argv);
+ }
+
+ strbuf_release(&branch);
+ return bisect_clean_state();
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
NEXT_ALL = 1,
WRITE_TERMS,
BISECT_CLEAN_STATE,
- CHECK_EXPECTED_REVS
+ CHECK_EXPECTED_REVS,
+ BISECT_RESET
} cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
@@ -124,6 +167,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
OPT_CMDMODE(0, "check-expected-revs", &cmdmode,
N_("check for expected revs"), CHECK_EXPECTED_REVS),
+ OPT_CMDMODE(0, "bisect-reset", &cmdmode,
+ N_("reset the bisection state"), BISECT_RESET),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
OPT_END()
@@ -149,6 +194,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
case CHECK_EXPECTED_REVS:
check_expected_revs(argv, argc);
return 0;
+ case BISECT_RESET:
+ if (argc > 1)
+ return error(_("--bisect-reset requires either no argument or a commit"));
+ return !!bisect_reset(argc ? argv[0] : NULL);
default:
return error("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 54cbfecc5a..afbfbc1f8e 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -393,35 +393,11 @@ bisect_visualize() {
eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
}
-bisect_reset() {
- test -s "$GIT_DIR/BISECT_START" || {
- gettextln "We are not bisecting."
- return
- }
- case "$#" in
- 0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
- 1) git rev-parse --quiet --verify "$1^{commit}" >/dev/null || {
- invalid="$1"
- die "$(eval_gettext "'\$invalid' is not a valid commit")"
- }
- branch="$1" ;;
- *)
- usage ;;
- esac
-
- if ! test -f "$GIT_DIR/BISECT_HEAD" && ! git checkout "$branch" --
- then
- die "$(eval_gettext "Could not check out original HEAD '\$branch'.
-Try 'git bisect reset <commit>'.")"
- fi
- git bisect--helper --bisect-clean-state || exit
-}
-
bisect_replay () {
file="$1"
test "$#" -eq 1 || die "$(gettext "No logfile given")"
test -r "$file" || die "$(eval_gettext "cannot read \$file for replaying")"
- bisect_reset
+ git bisect--helper --bisect-reset || exit
while read git bisect command rev
do
test "$git $bisect" = "git bisect" || test "$git" = "git-bisect" || continue
@@ -613,7 +589,7 @@ case "$#" in
visualize|view)
bisect_visualize "$@" ;;
reset)
- bisect_reset "$@" ;;
+ git bisect--helper --bisect-reset "$@" ;;
replay)
bisect_replay "$@" ;;
log)
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2] doc: remove unneeded TODO for release_commit_memory
From: Duy Nguyen @ 2019-01-02 11:17 UTC (permalink / raw)
To: Albert Burt; +Cc: Git Mailing List, Junio C Hamano, Stefan Beller
In-Reply-To: <20190101200818.81273-1-aburthinds@gmail.com>
On Wed, Jan 2, 2019 at 3:09 AM Albert Burt <aburthinds@gmail.com> wrote:
>
> Remove TODO that was left in from:
> commit 110240588d (Merge branch 'sb/object-store-alloc' - 2018-06-25)
>
> Todo can be removed as:
> 9d2c97016f (commit.h: delete 'util' field in struct commit - 2018-05-19)
> deletes commit->util.
>
> Signed-off-by: Albert Burt <aburthinds@gmail.com>
> ---
>
> Thanks for looking at this for me Duy. I updated some of the changes you
> suggested.
>
> Let me know if there's anything else that I would need to clean up, or do better.
> :)
Nope. The patch looks good to me.
>
> commit.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/commit.c b/commit.c
> index 2d94e0b199..2ff6dca0bc 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -357,8 +357,6 @@ void release_commit_memory(struct commit *c)
> c->index = 0;
> free_commit_buffer(c);
> free_commit_list(c->parents);
> - /* TODO: what about commit->util? */
> -
> c->object.parsed = 0;
> }
>
> --
> 2.17.2 (Apple Git-113)
>
--
Duy
^ permalink raw reply
* Re: Regression `git checkout $rev -b branch` while in a `--no-checkout` clone does not check out files
From: Duy Nguyen @ 2019-01-02 11:08 UTC (permalink / raw)
To: Anthony Sottile; +Cc: Git Mailing List
In-Reply-To: <CA+dzEB=DH0irkFaRzkKERSjdZ=EJ+mG3Ri2Xeobx9Yu_eDd+jg@mail.gmail.com>
On Wed, Jan 2, 2019 at 6:36 AM Anthony Sottile <asottile@umich.edu> wrote:
>
> Here's a simple regression test -- haven't had time to bisect this
I can't reproduce with either 2.20.0, 2.20.1 or 'master'. It would be
great if you could bisect this.
There are no suspicious commits from 2.27.1 touching
builtin/checkout.c. Though there are some more changes in
unpack-trees.c that might cause this (big wild guess).
--
Duy
^ permalink raw reply
* [PATCH] banned.h: mark strncat() as banned
From: Eric Wong @ 2019-01-02 9:38 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git
strncat() has the same quadratic behavior as strcat() and is
difficult-to-read and bug-prone. While it hasn't yet been a
problem in git iself, strncat() found it's way into 'master'
of cgit and caused segfaults on my system.
Signed-off-by: Eric Wong <e@80x24.org>
---
banned.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/banned.h b/banned.h
index 28f5937035..447af24807 100644
--- a/banned.h
+++ b/banned.h
@@ -16,6 +16,8 @@
#define strcat(x,y) BANNED(strcat)
#undef strncpy
#define strncpy(x,y,n) BANNED(strncpy)
+#undef strncat
+#define strncat(x,y,n) BANNED(strncat)
#undef sprintf
#undef vsprintf
--
EW
^ permalink raw reply related
* Re: Regression in git-subtree.sh, introduced in 2.20.1, after 315a84f9aa0e2e629b0680068646b0032518ebed
From: Marc Balmer @ 2019-01-02 9:13 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, roger.strain, Junio C Hamano
In-Reply-To: <20190101131947.GA18907@ash>
> [...]
>
>>
>
> Hmm.. I'm not that familiar with git-subtree.sh, so here's one last
> blind shot.
>
> There's a format change between git-show and git-rev-parse. The former
> separates commits by spaces while the latter by newlines. Will this
> help?
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 147201dc6c..23f570beee 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -633,7 +633,7 @@ process_split_commit () {
> else
> # processing commit without normal parent information;
> # fetch from repo
> - parents=$(git rev-parse "$rev^@")
> + parents=$(git rev-parse "$rev^@" | tr '\n' ' ')
> extracount=$(($extracount + 1))
> fi
>
Unfortunately, this did not change the situation. It still segfaults.
> --
> Duy
^ permalink raw reply
* [PATCH v3 3/3] pack-redundant: remove unused functions
From: Jiang Xin @ 2019-01-02 4:34 UTC (permalink / raw)
To: Sun Chao, Git List, Junio C Hamano; +Cc: Jiang Xin
In-Reply-To: <20181219121451.21697-1-worldhello.net@gmail.com>
From: Sun Chao <sunchao9@huawei.com>
Remove unused functions to find `min` packs, such as `get_permutations`,
`pll_free`, etc.
Signed-off-by: Sun Chao <sunchao9@huawei.com>
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
builtin/pack-redundant.c | 72 ------------------------------------------------
1 file changed, 72 deletions(-)
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 3655cc7dc6..9630117c90 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -285,78 +285,6 @@ static void cmp_two_packs(struct pack_list *p1, struct pack_list *p2)
}
}
-static void pll_free(struct pll *l)
-{
- struct pll *old;
- struct pack_list *opl;
-
- while (l) {
- old = l;
- while (l->pl) {
- opl = l->pl;
- l->pl = opl->next;
- free(opl);
- }
- l = l->next;
- free(old);
- }
-}
-
-/* all the permutations have to be free()d at the same time,
- * since they refer to each other
- */
-static struct pll * get_permutations(struct pack_list *list, int n)
-{
- struct pll *subset, *ret = NULL, *new_pll = NULL;
-
- if (list == NULL || pack_list_size(list) < n || n == 0)
- return NULL;
-
- if (n == 1) {
- while (list) {
- new_pll = xmalloc(sizeof(*new_pll));
- new_pll->pl = NULL;
- pack_list_insert(&new_pll->pl, list);
- new_pll->next = ret;
- ret = new_pll;
- list = list->next;
- }
- return ret;
- }
-
- while (list->next) {
- subset = get_permutations(list->next, n - 1);
- while (subset) {
- new_pll = xmalloc(sizeof(*new_pll));
- new_pll->pl = subset->pl;
- pack_list_insert(&new_pll->pl, list);
- new_pll->next = ret;
- ret = new_pll;
- subset = subset->next;
- }
- list = list->next;
- }
- return ret;
-}
-
-static int is_superset(struct pack_list *pl, struct llist *list)
-{
- struct llist *diff;
-
- diff = llist_copy(list);
-
- while (pl) {
- llist_sorted_difference_inplace(diff, pl->all_objects);
- if (diff->size == 0) { /* we're done */
- llist_free(diff);
- return 1;
- }
- pl = pl->next;
- }
- llist_free(diff);
- return 0;
-}
-
static size_t sizeof_union(struct packed_git *p1, struct packed_git *p2)
{
size_t ret = 0;
--
2.14.5.agit.2
^ permalink raw reply related
* [PATCH v3 2/3] pack-redundant: new algorithm to find min packs
From: Jiang Xin @ 2019-01-02 4:34 UTC (permalink / raw)
To: Sun Chao, Git List, Junio C Hamano; +Cc: Jiang Xin
In-Reply-To: <20181219121451.21697-1-worldhello.net@gmail.com>
From: Sun Chao <sunchao9@huawei.com>
When calling `git pack-redundant --all`, if there are too many local
packs and too many redundant objects within them, the too deep iteration
of `get_permutations` will exhaust all the resources, and the process of
`git pack-redundant` will be killed.
The following script could create a repository with too many redundant
packs, and running `git pack-redundant --all` in the `test.git` repo
will die soon.
#!/bin/sh
repo="$(pwd)/test.git"
work="$(pwd)/test"
i=1
max=199
if test -d "$repo" || test -d "$work"; then
echo >&2 "ERROR: '$repo' or '$work' already exist"
exit 1
fi
git init -q --bare "$repo"
git --git-dir="$repo" config gc.auto 0
git --git-dir="$repo" config transfer.unpackLimit 0
git clone -q "$repo" "$work" 2>/dev/null
while :; do
cd "$work"
echo "loop $i: $(date +%s)" >$i
git add $i
git commit -q -sm "loop $i"
git push -q origin HEAD:master
printf "\rCreate pack %4d/%d\t" $i $max
if test $i -ge $max; then break; fi
cd "$repo"
git repack -q
if test $(($i % 2)) -eq 0; then
git repack -aq
pack=$(ls -t $repo/objects/pack/*.pack | head -1)
touch "${pack%.pack}.keep"
fi
i=$((i+1))
done
printf "\ndone\n"
To get the `min` unique pack list, we can replace the iteration in
`minimize` function with a new algorithm, and this could solve this
issue:
1. Get the unique and non_uniqe packs, add the unique packs to the
`min` list.
2. Remove the objects of unique packs from non_unique packs, then each
object left in the non_unique packs will have at least two copies.
3. Sort the non_unique packs by the objects' size, more objects first,
and add the first non_unique pack to `min` list.
4. Drop the duplicated objects from other packs in the ordered
non_unique pack list, and repeat step 3.
Original PR and discussions: https://github.com/jiangxin/git/pull/25
Signed-off-by: Sun Chao <sunchao9@huawei.com>
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
builtin/pack-redundant.c | 109 +++++++++++++++++++++++++++++------------------
1 file changed, 68 insertions(+), 41 deletions(-)
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index cf9a9aabd4..3655cc7dc6 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -421,14 +421,52 @@ static inline off_t pack_set_bytecount(struct pack_list *pl)
return ret;
}
+static int cmp_pack_list_reverse(const void *a, const void *b)
+{
+ struct pack_list *pl_a = *((struct pack_list **)a);
+ struct pack_list *pl_b = *((struct pack_list **)b);
+ size_t sz_a = pl_a->all_objects->size;
+ size_t sz_b = pl_b->all_objects->size;
+
+ if (sz_a == sz_b)
+ return 0;
+ else if (sz_a < sz_b)
+ return 1;
+ else
+ return -1;
+}
+
+/* Sort pack_list, greater size of all_objects first */
+static void sort_pack_list(struct pack_list **pl)
+{
+ struct pack_list **ary, *p;
+ int i;
+ size_t n = pack_list_size(*pl);
+
+ if (n < 2)
+ return;
+
+ /* prepare an array of packed_list for easier sorting */
+ ary = xcalloc(n, sizeof(struct pack_list *));
+ for (n = 0, p = *pl; p; p = p->next)
+ ary[n++] = p;
+
+ QSORT(ary, n, cmp_pack_list_reverse);
+
+ /* link them back again */
+ for (i = 0; i < n - 1; i++)
+ ary[i]->next = ary[i + 1];
+ ary[n - 1]->next = NULL;
+ *pl = ary[0];
+
+ free(ary);
+}
+
+
static void minimize(struct pack_list **min)
{
- struct pack_list *pl, *unique = NULL,
- *non_unique = NULL, *min_perm = NULL;
- struct pll *perm, *perm_all, *perm_ok = NULL, *new_perm;
- struct llist *missing;
- off_t min_perm_size = 0, perm_size;
- int n;
+ struct pack_list *pl, *unique = NULL, *non_unique = NULL;
+ struct llist *missing, *unique_pack_objects;
pl = local_packs;
while (pl) {
@@ -446,49 +484,37 @@ static void minimize(struct pack_list **min)
pl = pl->next;
}
+ *min = unique;
+
/* return if there are no objects missing from the unique set */
if (missing->size == 0) {
- *min = unique;
free(missing);
return;
}
- /* find the permutations which contain all missing objects */
- for (n = 1; n <= pack_list_size(non_unique) && !perm_ok; n++) {
- perm_all = perm = get_permutations(non_unique, n);
- while (perm) {
- if (is_superset(perm->pl, missing)) {
- new_perm = xmalloc(sizeof(struct pll));
- memcpy(new_perm, perm, sizeof(struct pll));
- new_perm->next = perm_ok;
- perm_ok = new_perm;
- }
- perm = perm->next;
- }
- if (perm_ok)
- break;
- pll_free(perm_all);
- }
- if (perm_ok == NULL)
- die("Internal error: No complete sets found!");
-
- /* find the permutation with the smallest size */
- perm = perm_ok;
- while (perm) {
- perm_size = pack_set_bytecount(perm->pl);
- if (!min_perm_size || min_perm_size > perm_size) {
- min_perm_size = perm_size;
- min_perm = perm->pl;
- }
- perm = perm->next;
- }
- *min = min_perm;
- /* add the unique packs to the list */
- pl = unique;
+ unique_pack_objects = llist_copy(all_objects);
+ llist_sorted_difference_inplace(unique_pack_objects, missing);
+
+ /* remove unique pack objects from the non_unique packs */
+ pl = non_unique;
while (pl) {
- pack_list_insert(min, pl);
+ llist_sorted_difference_inplace(pl->all_objects, unique_pack_objects);
pl = pl->next;
}
+
+ while (non_unique) {
+ /* sort the non_unique packs, greater size of all_objects first */
+ sort_pack_list(&non_unique);
+ if (non_unique->all_objects->size == 0)
+ break;
+
+ pack_list_insert(min, non_unique);
+
+ for (pl = non_unique->next; pl && pl->all_objects->size > 0; pl = pl->next)
+ llist_sorted_difference_inplace(pl->all_objects, non_unique->all_objects);
+
+ non_unique = non_unique->next;
+ }
}
static void load_all_objects(void)
@@ -603,7 +629,7 @@ static void load_all(void)
int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
{
int i;
- struct pack_list *min, *red, *pl;
+ struct pack_list *min = NULL, *red, *pl;
struct llist *ignore;
struct object_id *oid;
char buf[GIT_MAX_HEXSZ + 2]; /* hex hash + \n + \0 */
@@ -664,6 +690,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
pl = local_packs;
while (pl) {
llist_sorted_difference_inplace(pl->unique_objects, ignore);
+ llist_sorted_difference_inplace(pl->all_objects, ignore);
pl = pl->next;
}
--
2.14.5.agit.2
^ permalink raw reply related
* [PATCH v3 1/3] t5323: test cases for git-pack-redundant
From: Jiang Xin @ 2019-01-02 4:34 UTC (permalink / raw)
To: Sun Chao, Git List, Junio C Hamano; +Cc: Jiang Xin
In-Reply-To: <20181219121451.21697-1-worldhello.net@gmail.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Add test cases for git pack-redundant to validate new algorithm for git
pack-redundant.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
t/t5323-pack-redundant.sh | 84 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)
create mode 100755 t/t5323-pack-redundant.sh
diff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh
new file mode 100755
index 0000000000..ef6076f065
--- /dev/null
+++ b/t/t5323-pack-redundant.sh
@@ -0,0 +1,84 @@
+#!/bin/sh
+#
+# Copyright (c) 2018 Jiang Xin
+#
+
+test_description='git redundant test'
+
+. ./test-lib.sh
+
+create_commits()
+{
+ set -e
+ parent=
+ for name in A B C D E F G H I J K L M
+ do
+ test_tick
+ T=$(git write-tree)
+ if test -z "$parent"
+ then
+ sha1=$(echo $name | git commit-tree $T)
+ else
+ sha1=$(echo $name | git commit-tree -p $parent $T)
+ fi
+ eval $name=$sha1
+ parent=$sha1
+ done
+ git update-ref refs/heads/master $M
+}
+
+create_redundant_packs()
+{
+ set -e
+ cd .git/objects/pack
+ P1=$(printf "$T\n$A\n" | git pack-objects pack 2>/dev/null)
+ P2=$(printf "$T\n$A\n$B\n$C\n$D\n$E\n" | git pack-objects pack 2>/dev/null)
+ P3=$(printf "$C\n$D\n$F\n$G\n$I\n$J\n" | git pack-objects pack 2>/dev/null)
+ P4=$(printf "$D\n$E\n$G\n$H\n$J\n$K\n" | git pack-objects pack 2>/dev/null)
+ P5=$(printf "$F\n$G\n$H\n" | git pack-objects pack 2>/dev/null)
+ P6=$(printf "$F\n$I\n$L\n" | git pack-objects pack 2>/dev/null)
+ P7=$(printf "$H\n$K\n$M\n" | git pack-objects pack 2>/dev/null)
+ P8=$(printf "$L\n$M\n" | git pack-objects pack 2>/dev/null)
+ cd -
+ eval P$P1=P1:$P1
+ eval P$P2=P2:$P2
+ eval P$P3=P3:$P3
+ eval P$P4=P4:$P4
+ eval P$P5=P5:$P5
+ eval P$P6=P6:$P6
+ eval P$P7=P7:$P7
+ eval P$P8=P8:$P8
+}
+
+# Create commits and packs
+create_commits
+create_redundant_packs
+
+test_expect_success 'clear loose objects' '
+ git prune-packed &&
+ test $(find .git/objects -type f | grep -v pack | wc -l) -eq 0
+'
+
+cat >expected <<EOF
+P1:$P1
+P4:$P4
+P5:$P5
+P6:$P6
+EOF
+
+test_expect_success 'git pack-redundant --all' '
+ git pack-redundant --all | \
+ sed -e "s#^.*/pack-\(.*\)\.\(idx\|pack\)#\1#g" | \
+ sort -u | \
+ while read p; do eval echo "\${P$p}"; done | \
+ sort > actual && \
+ test_cmp expected actual
+'
+
+test_expect_success 'remove redundant packs' '
+ git pack-redundant --all | xargs rm &&
+ git fsck &&
+ test $(git pack-redundant --all | wc -l) -eq 0
+'
+
+test_done
--
2.14.5.agit.2
^ permalink raw reply related
* [PATCH v3 0/3] pack-redundant: new algorithm to find min packs
From: Jiang Xin @ 2019-01-02 4:34 UTC (permalink / raw)
To: Sun Chao, Git List, Junio C Hamano; +Cc: Jiang Xin
In-Reply-To: <20181219121451.21697-1-worldhello.net@gmail.com>
Sun Chao (my former colleague at Huawei) found a bug of
git-pack-redundant. If there are too many packs and many of them overlap
each other, running `git pack-redundant --all` will exhaust all memories
and the process will be killed by kernel.
There is a script in commit log of commit 2/3, which can be used to
create a repository with lots of redundant packs. Running `git
pack-redundant --all` in it can reproduce this issue.
Updates of reroll v3:
* Rename test case file from t5322 to t5323, for I see t5322 exist in
commit 404dead121: "pack-objects: add --sparse option".
Jiang Xin (1):
t5323: test cases for git-pack-redundant
Sun Chao (2):
pack-redundant: new algorithm to find min packs
pack-redundant: remove unused functions
builtin/pack-redundant.c | 181 +++++++++++++++++-----------------------------
t/t5323-pack-redundant.sh | 84 +++++++++++++++++++++
2 files changed, 152 insertions(+), 113 deletions(-)
create mode 100755 t/t5323-pack-redundant.sh
--
2.14.5.agit.2
^ permalink raw reply
* Re: [PATCH] test-lib: check Bash version for '-x' without using shell arrays
From: Johannes Sixt @ 2019-01-02 0:20 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Junio C Hamano, Max Kirillov, Carlo Arenas, Jeff King, git
In-Reply-To: <20190101231949.8184-1-szeder.dev@gmail.com>
Am 02.01.19 um 00:19 schrieb SZEDER Gábor:
> Alas, it has been reported that NetBSD's /bin/sh does complain about
> them:
>
> ./test-lib.sh: 327: Syntax error: Bad substitution
>
> where line 327 contains the first ${BASH_VERSINFO[0]} array access.
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 0f1faa24b2..f47a191e3b 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -324,9 +324,12 @@ do
> # isn't executed with a suitable Bash version.
> if test -z "$test_untraceable" || {
> test -n "$BASH_VERSION" && {
> - test ${BASH_VERSINFO[0]} -gt 4 || {
> - test ${BASH_VERSINFO[0]} -eq 4 &&
> - test ${BASH_VERSINFO[1]} -ge 1
> + bash_major=${BASH_VERSION%%.*}
> + bash_minor=${BASH_VERSION#*.}
> + bash_minor=${bash_minor%%.*}
> + test $bash_major -gt 4 || {
> + test $bash_major -eq 4 &&
> + test $bash_minor -ge 1
> }
> }
> }
>
Would it perhaps be simpler to just hide the syntax behind eval? Like
if test -z "$test_untraceable" || {
test -n "$BASH_VERSION" && eval '
test ${BASH_VERSINFO[0]} -gt 4 || {
test ${BASH_VERSINFO[0]} -eq 4 &&
test ${BASH_VERSINFO[1]} -ge 1
}
'
-- Hannes
^ permalink raw reply
* git-send-email warnings & process dying of signal 11
From: Rafał Miłecki @ 2019-01-01 22:45 UTC (permalink / raw)
To: git
Hello & Happy New Year!
I've recently switched from openSUSE 42.3 (perl 5.18.2 & git 2.13.7) to
the openSUSE Tumbleweed (perl 5.28.1 & git 2.20.1) and send-email
doesn't work for me anymore.
FWIW it doesn't seem like a git regression. I've manually installed
2.13.7 in my new OS and it also fails.
It basically fails with the:
Warning: unable to close filehandle __ANONIO__ properly: Bad file descriptor at /usr/lib/git/git-send-email line 812.
Warning: unable to close filehandle __ANONIO__ properly: Bad file descriptor at /usr/lib/git/git-send-email line 812.
error: git-send-email died of signal 11
Relevant perl source:
802 sub ask {
803 my ($prompt, %arg) = @_;
804 my $valid_re = $arg{valid_re};
805 my $default = $arg{default};
806 my $confirm_only = $arg{confirm_only};
807 my $resp;
808 my $i = 0;
809 return defined $default ? $default : undef
810 unless defined $term->IN and defined fileno($term->IN) and
811 defined $term->OUT and defined fileno($term->OUT);
812 while ($i++ < 10) {
813 $resp = $term->readline($prompt);
814 if (!defined $resp) { # EOF
815 print "\n";
816 return defined $default ? $default : undef;
817 }
818 if ($resp eq '' and defined $default) {
819 return $default;
820 }
821 if (!defined $valid_re or $resp =~ /$valid_re/) {
822 return $resp;
823 }
824 if ($confirm_only) {
825 my $yesno = $term->readline(
826 # TRANSLATORS: please keep [y/N] as is.
827 sprintf(__("Are you sure you want to use <%s> [y/N]? "), $resp));
828 if (defined $yesno && $yesno =~ /y/i) {
829 return $resp;
830 }
831 }
832 }
833 return;
834 }
It seems that the "return defined" line causes warnings.
I don't know perl but I tried patching /usr/lib/git/git-send-email with
the attached diff and got a following output:
Warning: unable to close filehandle __ANONIO__ properly: Bad file descriptor at /usr/lib/git/git-send-email line 812.
Warning: unable to close filehandle __ANONIO__ properly: Bad file descriptor at /usr/lib/git/git-send-email line 812.
term: Term::ReadLine=HASH(0x56138dd97c50)
*** buffer overflow detected ***: /usr/bin/perl terminated
error: git-send-email died of signal 6
It makes me suspect that maybe readline() call causes a crash.
Does this report make sense? Is there anything else I can provide?
--- /usr/lib/git/git-send-email.orig 2019-01-01 23:38:35.980954492 +0100
+++ /usr/lib/git/git-send-email 2019-01-01 23:37:58.964956240 +0100
@@ -810,7 +810,9 @@
unless defined $term->IN and defined fileno($term->IN) and
defined $term->OUT and defined fileno($term->OUT);
while ($i++ < 10) {
+ print STDERR "term: $term\n";
$resp = $term->readline($prompt);
+ print STDERR "readline done\n";
if (!defined $resp) { # EOF
print "\n";
return defined $default ? $default : undef;
^ permalink raw reply
* [PATCH] test-lib: check Bash version for '-x' without using shell arrays
From: SZEDER Gábor @ 2019-01-01 23:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: Max Kirillov, Carlo Arenas, Jeff King, git, SZEDER Gábor
One of our test scripts, 't1510-repo-setup.sh' [1], still can't be
reliably run with '-x' tracing enabled, unless it's executed with a
Bash version supporting BASH_XTRACEFD (since v4.1). We have a lengthy
condition to check the version of the shell running the test script,
and disable tracing if it's not executed with a suitable Bash version
[2].
This condition uses non-portable shell array accesses to easily get
Bash's major and minor version number. This didn't seem to be
problematic, because the simple commands expanding those array
accesses are only executed when the test script is actually run with
Bash. When run with Dash, the only shell I have at hand that doesn't
support shell arrays, there are no issues, as it apparently skips
right over the non-executed simple commands without noticing the
non-supported constructs.
Alas, it has been reported that NetBSD's /bin/sh does complain about
them:
./test-lib.sh: 327: Syntax error: Bad substitution
where line 327 contains the first ${BASH_VERSINFO[0]} array access.
To my understanding both shells are right and conform to POSIX,
because the standard allows both behavior by stating the following
under '2.8.1 Consequences of Shell Errors':
"An expansion error is one that occurs when the shell expansions
define in wordexp are carried out (for example, "${x!y}", because
'!' is not a valid operator); an implementation may treat these as
syntax errors if it is able to detect them during tokenization,
rather than during expansion."
Avoid this issue with NetBSD's /bin/sh (and potentially with other,
less common shells) by using a couple of parameter expansions to
extract the major and minor version numbers from $BASH_VERSION.
[1] 5827506928 (t1510-repo-setup: mark as untraceable with '-x',
2018-02-24)
[2] 5fc98e79fc (t: add means to disable '-x' tracing for individual
test scripts, 2018-02-24)
Reported-by: Max Kirillov <max@max630.net>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
This will certainly conflict with patches 3 and 6 in my stress testing
patch series at
https://public-inbox.org/git/20181230191629.3232-1-szeder.dev@gmail.com/T/#u
t/test-lib.sh | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0f1faa24b2..f47a191e3b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -324,9 +324,12 @@ do
# isn't executed with a suitable Bash version.
if test -z "$test_untraceable" || {
test -n "$BASH_VERSION" && {
- test ${BASH_VERSINFO[0]} -gt 4 || {
- test ${BASH_VERSINFO[0]} -eq 4 &&
- test ${BASH_VERSINFO[1]} -ge 1
+ bash_major=${BASH_VERSION%%.*}
+ bash_minor=${BASH_VERSION#*.}
+ bash_minor=${bash_minor%%.*}
+ test $bash_major -gt 4 || {
+ test $bash_major -eq 4 &&
+ test $bash_minor -ge 1
}
}
}
--
2.20.1.151.gec613c4b75
^ permalink raw reply related
* Regression `git checkout $rev -b branch` while in a `--no-checkout` clone does not check out files
From: Anthony Sottile @ 2019-01-01 23:17 UTC (permalink / raw)
To: Git Mailing List
Here's a simple regression test -- haven't had time to bisect this
```
#!/usr/bin/env bash
set -euxo pipefail
rm -rf src dest
git --version
git init src
echo hi > src/a
git -C src add .
git -C src commit -m "initial commit"
rev="$(git -C src rev-parse HEAD)"
git clone --no-checkout src dest
git -C dest checkout "$rev" -b branch
test -f dest/a
: 'SUCCESS!'
```
With git 2.17.1
```
+ set -euo pipefail
+ rm -rf src dest
+ git --version
git version 2.17.1
+ git init src
Initialized empty Git repository in /tmp/t/src/.git/
+ echo hi
+ git -C src add .
+ git -C src commit -m 'initial commit'
[master (root-commit) 61ae2ae] initial commit
1 file changed, 1 insertion(+)
create mode 100644 a
++ git -C src rev-parse HEAD
+ rev=61ae2ae9c9a96de7b1688b095f20a6adf9c20db1
+ git clone --no-checkout src dest
Cloning into 'dest'...
done.
+ git -C dest checkout 61ae2ae9c9a96de7b1688b095f20a6adf9c20db1 -b branch
Switched to a new branch 'branch'
+ test -f dest/a
+ : 'SUCCESS!'
```
With git 2.20.GIT (b21ebb671bb7dea8d342225f0d66c41f4e54d5ca)
```
+ set -euo pipefail
+ rm -rf src dest
+ git --version
git version 2.20.GIT
+ git init src
Initialized empty Git repository in /tmp/t/src/.git/
+ echo hi
+ git -C src add .
+ git -C src commit -m 'initial commit'
[master (root-commit) df4d6dc] initial commit
1 file changed, 1 insertion(+)
create mode 100644 a
++ git -C src rev-parse HEAD
+ rev=df4d6dcf02b15bfe2b3f0e4a8aa25ac165b1368c
+ git clone --no-checkout src dest
Cloning into 'dest'...
done.
+ git -C dest checkout df4d6dcf02b15bfe2b3f0e4a8aa25ac165b1368c -b branch
D a
Switched to a new branch 'branch'
+ test -f dest/a
```
A workaround for my use case is to not use `--no-checkout`, though
this is potentially slow
Anthony
^ permalink raw reply
* [PATCH v2] doc: remove unneeded TODO for release_commit_memory
From: Albert Burt @ 2019-01-01 20:08 UTC (permalink / raw)
To: pclouds; +Cc: git, gitster, stefanbeller, Albert Burt
In-Reply-To: <CACsJy8D_gTKWXogPDNW7NQk_a0ChBu28HfGu388hFn3-by_cRw@mail.gmail.com>
Remove TODO that was left in from:
commit 110240588d (Merge branch 'sb/object-store-alloc' - 2018-06-25)
Todo can be removed as:
9d2c97016f (commit.h: delete 'util' field in struct commit - 2018-05-19)
deletes commit->util.
Signed-off-by: Albert Burt <aburthinds@gmail.com>
---
Thanks for looking at this for me Duy. I updated some of the changes you
suggested.
Let me know if there's anything else that I would need to clean up, or do better.
:)
commit.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/commit.c b/commit.c
index 2d94e0b199..2ff6dca0bc 100644
--- a/commit.c
+++ b/commit.c
@@ -357,8 +357,6 @@ void release_commit_memory(struct commit *c)
c->index = 0;
free_commit_buffer(c);
free_commit_list(c->parents);
- /* TODO: what about commit->util? */
-
c->object.parsed = 0;
}
--
2.17.2 (Apple Git-113)
^ permalink raw reply related
* [PATCH v3 2/2] completion: treat results of git ls-tree as file paths
From: Chayoung You @ 2019-01-01 14:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, SZEDER Gábor
In-Reply-To: <20190101140509.1857-1-yousbe@gmail.com>
Let's say there are files named 'foo bar.txt', and 'abc def/test.txt' in
repository. When following commands trigger a completion:
git show HEAD:fo<Tab>
git show HEAD:ab<Tab>
The completion results in bash/zsh:
git show HEAD:foo bar.txt
git show HEAD:abc def/
Where the both of them have an unescaped space in paths, so they'll be
misread by git. All entries of git ls-tree either a filename or a
directory, so __gitcomp_file() is proper rather than __gitcomp_nl().
Note the commit f12785a3, which handles quoted paths properly. Like this
case, we should dequote $cur_ for ?*:* case. For example, let's say
there is untracked directory 'abc deg', then trigger a completion:
git show HEAD:abc\ de<Tab>
git show HEAD:'abc de<Tab>
git show HEAD:"abc de<Tab>
should uniquely complete 'abc def', but bash completes 'abc def' and
'abc deg' instead. In zsh, triggering a completion:
git show HEAD:abc\ def/<Tab>
should complete 'test.txt', but nothing comes. The both problems will be
resolved by dequoting paths.
__git_complete_revlist_file() passes arguments to __gitcomp_nl() where
the first one is a list something like:
abc def/Z
foo bar.txt Z
where Z is the mark of the EOL.
- The trailing space of blob in __git ls-tree | sed.
It makes the completion results become:
git show HEAD:foo\ bar.txt\ <CURSOR>
So git will try to find a file named 'foo bar.txt ' instead.
- The trailing slash of tree in __git ls-tree | sed.
It makes the completion results on zsh become:
git show HEAD:abc\ def/ <CURSOR>
So that the last space on command like should be removed on zsh to
complete filenames under 'abc def/'.
Signed-off-by: Chayoung You <yousbe@gmail.com>
---
contrib/completion/git-completion.bash | 31 ++++++++++----------------
t/t9902-completion.sh | 10 ++++-----
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 816ee3280..26ea310fd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -855,7 +855,7 @@ __git_compute_merge_strategies ()
__git_complete_revlist_file ()
{
- local pfx ls ref cur_="$cur"
+ local dequoted_word pfx ls ref cur_="$cur"
case "$cur_" in
*..?*:*)
return
@@ -863,14 +863,18 @@ __git_complete_revlist_file ()
?*:*)
ref="${cur_%%:*}"
cur_="${cur_#*:}"
- case "$cur_" in
+
+ __git_dequote "$cur_"
+
+ case "$dequoted_word" in
?*/*)
- pfx="${cur_%/*}"
- cur_="${cur_##*/}"
+ pfx="${dequoted_word%/*}"
+ cur_="${dequoted_word##*/}"
ls="$ref:$pfx"
pfx="$pfx/"
;;
*)
+ cur_="$dequoted_word"
ls="$ref"
;;
esac
@@ -880,21 +884,10 @@ __git_complete_revlist_file ()
*) pfx="$ref:$pfx" ;;
esac
- __gitcomp_nl "$(__git ls-tree "$ls" \
- | sed '/^100... blob /{
- s,^.* ,,
- s,$, ,
- }
- /^120000 blob /{
- s,^.* ,,
- s,$, ,
- }
- /^040000 tree /{
- s,^.* ,,
- s,$,/,
- }
- s/^.* //')" \
- "$pfx" "$cur_" ""
+ __gitcomp_file "$(__git ls-tree "$ls" \
+ | sed 's/^.* //
+ s/$//')" \
+ "$pfx" "$cur_"
;;
*...*)
pfx="${cur_%...*}..."
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 137fdc9bd..94157e587 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1515,8 +1515,8 @@ test_expect_success 'show completes all refs' '
test_expect_success '<ref>: completes paths' '
test_completion "git show mytag:f" <<-\EOF
- file1 Z
- file2 Z
+ file1Z
+ file2Z
EOF
'
@@ -1525,7 +1525,7 @@ test_expect_success 'complete tree filename with spaces' '
git add "name with spaces" &&
git commit -m spaces &&
test_completion "git show HEAD:nam" <<-\EOF
- name with spaces Z
+ name with spacesZ
EOF
'
@@ -1534,8 +1534,8 @@ test_expect_success 'complete tree filename with metacharacters' '
git add "name with \${meta}" &&
git commit -m meta &&
test_completion "git show HEAD:nam" <<-\EOF
- name with ${meta} Z
- name with spaces Z
+ name with ${meta}Z
+ name with spacesZ
EOF
'
--
2.17.1
^ permalink raw reply related
* [PATCH v3 1/2] zsh: complete unquoted paths with spaces correctly
From: Chayoung You @ 2019-01-01 14:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, SZEDER Gábor
In-Reply-To: <20190101140509.1857-1-yousbe@gmail.com>
The following is the description of -Q flag of zsh compadd [1]:
This flag instructs the completion code not to quote any
metacharacters in the words when inserting them into the command
line.
Let's say there is a file named 'foo bar.txt' in repository, but it's
not yet added to the repository. Then the following command triggers a
completion:
git add fo<Tab>
git add 'fo<Tab>
git add "fo<Tab>
The completion results in bash:
git add foo\ bar.txt
git add 'foo bar.txt'
git add "foo bar.txt"
While them in zsh:
git add foo bar.txt
git add 'foo bar.txt'
git add "foo bar.txt"
The first one, where the pathname is not enclosed in quotes, should
escape the space with a backslash, just like bash completion does.
Otherwise, this leads git to think there are two files; foo, and
bar.txt.
The main cause of this behavior is __gitcomp_file_direct(). The both
implementions of bash and zsh are called with an argument 'foo bar.txt',
but only bash adds a backslash before a space on command line.
[1]: http://zsh.sourceforge.net/Doc/Release/Completion-Widgets.html
Signed-off-by: Chayoung You <yousbe@gmail.com>
Reviewed-by: Junio C Hamano <gitster@pobox.com>
---
contrib/completion/git-completion.bash | 4 ++--
contrib/completion/git-completion.zsh | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9e8ec95c3..816ee3280 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2993,7 +2993,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
local IFS=$'\n'
compset -P '*[=:]'
- compadd -Q -f -- ${=1} && _ret=0
+ compadd -f -- ${=1} && _ret=0
}
__gitcomp_file ()
@@ -3002,7 +3002,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
local IFS=$'\n'
compset -P '*[=:]'
- compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+ compadd -p "${2-}" -f -- ${=1} && _ret=0
}
_git ()
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 049d6b80f..886bf95d1 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -99,7 +99,7 @@ __gitcomp_file_direct ()
local IFS=$'\n'
compset -P '*[=:]'
- compadd -Q -f -- ${=1} && _ret=0
+ compadd -f -- ${=1} && _ret=0
}
__gitcomp_file ()
@@ -108,7 +108,7 @@ __gitcomp_file ()
local IFS=$'\n'
compset -P '*[=:]'
- compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+ compadd -p "${2-}" -f -- ${=1} && _ret=0
}
__git_zsh_bash_func ()
--
2.17.1
^ permalink raw reply related
* [PATCH v3 0/2] completion: handle paths with spaces correctly
From: Chayoung You @ 2019-01-01 14:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, SZEDER Gábor
In-Reply-To: <20181230053125.55896-1-yousbe@gmail.com>
I found more issues with completion for git show ref:path, so here I add one
more patch.
Chayoung You (2):
zsh: complete unquoted paths with spaces correctly
completion: treat results of git ls-tree as file paths
contrib/completion/git-completion.bash | 35 +++++++++++---------------
contrib/completion/git-completion.zsh | 4 +--
t/t9902-completion.sh | 10 ++++----
3 files changed, 21 insertions(+), 28 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: Regression in git-subtree.sh, introduced in 2.20.1, after 315a84f9aa0e2e629b0680068646b0032518ebed
From: Duy Nguyen @ 2019-01-01 13:19 UTC (permalink / raw)
To: Marc Balmer; +Cc: Git Mailing List, roger.strain, Junio C Hamano
In-Reply-To: <DE854533-FC7C-4DD4-8F42-C02C4D4524CB@msys.ch>
On Mon, Dec 31, 2018 at 01:31:21PM +0100, Marc Balmer wrote:
>
>
> > Am 31.12.2018 um 12:36 schrieb Duy Nguyen <pclouds@gmail.com>:
> >
> > On Mon, Dec 31, 2018 at 6:24 PM Marc Balmer <marc@msys.ch> wrote:
> >> In a (private) Email to me, he indicated that had no time for a fix. Maybe he can speak up here?
> >
> > Well, I guess Junio will revert when he's back after the holidays
> > then. Meanwhile..
> >
> >> In any case, if I can help testing, I am in. I just don't know the inner workings of git-subtree.sh (I am a mere user of it...)
> >
> > If the repo you're facing the problem is publicly available, that
> > would be great so some of us could try reproduce.
>
> Unfortunately it is not.
>
> >
> > Otherwise we'll need your help to track this problem down. in
> > git-subtree script line 640 (or somewhere close)
> >
> > progress "$revcount/$revmax ($createcount) [$extracount]"
> >
> > could you update it to show $parents and $rev as well, e.g.
> >
> > progress "$revcount/$revmax ($createcount) [$extracount] ($parents) ($rev)"
>
> I did add this, plus changed progress to output a linefeed, and now just before the crash, the output looks like this:
>
> 436/627 (2013) [1649] (6e54a90a29e4e01fa2d6a42c232e02e08e912b2d) (2ca7b24e731ff91c94c9abf214686cb29cdc367e)
> 436/627 (2014) [1650] (1ef866e5a18012e80eed36315deb932c2b66d34a) (6e54a90a29e4e01fa2d6a42c232e02e08e912b2d)
> 436/627 (2015) [1651] (c8585f441548dd43f113a96ba48f6fa70363d388) (1ef866e5a18012e80eed36315deb932c2b66d34a)
> 436/627 (2016) [1652] (663bb110a58decfe889cf7c6b766f1d0c032ba39) (c8585f441548dd43f113a96ba48f6fa70363d388)
> 436/627 (2017) [1653] (edbdd28e009e52c8001bb54e53a56b059167e07d) (663bb110a58decfe889cf7c6b766f1d0c032ba39)
> 436/627 (2018) [1654] (c47739713912ae6e94714b9a1a6732407b236932) (edbdd28e009e52c8001bb54e53a56b059167e07d)
> 436/627 (2019) [1655] (d444823b97d9a8e53c4e721a44e4c49619d0b372) (c47739713912ae6e94714b9a1a6732407b236932)
> 436/627 (2020) [1656] (15a7ccecb2ca8bc47c77a997f8c74e7ac3b13325) (d444823b97d9a8e53c4e721a44e4c49619d0b372)
> 436/627 (2021) [1657] (b9bc5c9b33b100b57e23626ff422dac73f94384e) (15a7ccecb2ca8bc47c77a997f8c74e7ac3b13325)
> 436/627 (2022) [1658] (eec0f28c6fe5f7d664c41a913883d64cdf53c111) (b9bc5c9b33b100b57e23626ff422dac73f94384e)
> 436/627 (2023) [1659] (e0ddd9c60f71283996cfb169f1dbb77e8f7c4b94) (eec0f28c6fe5f7d664c41a913883d64cdf53c111)
> 436/627 (2024) [1660] (27b96988847caf3bfd71df2d7f58cbe6ba78208a) (e0ddd9c60f71283996cfb169f1dbb77e8f7c4b94)
> 436/627 (2025) [1661] (11e5861e50f88237ce362b6c7531e4e90bac86ac) (27b96988847caf3bfd71df2d7f58cbe6ba78208a)
> /usr/libexec/git-core/git-subtree: line 751: 122202 Done eval "$grl"
> 122203 Segmentation fault (core dumped) | while read rev parents; do
> process_split_commit "$rev" "$parents" 0;
> done
>
>
> >
> > Then please run these commands and post the output here
> >
> > git rev-parse <that-rev>^@
>
> Did that with the last three lines:
>
> $ git rev-parse 27b96988847caf3bfd71df2d7f58cbe6ba78208a^@
> 11e5861e50f88237ce362b6c7531e4e90bac86ac
> $ git rev-parse e0ddd9c60f71283996cfb169f1dbb77e8f7c4b94^@
> 27b96988847caf3bfd71df2d7f58cbe6ba78208a
> $ git rev-parse eec0f28c6fe5f7d664c41a913883d64cdf53c111^@
> e0ddd9c60f71283996cfb169f1dbb77e8f7c4b94
>
> >
> > and
> >
> > git show -s --pretty=%P <that-rev>
>
> $ git show -s --pretty=%P 27b96988847caf3bfd71df2d7f58cbe6ba78208a
> 11e5861e50f88237ce362b6c7531e4e90bac86ac
> $ git show -s --pretty=%P e0ddd9c60f71283996cfb169f1dbb77e8f7c4b94
> 27b96988847caf3bfd71df2d7f58cbe6ba78208a
> $ git show -s --pretty=%P eec0f28c6fe5f7d664c41a913883d64cdf53c111
> e0ddd9c60f71283996cfb169f1dbb77e8f7c4b94
>
Hmm.. I'm not that familiar with git-subtree.sh, so here's one last
blind shot.
There's a format change between git-show and git-rev-parse. The former
separates commits by spaces while the latter by newlines. Will this
help?
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 147201dc6c..23f570beee 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -633,7 +633,7 @@ process_split_commit () {
else
# processing commit without normal parent information;
# fetch from repo
- parents=$(git rev-parse "$rev^@")
+ parents=$(git rev-parse "$rev^@" | tr '\n' ' ')
extracount=$(($extracount + 1))
fi
--
Duy
^ permalink raw reply related
* Re: [PATCH] doc: remove unneeded TODO for release_commit_memory
From: Duy Nguyen @ 2019-01-01 10:40 UTC (permalink / raw)
To: Albert Burt; +Cc: Git Mailing List, Junio C Hamano, Stefan Beller
In-Reply-To: <20181231235713.76200-1-aburthinds@gmail.com>
On Tue, Jan 1, 2019 at 7:02 AM Albert Burt <aburthinds@gmail.com> wrote:
>
> The code that was merged with commit 1102405 left in an TODO that
People usually use something this to produce the commit reference
git show -s --date=short --pretty='format:%h (%s - %ad)'
which produces
110240588d (Merge branch 'sb/object-store-alloc' - 2018-06-25)
> is no longer relevant. It seems as if we can remove this todo.
> util seems to not be a field of the struct commit,
> definition of struct commit --> commit.c:27
> The commit list also, does not contain a field for util.
There was commit->util, which was deleted in 9d2c97016f (commit.h:
delete 'util' field in struct commit - 2018-05-19). You can mention it
too if you reroll this patch.
> ____
> By making a contribution to this project, I certify that:
>
> a. The contribution was created in whole or in part by me and I
> have the right to submit it under the open source license
> indicated in the file; or
>
> b. The contribution is based upon previous work that, to the best
> of my knowledge, is covered under an appropriate open source
> license and I have the right under that license to submit that
> work with modifications, whether created in whole or in part
> by me, under the same open source license (unless I am
> permitted to submit under a different license), as indicated
> in the file; or
>
> c. The contribution was provided directly to me by some other
> person who certified (a), (b) or (c) and I have not modified
> it.
>
> d. I understand and agree that this project and the contribution
> are public and that a record of the contribution (including all
> personal information I submit with it, including my sign-off) is
> maintained indefinitely and may be redistributed consistent with
> this project or the open source license(s) involved.
> ____
I think you can drop this. Your S-o-b below implies all this.
> Signed-off-by: Albert Burt <aburthinds@gmail.com>
> ---
> commit.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/commit.c b/commit.c
> index 2d94e0b199..2ff6dca0bc 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -357,8 +357,6 @@ void release_commit_memory(struct commit *c)
> c->index = 0;
> free_commit_buffer(c);
> free_commit_list(c->parents);
> - /* TODO: what about commit->util? */
> -
Obviously correct :)
> c->object.parsed = 0;
> }
>
> --
> 2.17.2 (Apple Git-113)
>
--
Duy
^ permalink raw reply
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