* [PATCH v3] revision: add separate field for "-m" of "diff-index -m"
From: Sergey Organov @ 2020-08-31 12:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, Sergey Organov
In-Reply-To: <20200829194634.23306-1-sorganov@gmail.com>
Add separate 'diff_index_match_missing' field for diff-index to use and set it
when we encounter "-m" option. This field won't then be cleared when another
meaning of "-m" is reverted (e.g., by "--no-diff-merges"), nor it will be
affected by future option(s) that might drive 'ignore_merges' field.
Use this new field from diff-lib:do_oneway_diff() instead of reusing
'ignore_merges' field.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
v3: improve commit message
v2: rebased from 'maint' onto 'master'
diff-lib.c | 10 ++--------
revision.c | 6 ++++++
revision.h | 1 +
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index 50521e2093fc..f2aee78e7aa2 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -405,14 +405,8 @@ static void do_oneway_diff(struct unpack_trees_options *o,
/* if the entry is not checked out, don't examine work tree */
cached = o->index_only ||
(idx && ((idx->ce_flags & CE_VALID) || ce_skip_worktree(idx)));
- /*
- * Backward compatibility wart - "diff-index -m" does
- * not mean "do not ignore merges", but "match_missing".
- *
- * But with the revision flag parsing, that's found in
- * "!revs->ignore_merges".
- */
- match_missing = !revs->ignore_merges;
+
+ match_missing = revs->diff_index_match_missing;
if (cached && idx && ce_stage(idx)) {
struct diff_filepair *pair;
diff --git a/revision.c b/revision.c
index 96630e31867d..64b16f7d1033 100644
--- a/revision.c
+++ b/revision.c
@@ -2345,6 +2345,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->diffopt.flags.tree_in_recursive = 1;
} else if (!strcmp(arg, "-m")) {
revs->ignore_merges = 0;
+ /*
+ * Backward compatibility wart - "diff-index -m" does
+ * not mean "do not ignore merges", but "match_missing",
+ * so set separate flag for it.
+ */
+ revs->diff_index_match_missing = 1;
} else if ((argcount = parse_long_opt("diff-merges", argv, &optarg))) {
if (!strcmp(optarg, "off")) {
revs->ignore_merges = 1;
diff --git a/revision.h b/revision.h
index c1e5bcf139d7..5ae8254ffaed 100644
--- a/revision.h
+++ b/revision.h
@@ -188,6 +188,7 @@ struct rev_info {
unsigned int diff:1,
full_diff:1,
show_root_diff:1,
+ diff_index_match_missing:1,
no_commit_id:1,
verbose_header:1,
combine_merges:1,
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] revision: add separate field for "-m" of "diff-index -m"
From: Sergey Organov @ 2020-08-31 12:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqqo8mrh12b.fsf@gitster.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> Historically, in "diff-index -m", "-m" does not mean "do not ignore
>> merges", but
>> "match missing". Despite this, diff-index abuses 'ignore_merges'
>> field being set
>> by "-m", that in turn causes more troubles.
>
> "causes more troubles"? When there is no trouble, and no "more"
> trouble, concretely mentioned, it is a quite weak justfiication.
Well, existed comment says "Backward compatibility wart" that sounds
like a trouble to me already. No?
Then, since "--[no-]diff-merges" is introduced, we have:
$ git diff-index HEAD
:100644 000000 4aec621a6d1a9a5892f0b4b6feb2ed329fd04bf2 0000000000000000000000000000000000000000 D main/main.cc
$ git diff-index -m HEAD
$ git diff-index -m --no-diff-merges HEAD
:100644 000000 4aec621a6d1a9a5892f0b4b6feb2ed329fd04bf2 0000000000000000000000000000000000000000 D main/main.cc
that sounds like yet another trouble. That's why I used "more trouble" in my
commit message.
If you say "compatibility wart" is not a trouble by itself, -- I'm fine
with it, -- then "more" in my commit message is misplaced indeed.
>
> There is no reason to say "historically" here, as it has been like
> so from beginning of the time, it still is so and it is relied
> upon. "diff-{files,index,tree}" are about comparing two things, and
> not about history (where a "merge" might influence "now we are
> showing this commit. which parent do we compare it with?"), so
> giving short-and-sweet "-m" its own meaning that is sensible within
> the context of "diff" was and is perfectly sensible thing to do.
Well, if "historically" makes you feel uncomfortable, -- I'm willing to
get rid of it.
>
> What is worth fixing is not "-m" in diff-index means "match missing"
> while "-m" in log wants to mean "show merges". It is that, even both
> commands use the same option parsing machinery, and the use of these
> two options are mutually exclusive so there is no risk of confusion,
> the flag internally used to record the presense of the "em" option is
> not named neutrally (e.g. "revs->seen_em_option").
>
> The "log" family of commands and "diff" family of commands
> share the same command line parsiong machinery. For the
> former, "-m" means "show merges" while for the latter it
> means "match missing". Tnis is not a problem at the UI
> level, as "show/not show merges" is meaningless in the
> context of "diff", and similarly "match/not match missing"
> is meaningless in the context of "log".
>
> But there are two problems with this arrangement.
>
> 1. the field the presense of the option on the command line
> is recorded in has to be given a name. It is currently
> called "ignore_merges", which gives an incorrect
> impression that using it for "diff" family is somehow a
> mistake, and renaming it to "match_missing" would not be
> a solution, as it will give an incorrect impression that
> "log" family is abusing it. However, naming the field to
> something neutral, e.g. "em_option", would make the code
> harder to understand.
>
> 2. because it uses the same command line parser, giving a
> default for "diff -m" in a way that is different from the
> default for "log -m" is quite cumbersome if they use the
> same field to record it.
>
> Introduce a separate "match_missing" field, and flip it and
> "ignore_merges" when we see the "-m" option on the command
> line. That way, even when ignore_merges's default is
> affected by end-user configuration, the default for
> "match_missing" would not be affected.
>
> I think the above would be in line with what you wanted to say but
> didn't, and I think it supports the split fairly well.
>
> I have a very strong objection against changing the built-in default
> of "log -m", but I do agree that this split of the single field into
> two is a fairly good idea. So I do not want to be in the position
> that must reject this change because "log -m" and "diff-index -m"
> will never be on by default. Basing the justification of this
> change on end-user configurability would be a good way to sidestep
> the issue, and avoids taking this change hostage to the discussion
> on what should be the built-in default for "log/diff-index -m".
This change has nothing to do with defaults. It rather about correct and
clear code.
I'll re-roll with better commit message.
Thanks,
-- Sergey
^ permalink raw reply
* [PATCH v7 06/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
Reimplement the `bisect_next()` and the `bisect_auto_next()` shell functions
in C and add the subcommands to `git bisect--helper` to call them from
git-bisect.sh .
bisect_auto_next() function returns an enum bisect_error type as whole
`git bisect` can exit with an error code when bisect_next() does.
Using `--bisect-next` and `--bisect-auto-next` subcommands is a
temporary measure to port shell function to C so as to use the existing
test suite. As more functions are ported, `--bisect-auto-next`
subcommand will be retired and will be called by some other methods.
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>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
bisect.c | 11 ++-
builtin/bisect--helper.c | 180 ++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 47 +---------
3 files changed, 190 insertions(+), 48 deletions(-)
diff --git a/bisect.c b/bisect.c
index c6aba2b9f2..f5b1368128 100644
--- a/bisect.c
+++ b/bisect.c
@@ -988,8 +988,11 @@ void read_bisect_terms(const char **read_bad, const char **read_good)
* the bisection process finished successfully.
* In this case the calling function or command should not turn a
* BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND return code into an error or a non zero exit code.
- * If no_checkout is non-zero, the bisection process does not
- * checkout the trial commit but instead simply updates BISECT_HEAD.
+ *
+ * Checking BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND
+ * in bisect_helper::bisect_next() and only transforming it to 0 at
+ * the end of bisect_helper::cmd_bisect__helper() helps bypassing
+ * all the code related to finding a commit to test.
*/
enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
{
@@ -999,6 +1002,10 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
enum bisect_error res = BISECT_OK;
struct object_id *bisect_rev;
char *steps_msg;
+ /*
+ * If no_checkout is non-zero, the bisection process does not
+ * checkout the trial commit but instead simply updates BISECT_HEAD.
+ */
int no_checkout = ref_exists("BISECT_HEAD");
unsigned bisect_flags = 0;
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index f71e8e54d2..e29e86142a 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -8,6 +8,7 @@
#include "run-command.h"
#include "prompt.h"
#include "quote.h"
+#include "revision.h"
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -29,10 +30,17 @@ static const char * const git_bisect_helper_usage[] = {
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] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]"),
+ N_("git bisect--helper --bisect-next"),
+ N_("git bisect--helper --bisect-auto-next"),
N_("git bisect--helper --bisect-autostart"),
NULL
};
+struct add_bisect_ref_data {
+ struct rev_info *revs;
+ unsigned int object_flags;
+};
+
struct bisect_terms {
char *term_good;
char *term_bad;
@@ -56,6 +64,8 @@ static void set_terms(struct bisect_terms *terms, const char *bad,
static const char vocab_bad[] = "bad|new";
static const char vocab_good[] = "good|old";
+static int bisect_autostart(struct bisect_terms *terms);
+
/*
* Check whether the string `term` belongs to the set of strings
* included in the variable arguments.
@@ -80,7 +90,7 @@ static int write_in_file(const char *path, const char *mode, const char *format,
FILE *fp = NULL;
int res = 0;
- if (strcmp(mode, "w"))
+ if (strcmp(mode, "w") && strcmp(mode, "a"))
BUG("write-in-file does not support '%s' mode", mode);
fp = fopen(path, mode);
if (!fp)
@@ -109,6 +119,18 @@ static int write_to_file(const char *path, const char *format, ...)
return res;
}
+static int append_to_file(const char *path, const char *format, ...)
+{
+ int res;
+ va_list args;
+
+ va_start(args, format);
+ res = write_in_file(path, "a", format, args);
+ va_end(args);
+
+ return res;
+}
+
static int check_term_format(const char *term, const char *orig_term)
{
int res;
@@ -451,6 +473,140 @@ static int bisect_append_log_quoted(const char **argv)
return res;
}
+static int add_bisect_ref(const char *refname, const struct object_id *oid,
+ int flags, void *cb)
+{
+ struct add_bisect_ref_data *data = cb;
+
+ add_pending_oid(data->revs, refname, oid, data->object_flags);
+
+ return 0;
+}
+
+static int prepare_revs(struct bisect_terms *terms, struct rev_info *revs)
+{
+ int res = 0;
+ struct add_bisect_ref_data cb = { revs };
+ char *good = xstrfmt("%s-*", terms->term_good);
+
+ /*
+ * We cannot use terms->term_bad directly in
+ * for_each_glob_ref_in() and we have to append a '*' to it,
+ * otherwise for_each_glob_ref_in() will append '/' and '*'.
+ */
+ char *bad = xstrfmt("%s*", terms->term_bad);
+
+ /*
+ * It is important to reset the flags used by revision walks
+ * as the previous call to bisect_next_all() in turn
+ * sets up a revision walk.
+ */
+ reset_revision_walk();
+ init_revisions(revs, NULL);
+ setup_revisions(0, NULL, revs, NULL);
+ for_each_glob_ref_in(add_bisect_ref, bad, "refs/bisect/", &cb);
+ cb.object_flags = UNINTERESTING;
+ for_each_glob_ref_in(add_bisect_ref, good, "refs/bisect/", &cb);
+ if (prepare_revision_walk(revs))
+ res = error(_("revision walk setup failed\n"));
+
+ free(good);
+ free(bad);
+ return res;
+}
+
+static int bisect_skipped_commits(struct bisect_terms *terms)
+{
+ int res;
+ FILE *fp = NULL;
+ struct rev_info revs;
+ struct commit *commit;
+ struct pretty_print_context pp = {0};
+ struct strbuf commit_name = STRBUF_INIT;
+
+ res = prepare_revs(terms, &revs);
+ if (res)
+ return res;
+
+ fp = fopen(git_path_bisect_log(), "a");
+ if (!fp)
+ return error_errno(_("could not open '%s' for appending"),
+ git_path_bisect_log());
+
+ if (fprintf(fp, "# only skipped commits left to test\n") < 0)
+ return error_errno(_("failed to write to '%s'"), git_path_bisect_log());
+
+ while ((commit = get_revision(&revs)) != NULL) {
+ strbuf_reset(&commit_name);
+ format_commit_message(commit, "%s",
+ &commit_name, &pp);
+ fprintf(fp, "# possible first %s commit: [%s] %s\n",
+ terms->term_bad, oid_to_hex(&commit->object.oid),
+ commit_name.buf);
+ }
+
+ /*
+ * Reset the flags used by revision walks in case
+ * there is another revision walk after this one.
+ */
+ reset_revision_walk();
+
+ strbuf_release(&commit_name);
+ fclose(fp);
+ return 0;
+}
+
+static int bisect_successful(struct bisect_terms *terms)
+{
+ struct object_id oid;
+ struct commit *commit;
+ struct pretty_print_context pp = {0};
+ struct strbuf commit_name = STRBUF_INIT;
+ char *bad_ref = xstrfmt("refs/bisect/%s",terms->term_bad);
+ int res;
+
+ read_ref(bad_ref, &oid);
+ commit = lookup_commit_reference_by_name(bad_ref);
+ format_commit_message(commit, "%s", &commit_name, &pp);
+
+ res = append_to_file(git_path_bisect_log(), "# first %s commit: [%s] %s\n",
+ terms->term_bad, oid_to_hex(&commit->object.oid),
+ commit_name.buf);
+
+ strbuf_release(&commit_name);
+ free(bad_ref);
+ return res;
+}
+
+static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)
+{
+ enum bisect_error res;
+
+ bisect_autostart(terms);
+ if (bisect_next_check(terms, terms->term_good))
+ return BISECT_FAILED;
+
+ /* Perform all bisection computation */
+ res = bisect_next_all(the_repository, prefix);
+
+ if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
+ res = bisect_successful(terms);
+ return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
+ } else if (res == BISECT_ONLY_SKIPPED_LEFT) {
+ res = bisect_skipped_commits(terms);
+ return res ? res : BISECT_ONLY_SKIPPED_LEFT;
+ }
+ return res;
+}
+
+static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char *prefix)
+{
+ if (bisect_next_check(terms, NULL))
+ return BISECT_OK;
+
+ return bisect_next(terms, prefix);
+}
+
static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)
{
int no_checkout = 0;
@@ -699,7 +855,9 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_NEXT_CHECK,
BISECT_TERMS,
BISECT_START,
- BISECT_AUTOSTART,
+ BISECT_NEXT,
+ BISECT_AUTO_NEXT,
+ BISECT_AUTOSTART
} cmdmode = 0;
int res = 0, nolog = 0;
struct option options[] = {
@@ -723,6 +881,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("print out the bisect terms"), BISECT_TERMS),
OPT_CMDMODE(0, "bisect-start", &cmdmode,
N_("start the bisect session"), BISECT_START),
+ OPT_CMDMODE(0, "bisect-next", &cmdmode,
+ N_("find the next bisection commit"), BISECT_NEXT),
+ OPT_CMDMODE(0, "bisect-auto-next", &cmdmode,
+ N_("verify the next bisection state then checkout the next bisection commit"), BISECT_AUTO_NEXT),
OPT_CMDMODE(0, "bisect-autostart", &cmdmode,
N_("start the bisection if it has not yet been started"), BISECT_AUTOSTART),
OPT_BOOL(0, "no-log", &nolog,
@@ -784,6 +946,18 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
set_terms(&terms, "bad", "good");
res = bisect_start(&terms, argv, argc);
break;
+ case BISECT_NEXT:
+ if (argc)
+ return error(_("--bisect-next requires 0 arguments"));
+ get_terms(&terms);
+ res = bisect_next(&terms, prefix);
+ break;
+ case BISECT_AUTO_NEXT:
+ if (argc)
+ return error(_("--bisect-auto-next requires 0 arguments"));
+ get_terms(&terms);
+ res = bisect_auto_next(&terms, prefix);
+ break;
case BISECT_AUTOSTART:
if (argc)
return error(_("--bisect-autostart does not accept arguments"));
@@ -799,7 +973,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
* Handle early success
* From check_merge_bases > check_good_are_ancestors_of_bad > bisect_next_all
*/
- if (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE)
+ if ((res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) || (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND))
res = BISECT_OK;
return -res;
diff --git a/git-bisect.sh b/git-bisect.sh
index 9ca583d964..59424f5b37 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -65,8 +65,7 @@ bisect_start() {
#
# Check if we can proceed to the next bisect state.
#
- get_terms
- bisect_auto_next
+ git bisect--helper --bisect-auto-next || exit
trap '-' 0
}
@@ -119,45 +118,7 @@ bisect_state() {
*)
usage ;;
esac
- bisect_auto_next
-}
-
-bisect_auto_next() {
- git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD && bisect_next || :
-}
-
-bisect_next() {
- case "$#" in 0) ;; *) usage ;; esac
- git bisect--helper --bisect-autostart
- 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
- res=$?
-
- # Check if we should exit because bisection is finished
- if test $res -eq 10
- then
- bad_rev=$(git show-ref --hash --verify refs/bisect/$TERM_BAD)
- bad_commit=$(git show-branch $bad_rev)
- echo "# first $TERM_BAD commit: $bad_commit" >>"$GIT_DIR/BISECT_LOG"
- exit 0
- elif test $res -eq 2
- then
- echo "# only skipped commits left to test" >>"$GIT_DIR/BISECT_LOG"
- good_revs=$(git for-each-ref --format="%(objectname)" "refs/bisect/$TERM_GOOD-*")
- for skipped in $(git rev-list refs/bisect/$TERM_BAD --not $good_revs)
- do
- skipped_commit=$(git show-branch $skipped)
- echo "# possible first $TERM_BAD commit: $skipped_commit" >>"$GIT_DIR/BISECT_LOG"
- done
- exit $res
- fi
-
- # Check for an error in the bisection process
- test $res -ne 0 && exit $res
-
- return 0
+ git bisect--helper --bisect-auto-next
}
bisect_visualize() {
@@ -213,7 +174,7 @@ bisect_replay () {
esac
done <"$file"
IFS="$oIFS"
- bisect_auto_next
+ git bisect--helper --bisect-auto-next
}
bisect_run () {
@@ -310,7 +271,7 @@ case "$#" in
bisect_skip "$@" ;;
next)
# Not sure we want "next" at the UI level anymore.
- bisect_next "$@" ;;
+ git bisect--helper --bisect-next "$@" || exit ;;
visualize|view)
bisect_visualize "$@" ;;
reset)
--
2.25.0
^ permalink raw reply related
* [PATCH v7 07/13] bisect--helper: finish porting `bisect_start()` to C
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Christian Couder, Johannes Schindelin,
Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
Add the subcommand to `git bisect--helper` and call it from
git-bisect.sh.
With the conversion of `bisect_auto_next()` from shell to C in a
previous commit, `bisect_start()` can now be fully ported to C.
So let's complete the `--bisect-start` subcommand of
`git bisect--helper` so that it fully implements `bisect_start()`,
and let's use this subcommand in `git-bisect.sh` instead of
`bisect_start()`.
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>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 56 ++++++++++++++++++++++++++++------------
git-bisect.sh | 26 ++-----------------
2 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index e29e86142a..b40369e8aa 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -85,6 +85,19 @@ static int one_of(const char *term, ...)
return res;
}
+/*
+ * return code BISECT_INTERNAL_SUCCESS_MERGE_BASE
+ * and BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND are codes
+ * that indicate special success.
+ */
+
+static int is_bisect_success(enum bisect_error res)
+{
+ return !res ||
+ res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND ||
+ res == BISECT_INTERNAL_SUCCESS_MERGE_BASE;
+}
+
static int write_in_file(const char *path, const char *mode, const char *format, va_list args)
{
FILE *fp = NULL;
@@ -607,12 +620,13 @@ static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char
return bisect_next(terms, prefix);
}
-static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)
+static enum bisect_error bisect_start(struct bisect_terms *terms, const char **argv, int argc)
{
int no_checkout = 0;
int first_parent_only = 0;
int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
- int flags, pathspec_pos, res = 0;
+ int flags, pathspec_pos;
+ enum bisect_error res = BISECT_OK;
struct string_list revs = STRING_LIST_INIT_DUP;
struct string_list states = STRING_LIST_INIT_DUP;
struct strbuf start_head = STRBUF_INIT;
@@ -672,9 +686,12 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)
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);
+ if (get_oid(commit_id, &oid) && has_double_dash) {
+ error(_("'%s' does not appear to be a valid "
+ "revision"), arg);
+ free(commit_id);
+ return BISECT_FAILED;
+ }
string_list_append(&revs, oid_to_hex(&oid));
free(commit_id);
@@ -752,14 +769,7 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)
* 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".
- */
+ return BISECT_FAILED;
/*
* Write new start state
@@ -776,7 +786,7 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)
}
if (update_ref(NULL, "BISECT_HEAD", &oid, NULL, 0,
UPDATE_REFS_MSG_ON_ERR)) {
- res = -1;
+ res = BISECT_FAILED;
goto finish;
}
}
@@ -788,25 +798,37 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)
for (i = 0; i < states.nr; i++)
if (bisect_write(states.items[i].string,
revs.items[i].string, terms, 1)) {
- res = -1;
+ res = BISECT_FAILED;
goto finish;
}
if (must_write_terms && write_terms(terms->term_bad,
terms->term_good)) {
- res = -1;
+ res = BISECT_FAILED;
goto finish;
}
res = bisect_append_log_quoted(argv);
if (res)
- res = -1;
+ res = BISECT_FAILED;
finish:
string_list_clear(&revs, 0);
string_list_clear(&states, 0);
strbuf_release(&start_head);
strbuf_release(&bisect_names);
+ if (res)
+ return res;
+
+ res = bisect_auto_next(terms, NULL);
+ /*
+ * In case of mistaken revs or checkout error,
+ * "bisect_auto_next" above may exit or misbehave.
+ * We have to handle this to be able to clean up using
+ * "bisect_clean_state".
+ */
+ if (!is_bisect_success(res))
+ bisect_clean_state();
return res;
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 59424f5b37..356264caf0 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -49,27 +49,6 @@ bisect_head()
fi
}
-bisect_start() {
- git bisect--helper --bisect-start $@ || exit
-
- #
- # Change state.
- # 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".
- #
- trap 'git bisect--helper --bisect-clean-state' 0
- trap 'exit 255' 1 2 3 15
-
- #
- # Check if we can proceed to the next bisect state.
- #
- git bisect--helper --bisect-auto-next || exit
-
- trap '-' 0
-}
-
bisect_skip() {
all=''
for arg in "$@"
@@ -163,8 +142,7 @@ bisect_replay () {
get_terms
case "$command" in
start)
- cmd="bisect_start $rev $tail"
- eval "$cmd" ;;
+ eval "git bisect--helper --bisect-start $rev $tail" ;;
"$TERM_GOOD"|"$TERM_BAD"|skip)
git bisect--helper --bisect-write "$command" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit;;
terms)
@@ -264,7 +242,7 @@ case "$#" in
help)
git bisect -h ;;
start)
- bisect_start "$@" ;;
+ git bisect--helper --bisect-start "$@" ;;
bad|good|new|old|"$TERM_BAD"|"$TERM_GOOD")
bisect_state "$cmd" "$@" ;;
skip)
--
2.25.0
^ permalink raw reply related
* [PATCH v7 09/13] bisect--helper: retire `--next-all` subcommand
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder, Tanushree Tumane,
Miriam Rubio
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
The `--next-all` subcommand is no longer used from the git-bisect.sh
shell script. Instead the function `bisect_next_all()` is called from
the C implementation of `bisect_next()`.
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: Tanushree Tumane <tanushreetumane@gmail.com>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index b8b8a7473c..0796e51672 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -20,7 +20,6 @@ static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
static const char * const git_bisect_helper_usage[] = {
- N_("git bisect--helper --next-all"),
N_("git bisect--helper --write-terms <bad_term> <good_term>"),
N_("git bisect--helper --bisect-reset [<commit>]"),
N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
@@ -866,8 +865,7 @@ static int bisect_autostart(struct bisect_terms *terms)
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
- NEXT_ALL = 1,
- WRITE_TERMS,
+ WRITE_TERMS = 1,
CHECK_EXPECTED_REVS,
BISECT_RESET,
BISECT_WRITE,
@@ -881,8 +879,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
} cmdmode = 0;
int res = 0, nolog = 0;
struct option options[] = {
- OPT_CMDMODE(0, "next-all", &cmdmode,
- N_("perform 'git bisect next'"), NEXT_ALL),
OPT_CMDMODE(0, "write-terms", &cmdmode,
N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS),
OPT_CMDMODE(0, "check-expected-revs", &cmdmode,
@@ -919,9 +915,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
usage_with_options(git_bisect_helper_usage, options);
switch (cmdmode) {
- case NEXT_ALL:
- res = bisect_next_all(the_repository, prefix);
- break;
case WRITE_TERMS:
if (argc != 2)
return error(_("--write-terms requires two arguments"));
--
2.25.0
^ permalink raw reply related
* [PATCH v7 10/13] bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions in C
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
Reimplement the `bisect_state()` shell functions in C and also add a
subcommand `--bisect-state` to `git-bisect--helper` to call them from
git-bisect.sh .
Using `--bisect-state` subcommand is a temporary measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other methods.
`bisect_head()` is only called from `bisect_state()`, thus it is not
required to introduce another 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>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 78 +++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 55 +++-------------------------
2 files changed, 81 insertions(+), 52 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 0796e51672..a976f4739c 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -31,6 +31,8 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-next"),
N_("git bisect--helper --bisect-auto-next"),
N_("git bisect--helper --bisect-autostart"),
+ N_("git bisect--helper --bisect-state (bad|new) [<rev>]"),
+ N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
NULL
};
@@ -862,6 +864,72 @@ static int bisect_autostart(struct bisect_terms *terms)
return res;
}
+static enum bisect_error bisect_state(struct bisect_terms *terms, const char **argv,
+ int argc)
+{
+ const char *state;
+ int i, verify_expected = 1;
+ struct object_id oid, expected;
+ struct strbuf buf = STRBUF_INIT;
+ struct oid_array revs = OID_ARRAY_INIT;
+
+ if (!argc)
+ return error(_("Please call `--bisect-state` with at least one argument"));
+
+ state = argv[0];
+ if (check_and_set_terms(terms, state) ||
+ !one_of(state, terms->term_good, terms->term_bad, "skip", NULL))
+ return BISECT_FAILED;
+
+ argv++;
+ argc--;
+ if (argc > 1 && !strcmp(state, terms->term_bad))
+ return error(_("'git bisect %s' can take only one argument."), terms->term_bad);
+
+ if (argc == 0) {
+ enum get_oid_result res_head = get_oid("BISECT_HEAD", &oid);
+ if (res_head == MISSING_OBJECT)
+ res_head = get_oid("HEAD", &oid);
+ if (res_head) {
+ error(_("Bad bisect_head rev input"));
+ return BISECT_FAILED;
+ }
+ oid_array_append(&revs, &oid);
+ }
+
+ /*
+ * All input revs must be checked before executing bisect_write()
+ * to discard junk revs.
+ */
+
+ for (; argc; argc--, argv++) {
+ if (get_oid(*argv, &oid)){
+ error(_("Bad rev input: %s"), *argv);
+ return BISECT_FAILED;
+ }
+ oid_array_append(&revs, &oid);
+ }
+
+ if (strbuf_read_file(&buf, git_path_bisect_expected_rev(), 0) < the_hash_algo->hexsz ||
+ get_oid_hex(buf.buf, &expected) < 0)
+ verify_expected = 0; /* Ignore invalid file contents */
+ strbuf_release(&buf);
+
+ for (i = 0; i < revs.nr; i++) {
+ if (bisect_write(state, oid_to_hex(&revs.oid[i]), terms, 0))
+ return BISECT_FAILED;
+
+ if (verify_expected && !oideq(&revs.oid[i], &expected)) {
+ unlink_or_warn(git_path_bisect_ancestors_ok());
+ unlink_or_warn(git_path_bisect_expected_rev());
+ verify_expected = 0;
+ }
+ }
+
+ oid_array_clear(&revs);
+ return bisect_auto_next(terms, NULL);
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -875,7 +943,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_START,
BISECT_NEXT,
BISECT_AUTO_NEXT,
- BISECT_AUTOSTART
+ BISECT_AUTOSTART,
+ BISECT_STATE
} cmdmode = 0;
int res = 0, nolog = 0;
struct option options[] = {
@@ -901,6 +970,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("verify the next bisection state then checkout the next bisection commit"), BISECT_AUTO_NEXT),
OPT_CMDMODE(0, "bisect-autostart", &cmdmode,
N_("start the bisection if it has not yet been started"), BISECT_AUTOSTART),
+ OPT_CMDMODE(0, "bisect-state", &cmdmode,
+ N_("mark the state of ref (or refs)"), BISECT_STATE),
OPT_BOOL(0, "no-log", &nolog,
N_("no log for BISECT_WRITE")),
OPT_END()
@@ -971,6 +1042,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
set_terms(&terms, "bad", "good");
res = bisect_autostart(&terms);
break;
+ case BISECT_STATE:
+ set_terms(&terms, "bad", "good");
+ get_terms(&terms);
+ res = bisect_state(&terms, argv, argc);
+ break;
default:
BUG("unknown subcommand %d", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 356264caf0..7a8f796251 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -39,16 +39,6 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
TERM_BAD=bad
TERM_GOOD=good
-bisect_head()
-{
- if git rev-parse --verify -q BISECT_HEAD > /dev/null
- then
- echo BISECT_HEAD
- else
- echo HEAD
- fi
-}
-
bisect_skip() {
all=''
for arg in "$@"
@@ -61,43 +51,7 @@ bisect_skip() {
esac
all="$all $revs"
done
- eval bisect_state 'skip' $all
-}
-
-bisect_state() {
- git bisect--helper --bisect-autostart
- state=$1
- 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." ;;
- 1,"$TERM_BAD"|1,"$TERM_GOOD"|1,skip)
- bisected_head=$(bisect_head)
- rev=$(git rev-parse --verify "$bisected_head") ||
- die "$(eval_gettext "Bad rev input: \$bisected_head")"
- 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
- hash_list=''
- for rev in "$@"
- do
- sha=$(git rev-parse --verify "$rev^{commit}") ||
- die "$(eval_gettext "Bad rev input: \$rev")"
- hash_list="$hash_list $sha"
- done
- for rev in $hash_list
- do
- git bisect--helper --bisect-write "$state" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit
- done
- git bisect--helper --check-expected-revs $hash_list ;;
- *,"$TERM_BAD")
- die "$(eval_gettext "'git bisect \$TERM_BAD' can take only one argument.")" ;;
- *)
- usage ;;
- esac
- git bisect--helper --bisect-auto-next
+ eval git bisect--helper --bisect-state 'skip' $all
}
bisect_visualize() {
@@ -187,8 +141,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
state="$TERM_GOOD"
fi
- # We have to use a subshell because "bisect_state" can exit.
- ( bisect_state $state >"$GIT_DIR/BISECT_RUN" )
+ git bisect--helper --bisect-state $state >"$GIT_DIR/BISECT_RUN"
res=$?
cat "$GIT_DIR/BISECT_RUN"
@@ -203,7 +156,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
if [ $res -ne 0 ]
then
eval_gettextln "bisect run failed:
-'bisect_state \$state' exited with error code \$res" >&2
+'git bisect--helper --bisect-state \$state' exited with error code \$res" >&2
exit $res
fi
@@ -244,7 +197,7 @@ case "$#" in
start)
git bisect--helper --bisect-start "$@" ;;
bad|good|new|old|"$TERM_BAD"|"$TERM_GOOD")
- bisect_state "$cmd" "$@" ;;
+ git bisect--helper --bisect-state "$cmd" "$@" ;;
skip)
bisect_skip "$@" ;;
next)
--
2.25.0
^ permalink raw reply related
* [PATCH v7 12/13] bisect--helper: retire `--write-terms` subcommand
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
The `--write-terms` subcommand is no longer used from the
git-bisect.sh shell script. Instead the function `write_terms()`
is called from the C implementation of `set_terms()` and
`bisect_start()`.
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>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index da203c2091..46ddfd5710 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -20,7 +20,6 @@ static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
static const char * const git_bisect_helper_usage[] = {
- N_("git bisect--helper --write-terms <bad_term> <good_term>"),
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>"),
@@ -909,8 +908,7 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
- WRITE_TERMS = 1,
- BISECT_RESET,
+ BISECT_RESET = 1,
BISECT_WRITE,
CHECK_AND_SET_TERMS,
BISECT_NEXT_CHECK,
@@ -923,8 +921,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
} cmdmode = 0;
int res = 0, nolog = 0;
struct option options[] = {
- OPT_CMDMODE(0, "write-terms", &cmdmode,
- N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS),
OPT_CMDMODE(0, "bisect-reset", &cmdmode,
N_("reset the bisection state"), BISECT_RESET),
OPT_CMDMODE(0, "bisect-write", &cmdmode,
@@ -959,10 +955,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
usage_with_options(git_bisect_helper_usage, options);
switch (cmdmode) {
- case WRITE_TERMS:
- if (argc != 2)
- return error(_("--write-terms requires two arguments"));
- return write_terms(argv[0], argv[1]);
case BISECT_RESET:
if (argc > 1)
return error(_("--bisect-reset requires either no argument or a commit"));
--
2.25.0
^ permalink raw reply related
* [PATCH v7 11/13] bisect--helper: retire `--check-expected-revs` subcommand
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
The `--check-expected-revs` subcommand is no longer used from the
git-bisect.sh shell script. Functions `check_expected_revs` and
`is_expected_revs` are also deleted.
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>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 30 ------------------------------
1 file changed, 30 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index a976f4739c..da203c2091 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -187,30 +187,6 @@ static int write_terms(const char *bad, const char *good)
return res;
}
-static int is_expected_rev(const char *expected_hex)
-{
- struct strbuf actual_hex = STRBUF_INIT;
- int res = 0;
- if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) >= 40) {
- strbuf_trim(&actual_hex);
- res = !strcmp(actual_hex.buf, expected_hex);
- }
- strbuf_release(&actual_hex);
- return res;
-}
-
-static void check_expected_revs(const char **revs, int rev_nr)
-{
- int i;
-
- for (i = 0; i < rev_nr; i++) {
- if (!is_expected_rev(revs[i])) {
- unlink_or_warn(git_path_bisect_ancestors_ok());
- unlink_or_warn(git_path_bisect_expected_rev());
- }
- }
-}
-
static int bisect_reset(const char *commit)
{
struct strbuf branch = STRBUF_INIT;
@@ -934,7 +910,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
WRITE_TERMS = 1,
- CHECK_EXPECTED_REVS,
BISECT_RESET,
BISECT_WRITE,
CHECK_AND_SET_TERMS,
@@ -950,8 +925,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
struct option options[] = {
OPT_CMDMODE(0, "write-terms", &cmdmode,
N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS),
- 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_CMDMODE(0, "bisect-write", &cmdmode,
@@ -990,9 +963,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
if (argc != 2)
return error(_("--write-terms requires two arguments"));
return write_terms(argv[0], argv[1]);
- 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"));
--
2.25.0
^ permalink raw reply related
* [PATCH v7 13/13] bisect--helper: retire `--bisect-autostart` subcommand
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
The `--bisect-autostart` subcommand is no longer used from the
git-bisect.sh shell script. Instead the function
`bisect_autostart()` is directly called from the C implementation.
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>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 46ddfd5710..8a48c1ef8e 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -29,7 +29,6 @@ static const char * const git_bisect_helper_usage[] = {
" [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]"),
N_("git bisect--helper --bisect-next"),
N_("git bisect--helper --bisect-auto-next"),
- N_("git bisect--helper --bisect-autostart"),
N_("git bisect--helper --bisect-state (bad|new) [<rev>]"),
N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
NULL
@@ -916,7 +915,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_START,
BISECT_NEXT,
BISECT_AUTO_NEXT,
- BISECT_AUTOSTART,
BISECT_STATE
} cmdmode = 0;
int res = 0, nolog = 0;
@@ -937,8 +935,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("find the next bisection commit"), BISECT_NEXT),
OPT_CMDMODE(0, "bisect-auto-next", &cmdmode,
N_("verify the next bisection state then checkout the next bisection commit"), BISECT_AUTO_NEXT),
- OPT_CMDMODE(0, "bisect-autostart", &cmdmode,
- N_("start the bisection if it has not yet been started"), BISECT_AUTOSTART),
OPT_CMDMODE(0, "bisect-state", &cmdmode,
N_("mark the state of ref (or refs)"), BISECT_STATE),
OPT_BOOL(0, "no-log", &nolog,
@@ -998,12 +994,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
get_terms(&terms);
res = bisect_auto_next(&terms, prefix);
break;
- case BISECT_AUTOSTART:
- if (argc)
- return error(_("--bisect-autostart does not accept arguments"));
- set_terms(&terms, "bad", "good");
- res = bisect_autostart(&terms);
- break;
case BISECT_STATE:
set_terms(&terms, "bad", "good");
get_terms(&terms);
--
2.25.0
^ permalink raw reply related
* [PATCH v7 00/13] Finish converting git bisect to C part 2
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git; +Cc: Miriam Rubio
These patches correspond to a second part of patch series
of Outreachy project "Finish converting `git bisect` from shell to C"
started by Pranit Bauva and Tanushree Tumane
(https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and
continued by me.
These patch series emails were generated from:
https://gitlab.com/mirucam/git/commits/git-bisect-work-part2-v7.
I would like to thank Junio Hamano for reviewing this patch series.
General changes
---------------
* Rebased on e9b77c84a0 (Tenth batch, 2020-08-24), to build on the
strvec API (instead of argv_array) and adjust to the codebase
after the "--first-parent" feature was added.
Specific changes
----------------
[1/13] bisect--helper: BUG() in cmd_*() on invalid subcommand
* Amend commit message.
* Remove casting to int.
---
[2/13] bisect--helper: use '-res' in 'cmd_bisect__helper' return
* Amend commit message.
---
[3/13] bisect--helper: introduce new `write_in_file()` function
* Use saved_errno variable.
---
[5/13] bisect--helper: reimplement `bisect_autostart` shell function in C
* Fix bug using empty_strvec instead of NULL in a `bisect_start()` call.
---
[6/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell
functions in C
* Remove unused `no-checkout` variable.
* Move a comment to more appropriate place.
---
.iriam Rubio (4):
bisect--helper: BUG() in cmd_*() on invalid subcommand
bisect--helper: use '-res' in 'cmd_bisect__helper' return
bisect--helper: introduce new `write_in_file()` function
bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'
Pranit Bauva (9):
bisect--helper: reimplement `bisect_autostart` shell function in C
bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell
functions in C
bisect--helper: finish porting `bisect_start()` to C
bisect--helper: retire `--bisect-clean-state` subcommand
bisect--helper: retire `--next-all` subcommand
bisect--helper: reimplement `bisect_state` & `bisect_head` shell
functions in C
bisect--helper: retire `--check-expected-revs` subcommand
bisect--helper: retire `--write-terms` subcommand
bisect--helper: retire `--bisect-autostart` subcommand
bisect.c | 13 +-
builtin/bisect--helper.c | 442 ++++++++++++++++++++++++++++++++-------
git-bisect.sh | 145 +------------
3 files changed, 380 insertions(+), 220 deletions(-)
--
2.25.0
^ permalink raw reply
* [PATCH v7 08/13] bisect--helper: retire `--bisect-clean-state` subcommand
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder, Tanushree Tumane,
Miriam Rubio
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
The `--bisect-clean-state` subcommand is no longer used from the
git-bisect.sh shell script. Instead the function
`bisect_clean_state()` is directly called from the C
implementation.
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: Tanushree Tumane <tanushreetumane@gmail.com>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index b40369e8aa..b8b8a7473c 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -22,7 +22,6 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all"),
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>"),
N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
@@ -869,7 +868,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
enum {
NEXT_ALL = 1,
WRITE_TERMS,
- BISECT_CLEAN_STATE,
CHECK_EXPECTED_REVS,
BISECT_RESET,
BISECT_WRITE,
@@ -887,8 +885,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("perform 'git bisect next'"), NEXT_ALL),
OPT_CMDMODE(0, "write-terms", &cmdmode,
N_("write the terms to .git/BISECT_TERMS"), WRITE_TERMS),
- OPT_CMDMODE(0, "bisect-clean-state", &cmdmode,
- N_("cleanup the bisection state"), BISECT_CLEAN_STATE),
OPT_CMDMODE(0, "check-expected-revs", &cmdmode,
N_("check for expected revs"), CHECK_EXPECTED_REVS),
OPT_CMDMODE(0, "bisect-reset", &cmdmode,
@@ -930,10 +926,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
if (argc != 2)
return error(_("--write-terms requires two arguments"));
return write_terms(argv[0], argv[1]);
- case BISECT_CLEAN_STATE:
- if (argc != 0)
- return error(_("--bisect-clean-state requires no arguments"));
- return bisect_clean_state();
case CHECK_EXPECTED_REVS:
check_expected_revs(argv, argc);
return 0;
--
2.25.0
^ permalink raw reply related
* [PATCH v7 05/13] bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git; +Cc: Miriam Rubio, Christian Couder
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
As there can be other revision walks after bisect_next_all(),
let's add a call to a function to clear all the marks at the
end of bisect_next_all().
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
bisect.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/bisect.c b/bisect.c
index d42a3a3767..c6aba2b9f2 100644
--- a/bisect.c
+++ b/bisect.c
@@ -1082,6 +1082,8 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
"Bisecting: %d revisions left to test after this %s\n",
nr), nr, steps_msg);
free(steps_msg);
+ /* Clean up objects used, as they will be reused. */
+ clear_commit_marks_all(ALL_REV_FLAGS);
return bisect_checkout(bisect_rev, no_checkout);
}
--
2.25.0
^ permalink raw reply related
* [PATCH v7 04/13] bisect--helper: reimplement `bisect_autostart` shell function in C
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
From: Pranit Bauva <pranit.bauva@gmail.com>
Reimplement the `bisect_autostart()` shell function in C and add the
C implementation from `bisect_next()` which was previously left
uncovered.
Add `--bisect-autostart` subcommand to be called from git-bisect.sh.
Using `--bisect-autostart` subcommand is a temporary measure to port
the shell function to C so as to use the existing test suite. As more
functions are ported, this subcommand will be retired and
bisect_autostart() will be called directly by `bisect_state()`.
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>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 44 +++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 25 ++---------------------
2 files changed, 45 insertions(+), 24 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index bae09ce65d..f71e8e54d2 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -29,6 +29,7 @@ static const char * const git_bisect_helper_usage[] = {
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] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]"),
+ N_("git bisect--helper --bisect-autostart"),
NULL
};
@@ -653,6 +654,38 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)
return res;
}
+static inline int file_is_not_empty(const char *path)
+{
+ return !is_empty_or_missing_file(path);
+}
+
+static int bisect_autostart(struct bisect_terms *terms)
+{
+ int res;
+ const char *yesno;
+
+ if (file_is_not_empty(git_path_bisect_start()))
+ return 0;
+
+ fprintf_ln(stderr, _("You need to start by \"git bisect "
+ "start\"\n"));
+
+ if (!isatty(STDIN_FILENO))
+ return 0;
+
+ /*
+ * TRANSLATORS: Make sure to include [Y] and [n] in your
+ * translation. The program will only accept English input
+ * at this point.
+ */
+ yesno = git_prompt(_("Do you want me to do it for you "
+ "[Y/n]? "), PROMPT_ECHO);
+ res = tolower(*yesno) == 'n' ?
+ -1 : bisect_start(terms, empty_strvec, 0);
+
+ return res;
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -665,7 +698,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
CHECK_AND_SET_TERMS,
BISECT_NEXT_CHECK,
BISECT_TERMS,
- BISECT_START
+ BISECT_START,
+ BISECT_AUTOSTART,
} cmdmode = 0;
int res = 0, nolog = 0;
struct option options[] = {
@@ -689,6 +723,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("print out the bisect terms"), BISECT_TERMS),
OPT_CMDMODE(0, "bisect-start", &cmdmode,
N_("start the bisect session"), BISECT_START),
+ OPT_CMDMODE(0, "bisect-autostart", &cmdmode,
+ N_("start the bisection if it has not yet been started"), BISECT_AUTOSTART),
OPT_BOOL(0, "no-log", &nolog,
N_("no log for BISECT_WRITE")),
OPT_END()
@@ -748,6 +784,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
set_terms(&terms, "bad", "good");
res = bisect_start(&terms, argv, argc);
break;
+ case BISECT_AUTOSTART:
+ if (argc)
+ return error(_("--bisect-autostart does not accept arguments"));
+ set_terms(&terms, "bad", "good");
+ res = bisect_autostart(&terms);
+ break;
default:
BUG("unknown subcommand %d", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index c7580e51a0..9ca583d964 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -49,27 +49,6 @@ bisect_head()
fi
}
-bisect_autostart() {
- test -s "$GIT_DIR/BISECT_START" || {
- gettextln "You need to start by \"git bisect start\"" >&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 "Do you want me to do it for you [Y/n]? " >&2
- read yesno
- case "$yesno" in
- [Nn]*)
- exit ;;
- esac
- bisect_start
- else
- exit 1
- fi
- }
-}
-
bisect_start() {
git bisect--helper --bisect-start $@ || exit
@@ -108,7 +87,7 @@ bisect_skip() {
}
bisect_state() {
- bisect_autostart
+ git bisect--helper --bisect-autostart
state=$1
git bisect--helper --check-and-set-terms $state $TERM_GOOD $TERM_BAD || exit
get_terms
@@ -149,7 +128,7 @@ bisect_auto_next() {
bisect_next() {
case "$#" in 0) ;; *) usage ;; esac
- bisect_autostart
+ git bisect--helper --bisect-autostart
git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD $TERM_GOOD|| exit
# Perform all bisection computation, display and checkout
--
2.25.0
^ permalink raw reply related
* [PATCH v7 03/13] bisect--helper: introduce new `write_in_file()` function
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git; +Cc: Miriam Rubio, Christian Couder, Johannes Schindelin
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
Let's refactor code adding a new `write_in_file()` function
that opens a file for writing a message and closes it and a
wrapper for writing mode.
This helper will be used in later steps and makes the code
simpler and easier to understand.
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 43 +++++++++++++++++++++++++++++++++-------
1 file changed, 36 insertions(+), 7 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index b7345be3a5..bae09ce65d 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -74,6 +74,40 @@ static int one_of(const char *term, ...)
return res;
}
+static int write_in_file(const char *path, const char *mode, const char *format, va_list args)
+{
+ FILE *fp = NULL;
+ int res = 0;
+
+ if (strcmp(mode, "w"))
+ BUG("write-in-file does not support '%s' mode", mode);
+ fp = fopen(path, mode);
+ if (!fp)
+ return error_errno(_("cannot open file '%s' in mode '%s'"), path, mode);
+ res = vfprintf(fp, format, args);
+
+ if (res < 0) {
+ int saved_errno = errno;
+ fclose(fp);
+ errno = saved_errno;
+ return error_errno(_("could not write to file '%s'"), path);
+ }
+
+ return fclose(fp);
+}
+
+static int write_to_file(const char *path, const char *format, ...)
+{
+ int res;
+ va_list args;
+
+ va_start(args, format);
+ res = write_in_file(path, "w", format, args);
+ va_end(args);
+
+ return res;
+}
+
static int check_term_format(const char *term, const char *orig_term)
{
int res;
@@ -104,7 +138,6 @@ static int check_term_format(const char *term, const char *orig_term)
static int write_terms(const char *bad, const char *good)
{
- FILE *fp = NULL;
int res;
if (!strcmp(bad, good))
@@ -113,13 +146,9 @@ static int write_terms(const char *bad, const char *good)
if (check_term_format(bad, "bad") || check_term_format(good, "good"))
return -1;
- fp = fopen(git_path_bisect_terms(), "w");
- if (!fp)
- return error_errno(_("could not open the file BISECT_TERMS"));
+ res = write_to_file(git_path_bisect_terms(), "%s\n%s\n", bad, good);
- res = fprintf(fp, "%s\n%s\n", bad, good);
- res |= fclose(fp);
- return (res < 0) ? -1 : 0;
+ return res;
}
static int is_expected_rev(const char *expected_hex)
--
2.25.0
^ permalink raw reply related
* [PATCH v7 02/13] bisect--helper: use '-res' in 'cmd_bisect__helper' return
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git; +Cc: Miriam Rubio, Christian Couder, Junio C Hamano
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
Following 'enum bisect_error' vocabulary, return variable 'res' is
always non-positive.
Let's use '-res' instead of 'abs(res)' to make the code clearer.
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index f464e95792..b7345be3a5 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -731,5 +731,5 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
if (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE)
res = BISECT_OK;
- return abs(res);
+ return -res;
}
--
2.25.0
^ permalink raw reply related
* [PATCH v7 01/13] bisect--helper: BUG() in cmd_*() on invalid subcommand
From: Miriam Rubio @ 2020-08-31 10:50 UTC (permalink / raw)
To: git; +Cc: Miriam Rubio, Christian Couder, Johannes Schindelin
In-Reply-To: <20200831105043.97665-1-mirucam@gmail.com>
In cmd_bisect__helper() function, if an invalid or no
subcommand is passed there is a BUG.
BUG() out instead of returning an error.
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
builtin/bisect--helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index cdda279b23..f464e95792 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -720,7 +720,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
res = bisect_start(&terms, argv, argc);
break;
default:
- return error("BUG: unknown subcommand '%d'", cmdmode);
+ BUG("unknown subcommand %d", cmdmode);
}
free_terms(&terms);
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v6 06/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C
From: Miriam R. @ 2020-08-31 10:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq8sdxi70a.fsf@gitster.c.googlers.com>
El sáb., 29 ago. 2020 a las 21:31, Junio C Hamano
(<gitster@pobox.com>) escribió:
>
> Miriam Rubio <mirucam@gmail.com> writes:
>
> > diff --git a/bisect.c b/bisect.c
> > index c6aba2b9f2..f0fca5c6f3 100644
> > --- a/bisect.c
> > +++ b/bisect.c
> > @@ -988,6 +988,12 @@ void read_bisect_terms(const char **read_bad, const char **read_good)
> > * the bisection process finished successfully.
> > * In this case the calling function or command should not turn a
> > * BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND return code into an error or a non zero exit code.
> > + *
> > + * Checking BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND
> > + * in bisect_helper::bisect_next() and only transforming it to 0 at
> > + * the end of bisect_helper::cmd_bisect__helper() helps bypassing
> > + * all the code related to finding a commit to test.
> > + *
> > * If no_checkout is non-zero, the bisection process does not
> > * checkout the trial commit but instead simply updates BISECT_HEAD.
> > */
>
> Not a problem introduced by this step, but the above description on
> no_checkout describes a parameter that no longer exists.
>
> The comments before a function is to guide the developers how to
> call the function correctly, so it should have been removed, moved
> to where no_checkout is used in the function, or moved to where
> BISECT_HEAD ref gets created, as necessary, but by mistake be5fe200
> (cmd_bisect__helper: defer parsing no-checkout flag, 2020-08-07),
> forgot to do any of the three.
>
>
> > +static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)
> > +{
> > + int no_checkout;
> > + enum bisect_error res;
> > +
> > + bisect_autostart(terms);
> > + if (bisect_next_check(terms, terms->term_good))
> > + return BISECT_FAILED;
> > +
> > + no_checkout = ref_exists("BISECT_HEAD");
> > +
> > + /* Perform all bisection computation */
> > + res = bisect_next_all(the_repository, prefix);
> > +
> > + if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
> > + res = bisect_successful(terms);
> > + return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
> > + } else if (res == BISECT_ONLY_SKIPPED_LEFT) {
> > + res = bisect_skipped_commits(terms);
> > + return res ? res : BISECT_ONLY_SKIPPED_LEFT;
> > + }
> > + return res;
> > +}
> > +
>
> The no_checkout local variable is assigned but never used. It is
> understandable if a variable that used to be used becomes unused
> when some part (i.e. the part that used to use the variable) of a
> function is factored out, but it is rather unusual how a brand new
> function has such an unused code and stay to be that way throughout
> a topic. Makes a reviewer suspect that there may be a code missing,
> that has to use the variable to decide to do things differently, in
> this function. It seems to break -Werror builds.
Ok, I will send a new version with this local variable removed.
I don't know if it is something related to my compiler but my -Werror
build does not break. Something similar happened with a previous patch
series version and a warning related with missing-prototypes.
I always compile with : make -j 16 DEVELOPER=1 CFLAGS="-O0 -g3" install;
and I have also tested adding the -Werror flag and commits always
compile without problems.
Maybe someone in the community knows what my problem is, because it
could avoid extra patch series versions and reviewers work.
Thank you.
>
> Thanks.
>
>
^ permalink raw reply
* Re: [PATCH v1 3/3] git: catch an attempt to run "git-foo"
From: Johannes Schindelin @ 2020-08-31 9:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <xmqq1rjqo2bp.fsf@gitster.c.googlers.com>
Hi Junio,
On Fri, 28 Aug 2020, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > git.c | 3 ++-
> > help.c | 5 ++++-
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/git.c b/git.c
> > index 71ef4835b20e..863fd0c58a66 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -851,7 +851,8 @@ int cmd_main(int argc, const char **argv)
> > * that one cannot handle it.
> > */
> > if (skip_prefix(cmd, "git-", &cmd)) {
> > - warn_on_dashed_git(argv[0]);
> > + strip_extension(&cmd);
> > + warn_on_dashed_git(cmd);
>
> The argv[0] may have been NULL from the beginning of cmd_main(), and
> cmd would be "git-help" in such a case. We used to pass NULL to
> warn_on_dashed_git() in such a case because "git-help" is not what the
> user typed, but what we internally trigger, so we didn't want
> warn_on_dashed_git() to do anything based on that internal string.
True. The test suite passes with this, though, which means we haven't
covered that case.
> As there is no special casing of "help" in warn_on_dashed_git()
> mechanism, it probably would start triggering a warning/die when
> "git<ENTER>" is typed, no?
>
> + if (argv[0]) {
> strip_extension(&cmd);
> warn_on_dashed_git(cmd);
>
> may be the minimum fix, but I would strongly prefer to keep the
> interface into warn_on_dashed_git() (eh, the most important is the
> interface into find_cmdname_help() helper function, which is
> designed to be reused by other parts of help.c) to take the full
> command name, not without "git-" prefix. This is primarily because
> the entirety of the help.c API is driven by full command names,
> without removing "git-" prefix, and it has to, because the help.c
> API needs to handle "gitk", from which you cannot remove any "git-"
> prefix.
>
> Perhaps
>
> if (starts_with(cmd, "git-")) {
> strip_extension(&cmd);
> if (argv[0])
> warn_on_dashed_git(cmd);
> argv[0] = cmd + 4;
> handle_builtin(argc, argv);
> die(...);
Sure.
> How does your handle_builtin() work, by the way?
>
> The original code (even before we added warn_on_dashed_git() in this
> codepath) did not do any strip_extension(), so handle_builtin() can
> take commands with ".exe" suffix, but now we are feeding the result
> of strip_extension() to it, so it can deal with both?
Yes, it can deal with both. It calls `strip_extension()`, which on Windows
removes the `.exe` suffix, if found.
> Sounds convenient and sloppy (not the handle_builtin's
> implementation, but its callers that sometimes feeds the full
> executable name, and feeds only the basename some other times) at
> the same time.
Right, it does not _require_ the extension. I do not necessarily agree
that it is sloppy, but I do agree that it is convenient ;-)
Ciao,
Dscho
> > argv[0] = cmd;
> > handle_builtin(argc, argv);
> > diff --git a/help.c b/help.c
> > index c93a76944b00..27b1b26890be 100644
> > --- a/help.c
> > +++ b/help.c
> > @@ -724,9 +724,12 @@ NORETURN void help_unknown_ref(const char *ref, const char *cmd,
> > static struct cmdname_help *find_cmdname_help(const char *name)
> > {
> > int i;
> > + const char *p;
> >
> > + skip_prefix(name, "git-", &name);
> > for (i = 0; i < ARRAY_SIZE(command_list); i++) {
> > - if (!strcmp(command_list[i].name, name))
> > + if (skip_prefix(command_list[i].name, "git-", &p) &&
> > + !strcmp(p, name))
> > return &command_list[i];
> > }
> > return NULL;
> > --
> > 2.28.0.windows.1
>
^ permalink raw reply
* Re: [PATCH] Avoid infinite loop in malformed packfiles
From: Jeff King @ 2020-08-31 9:29 UTC (permalink / raw)
To: ori; +Cc: Junio C Hamano, René Scharfe, git
In-Reply-To: <xmqqwo1gglf5.fsf@gitster.c.googlers.com>
On Sun, Aug 30, 2020 at 09:15:10AM -0700, Junio C Hamano wrote:
> René Scharfe <l.s.r@web.de> writes:
>
> >> Will that work? I'd expect that modern pack files end up being
> >> offset deltas, rather than reference deltas.
> >
> > True, but going down all the way would work:
>
> Perhaps, but I'd rather use pack-objects to prepare the repository
> with no-delta-base-offset to force ref deltas.
Yeah, that seems like a much better test setup.
It does raise an interesting question, though. I had imagined we would
limit the depth of all delta chains here, not just ref-deltas. But it is
true that ofs deltas can't cycle. Without cycles, neither type can go on
indefinitely (they are limited by the number of entries in the
packfile). I could see arguments going either way:
- ofs deltas cannot cycle, so we do not need a counter that limits
them (and which _could_ find a false positive). So we should not
limit them.
- a counter is preventing us from following cycles indefinitely, but
also hardening us against misbehavior due to bugs or insanely large
delta chains (intentional or not). So we should include ofs deltas
in our limit.
A related point is that delta chains might be composed of both types. If
we don't differentiate between the two types, then the limit is clearly
total chain length. If we do, then is the limit the total number of
ref-deltas found in the current lookup, or is it the number of
consecutive ref-deltas? I guess it would have to be the former if our
goal is to catch cycles (since a cycle could include an ofs-delta, as
long as a ref-delta is the part that forms the loop).
-Peff
^ permalink raw reply
* [PATCH v2 4/5] init: teach --separate-git-dir to repair linked worktrees
From: Eric Sunshine @ 2020-08-31 6:57 UTC (permalink / raw)
To: git
Cc: Henré Botha, Jeff King, Junio C Hamano, Johannes Schindelin,
Ramsay Jones, Eric Sunshine
In-Reply-To: <20200831065800.62502-1-sunshine@sunshineco.com>
A linked worktree's .git file is a "gitfile" pointing at the
.git/worktrees/<id> directory within the repository. When `git init
--separate-git-dir=<path>` is used on an existing repository to relocate
the repository's .git/ directory to a different location, it neglects to
update the .git files of linked worktrees, thus breaking the worktrees
by making it impossible for them to locate the repository. Fix this by
teaching --separate-git-dir to repair the .git file of each linked
worktree to point at the new repository location.
Reported-by: Henré Botha <henrebotha@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
builtin/init-db.c | 2 ++
t/t0001-init.sh | 11 +++++++++++
2 files changed, 13 insertions(+)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index bbc9bc78f9..7b915d88ab 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -9,6 +9,7 @@
#include "builtin.h"
#include "exec-cmd.h"
#include "parse-options.h"
+#include "worktree.h"
#ifndef DEFAULT_GIT_TEMPLATE_DIR
#define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
@@ -364,6 +365,7 @@ static void separate_git_dir(const char *git_dir, const char *git_link)
if (rename(src, git_dir))
die_errno(_("unable to move %s to %s"), src, git_dir);
+ repair_worktrees(NULL, NULL);
}
write_file(git_link, "gitdir: %s", git_dir);
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 50222a10c5..e489eb4ddb 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -405,6 +405,17 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' '
test_path_is_dir realgitdir/refs
'
+test_expect_success 're-init to move gitdir with linked worktrees' '
+ test_when_finished "rm -rf mainwt linkwt seprepo" &&
+ git init mainwt &&
+ test_commit -C mainwt gumby &&
+ git -C mainwt worktree add --detach ../linkwt &&
+ git -C mainwt init --separate-git-dir ../seprepo &&
+ git -C mainwt rev-parse --git-common-dir >expect &&
+ git -C linkwt rev-parse --git-common-dir >actual &&
+ test_cmp expect actual
+'
+
test_expect_success MINGW '.git hidden' '
rm -rf newdir &&
(
--
2.28.0.531.g41c3d8a546
^ permalink raw reply related
* [PATCH v2 2/5] worktree: teach "repair" to fix worktree back-links to main worktree
From: Eric Sunshine @ 2020-08-31 6:57 UTC (permalink / raw)
To: git
Cc: Henré Botha, Jeff King, Junio C Hamano, Johannes Schindelin,
Ramsay Jones, Eric Sunshine
In-Reply-To: <20200831065800.62502-1-sunshine@sunshineco.com>
The .git file in a linked worktree is a "gitfile" which points back to
the .git/worktrees/<id> entry in the main worktree or bare repository.
If a worktree's .git file is deleted or becomes corrupted or outdated,
then the linked worktree won't know how to find the repository or any of
its own administrative files (such as 'index', 'HEAD', etc.). An easy
way for the .git file to become outdated is for the user to move the
main worktree or bare repository. Although it is possible to manually
update each linked worktree's .git file to reflect the new repository
location, doing so requires a level of knowledge about worktree
internals beyond what a user should be expected to know offhand.
Therefore, teach "git worktree repair" how to repair broken or outdated
worktree .git files automatically. (For this to work, the command must
be invoked from within the main worktree or bare repository, or from
within a worktree which has not become disconnected from the repository
-- such as one which was created after the repository was moved.)
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
Documentation/git-worktree.txt | 10 ++++-
builtin/worktree.c | 12 +++++
t/t2406-worktree-repair.sh | 82 ++++++++++++++++++++++++++++++++++
worktree.c | 61 +++++++++++++++++++++++++
worktree.h | 11 +++++
5 files changed, 175 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index ae432d39a8..34fe47cecd 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -98,7 +98,10 @@ with `--reason`.
move::
Move a working tree to a new location. Note that the main working tree
-or linked working trees containing submodules cannot be moved.
+or linked working trees containing submodules cannot be moved with this
+command. (The `git worktree repair` command, however, can reestablish
+the connection with linked working trees if you move the main working
+tree manually.)
prune::
@@ -115,6 +118,11 @@ repair::
Repair working tree administrative files, if possible, if they have
become corrupted or outdated due to external factors.
++
+For instance, if the main working tree (or bare repository) is moved,
+linked working trees will be unable to locate it. Running `repair` in
+the main working tree will reestablish the connection from linked
+working trees back to the main working tree.
unlock::
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 88af412d4f..68b0032428 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1030,6 +1030,17 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
return ret;
}
+static void report_repair(int iserr, const char *path, const char *msg, void *cb_data)
+{
+ if (!iserr) {
+ printf_ln(_("repair: %s: %s"), msg, path);
+ } else {
+ int *exit_status = (int *)cb_data;
+ fprintf_ln(stderr, _("error: %s: %s"), msg, path);
+ *exit_status = 1;
+ }
+}
+
static int repair(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -1040,6 +1051,7 @@ static int repair(int ac, const char **av, const char *prefix)
ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
if (ac)
usage_with_options(worktree_usage, options);
+ repair_worktrees(report_repair, &rc);
return rc;
}
diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh
index cc679e1a21..ef59cdce95 100755
--- a/t/t2406-worktree-repair.sh
+++ b/t/t2406-worktree-repair.sh
@@ -8,4 +8,86 @@ test_expect_success setup '
test_commit init
'
+test_expect_success 'skip missing worktree' '
+ test_when_finished "git worktree prune" &&
+ git worktree add --detach missing &&
+ rm -rf missing &&
+ git worktree repair >out 2>err &&
+ test_must_be_empty out &&
+ test_must_be_empty err
+'
+
+test_expect_success 'worktree path not directory' '
+ test_when_finished "git worktree prune" &&
+ git worktree add --detach notdir &&
+ rm -rf notdir &&
+ >notdir &&
+ test_must_fail git worktree repair >out 2>err &&
+ test_must_be_empty out &&
+ test_i18ngrep "not a directory" err
+'
+
+test_expect_success "don't clobber .git repo" '
+ test_when_finished "rm -rf repo && git worktree prune" &&
+ git worktree add --detach repo &&
+ rm -rf repo &&
+ test_create_repo repo &&
+ test_must_fail git worktree repair >out 2>err &&
+ test_must_be_empty out &&
+ test_i18ngrep ".git is not a file" err
+'
+
+test_corrupt_gitfile () {
+ butcher=$1 &&
+ problem=$2 &&
+ repairdir=${3:-.} &&
+ test_when_finished 'rm -rf corrupt && git worktree prune' &&
+ git worktree add --detach corrupt &&
+ git -C corrupt rev-parse --absolute-git-dir >expect &&
+ eval "$butcher" &&
+ git -C "$repairdir" worktree repair >out 2>err &&
+ test_i18ngrep "$problem" out &&
+ test_must_be_empty err &&
+ git -C corrupt rev-parse --absolute-git-dir >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'repair missing .git file' '
+ test_corrupt_gitfile "rm -f corrupt/.git" ".git file broken"
+'
+
+test_expect_success 'repair bogus .git file' '
+ test_corrupt_gitfile "echo \"gitdir: /nowhere\" >corrupt/.git" \
+ ".git file broken"
+'
+
+test_expect_success 'repair incorrect .git file' '
+ test_when_finished "rm -rf other && git worktree prune" &&
+ test_create_repo other &&
+ other=$(git -C other rev-parse --absolute-git-dir) &&
+ test_corrupt_gitfile "echo \"gitdir: $other\" >corrupt/.git" \
+ ".git file incorrect"
+'
+
+test_expect_success 'repair .git file from main/.git' '
+ test_corrupt_gitfile "rm -f corrupt/.git" ".git file broken" .git
+'
+
+test_expect_success 'repair .git file from linked worktree' '
+ test_when_finished "rm -rf other && git worktree prune" &&
+ git worktree add --detach other &&
+ test_corrupt_gitfile "rm -f corrupt/.git" ".git file broken" other
+'
+
+test_expect_success 'repair .git file from bare.git' '
+ test_when_finished "rm -rf bare.git corrupt && git worktree prune" &&
+ git clone --bare . bare.git &&
+ git -C bare.git worktree add --detach ../corrupt &&
+ git -C corrupt rev-parse --absolute-git-dir >expect &&
+ rm -f corrupt/.git &&
+ git -C bare.git worktree repair &&
+ git -C corrupt rev-parse --absolute-git-dir >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/worktree.c b/worktree.c
index 62217b4a6b..3ad93cc4aa 100644
--- a/worktree.c
+++ b/worktree.c
@@ -571,3 +571,64 @@ int other_head_refs(each_ref_fn fn, void *cb_data)
free_worktrees(worktrees);
return ret;
}
+
+/*
+ * Repair worktree's /path/to/worktree/.git file if missing, corrupt, or not
+ * pointing at <repo>/worktrees/<id>.
+ */
+static void repair_gitfile(struct worktree *wt,
+ worktree_repair_fn fn, void *cb_data)
+{
+ struct strbuf dotgit = STRBUF_INIT;
+ struct strbuf repo = STRBUF_INIT;
+ char *backlink;
+ const char *repair = NULL;
+ int err;
+
+ /* missing worktree can't be repaired */
+ if (!file_exists(wt->path))
+ return;
+
+ if (!is_directory(wt->path)) {
+ fn(1, wt->path, _("not a directory"), cb_data);
+ return;
+ }
+
+ strbuf_realpath(&repo, git_common_path("worktrees/%s", wt->id), 1);
+ strbuf_addf(&dotgit, "%s/.git", wt->path);
+ backlink = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
+
+ if (err == READ_GITFILE_ERR_NOT_A_FILE)
+ fn(1, wt->path, _(".git is not a file"), cb_data);
+ else if (err)
+ repair = _(".git file broken");
+ else if (fspathcmp(backlink, repo.buf))
+ repair = _(".git file incorrect");
+
+ if (repair) {
+ fn(0, wt->path, repair, cb_data);
+ write_file(dotgit.buf, "gitdir: %s", repo.buf);
+ }
+
+ free(backlink);
+ strbuf_release(&repo);
+ strbuf_release(&dotgit);
+}
+
+static void repair_noop(int iserr, const char *path, const char *msg,
+ void *cb_data)
+{
+ /* nothing */
+}
+
+void repair_worktrees(worktree_repair_fn fn, void *cb_data)
+{
+ struct worktree **worktrees = get_worktrees();
+ struct worktree **wt = worktrees + 1; /* +1 skips main worktree */
+
+ if (!fn)
+ fn = repair_noop;
+ for (; *wt; wt++)
+ repair_gitfile(*wt, fn, cb_data);
+ free_worktrees(worktrees);
+}
diff --git a/worktree.h b/worktree.h
index 516744c433..4fcb01348c 100644
--- a/worktree.h
+++ b/worktree.h
@@ -89,6 +89,17 @@ int validate_worktree(const struct worktree *wt,
void update_worktree_location(struct worktree *wt,
const char *path_);
+typedef void (* worktree_repair_fn)(int iserr, const char *path,
+ const char *msg, void *cb_data);
+
+/*
+ * Visit each registered linked worktree and repair corruptions. For each
+ * repair made or error encountered while attempting a repair, the callback
+ * function, if non-NULL, is called with the path of the worktree and a
+ * description of the repair or error, along with the callback user-data.
+ */
+void repair_worktrees(worktree_repair_fn, void *cb_data);
+
/*
* Free up the memory for worktree(s)
*/
--
2.28.0.531.g41c3d8a546
^ permalink raw reply related
* [PATCH v2 5/5] init: make --separate-git-dir work from within linked worktree
From: Eric Sunshine @ 2020-08-31 6:58 UTC (permalink / raw)
To: git
Cc: Henré Botha, Jeff King, Junio C Hamano, Johannes Schindelin,
Ramsay Jones, Eric Sunshine
In-Reply-To: <20200831065800.62502-1-sunshine@sunshineco.com>
The intention of `git init --separate-work-dir=<path>` is to move the
.git/ directory to a location outside of the main worktree. When used
within a linked worktree, however, rather than moving the .git/
directory as intended, it instead incorrectly moves the worktree's
.git/worktrees/<id> directory to <path>, thus disconnecting the linked
worktree from its parent repository and breaking the worktree in the
process since its local .git file no longer points at a location at
which it can find the object database. Fix this broken behavior.
An intentional side-effect of this change is that it also closes a
loophole not caught by ccf236a23a (init: disallow --separate-git-dir
with bare repository, 2020-08-09) in which the check to prevent
--separate-git-dir being used in conjunction with a bare repository was
unable to detect the invalid combination when invoked from within a
linked worktree. Therefore, add a test to verify that this loophole is
closed, as well.
Reported-by: Henré Botha <henrebotha@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
builtin/init-db.c | 24 ++++++++++++++++++++++++
t/t0001-init.sh | 21 +++++++++++++++++++--
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 7b915d88ab..cd3e760541 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -642,6 +642,30 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
if (!git_dir)
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+ /*
+ * When --separate-git-dir is used inside a linked worktree, take
+ * care to ensure that the common .git/ directory is relocated, not
+ * the worktree-specific .git/worktrees/<id>/ directory.
+ */
+ if (real_git_dir) {
+ int err;
+ const char *p;
+ struct strbuf sb = STRBUF_INIT;
+
+ p = read_gitfile_gently(git_dir, &err);
+ if (p && get_common_dir(&sb, p)) {
+ struct strbuf mainwt = STRBUF_INIT;
+
+ strbuf_addbuf(&mainwt, &sb);
+ strbuf_strip_suffix(&mainwt, "/.git");
+ if (chdir(mainwt.buf) < 0)
+ die_errno(_("cannot chdir to %s"), mainwt.buf);
+ strbuf_release(&mainwt);
+ git_dir = strbuf_detach(&sb, NULL);
+ }
+ strbuf_release(&sb);
+ }
+
if (is_bare_repository_cfg < 0)
is_bare_repository_cfg = guess_repository_type(git_dir);
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index e489eb4ddb..2f7c3dcd0f 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -329,6 +329,15 @@ test_expect_success 'implicit bare & --separate-git-dir incompatible' '
test_i18ngrep "incompatible" err
'
+test_expect_success 'bare & --separate-git-dir incompatible within worktree' '
+ test_when_finished "rm -rf bare.git linkwt seprepo" &&
+ test_commit gumby &&
+ git clone --bare . bare.git &&
+ git -C bare.git worktree add --detach ../linkwt &&
+ test_must_fail git -C linkwt init --separate-git-dir seprepo 2>err &&
+ test_i18ngrep "incompatible" err
+'
+
test_lazy_prereq GETCWD_IGNORES_PERMS '
base=GETCWD_TEST_BASE_DIR &&
mkdir -p $base/dir &&
@@ -405,15 +414,23 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' '
test_path_is_dir realgitdir/refs
'
-test_expect_success 're-init to move gitdir with linked worktrees' '
+sep_git_dir_worktree () {
test_when_finished "rm -rf mainwt linkwt seprepo" &&
git init mainwt &&
test_commit -C mainwt gumby &&
git -C mainwt worktree add --detach ../linkwt &&
- git -C mainwt init --separate-git-dir ../seprepo &&
+ git -C "$1" init --separate-git-dir ../seprepo &&
git -C mainwt rev-parse --git-common-dir >expect &&
git -C linkwt rev-parse --git-common-dir >actual &&
test_cmp expect actual
+}
+
+test_expect_success 're-init to move gitdir with linked worktrees' '
+ sep_git_dir_worktree mainwt
+'
+
+test_expect_success 're-init to move gitdir within linked worktree' '
+ sep_git_dir_worktree linkwt
'
test_expect_success MINGW '.git hidden' '
--
2.28.0.531.g41c3d8a546
^ permalink raw reply related
* [PATCH v2 3/5] worktree: teach "repair" to fix outgoing links to worktrees
From: Eric Sunshine @ 2020-08-31 6:57 UTC (permalink / raw)
To: git
Cc: Henré Botha, Jeff King, Junio C Hamano, Johannes Schindelin,
Ramsay Jones, Eric Sunshine
In-Reply-To: <20200831065800.62502-1-sunshine@sunshineco.com>
The .git/worktrees/<id>/gitdir file points at the location of a linked
worktree's .git file. Its content must be of the form
/path/to/worktree/.git (from which the location of the worktree itself
can be derived by stripping the "/.git" suffix). If the gitdir file is
deleted or becomes corrupted or outdated, then Git will be unable to
find the linked worktree. An easy way for the gitdir file to become
outdated is for the user to move the worktree manually (without using
"git worktree move"). Although it is possible to manually update the
gitdir file to reflect the new linked worktree location, doing so
requires a level of knowledge about worktree internals beyond what a
user should be expected to know offhand.
Therefore, teach "git worktree repair" how to repair broken or outdated
.git/worktrees/<id>/gitdir files automatically. (For this to work, the
command must either be invoked from within the worktree whose gitdir
file requires repair, or from within the main or any linked worktree by
providing the path of the broken worktree as an argument to "git
worktree repair".)
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
Documentation/git-worktree.txt | 14 ++++--
builtin/worktree.c | 7 ++-
t/t2406-worktree-repair.sh | 86 ++++++++++++++++++++++++++++++++++
worktree.c | 74 +++++++++++++++++++++++++++++
worktree.h | 12 +++++
5 files changed, 188 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 34fe47cecd..f70cda4b36 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -15,7 +15,7 @@ SYNOPSIS
'git worktree move' <worktree> <new-path>
'git worktree prune' [-n] [-v] [--expire <expire>]
'git worktree remove' [-f] <worktree>
-'git worktree repair'
+'git worktree repair' [<path>...]
'git worktree unlock' <worktree>
DESCRIPTION
@@ -114,7 +114,7 @@ and no modification in tracked files) can be removed. Unclean working
trees or ones with submodules can be removed with `--force`. The main
working tree cannot be removed.
-repair::
+repair [<path>...]::
Repair working tree administrative files, if possible, if they have
become corrupted or outdated due to external factors.
@@ -123,6 +123,13 @@ For instance, if the main working tree (or bare repository) is moved,
linked working trees will be unable to locate it. Running `repair` in
the main working tree will reestablish the connection from linked
working trees back to the main working tree.
++
+Similarly, if a linked working tree is moved without using `git worktree
+move`, the main working tree (or bare repository) will be unable to
+locate it. Running `repair` within the recently-moved working tree will
+reestablish the connection. If multiple linked working trees are moved,
+running `repair` from any working tree with each tree's new `<path>` as
+an argument, will reestablish the connection to all the specified paths.
unlock::
@@ -317,7 +324,8 @@ in the entry's directory. For example, if a linked working tree is moved
to `/newpath/test-next` and its `.git` file points to
`/path/main/.git/worktrees/test-next`, then update
`/path/main/.git/worktrees/test-next/gitdir` to reference `/newpath/test-next`
-instead.
+instead. Better yet, run `git worktree repair` to reestablish the connection
+automatically.
To prevent a `$GIT_DIR/worktrees` entry from being pruned (which
can be useful in some situations, such as when the
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 68b0032428..8165343145 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1043,15 +1043,18 @@ static void report_repair(int iserr, const char *path, const char *msg, void *cb
static int repair(int ac, const char **av, const char *prefix)
{
+ const char **p;
+ const char *self[] = { ".", NULL };
struct option options[] = {
OPT_END()
};
int rc = 0;
ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
- if (ac)
- usage_with_options(worktree_usage, options);
repair_worktrees(report_repair, &rc);
+ p = ac > 0 ? av : self;
+ for (; *p; p++)
+ repair_worktree_at_path(*p, report_repair, &rc);
return rc;
}
diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh
index ef59cdce95..1fe468bfe8 100755
--- a/t/t2406-worktree-repair.sh
+++ b/t/t2406-worktree-repair.sh
@@ -90,4 +90,90 @@ test_expect_success 'repair .git file from bare.git' '
test_cmp expect actual
'
+test_expect_success 'invalid worktree path' '
+ test_must_fail git worktree repair /notvalid >out 2>err &&
+ test_must_be_empty out &&
+ test_i18ngrep "not a valid path" err
+'
+
+test_expect_success 'repo not found; .git not file' '
+ test_when_finished "rm -rf not-a-worktree" &&
+ test_create_repo not-a-worktree &&
+ test_must_fail git worktree repair not-a-worktree >out 2>err &&
+ test_must_be_empty out &&
+ test_i18ngrep ".git is not a file" err
+'
+
+test_expect_success 'repo not found; .git file broken' '
+ test_when_finished "rm -rf orig moved && git worktree prune" &&
+ git worktree add --detach orig &&
+ echo /invalid >orig/.git &&
+ mv orig moved &&
+ test_must_fail git worktree repair moved >out 2>err &&
+ test_must_be_empty out &&
+ test_i18ngrep ".git file broken" err
+'
+
+test_expect_success 'repair broken gitdir' '
+ test_when_finished "rm -rf orig moved && git worktree prune" &&
+ git worktree add --detach orig &&
+ sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
+ rm .git/worktrees/orig/gitdir &&
+ mv orig moved &&
+ git worktree repair moved >out 2>err &&
+ test_cmp expect .git/worktrees/orig/gitdir &&
+ test_i18ngrep "gitdir unreadable" out &&
+ test_must_be_empty err
+'
+
+test_expect_success 'repair incorrect gitdir' '
+ test_when_finished "rm -rf orig moved && git worktree prune" &&
+ git worktree add --detach orig &&
+ sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
+ mv orig moved &&
+ git worktree repair moved >out 2>err &&
+ test_cmp expect .git/worktrees/orig/gitdir &&
+ test_i18ngrep "gitdir incorrect" out &&
+ test_must_be_empty err
+'
+
+test_expect_success 'repair gitdir (implicit) from linked worktree' '
+ test_when_finished "rm -rf orig moved && git worktree prune" &&
+ git worktree add --detach orig &&
+ sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
+ mv orig moved &&
+ git -C moved worktree repair >out 2>err &&
+ test_cmp expect .git/worktrees/orig/gitdir &&
+ test_i18ngrep "gitdir incorrect" out &&
+ test_must_be_empty err
+'
+
+test_expect_success 'unable to repair gitdir (implicit) from main worktree' '
+ test_when_finished "rm -rf orig moved && git worktree prune" &&
+ git worktree add --detach orig &&
+ cat .git/worktrees/orig/gitdir >expect &&
+ mv orig moved &&
+ git worktree repair >out 2>err &&
+ test_cmp expect .git/worktrees/orig/gitdir &&
+ test_must_be_empty out &&
+ test_must_be_empty err
+'
+
+test_expect_success 'repair multiple gitdir files' '
+ test_when_finished "rm -rf orig1 orig2 moved1 moved2 &&
+ git worktree prune" &&
+ git worktree add --detach orig1 &&
+ git worktree add --detach orig2 &&
+ sed s,orig1/\.git$,moved1/.git, .git/worktrees/orig1/gitdir >expect1 &&
+ sed s,orig2/\.git$,moved2/.git, .git/worktrees/orig2/gitdir >expect2 &&
+ mv orig1 moved1 &&
+ mv orig2 moved2 &&
+ git worktree repair moved1 moved2 >out 2>err &&
+ test_cmp expect1 .git/worktrees/orig1/gitdir &&
+ test_cmp expect2 .git/worktrees/orig2/gitdir &&
+ test_i18ngrep "gitdir incorrect:.*orig1/gitdir$" out &&
+ test_i18ngrep "gitdir incorrect:.*orig2/gitdir$" out &&
+ test_must_be_empty err
+'
+
test_done
diff --git a/worktree.c b/worktree.c
index 3ad93cc4aa..46a5fb8447 100644
--- a/worktree.c
+++ b/worktree.c
@@ -632,3 +632,77 @@ void repair_worktrees(worktree_repair_fn fn, void *cb_data)
repair_gitfile(*wt, fn, cb_data);
free_worktrees(worktrees);
}
+
+static int is_main_worktree_path(const char *path)
+{
+ struct strbuf target = STRBUF_INIT;
+ struct strbuf maindir = STRBUF_INIT;
+ int cmp;
+
+ strbuf_add_real_path(&target, path);
+ strbuf_strip_suffix(&target, "/.git");
+ strbuf_add_real_path(&maindir, get_git_common_dir());
+ strbuf_strip_suffix(&maindir, "/.git");
+ cmp = fspathcmp(maindir.buf, target.buf);
+
+ strbuf_release(&maindir);
+ strbuf_release(&target);
+ return !cmp;
+}
+
+/*
+ * Repair <repo>/worktrees/<id>/gitdir if missing, corrupt, or not pointing at
+ * the worktree's path.
+ */
+void repair_worktree_at_path(const char *path,
+ worktree_repair_fn fn, void *cb_data)
+{
+ struct strbuf dotgit = STRBUF_INIT;
+ struct strbuf realdotgit = STRBUF_INIT;
+ struct strbuf gitdir = STRBUF_INIT;
+ struct strbuf olddotgit = STRBUF_INIT;
+ char *backlink = NULL;
+ const char *repair = NULL;
+ int err;
+
+ if (!fn)
+ fn = repair_noop;
+
+ if (is_main_worktree_path(path))
+ goto done;
+
+ strbuf_addf(&dotgit, "%s/.git", path);
+ if (!strbuf_realpath(&realdotgit, dotgit.buf, 0)) {
+ fn(1, path, _("not a valid path"), cb_data);
+ goto done;
+ }
+
+ backlink = xstrdup_or_null(read_gitfile_gently(realdotgit.buf, &err));
+ if (err == READ_GITFILE_ERR_NOT_A_FILE) {
+ fn(1, realdotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
+ goto done;
+ } else if (err) {
+ fn(1, realdotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
+ goto done;
+ }
+
+ strbuf_addf(&gitdir, "%s/gitdir", backlink);
+ if (strbuf_read_file(&olddotgit, gitdir.buf, 0) < 0)
+ repair = _("gitdir unreadable");
+ else {
+ strbuf_rtrim(&olddotgit);
+ if (fspathcmp(olddotgit.buf, realdotgit.buf))
+ repair = _("gitdir incorrect");
+ }
+
+ if (repair) {
+ fn(0, gitdir.buf, repair, cb_data);
+ write_file(gitdir.buf, "%s", realdotgit.buf);
+ }
+done:
+ free(backlink);
+ strbuf_release(&olddotgit);
+ strbuf_release(&gitdir);
+ strbuf_release(&realdotgit);
+ strbuf_release(&dotgit);
+}
diff --git a/worktree.h b/worktree.h
index 4fcb01348c..ff7b62e434 100644
--- a/worktree.h
+++ b/worktree.h
@@ -100,6 +100,18 @@ typedef void (* worktree_repair_fn)(int iserr, const char *path,
*/
void repair_worktrees(worktree_repair_fn, void *cb_data);
+/*
+ * Repair administrative files corresponding to the worktree at the given path.
+ * The worktree's .git file pointing at the repository must be intact for the
+ * repair to succeed. Useful for re-associating an orphaned worktree with the
+ * repository if the worktree has been moved manually (without using "git
+ * worktree move"). For each repair made or error encountered while attempting
+ * a repair, the callback function, if non-NULL, is called with the path of the
+ * worktree and a description of the repair or error, along with the callback
+ * user-data.
+ */
+void repair_worktree_at_path(const char *, worktree_repair_fn, void *cb_data);
+
/*
* Free up the memory for worktree(s)
*/
--
2.28.0.531.g41c3d8a546
^ permalink raw reply related
* [PATCH v2 0/5] add "git worktree repair" command
From: Eric Sunshine @ 2020-08-31 6:57 UTC (permalink / raw)
To: git
Cc: Henré Botha, Jeff King, Junio C Hamano, Johannes Schindelin,
Ramsay Jones, Eric Sunshine
In-Reply-To: <20200827082129.56149-1-sunshine@sunshineco.com>
This is a re-roll of [1] which adds a "git worktree repair" command
and which fixes bugs with "git init --separate-git-dir" related to
worktrees.
Changes since v1:
* employ proper terminology s/gitlink/gitfile/g (junio[2])
* tighten documentation slightly (junio[2])
* rename callback function and typedef s/cb/fn/g (junio[2])
* give concrete callback function more meaningful name (junio[2])
* employ explicit name for callback data (junio[2])
* explicitly check that worktree path is directory, not file, and bail
with meaningful error message; add associated test (junio[2])
* fix empty-function style violation (junio[2])
* refine worktree repair callback typedef (sunshine)
* pacify GCC -Werror=main (dscho[3])
* pacify Sparse s/0/NULL/ (ramsay[4])
[1]: https://lore.kernel.org/git/20200827082129.56149-1-sunshine@sunshineco.com/
[2]: https://lore.kernel.org/git/xmqqlfi0qaru.fsf@gitster.c.googlers.com/
[3]: https://lore.kernel.org/git/nycvar.QRO.7.76.6.2008280413450.56@tvgsbejvaqbjf.bet/
[4]: https://lore.kernel.org/git/8d4b4011-b8a2-c0e0-a3f2-28c7bbec040b@ramsayjones.plus.com/
Eric Sunshine (5):
worktree: add skeleton "repair" command
worktree: teach "repair" to fix worktree back-links to main worktree
worktree: teach "repair" to fix outgoing links to worktrees
init: teach --separate-git-dir to repair linked worktrees
init: make --separate-git-dir work from within linked worktree
Documentation/git-worktree.txt | 26 ++++-
builtin/init-db.c | 26 +++++
builtin/worktree.c | 30 ++++++
t/t0001-init.sh | 28 ++++++
t/t2406-worktree-repair.sh | 179 +++++++++++++++++++++++++++++++++
worktree.c | 135 +++++++++++++++++++++++++
worktree.h | 23 +++++
7 files changed, 445 insertions(+), 2 deletions(-)
create mode 100755 t/t2406-worktree-repair.sh
Interdiff against v1:
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index a43a0af0af..f70cda4b36 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -121,8 +121,8 @@ become corrupted or outdated due to external factors.
+
For instance, if the main working tree (or bare repository) is moved,
linked working trees will be unable to locate it. Running `repair` in
-the recently-moved main working tree will reestablish the connection
-from linked working trees back to the main working tree.
+the main working tree will reestablish the connection from linked
+working trees back to the main working tree.
+
Similarly, if a linked working tree is moved without using `git worktree
move`, the main working tree (or bare repository) will be unable to
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 6a94d20a2e..cd3e760541 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -661,7 +661,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
if (chdir(mainwt.buf) < 0)
die_errno(_("cannot chdir to %s"), mainwt.buf);
strbuf_release(&mainwt);
- git_dir = strbuf_detach(&sb, 0);
+ git_dir = strbuf_detach(&sb, NULL);
}
strbuf_release(&sb);
}
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 19bbc246ad..8165343145 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1030,13 +1030,14 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
return ret;
}
-static void repair_cb(int iserr, const char *path, const char *msg, void *cb_data)
+static void report_repair(int iserr, const char *path, const char *msg, void *cb_data)
{
- if (!iserr)
+ if (!iserr) {
printf_ln(_("repair: %s: %s"), msg, path);
- else {
+ } else {
+ int *exit_status = (int *)cb_data;
fprintf_ln(stderr, _("error: %s: %s"), msg, path);
- *(int *)cb_data = 1;
+ *exit_status = 1;
}
}
@@ -1050,10 +1051,10 @@ static int repair(int ac, const char **av, const char *prefix)
int rc = 0;
ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
- repair_worktrees(repair_cb, &rc);
+ repair_worktrees(report_repair, &rc);
p = ac > 0 ? av : self;
for (; *p; p++)
- repair_worktree_at_path(*p, repair_cb, &rc);
+ repair_worktree_at_path(*p, report_repair, &rc);
return rc;
}
diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh
index 87bd8fc526..1fe468bfe8 100755
--- a/t/t2406-worktree-repair.sh
+++ b/t/t2406-worktree-repair.sh
@@ -17,6 +17,16 @@ test_expect_success 'skip missing worktree' '
test_must_be_empty err
'
+test_expect_success 'worktree path not directory' '
+ test_when_finished "git worktree prune" &&
+ git worktree add --detach notdir &&
+ rm -rf notdir &&
+ >notdir &&
+ test_must_fail git worktree repair >out 2>err &&
+ test_must_be_empty out &&
+ test_i18ngrep "not a directory" err
+'
+
test_expect_success "don't clobber .git repo" '
test_when_finished "rm -rf repo && git worktree prune" &&
git worktree add --detach repo &&
@@ -27,7 +37,7 @@ test_expect_success "don't clobber .git repo" '
test_i18ngrep ".git is not a file" err
'
-test_corrupt_gitlink () {
+test_corrupt_gitfile () {
butcher=$1 &&
problem=$2 &&
repairdir=${3:-.} &&
@@ -43,30 +53,30 @@ test_corrupt_gitlink () {
}
test_expect_success 'repair missing .git file' '
- test_corrupt_gitlink "rm -f corrupt/.git" ".git link broken"
+ test_corrupt_gitfile "rm -f corrupt/.git" ".git file broken"
'
test_expect_success 'repair bogus .git file' '
- test_corrupt_gitlink "echo \"gitdir: /nowhere\" >corrupt/.git" \
- ".git link broken"
+ test_corrupt_gitfile "echo \"gitdir: /nowhere\" >corrupt/.git" \
+ ".git file broken"
'
test_expect_success 'repair incorrect .git file' '
test_when_finished "rm -rf other && git worktree prune" &&
test_create_repo other &&
other=$(git -C other rev-parse --absolute-git-dir) &&
- test_corrupt_gitlink "echo \"gitdir: $other\" >corrupt/.git" \
- ".git link incorrect"
+ test_corrupt_gitfile "echo \"gitdir: $other\" >corrupt/.git" \
+ ".git file incorrect"
'
test_expect_success 'repair .git file from main/.git' '
- test_corrupt_gitlink "rm -f corrupt/.git" ".git link broken" .git
+ test_corrupt_gitfile "rm -f corrupt/.git" ".git file broken" .git
'
test_expect_success 'repair .git file from linked worktree' '
test_when_finished "rm -rf other && git worktree prune" &&
git worktree add --detach other &&
- test_corrupt_gitlink "rm -f corrupt/.git" ".git link broken" other
+ test_corrupt_gitfile "rm -f corrupt/.git" ".git file broken" other
'
test_expect_success 'repair .git file from bare.git' '
@@ -94,14 +104,14 @@ test_expect_success 'repo not found; .git not file' '
test_i18ngrep ".git is not a file" err
'
-test_expect_success 'repo not found; .git link broken' '
+test_expect_success 'repo not found; .git file broken' '
test_when_finished "rm -rf orig moved && git worktree prune" &&
git worktree add --detach orig &&
echo /invalid >orig/.git &&
mv orig moved &&
test_must_fail git worktree repair moved >out 2>err &&
test_must_be_empty out &&
- test_i18ngrep ".git link broken" err
+ test_i18ngrep ".git file broken" err
'
test_expect_success 'repair broken gitdir' '
diff --git a/worktree.c b/worktree.c
index 6ade4f0d8b..46a5fb8447 100644
--- a/worktree.c
+++ b/worktree.c
@@ -573,11 +573,11 @@ int other_head_refs(each_ref_fn fn, void *cb_data)
}
/*
- * Repair worktree's /path/to/worktree/.git link if missing, corrupt, or not
+ * Repair worktree's /path/to/worktree/.git file if missing, corrupt, or not
* pointing at <repo>/worktrees/<id>.
*/
-static void repair_dotgit(struct worktree *wt,
- worktree_repair_cb *cb, void *cb_data)
+static void repair_gitfile(struct worktree *wt,
+ worktree_repair_fn fn, void *cb_data)
{
struct strbuf dotgit = STRBUF_INIT;
struct strbuf repo = STRBUF_INIT;
@@ -589,19 +589,24 @@ static void repair_dotgit(struct worktree *wt,
if (!file_exists(wt->path))
return;
+ if (!is_directory(wt->path)) {
+ fn(1, wt->path, _("not a directory"), cb_data);
+ return;
+ }
+
strbuf_realpath(&repo, git_common_path("worktrees/%s", wt->id), 1);
strbuf_addf(&dotgit, "%s/.git", wt->path);
backlink = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
if (err == READ_GITFILE_ERR_NOT_A_FILE)
- cb(1, wt->path, _(".git is not a file"), cb_data);
+ fn(1, wt->path, _(".git is not a file"), cb_data);
else if (err)
- repair = _(".git link broken");
+ repair = _(".git file broken");
else if (fspathcmp(backlink, repo.buf))
- repair = _(".git link incorrect");
+ repair = _(".git file incorrect");
if (repair) {
- cb(0, wt->path, repair, cb_data);
+ fn(0, wt->path, repair, cb_data);
write_file(dotgit.buf, "gitdir: %s", repo.buf);
}
@@ -610,34 +615,37 @@ static void repair_dotgit(struct worktree *wt,
strbuf_release(&dotgit);
}
-static void repair_noop_cb(int iserr, const char *path, const char *msg,
- void *cb_data) {}
+static void repair_noop(int iserr, const char *path, const char *msg,
+ void *cb_data)
+{
+ /* nothing */
+}
-void repair_worktrees(worktree_repair_cb *cb, void *cb_data)
+void repair_worktrees(worktree_repair_fn fn, void *cb_data)
{
struct worktree **worktrees = get_worktrees();
struct worktree **wt = worktrees + 1; /* +1 skips main worktree */
- if (!cb)
- cb = repair_noop_cb;
+ if (!fn)
+ fn = repair_noop;
for (; *wt; wt++)
- repair_dotgit(*wt, cb, cb_data);
+ repair_gitfile(*wt, fn, cb_data);
free_worktrees(worktrees);
}
static int is_main_worktree_path(const char *path)
{
struct strbuf target = STRBUF_INIT;
- struct strbuf main = STRBUF_INIT;
+ struct strbuf maindir = STRBUF_INIT;
int cmp;
strbuf_add_real_path(&target, path);
strbuf_strip_suffix(&target, "/.git");
- strbuf_add_real_path(&main, get_git_common_dir());
- strbuf_strip_suffix(&main, "/.git");
- cmp = fspathcmp(main.buf, target.buf);
+ strbuf_add_real_path(&maindir, get_git_common_dir());
+ strbuf_strip_suffix(&maindir, "/.git");
+ cmp = fspathcmp(maindir.buf, target.buf);
- strbuf_release(&main);
+ strbuf_release(&maindir);
strbuf_release(&target);
return !cmp;
}
@@ -647,7 +655,7 @@ static int is_main_worktree_path(const char *path)
* the worktree's path.
*/
void repair_worktree_at_path(const char *path,
- worktree_repair_cb *cb, void *cb_data)
+ worktree_repair_fn fn, void *cb_data)
{
struct strbuf dotgit = STRBUF_INIT;
struct strbuf realdotgit = STRBUF_INIT;
@@ -657,24 +665,24 @@ void repair_worktree_at_path(const char *path,
const char *repair = NULL;
int err;
- if (!cb)
- cb = repair_noop_cb;
+ if (!fn)
+ fn = repair_noop;
if (is_main_worktree_path(path))
goto done;
strbuf_addf(&dotgit, "%s/.git", path);
if (!strbuf_realpath(&realdotgit, dotgit.buf, 0)) {
- cb(1, path, _("not a valid path"), cb_data);
+ fn(1, path, _("not a valid path"), cb_data);
goto done;
}
backlink = xstrdup_or_null(read_gitfile_gently(realdotgit.buf, &err));
if (err == READ_GITFILE_ERR_NOT_A_FILE) {
- cb(1, realdotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
+ fn(1, realdotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
goto done;
} else if (err) {
- cb(1, realdotgit.buf, _("unable to locate repository; .git link broken"), cb_data);
+ fn(1, realdotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
goto done;
}
@@ -688,7 +696,7 @@ void repair_worktree_at_path(const char *path,
}
if (repair) {
- cb(0, gitdir.buf, repair, cb_data);
+ fn(0, gitdir.buf, repair, cb_data);
write_file(gitdir.buf, "%s", realdotgit.buf);
}
done:
diff --git a/worktree.h b/worktree.h
index 9cb7f05741..ff7b62e434 100644
--- a/worktree.h
+++ b/worktree.h
@@ -89,27 +89,28 @@ int validate_worktree(const struct worktree *wt,
void update_worktree_location(struct worktree *wt,
const char *path_);
-typedef void worktree_repair_cb(int iserr, const char *path, const char *msg,
- void *cb_data);
+typedef void (* worktree_repair_fn)(int iserr, const char *path,
+ const char *msg, void *cb_data);
/*
* Visit each registered linked worktree and repair corruptions. For each
- * repair made or error encountered while attempting a repair, the callback, if
- * non-NULL, is called with the path of the worktree and a description of the
- * repair or error, along with the callback user-data.
+ * repair made or error encountered while attempting a repair, the callback
+ * function, if non-NULL, is called with the path of the worktree and a
+ * description of the repair or error, along with the callback user-data.
*/
-void repair_worktrees(worktree_repair_cb *, void *cb_data);
+void repair_worktrees(worktree_repair_fn, void *cb_data);
/*
* Repair administrative files corresponding to the worktree at the given path.
- * The worktree's .git link pointing at the repository must be intact for the
+ * The worktree's .git file pointing at the repository must be intact for the
* repair to succeed. Useful for re-associating an orphaned worktree with the
* repository if the worktree has been moved manually (without using "git
* worktree move"). For each repair made or error encountered while attempting
- * a repair, the callback, if non-NULL, is called with the path of the worktree
- * and a description of the repair or error, along with the callback user-data.
+ * a repair, the callback function, if non-NULL, is called with the path of the
+ * worktree and a description of the repair or error, along with the callback
+ * user-data.
*/
-void repair_worktree_at_path(const char *, worktree_repair_cb *, void *cb_data);
+void repair_worktree_at_path(const char *, worktree_repair_fn, void *cb_data);
/*
* Free up the memory for worktree(s)
--
2.28.0.531.g41c3d8a546
^ permalink raw reply related
* [PATCH v2 1/5] worktree: add skeleton "repair" command
From: Eric Sunshine @ 2020-08-31 6:57 UTC (permalink / raw)
To: git
Cc: Henré Botha, Jeff King, Junio C Hamano, Johannes Schindelin,
Ramsay Jones, Eric Sunshine
In-Reply-To: <20200831065800.62502-1-sunshine@sunshineco.com>
Worktree administrative files can become corrupted or outdated due to
external factors. Although, it is often possible to recover from such
situations by hand-tweaking these files, doing so requires intimate
knowledge of worktree internals. While information necessary to make
such repairs manually can be obtained from git-worktree.txt and
gitrepository-layout.txt, we can assist users more directly by teaching
git-worktree how to repair its administrative files itself (at least to
some extent). Therefore, add a "git worktree repair" command which
attempts to correct common problems which may arise due to factors
beyond Git's control.
At this stage, the "repair" command is a mere skeleton; subsequent
commits will flesh out the functionality.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
Documentation/git-worktree.txt | 6 ++++++
builtin/worktree.c | 15 +++++++++++++++
t/t2406-worktree-repair.sh | 11 +++++++++++
3 files changed, 32 insertions(+)
create mode 100755 t/t2406-worktree-repair.sh
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 6ee6ec7982..ae432d39a8 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -15,6 +15,7 @@ SYNOPSIS
'git worktree move' <worktree> <new-path>
'git worktree prune' [-n] [-v] [--expire <expire>]
'git worktree remove' [-f] <worktree>
+'git worktree repair'
'git worktree unlock' <worktree>
DESCRIPTION
@@ -110,6 +111,11 @@ and no modification in tracked files) can be removed. Unclean working
trees or ones with submodules can be removed with `--force`. The main
working tree cannot be removed.
+repair::
+
+Repair working tree administrative files, if possible, if they have
+become corrupted or outdated due to external factors.
+
unlock::
Unlock a working tree, allowing it to be pruned, moved or deleted.
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 378f332b5d..88af412d4f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1030,6 +1030,19 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
return ret;
}
+static int repair(int ac, const char **av, const char *prefix)
+{
+ struct option options[] = {
+ OPT_END()
+ };
+ int rc = 0;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac)
+ usage_with_options(worktree_usage, options);
+ return rc;
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -1056,5 +1069,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
return move_worktree(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "remove"))
return remove_worktree(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], "repair"))
+ return repair(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh
new file mode 100755
index 0000000000..cc679e1a21
--- /dev/null
+++ b/t/t2406-worktree-repair.sh
@@ -0,0 +1,11 @@
+#!/bin/sh
+
+test_description='test git worktree repair'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ test_commit init
+'
+
+test_done
--
2.28.0.531.g41c3d8a546
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox