* [PATCH v6 09/13] bisect--helper: retire `--next-all` subcommand
From: Miriam Rubio @ 2020-08-28 12:46 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder, Tanushree Tumane,
Miriam Rubio
In-Reply-To: <20200828124617.60618-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 106e5b788e..0a32dbb68f 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>"),
@@ -869,8 +868,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,
@@ -884,8 +882,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,
@@ -922,9 +918,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 v6 10/13] bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions in C
From: Miriam Rubio @ 2020-08-28 12:46 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200828124617.60618-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 0a32dbb68f..6d2847dc9c 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
};
@@ -865,6 +867,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 {
@@ -878,7 +946,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[] = {
@@ -904,6 +973,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()
@@ -974,6 +1045,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 v6 11/13] bisect--helper: retire `--check-expected-revs` subcommand
From: Miriam Rubio @ 2020-08-28 12:46 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200828124617.60618-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 6d2847dc9c..d795f2e5c7 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;
@@ -937,7 +913,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,
@@ -953,8 +928,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,
@@ -993,9 +966,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 v6 12/13] bisect--helper: retire `--write-terms` subcommand
From: Miriam Rubio @ 2020-08-28 12:46 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200828124617.60618-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 d795f2e5c7..a6282d54a2 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>"),
@@ -912,8 +911,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,
@@ -926,8 +924,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,
@@ -962,10 +958,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 v6 13/13] bisect--helper: retire `--bisect-autostart` subcommand
From: Miriam Rubio @ 2020-08-28 12:46 UTC (permalink / raw)
To: git
Cc: Pranit Bauva, Lars Schneider, Christian Couder,
Johannes Schindelin, Tanushree Tumane, Miriam Rubio
In-Reply-To: <20200828124617.60618-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 a6282d54a2..be1291f036 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
@@ -919,7 +918,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;
@@ -940,8 +938,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,
@@ -1001,12 +997,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
* Re: [PATCH v1 3/3] git: catch an attempt to run "git-foo"
From: Johannes Schindelin @ 2020-08-28 2:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <20200826011718.3186597-4-gitster@pobox.com>
Hi Junio,
On Tue, 25 Aug 2020, Junio C Hamano wrote:
> diff --git a/git.c b/git.c
> index 8bd1d7551d..927018bda7 100644
> --- a/git.c
> +++ b/git.c
> @@ -839,6 +839,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]);
> +
> argv[0] = cmd;
> handle_builtin(argc, argv);
> die(_("cannot handle %s as a builtin"), cmd);
> diff --git a/help.c b/help.c
> index d478afb2af..490d2bc3ae 100644
> --- a/help.c
> +++ b/help.c
> @@ -720,3 +720,37 @@ NORETURN void help_unknown_ref(const char *ref, const char *cmd,
> string_list_clear(&suggested_refs, 0);
> exit(1);
> }
> +
> +static struct cmdname_help *find_cmdname_help(const char *name)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(command_list); i++) {
> + if (!strcmp(command_list[i].name, name))
> + return &command_list[i];
> + }
> + return NULL;
> +}
> +
> +void warn_on_dashed_git(const char *cmd)
> +{
> + struct cmdname_help *cmdname;
> + static const char *still_in_use_var = "GIT_I_STILL_USE_DASHED_GIT";
> + static const char *still_in_use_msg =
> + N_("Use of '%s' in the dashed-form is nominated for removal.\n"
> + "If you still use it, export '%s=true'\n"
> + "and send an e-mail to <git@vger.kernel.org>\n"
> + "to let us know and stop our removal plan. Thanks.\n");
> +
> + if (!cmd)
> + return; /* git-help is OK */
> +
> + cmdname = find_cmdname_help(cmd);
> + if (cmdname && (cmdname->category & CAT_onpath))
> + return; /* git-upload-pack and friends are OK */
> +
> + if (!git_env_bool(still_in_use_var, 0)) {
> + fprintf(stderr, _(still_in_use_msg), cmd, still_in_use_var);
> + exit(1);
> + }
> +}
I need this on top, to make it work on Windows:
-- snipsnap --
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH] fixup??? git: catch an attempt to run "git-foo"
This is needed to handle the case where `argv[0]` contains the full path
(which is the case on Windows) and the suffix `.exe` (which is also the
case on Windows).
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);
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 related
* Re: [PATCH 3/5] worktree: teach "repair" to fix outgoing links to worktrees
From: Johannes Schindelin @ 2020-08-28 2:15 UTC (permalink / raw)
To: Eric Sunshine; +Cc: git, Henré Botha, Jeff King
In-Reply-To: <20200827082129.56149-4-sunshine@sunshineco.com>
[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]
Hi Eric,
On Thu, 27 Aug 2020, Eric Sunshine wrote:
> diff --git a/worktree.c b/worktree.c
> index 029ce91fdf..6ade4f0d8b 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -624,3 +624,77 @@ void repair_worktrees(worktree_repair_cb *cb, void *cb_data)
> repair_dotgit(*wt, cb, cb_data);
> free_worktrees(worktrees);
> }
> +
> +static int is_main_worktree_path(const char *path)
> +{
> + struct strbuf target = STRBUF_INIT;
> + struct strbuf main = 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_release(&main);
> + strbuf_release(&target);
> + return !cmp;
> +}
This breaks our `linux-gcc` job, and I need this on top, to make it even
build:
-- snipsnap --
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH] fixup??? worktree: teach "repair" to fix outgoing links to worktrees
This is needed to shut up GCC's "‘main’ is usually a function
[-Werror=main]" error.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
worktree.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/worktree.c b/worktree.c
index 6ade4f0d8b2b..5471915d4680 100644
--- a/worktree.c
+++ b/worktree.c
@@ -628,16 +628,16 @@ void repair_worktrees(worktree_repair_cb *cb, void *cb_data)
static int is_main_worktree_path(const char *path)
{
struct strbuf target = STRBUF_INIT;
- struct strbuf main = STRBUF_INIT;
+ struct strbuf main_worktree = 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(&main_worktree, get_git_common_dir());
+ strbuf_strip_suffix(&main_worktree, "/.git");
+ cmp = fspathcmp(main_worktree.buf, target.buf);
- strbuf_release(&main);
+ strbuf_release(&main_worktree);
strbuf_release(&target);
return !cmp;
}
--
2.28.0.windows.1
^ permalink raw reply related
* [PATCH] refs: move REF_LOG_ONLY to refs-internal.h
From: Han-Wen Nienhuys via GitGitGadget @ 2020-08-28 15:25 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
REF_LOG_ONLY is used in the transaction preparation: if a symref is involved in
a transaction, the referent of the symref should be updated, and the symref
itself should only be updated in the reflog.
Other ref backends will need to duplicate this logic too, so move it to a
central place.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
refs: move REF_LOG_ONLY to refs-internal.h
REF_LOG_ONLY is used in the transaction preparation: if a symref is
involved in a transaction, the referent of the symref should be updated,
and the symref itself should only be updated in the reflog.
Other ref backends will need to duplicate this logic too, so move it to
a central place.
Signed-off-by: Han-Wen Nienhuys hanwen@google.com [hanwen@google.com]
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-712%2Fhanwen%2Flog-only-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-712/hanwen/log-only-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/712
refs/files-backend.c | 7 -------
refs/refs-internal.h | 7 +++++++
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 985631f33e..b1946dc583 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -38,13 +38,6 @@
*/
#define REF_NEEDS_COMMIT (1 << 6)
-/*
- * Used as a flag in ref_update::flags when we want to log a ref
- * update but not actually perform it. This is used when a symbolic
- * ref update is split up.
- */
-#define REF_LOG_ONLY (1 << 7)
-
/*
* Used as a flag in ref_update::flags when the ref_update was via an
* update to HEAD.
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 357359a0be..1f92861aeb 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -31,6 +31,13 @@ struct ref_transaction;
*/
#define REF_HAVE_OLD (1 << 3)
+/*
+ * Used as a flag in ref_update::flags when we want to log a ref
+ * update but not actually perform it. This is used when a symbolic
+ * ref update is split up.
+ */
+#define REF_LOG_ONLY (1 << 7)
+
/*
* Return the length of time to retry acquiring a loose reference lock
* before giving up, in milliseconds:
base-commit: 675a4aaf3b226c0089108221b96559e0baae5de9
--
gitgitgadget
^ permalink raw reply related
* [PATCH] RFC: refs: add GIT_DEBUG_REFS debugging mechanism
From: Han-Wen Nienhuys via GitGitGadget @ 2020-08-28 15:28 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
When set in the environment, GIT_DEBUG_REFS makes git print operations and
results as they flow through the ref storage backend. This helps debug
discrepancies between different ref backends.
This should be integrated with the trace2 sub-system, and I would appreciate
pointers on how to start.
Example:
$ GIT_DEBUG_REFS=1 ./git branch
ref_store for .git
read_raw_ref: HEAD: 0000000000000000000000000000000000000000 (=> refs/heads/ref-debug) type 1: 0
read_raw_ref: refs/heads/ref-debug: b0728cf8cb9dfd395bea41c646f48108c99230c0 (=> refs/heads/ref-debug) type 0: 0
ref_iterator_begin: refs/heads/ (0x1)
iterator_advance before: refs/heads/b4
iterator_advance before: refs/heads/bla
...
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
RFC: refs: add GIT_DEBUG_REFS debugging mechanism
When set in the environment, GIT_DEBUG_REFS makes git print operations
and results as they flow through the ref storage backend. This helps
debug discrepancies between different ref backends.
This should be integrated with the trace2 sub-system, and I would
appreciate pointers on how to start.
Example:
$ GIT_DEBUG_REFS=1 ./git branch
ref_store for .git
read_raw_ref: HEAD: 0000000000000000000000000000000000000000 (=> refs/heads/ref-debug) type 1: 0
read_raw_ref: refs/heads/ref-debug: b0728cf8cb9dfd395bea41c646f48108c99230c0 (=> refs/heads/ref-debug) type 0: 0
ref_iterator_begin: refs/heads/ (0x1)
iterator_advance before: refs/heads/b4
iterator_advance before: refs/heads/bla
...
Signed-off-by: Han-Wen Nienhuys hanwen@google.com [hanwen@google.com]
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-713%2Fhanwen%2Fdebug-refs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-713/hanwen/debug-refs-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/713
Makefile | 1 +
refs.c | 4 +
refs/debug.c | 387 +++++++++++++++++++++++++++++++++++++++++++
refs/refs-internal.h | 6 +
4 files changed, 398 insertions(+)
create mode 100644 refs/debug.c
diff --git a/Makefile b/Makefile
index 65f8cfb236..51408fe453 100644
--- a/Makefile
+++ b/Makefile
@@ -957,6 +957,7 @@ LIB_OBJS += rebase.o
LIB_OBJS += ref-filter.o
LIB_OBJS += reflog-walk.o
LIB_OBJS += refs.o
+LIB_OBJS += refs/debug.o
LIB_OBJS += refs/files-backend.o
LIB_OBJS += refs/iterator.o
LIB_OBJS += refs/packed-backend.o
diff --git a/refs.c b/refs.c
index cf91711968..06726f7544 100644
--- a/refs.c
+++ b/refs.c
@@ -1748,6 +1748,10 @@ struct ref_store *get_main_ref_store(struct repository *r)
BUG("attempting to get main_ref_store outside of repository");
r->refs_private = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
+ if (getenv("GIT_DEBUG_REFS")) {
+ // XXX - how to hook this up with the trace2 framework?
+ r->refs_private = debug_wrap(r->gitdir, r->refs_private);
+ }
return r->refs_private;
}
diff --git a/refs/debug.c b/refs/debug.c
new file mode 100644
index 0000000000..1482c5bd5f
--- /dev/null
+++ b/refs/debug.c
@@ -0,0 +1,387 @@
+
+#include "refs-internal.h"
+
+struct debug_ref_store {
+ struct ref_store base;
+ struct ref_store *refs;
+};
+
+extern struct ref_storage_be refs_be_debug;
+struct ref_store *debug_wrap(const char *gitdir, struct ref_store *store);
+
+struct ref_store *debug_wrap(const char *gitdir, struct ref_store *store)
+{
+ struct debug_ref_store *res = malloc(sizeof(struct debug_ref_store));
+ struct ref_storage_be *be_copy = malloc(sizeof(*be_copy));
+ *be_copy = refs_be_debug;
+ be_copy->name = store->be->name;
+ fprintf(stderr, "ref_store for %s\n", gitdir);
+ res->refs = store;
+ base_ref_store_init((struct ref_store *)res, be_copy);
+ return (struct ref_store *)res;
+}
+
+static int debug_init_db(struct ref_store *refs, struct strbuf *err)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
+ int res = drefs->refs->be->init_db(drefs->refs, err);
+ fprintf(stderr, "init_db: %d\n", res);
+ return res;
+}
+
+static int debug_transaction_prepare(struct ref_store *refs,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
+ int res;
+ transaction->ref_store = drefs->refs;
+ res = drefs->refs->be->transaction_prepare(drefs->refs, transaction,
+ err);
+ fprintf(stderr, "transaction_prepare: %d\n", res);
+ return res;
+}
+
+static void print_update(int i, const char *refname,
+ const struct object_id *old_oid,
+ const struct object_id *new_oid, unsigned int flags,
+ unsigned int type, const char *msg)
+{
+ char o[200] = "null";
+ char n[200] = "null";
+ if (old_oid)
+ oid_to_hex_r(o, old_oid);
+ if (new_oid)
+ oid_to_hex_r(n, new_oid);
+
+ type &= 0xf; /* see refs.h REF_* */
+ flags &= REF_HAVE_NEW | REF_HAVE_OLD | REF_NO_DEREF |
+ REF_FORCE_CREATE_REFLOG; // XXX: LOG_ONLY
+ fprintf(stderr, "%d: %s %s -> %s (F=0x%x, T=0x%x) \"%s\"\n", i, refname,
+ o, n, flags, type, msg);
+}
+
+static void print_transaction(struct ref_transaction *transaction)
+{
+ fprintf(stderr, "transaction {\n");
+ for (int i = 0; i < transaction->nr; i++) {
+ struct ref_update *u = transaction->updates[i];
+ print_update(i, u->refname, &u->old_oid, &u->new_oid, u->flags,
+ u->type, u->msg);
+ }
+ fprintf(stderr, "}\n");
+}
+
+static int debug_transaction_finish(struct ref_store *refs,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
+ int res;
+ transaction->ref_store = drefs->refs;
+ res = drefs->refs->be->transaction_finish(drefs->refs, transaction,
+ err);
+ print_transaction(transaction);
+ fprintf(stderr, "finish: %d\n", res);
+ return res;
+}
+
+static int debug_transaction_abort(struct ref_store *refs,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
+ int res;
+ transaction->ref_store = drefs->refs;
+ res = drefs->refs->be->transaction_abort(drefs->refs, transaction, err);
+ return res;
+}
+
+static int debug_initial_transaction_commit(struct ref_store *refs,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
+ int res;
+ transaction->ref_store = drefs->refs;
+ res = drefs->refs->be->initial_transaction_commit(drefs->refs,
+ transaction, err);
+ return res;
+}
+
+static int debug_pack_refs(struct ref_store *ref_store, unsigned int flags)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res = drefs->refs->be->pack_refs(drefs->refs, flags);
+ fprintf(stderr, "pack_refs: %d\n", res);
+ return res;
+}
+
+static int debug_create_symref(struct ref_store *ref_store,
+ const char *ref_name, const char *target,
+ const char *logmsg)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res = drefs->refs->be->create_symref(drefs->refs, ref_name, target,
+ logmsg);
+ fprintf(stderr, "create_symref: %s -> %s \"%s\": %d\n", ref_name,
+ target, logmsg, res);
+ return res;
+}
+
+static int debug_delete_refs(struct ref_store *ref_store, const char *msg,
+ struct string_list *refnames, unsigned int flags)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res =
+ drefs->refs->be->delete_refs(drefs->refs, msg, refnames, flags);
+ int i = 0;
+ fprintf(stderr, "delete_refs {\n");
+ for (i = 0; i < refnames->nr; i++)
+ fprintf(stderr, "%s\n", refnames->items[i].string);
+ fprintf(stderr, "}: %d\n", res);
+ return res;
+}
+
+static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
+ const char *newref, const char *logmsg)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res = drefs->refs->be->rename_ref(drefs->refs, oldref, newref,
+ logmsg);
+ fprintf(stderr, "rename_ref: %s -> %s \"%s\": %d\n", oldref, newref,
+ logmsg, res);
+ return res;
+}
+
+static int debug_copy_ref(struct ref_store *ref_store, const char *oldref,
+ const char *newref, const char *logmsg)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res =
+ drefs->refs->be->copy_ref(drefs->refs, oldref, newref, logmsg);
+ fprintf(stderr, "copy_ref: %s -> %s \"%s\": %d\n", oldref, newref,
+ logmsg, res);
+ return res;
+}
+
+struct debug_ref_iterator {
+ struct ref_iterator base;
+ struct ref_iterator *iter;
+};
+
+static int debug_ref_iterator_advance(struct ref_iterator *ref_iterator)
+{
+ struct debug_ref_iterator *diter =
+ (struct debug_ref_iterator *)ref_iterator;
+ int res = diter->iter->vtable->advance(diter->iter);
+ if (res)
+ fprintf(stderr, "iterator_advance: %d\n", res);
+ else
+ fprintf(stderr, "iterator_advance before: %s\n",
+ diter->iter->refname);
+
+ diter->base.ordered = diter->iter->ordered;
+ diter->base.refname = diter->iter->refname;
+ diter->base.oid = diter->iter->oid;
+ diter->base.flags = diter->iter->flags;
+ return res;
+}
+static int debug_ref_iterator_peel(struct ref_iterator *ref_iterator,
+ struct object_id *peeled)
+{
+ struct debug_ref_iterator *diter =
+ (struct debug_ref_iterator *)ref_iterator;
+ int res = diter->iter->vtable->peel(diter->iter, peeled);
+ fprintf(stderr, "iterator_peel: %s: %d\n", diter->iter->refname, res);
+ return res;
+}
+
+static int debug_ref_iterator_abort(struct ref_iterator *ref_iterator)
+{
+ struct debug_ref_iterator *diter =
+ (struct debug_ref_iterator *)ref_iterator;
+ int res = diter->iter->vtable->abort(diter->iter);
+ fprintf(stderr, "iterator_abort: %d\n", res);
+ return res;
+}
+
+static struct ref_iterator_vtable debug_ref_iterator_vtable = {
+ debug_ref_iterator_advance, debug_ref_iterator_peel,
+ debug_ref_iterator_abort
+};
+
+static struct ref_iterator *
+debug_ref_iterator_begin(struct ref_store *ref_store, const char *prefix,
+ unsigned int flags)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ struct ref_iterator *res =
+ drefs->refs->be->iterator_begin(drefs->refs, prefix, flags);
+ struct debug_ref_iterator *diter = xcalloc(1, sizeof(*diter));
+ base_ref_iterator_init(&diter->base, &debug_ref_iterator_vtable, 1);
+ diter->iter = res;
+ fprintf(stderr, "ref_iterator_begin: %s (0x%x)\n", prefix, flags);
+ return &diter->base;
+}
+
+static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,
+ struct object_id *oid, struct strbuf *referent,
+ unsigned int *type)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res = 0;
+
+ oidcpy(oid, &null_oid);
+ res = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent,
+ type);
+
+ if (res == 0) {
+ fprintf(stderr, "read_raw_ref: %s: %s (=> %s) type %x: %d\n",
+ refname, oid_to_hex(oid), referent->buf, *type, res);
+ } else {
+ fprintf(stderr, "read_raw_ref: %s err %d\n", refname, res);
+ }
+ return res;
+}
+
+static struct ref_iterator *
+debug_reflog_iterator_begin(struct ref_store *ref_store)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ struct ref_iterator *res =
+ drefs->refs->be->reflog_iterator_begin(drefs->refs);
+ fprintf(stderr, "for_each_reflog_iterator_begin\n");
+ return res;
+}
+
+struct debug_reflog {
+ const char *refname;
+ each_reflog_ent_fn *fn;
+ void *cb_data;
+};
+
+static int debug_print_reflog_ent(struct object_id *old_oid,
+ struct object_id *new_oid,
+ const char *committer, timestamp_t timestamp,
+ int tz, const char *msg, void *cb_data)
+{
+ struct debug_reflog *dbg = (struct debug_reflog *)cb_data;
+ int ret;
+ char o[100] = "null";
+ char n[100] = "null";
+ if (old_oid)
+ oid_to_hex_r(o, old_oid);
+ if (new_oid)
+ oid_to_hex_r(n, new_oid);
+
+ ret = dbg->fn(old_oid, new_oid, committer, timestamp, tz, msg,
+ dbg->cb_data);
+ fprintf(stderr, "reflog_ent %s (ret %d): %s -> %s, %s %ld \"%s\"\n",
+ dbg->refname, ret, o, n, committer, (long int)timestamp, msg);
+ return ret;
+}
+
+static int debug_for_each_reflog_ent(struct ref_store *ref_store,
+ const char *refname, each_reflog_ent_fn fn,
+ void *cb_data)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ struct debug_reflog dbg = {
+ .refname = refname,
+ .fn = fn,
+ .cb_data = cb_data,
+ };
+
+ int res = drefs->refs->be->for_each_reflog_ent(
+ drefs->refs, refname, &debug_print_reflog_ent, &dbg);
+ fprintf(stderr, "for_each_reflog: %s: %d\n", refname, res);
+ return res;
+}
+
+static int debug_for_each_reflog_ent_reverse(struct ref_store *ref_store,
+ const char *refname,
+ each_reflog_ent_fn fn,
+ void *cb_data)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ struct debug_reflog dbg = {
+ .refname = refname,
+ .fn = fn,
+ .cb_data = cb_data,
+ };
+ int res = drefs->refs->be->for_each_reflog_ent_reverse(
+ drefs->refs, refname, &debug_print_reflog_ent, &dbg);
+ fprintf(stderr, "for_each_reflog_reverse: %s: %d\n", refname, res);
+ return res;
+}
+
+static int debug_reflog_exists(struct ref_store *ref_store, const char *refname)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res = drefs->refs->be->reflog_exists(drefs->refs, refname);
+ fprintf(stderr, "reflog_exists: %s: %d\n", refname, res);
+ return res;
+}
+
+static int debug_create_reflog(struct ref_store *ref_store, const char *refname,
+ int force_create, struct strbuf *err)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res = drefs->refs->be->create_reflog(drefs->refs, refname,
+ force_create, err);
+ fprintf(stderr, "create_reflog: %s: %d\n", refname, res);
+ return res;
+}
+
+static int debug_delete_reflog(struct ref_store *ref_store, const char *refname)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res = drefs->refs->be->delete_reflog(drefs->refs, refname);
+ fprintf(stderr, "delete_reflog: %s: %d\n", refname, res);
+ return res;
+}
+
+static int debug_reflog_expire(struct ref_store *ref_store, const char *refname,
+ const struct object_id *oid, unsigned int flags,
+ reflog_expiry_prepare_fn prepare_fn,
+ reflog_expiry_should_prune_fn should_prune_fn,
+ reflog_expiry_cleanup_fn cleanup_fn,
+ void *policy_cb_data)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+ int res = drefs->refs->be->reflog_expire(drefs->refs, refname, oid,
+ flags, prepare_fn,
+ should_prune_fn, cleanup_fn,
+ policy_cb_data);
+ fprintf(stderr, "reflog_expire: %s: %d\n", refname, res);
+ return res;
+}
+
+struct ref_storage_be refs_be_debug = {
+ NULL,
+ "debug",
+ NULL,
+ debug_init_db,
+ debug_transaction_prepare,
+ debug_transaction_finish,
+ debug_transaction_abort,
+ debug_initial_transaction_commit,
+
+ debug_pack_refs,
+ debug_create_symref,
+ debug_delete_refs,
+ debug_rename_ref,
+ debug_copy_ref,
+
+ debug_ref_iterator_begin,
+ debug_read_raw_ref,
+
+ debug_reflog_iterator_begin,
+ debug_for_each_reflog_ent,
+ debug_for_each_reflog_ent_reverse,
+ debug_reflog_exists,
+ debug_create_reflog,
+ debug_delete_reflog,
+ debug_reflog_expire,
+};
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 357359a0be..ec28ae5d86 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -681,4 +681,10 @@ struct ref_store {
void base_ref_store_init(struct ref_store *refs,
const struct ref_storage_be *be);
+/*
+ * Print out ref operations as they occur. Useful for debugging alternate ref
+ * backends.
+ */
+struct ref_store *debug_wrap(const char *gitdir, struct ref_store *store);
+
#endif /* REFS_REFS_INTERNAL_H */
base-commit: 675a4aaf3b226c0089108221b96559e0baae5de9
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 0/5] Add struct strmap and associated utility functions
From: Elijah Newren @ 2020-08-28 15:29 UTC (permalink / raw)
To: Jeff King; +Cc: Elijah Newren via GitGitGadget, Git Mailing List
In-Reply-To: <20200828070335.GB2105050@coredump.intra.peff.net>
On Fri, Aug 28, 2020 at 12:03 AM Jeff King <peff@peff.net> wrote:
>
> On Fri, Aug 21, 2020 at 02:33:54PM -0700, Elijah Newren wrote:
>
> > However, there's an important difference here between what I've done
> > and what you've suggested for hashmap: my method did not deallocate
> > hashmap->table in hashmap_clear() and then use lazy initialization.
> > In fact, I think not deallocating the table was part of the charm --
> > the table had already naturally grown to the right size, and because
> > the repository has approximately the same number of paths in various
> > commits, this provided me a way of getting a table preallocated to a
> > reasonable size for all merges after the first (and there are multiple
> > merges either when recursiveness is needed due to multiple merge
> > bases, OR when rebasing or cherry-picking a sequence of commits).
> > This prevented, as hashmap.h puts it, "expensive resizing".
> >
> > So, once again, my performance ideas might be clashing with some of
> > your desires for the API. Any clever ideas for resolving that?
>
> If the magic is in pre-sizing the hash, then it seems like the callers
> ought to be feeding the size hint. That does make a little more work for
> them, but I think there's real value in having consistent semantics for
> "clear" across our data structures.
I thought about adding a size hint from the callers, but the thing is
I don't know how to get a good one short of running a merge and
querying how big things were sized in that merge. (In some common
cases I can get an upper bound, but I can't get it in all cases and
that upper bound might be a couple orders of magnitude too big.)
Thus, it's really a case where I just punt on pre-sizing for the first
merge, and use the size from the previous merge for subsequent ones.
If you have a non-recursive merge or are cherry-picking only a single
commit, then no sizing hint is used.
> However, one cheat would be to free the memory but retain the size hint
> after a clear. And then if we lazy-init, grow immediately to the hint
> size. That's more expensive than a true reuse, because we do reallocate
> the memory. But it avoids the repeated re-allocation during growth.
>
> It may also be a sign that we should be growing the hash more
> aggressively in the first place. Of course all of this is predicated
> having some benchmarks. It would be useful to know which part actually
> provided the speedup.
Your thoughts here are great; I also had another one this past week --
I could introduce a hashmap_partial_clear() (in addition to
hashmap_clear()) for the special usecase I have of leaving the table
allocated and pre-sized. It'd prevent people from accidentally using
it and forgetting to free stuff, while still allowing me to take
advantage. But, as you say, more benchmarks would be useful to find
which parts provided the speedup before taking any of these steps.
^ permalink raw reply
* [PATCH] bisect: swap command-line options in documentation
From: Hugo Locurcio via GitGitGadget @ 2020-08-28 15:31 UTC (permalink / raw)
To: git; +Cc: Hugo Locurcio, Hugo Locurcio
From: Hugo Locurcio <hugo.locurcio@hugo.pro>
The positional arguments are specified in this order: "bad" then "good".
To avoid confusion, the options above the positional arguments
are now specified in the same order. They can still be specified in any
order since they're options, not positional arguments.
Signed-off-by: Hugo Locurcio <hugo.locurcio@hugo.pro>
---
rebase: swap command-line options in documentation
I wasted an hour because I thought I'd have to specify "good" then "bad"
since I followed the order of the options instead of the positional
arguments. Let's make this harder to happen in the future :)
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-711%2FCalinou%2Frebase-doc-swap-options-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-711/Calinou/rebase-doc-swap-options-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/711
Documentation/git-bisect.txt | 2 +-
builtin/bisect--helper.c | 2 +-
git-bisect.sh | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 0e993e4587..fbb39fbdf5 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -16,7 +16,7 @@ DESCRIPTION
The command takes various subcommands, and different options depending
on the subcommand:
- git bisect start [--term-{old,good}=<term> --term-{new,bad}=<term>]
+ git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]
[--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index cdda279b23..7dcc1b5188 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -27,7 +27,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
- N_("git bisect--helper --bisect-start [--term-{old,good}=<term> --term-{new,bad}=<term>]"
+ N_("git bisect--helper --bisect-start [--term-{new,bad}=<term> --term-{old,good}=<term>]"
" [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]"),
NULL
};
diff --git a/git-bisect.sh b/git-bisect.sh
index c7580e51a0..3ec7558bdc 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -3,7 +3,7 @@
USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|view|replay|log|run]'
LONG_USAGE='git bisect help
print this long help message.
-git bisect start [--term-{old,good}=<term> --term-{new,bad}=<term>]
+git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]
[--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]
reset bisect state and start bisection.
git bisect (bad|new) [<rev>]
base-commit: 20de7e7e4f4e9ae52e6cc7cfaa6469f186ddb0fa
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/6] [RFC] Maintenance III: background maintenance
From: Derrick Stolee via GitGitGadget @ 2020-08-28 15:45 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.680.v2.git.1598380805.gitgitgadget@gmail.com>
This is based on v3 of Part II (ds/maintenance-part-2) [1].
[1]
https://lore.kernel.org/git/pull.696.v3.git.1598380599.gitgitgadget@gmail.com/
This RFC is intended to show how I hope to integrate true background
maintenance into Git. As opposed to my original RFC [2], this entirely
integrates with cron (through crontab [-e|-l]) to launch maintenance
commands in the background.
[2]
https://lore.kernel.org/git/pull.597.git.1585946894.gitgitgadget@gmail.com/
Some preliminary work is done to allow a new --schedule option that tells
the command which tasks to run based on a maintenance.<task>.schedule config
option. The timing is not enforced by Git, but instead is expected to be
provided as a hint from a cron schedule.
A new for-each-repo builtin runs Git commands on every repo in a given list.
Currently, the list is stored as a config setting, allowing a new
maintenance.repos config list to store the repositories registered for
background maintenance. Others may want to add a --file=<file> option for
their own workflows, but I focused on making this as simple as possible for
now.
The updates to the git maintenance builtin include new register/unregister
subcommands and start/stop subcommands. The register subcommand initializes
the config while the start subcommand does everything register does plus
update the cron table. The unregister and stop commands reverse this
process.
The very last patch is entirely optional. It sets a recommended schedule
based on my own experience with very large repositories. I'm open to other
suggestions, but these are ones that I think work well and don't cause a
"rewrite the world" scenario like running nightly 'gc' would do.
I've been testing this scenario on my macOS laptop for a while and my Linux
machine. I have modified my cron task to provide logging via trace2 so I can
see what's happening. A future direction here would be to add some
maintenance logs to the repository so we can track what is happening and
diagnose whether the maintenance strategy is working on real repos.
Note: git maintenance (start|stop) only works on machines with cron by
design. The proper thing to do on Windows will come later. Perhaps this
command should be marked as unavailable on Windows somehow, or at least a
better error than "cron may not be available on your system". I did find
that that message is helpful sometimes: macOS worker agents for CI builds
typically do not have cron available.
Updates since RFC v2
====================
* Update the cron schedule with three lines saying "run hourly except at
midnight", "run daily except on first day of week", and "run weekly".
This avoids parallel processes competing for the object database lock.
* Update the --schedule= and 'maintenance..schedule' config options. This
is reflected in the recommended schedule at the end.
* Drop the *.lastRun config option. It was going to trash config files but
it is also not needed by the new cron schedule.
I expect this to be my final RFC version before restarting the thread with a
v1 next week. Please throw any and all critique at the plan here!
Updates since RFC v1
====================
* Some fallout from rewriting the option parsing in "Maintenance I"
* This applies cleanly on v3 of "Maintenance II"
* Several helpful feedback items from Đoàn Trần Công Danh are applied.
* There is an unresolved comment around the use of approxidate("now").
These calls are untouched from v1.
Thanks, -Stolee
Cc: sandals@crustytoothpaste.net [sandals@crustytoothpaste.net],
steadmon@google.com [steadmon@google.com], jrnieder@gmail.com
[jrnieder@gmail.com], peff@peff.net [peff@peff.net], congdanhqx@gmail.com
[congdanhqx@gmail.com], phillip.wood123@gmail.com
[phillip.wood123@gmail.com], emilyshaffer@google.com
[emilyshaffer@google.com], sluongng@gmail.com [sluongng@gmail.com],
jonathantanmy@google.com [jonathantanmy@google.com]
Derrick Stolee (6):
maintenance: optionally skip --auto process
maintenance: add --schedule option and config
for-each-repo: run subcommands on configured repos
maintenance: add [un]register subcommands
maintenance: add start/stop subcommands
maintenance: recommended schedule in register/start
.gitignore | 1 +
Documentation/config/maintenance.txt | 10 +
Documentation/git-for-each-repo.txt | 59 ++++++
Documentation/git-maintenance.txt | 44 +++-
Makefile | 2 +
builtin.h | 1 +
builtin/for-each-repo.c | 58 ++++++
builtin/gc.c | 292 ++++++++++++++++++++++++++-
command-list.txt | 1 +
git.c | 1 +
run-command.c | 6 +
t/helper/test-crontab.c | 35 ++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0068-for-each-repo.sh | 30 +++
t/t7900-maintenance.sh | 114 ++++++++++-
t/test-lib.sh | 6 +
17 files changed, 654 insertions(+), 8 deletions(-)
create mode 100644 Documentation/git-for-each-repo.txt
create mode 100644 builtin/for-each-repo.c
create mode 100644 t/helper/test-crontab.c
create mode 100755 t/t0068-for-each-repo.sh
base-commit: e9bb32f53ade2067f773bfe6e5c13ed1a5d694a6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-680%2Fderrickstolee%2Fmaintenance%2Fscheduled-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-680/derrickstolee/maintenance/scheduled-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/680
Range-diff vs v2:
1: 5fdd8188b1 = 1: 5fdd8188b1 maintenance: optionally skip --auto process
2: e3ef0b9bea < -: ---------- maintenance: store the "last run" time in config
3: c728c57d85 ! 2: 41a067894d maintenance: add --scheduled option and config
@@ Metadata
Author: Derrick Stolee <dstolee@microsoft.com>
## Commit message ##
- maintenance: add --scheduled option and config
+ maintenance: add --schedule option and config
A user may want to run certain maintenance tasks based on frequency, not
conditions given in the repository. For example, the user may want to
perform a 'prefetch' task every hour, or 'gc' task every day. To assist,
- update the 'git maintenance run --scheduled' command to check the config
- for the last run of that task and add a number of seconds. The task
- would then run only if the current time is beyond that minimum
- timestamp.
+ update the 'git maintenance run' command to include a
+ '--schedule=<frequency>' option. The allowed frequencies are 'hourly',
+ 'daily', and 'weekly'. These values are also allowed in a new config
+ value 'maintenance.<task>.schedule'.
- Add a '--scheduled' option to 'git maintenance run' to only run tasks
- that have had enough time pass since their last run. This is done for
- each enabled task by checking if the current timestamp is at least as
- large as the sum of 'maintenance.<task>.lastRun' and
- 'maintenance.<task>.schedule' in the Git config. This second value is
- new to this commit, storing a number of seconds intended between runs.
+ The 'git maintenance run --schedule=<frequency>' checks the '*.schedule'
+ config value for each enabled task to see if the configured frequency is
+ at least as frequent as the frequency from the '--schedule' argument. We
+ use the following order, for full clarity:
- A user could then set up an hourly maintenance run with the following
- cron table:
+ 'hourly' > 'daily' > 'weekly'
- 0 * * * * git -C <repo> maintenance run --scheduled
+ Use new 'enum schedule_priority' to track these values numerically.
- Then, the user could configure the repository with the following config
- values:
+ The following cron table would run the scheduled tasks with the correct
+ frequencies:
- maintenance.prefetch.schedule 3000
- maintenance.gc.schedule 86000
+ 0 1-23 * * * git -C <repo> maintenance run --scheduled=hourly
+ 0 0 * * 1-6 git -C <repo> maintenance run --scheduled=daily
+ 0 0 * * 0 git -C <repo> maintenance run --scheduled=weekly
- These numbers are slightly lower than one hour and one day (in seconds).
- The cron schedule will enforce the hourly run rate, but we can use these
- schedules to ensure the 'gc' task runs once a day. The error is given
- because the *.lastRun config option is specified at the _start_ of the
- task run. Otherwise, a slow task run could shift the "daily" job of 'gc'
- from a 10:00pm run to 11:00pm run, or later.
+ This cron schedule will run --scheduled=hourly every hour except at
+ midnight. This avoids a concurrent run with the --scheduled=daily that
+ runs at midnight every day except the first day of the week. This avoids
+ a concurrent run with the --scheduled=weekly that runs at midnight on
+ the first day of the week. Since --scheduled=daily also runs the
+ 'hourly' tasks and --scheduled=weekly runs the 'hourly' and 'daily'
+ tasks, we will still see all tasks run with the proper frequencies.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
## Documentation/config/maintenance.txt ##
-@@ Documentation/config/maintenance.txt: maintenance.<task>.lastRun::
- `<task>` is run. It stores a timestamp representing the most-recent
- run of the `<task>`.
+@@ Documentation/config/maintenance.txt: maintenance.<task>.enabled::
+ `--task` option exists. By default, only `maintenance.gc.enabled`
+ is true.
+maintenance.<task>.schedule::
+ This config option controls whether or not the given `<task>` runs
-+ during a `git maintenance run --scheduled` command. If the option
-+ is an integer value `S`, then the `<task>` is run when the current
-+ time is `S` seconds after the timestamp stored in
-+ `maintenance.<task>.lastRun`. If the option has no value or a
-+ non-integer value, then the task will never run with the `--scheduled`
-+ option.
++ during a `git maintenance run --schedule=<frequency>` command. The
++ value must be one of "hourly", "daily", or "weekly".
+
maintenance.commit-graph.auto::
This integer config option controls how often the `commit-graph` task
@@ Documentation/git-maintenance.txt: OPTIONS
in the `gc.auto` config setting, or when the number of pack-files
- exceeds the `gc.autoPackLimit` config setting.
+ exceeds the `gc.autoPackLimit` config setting. Not compatible with
-+ the `--scheduled` option.
++ the `--schedule` option.
+
-+--scheduled::
++--schedule::
+ When combined with the `run` subcommand, run maintenance tasks
+ only if certain time conditions are met, as specified by the
+ `maintenance.<task>.schedule` config value for each `<task>`.
@@ Documentation/git-maintenance.txt: OPTIONS
## builtin/gc.c ##
@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
+ return 0;
}
- static const char * const builtin_maintenance_run_usage[] = {
+-static const char * const builtin_maintenance_run_usage[] = {
- N_("git maintenance run [--auto] [--[no-]quiet] [--task=<task>]"),
-+ N_("git maintenance run [--auto] [--[no-]quiet] [--task=<task>] [--scheduled]"),
++static const char *const builtin_maintenance_run_usage[] = {
++ N_("git maintenance run [--auto] [--[no-]quiet] [--task=<task>] [--schedule]"),
NULL
};
++enum schedule_priority {
++ SCHEDULE_NONE = 0,
++ SCHEDULE_WEEKLY = 1,
++ SCHEDULE_DAILY = 2,
++ SCHEDULE_HOURLY = 3,
++};
++
++static enum schedule_priority parse_schedule(const char *value)
++{
++ if (!value)
++ return SCHEDULE_NONE;
++ if (!strcasecmp(value, "hourly"))
++ return SCHEDULE_HOURLY;
++ if (!strcasecmp(value, "daily"))
++ return SCHEDULE_DAILY;
++ if (!strcasecmp(value, "weekly"))
++ return SCHEDULE_WEEKLY;
++ return SCHEDULE_NONE;
++}
++
++static int maintenance_opt_schedule(const struct option *opt, const char *arg,
++ int unset)
++{
++ enum schedule_priority *priority = opt->value;
++
++ if (unset)
++ die(_("--no-schedule is not allowed"));
++
++ *priority = parse_schedule(arg);
++
++ if (!*priority)
++ die(_("unrecognized --schedule argument '%s'"), arg);
++
++ return 0;
++}
++
struct maintenance_run_opts {
int auto_flag;
-+ int scheduled;
int quiet;
++ enum schedule_priority schedule;
};
+ /* Remember to update object flag allocation in object.h */
@@ builtin/gc.c: struct maintenance_task {
- const char *name;
- maintenance_task_fn *fn;
maintenance_auto_fn *auto_condition;
-- unsigned enabled:1;
-+ unsigned enabled:1,
-+ scheduled:1;
+ unsigned enabled:1;
++ enum schedule_priority schedule;
++
/* -1 if not selected. */
int selected_order;
+ };
@@ builtin/gc.c: static int maintenance_run_tasks(struct maintenance_run_opts *opts)
- !tasks[i].auto_condition()))
continue;
-+ if (opts->scheduled && !tasks[i].scheduled)
+ if (opts->auto_flag &&
+- (!tasks[i].auto_condition ||
+- !tasks[i].auto_condition()))
++ (!tasks[i].auto_condition || !tasks[i].auto_condition()))
+ continue;
+
- update_last_run(&tasks[i]);
++ if (opts->schedule && tasks[i].schedule < opts->schedule)
+ continue;
trace2_region_enter("maintenance", tasks[i].name, r);
-@@ builtin/gc.c: static int maintenance_run_tasks(struct maintenance_run_opts *opts)
- return result;
- }
-
-+static void fill_schedule_info(struct maintenance_task *task,
-+ const char *config_name,
-+ timestamp_t schedule_delay)
-+{
-+ timestamp_t now = approxidate("now");
-+ char *value = NULL;
-+ struct strbuf last_run = STRBUF_INIT;
-+ int64_t previous_run;
-+
-+ strbuf_addf(&last_run, "maintenance.%s.lastrun", task->name);
-+
-+ if (git_config_get_string(last_run.buf, &value))
-+ task->scheduled = 1;
-+ else {
-+ previous_run = git_config_int64(last_run.buf, value);
-+ if (now >= previous_run + schedule_delay)
-+ task->scheduled = 1;
-+ }
-+
-+ free(value);
-+ strbuf_release(&last_run);
-+}
-+
- static void initialize_task_config(void)
- {
- int i;
@@ builtin/gc.c: static void initialize_task_config(void)
for (i = 0; i < TASK__COUNT; i++) {
@@ builtin/gc.c: static void initialize_task_config(void)
+ tasks[i].name);
+
+ if (!git_config_get_string(config_name.buf, &config_str)) {
-+ timestamp_t schedule_delay = git_config_int64(
-+ config_name.buf,
-+ config_str);
-+ fill_schedule_info(&tasks[i],
-+ config_name.buf,
-+ schedule_delay);
++ tasks[i].schedule = parse_schedule(config_str);
+ free(config_str);
+ }
}
@@ builtin/gc.c: static int maintenance_run(int argc, const char **argv, const char
struct option builtin_maintenance_run_options[] = {
OPT_BOOL(0, "auto", &opts.auto_flag,
N_("run tasks based on the state of the repository")),
-+ OPT_BOOL(0, "scheduled", &opts.scheduled,
-+ N_("run tasks based on time intervals")),
++ OPT_CALLBACK(0, "schedule", &opts.schedule, N_("frequency"),
++ N_("run tasks based on frequency"),
++ maintenance_opt_schedule),
OPT_BOOL(0, "quiet", &opts.quiet,
N_("do not report progress or other information over stderr")),
OPT_CALLBACK_F(0, "task", NULL, N_("task"),
@@ builtin/gc.c: static int maintenance_run(int argc, const char **argv, const char
builtin_maintenance_run_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
-+ if (opts.auto_flag + opts.scheduled > 1)
-+ die(_("use at most one of the --auto and --scheduled options"));
++ if (opts.auto_flag && opts.schedule)
++ die(_("use at most one of --auto and --schedule=<frequency>"));
+
if (argc != 0)
usage_with_options(builtin_maintenance_run_usage,
@@ t/t7900-maintenance.sh: test_expect_success 'maintenance.incremental-repack.auto
done
'
-+test_expect_success '--auto and --scheduled incompatible' '
-+ test_must_fail git maintenance run --auto --scheduled 2>err &&
++test_expect_success '--auto and --schedule incompatible' '
++ test_must_fail git maintenance run --auto --schedule=daily 2>err &&
+ test_i18ngrep "at most one" err
+'
+
- test_expect_success 'tasks update maintenance.<task>.lastRun' '
- git config --unset maintenance.commit-graph.lastrun &&
- GIT_TRACE2_EVENT="$(pwd)/run.txt" \
-@@ t/t7900-maintenance.sh: test_expect_success 'tasks update maintenance.<task>.lastRun' '
- test_cmp_config 1595000000 maintenance.commit-graph.lastrun
- '
-
-+test_expect_success '--scheduled with specific time' '
-+ git config maintenance.commit-graph.schedule 100 &&
-+ GIT_TRACE2_EVENT="$(pwd)/too-soon.txt" \
-+ GIT_TEST_DATE_NOW=1595000099 \
-+ git maintenance run --scheduled 2>/dev/null &&
++test_expect_success 'invalid --schedule value' '
++ test_must_fail git maintenance run --schedule=annually 2>err &&
++ test_i18ngrep "unrecognized --schedule" err
++'
++
++test_expect_success '--schedule inheritance weekly -> daily -> hourly' '
++ git config maintenance.loose-objects.enabled true &&
++ git config maintenance.loose-objects.schedule hourly &&
++ git config maintenance.commit-graph.enabled true &&
++ git config maintenance.commit-graph.schedule daily &&
++ git config maintenance.incremental-repack.enabled true &&
++ git config maintenance.incremental-repack.schedule weekly &&
++
++ GIT_TRACE2_EVENT="$(pwd)/hourly.txt" \
++ git maintenance run --schedule=hourly 2>/dev/null &&
++ test_subcommand git prune-packed --quiet <hourly.txt &&
+ test_subcommand ! git commit-graph write --split --reachable \
-+ --no-progress <too-soon.txt &&
-+ GIT_TRACE2_EVENT="$(pwd)/long-enough.txt" \
-+ GIT_TEST_DATE_NOW=1595000100 \
-+ git maintenance run --scheduled 2>/dev/null &&
++ --no-progress <hourly.txt &&
++ test_subcommand ! git multi-pack-index write --no-progress <hourly.txt &&
++
++ GIT_TRACE2_EVENT="$(pwd)/daily.txt" \
++ git maintenance run --schedule=daily 2>/dev/null &&
++ test_subcommand git prune-packed --quiet <daily.txt &&
++ test_subcommand git commit-graph write --split --reachable \
++ --no-progress <daily.txt &&
++ test_subcommand ! git multi-pack-index write --no-progress <daily.txt &&
++
++ GIT_TRACE2_EVENT="$(pwd)/weekly.txt" \
++ git maintenance run --schedule=weekly 2>/dev/null &&
++ test_subcommand git prune-packed --quiet <weekly.txt &&
+ test_subcommand git commit-graph write --split --reachable \
-+ --no-progress <long-enough.txt &&
-+ test_cmp_config 1595000100 maintenance.commit-graph.lastrun
++ --no-progress <weekly.txt &&
++ test_subcommand git multi-pack-index write --no-progress <weekly.txt
+'
+
test_done
4: 0314258c5c = 3: b29b68614b for-each-repo: run subcommands on configured repos
5: c0ce1267a9 ! 4: fc741fab5a maintenance: add [un]register subcommands
@@ t/t7900-maintenance.sh: GIT_TEST_MULTI_PACK_INDEX=0
test_expect_code 128 git maintenance barf 2>err &&
test_i18ngrep "invalid subcommand: barf" err
'
-@@ t/t7900-maintenance.sh: test_expect_success '--scheduled with specific time' '
- test_cmp_config 1595000100 maintenance.commit-graph.lastrun
+@@ t/t7900-maintenance.sh: test_expect_success '--schedule inheritance weekly -> daily -> hourly' '
+ test_subcommand git multi-pack-index write --no-progress <weekly.txt
'
+test_expect_success 'register and unregister' '
6: 8a7c34035a ! 5: e9672c6a6c maintenance: add start/stop subcommands
@@ Commit message
maintenance using 'cron', when available. This integration is as simple
as I could make it, barring some implementation complications.
- For now, the background maintenance is scheduled to run hourly via the
- following cron table row (ignore line breaks):
+ The schedule is laid out as follows:
- 0 * * * * $p/git --exec-path=$p
- for-each-repo --config=maintenance.repo
- maintenance run --scheduled
+ 0 1-23 * * * $cmd maintenance run --schedule=hourly
+ 0 0 * * 1-6 $cmd maintenance run --schedule=daily
+ 0 0 * * 0 $cmd maintenance run --schedule=weekly
- Future extensions may want to add more complex schedules or some form of
- logging. For now, hourly runs seem frequent enough to satisfy the needs
- of tasks like 'prefetch' without being so frequent that users would
- complain about many no-op commands.
+ where $cmd is a properly-qualified 'git for-each-repo' execution:
- Here, "$p" is a placeholder for the path to the current Git executable.
- This is critical for systems with multiple versions of Git.
- Specifically, macOS has a system version at '/usr/bin/git' while the
- version that users can install resides at '/usr/local/bin/git' (symlinked
- to '/usr/local/libexec/git-core/git'). This will also use your
- locally-built version if you build and run this in your development
+ $cmd=$path/git --exec-path=$path for-each-repo --config=maintenance.repo
+
+ where $path points to the location of the Git executable running 'git
+ maintenance start'. This is critical for systems with multiple versions
+ of Git. Specifically, macOS has a system version at '/usr/bin/git' while
+ the version that users can install resides at '/usr/local/bin/git'
+ (symlinked to '/usr/local/libexec/git-core/git'). This will also use
+ your locally-built version if you build and run this in your development
environment without installing first.
+ This conditional schedule avoids having cron launch multiple 'git
+ for-each-repo' commands in parallel. Such parallel commands would likely
+ lead to the 'hourly' and 'daily' tasks competing over the object
+ database lock. This could lead to to some tasks never being run! Since
+ the --schedule=<frequency> argument will run all tasks with _at least_
+ the given frequency, the daily runs will also run the hourly tasks.
+ Similarly, the weekly runs will also run the daily and hourly tasks.
+
The GIT_TEST_CRONTAB environment variable is not intended for users to
edit, but instead as a way to mock the 'crontab [-l]' command. This
variable is set in test-lib.sh to avoid a future test from accidentally
@@ builtin/gc.c: static int maintenance_unregister(void)
+ }
+
+ if (run_maintenance) {
++ struct strbuf line_format = STRBUF_INIT;
+ const char *exec_path = git_exec_path();
+
-+ fprintf(cron_in, "\n%s\n", BEGIN_LINE);
-+ fprintf(cron_in, "# The following schedule was created by Git\n");
++ fprintf(cron_in, "%s\n", BEGIN_LINE);
++ fprintf(cron_in,
++ "# The following schedule was created by Git\n");
+ fprintf(cron_in, "# Any edits made in this region might be\n");
-+ fprintf(cron_in, "# replaced in the future by a Git command.\n\n");
-+
+ fprintf(cron_in,
-+ "0 * * * * \"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --scheduled\n",
-+ exec_path, exec_path);
++ "# replaced in the future by a Git command.\n\n");
++
++ strbuf_addf(&line_format,
++ "%%s %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\n",
++ exec_path, exec_path);
++ fprintf(cron_in, line_format.buf, "0", "1-23", "*", "hourly");
++ fprintf(cron_in, line_format.buf, "0", "0", "1-6", "daily");
++ fprintf(cron_in, line_format.buf, "0", "0", "0", "weekly");
++ strbuf_release(&line_format);
+
+ fprintf(cron_in, "\n%s\n", END_LINE);
+ }
@@ t/t7900-maintenance.sh: test_expect_success 'register and unregister' '
+ # start registers the repo
+ git config --get --global maintenance.repo "$(pwd)" &&
+
-+ grep "for-each-repo --config=maintenance.repo maintenance run --scheduled" cron.txt
++ grep "for-each-repo --config=maintenance.repo maintenance run --schedule=daily" cron.txt &&
++ grep "for-each-repo --config=maintenance.repo maintenance run --schedule=hourly" cron.txt &&
++ grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
+'
+
+test_expect_success 'stop from existing schedule' '
@@ t/t7900-maintenance.sh: test_expect_success 'register and unregister' '
+ # stop does not unregister the repo
+ git config --get --global maintenance.repo "$(pwd)" &&
+
-+ # The newline is preserved
-+ echo >empty &&
-+ test_cmp empty cron.txt &&
-+
+ # Operation is idempotent
+ GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
-+ test_cmp empty cron.txt
++ test_must_be_empty cron.txt
+'
+
+test_expect_success 'start preserves existing schedule' '
7: 9ecabeb055 ! 6: 62e8db8b2a maintenance: recommended schedule in register/start
@@ Commit message
repository. It does not specify what maintenance should occur or how
often.
- If a user sets any 'maintenance.<task>.scheduled' config value, then
+ If a user sets any 'maintenance.<task>.schedule' config value, then
they have chosen a specific schedule for themselves and Git should
respect that.
@@ Commit message
schedule we use in Scalar and VFS for Git for very large repositories
using the GVFS protocol. While the schedule works in that environment,
it is possible that "normal" Git repositories could benefit from
- something more obvious (such as running 'gc' once a day). However, this
+ something more obvious (such as running 'gc' weekly). However, this
patch gives us a place to start a conversation on what we should
recommend. For my purposes, Scalar will set these config values so we
can always differ from core Git's recommendations.
@@ builtin/gc.c: static int maintenance_run(int argc, const char **argv, const char
+ prefix = config_name.len;
+
+ for (i = 0; !found && i < TASK__COUNT; i++) {
-+ int value;
++ char *value;
+
+ strbuf_setlen(&config_name, prefix);
+ strbuf_addf(&config_name, "%s.schedule", tasks[i].name);
+
-+ if (!git_config_get_int(config_name.buf, &value))
++ if (!git_config_get_string(config_name.buf, &value)) {
+ found = 1;
++ FREE_AND_NULL(value);
++ }
+ }
+
+ strbuf_release(&config_name);
@@ builtin/gc.c: static int maintenance_run(int argc, const char **argv, const char
+ git_config_set("maintenance.gc.enabled", "false");
+
+ git_config_set("maintenance.prefetch.enabled", "true");
-+ git_config_set("maintenance.prefetch.schedule", "3500");
++ git_config_set("maintenance.prefetch.schedule", "hourly");
+
+ git_config_set("maintenance.commit-graph.enabled", "true");
-+ git_config_set("maintenance.commit-graph.schedule", "3500");
++ git_config_set("maintenance.commit-graph.schedule", "hourly");
+
+ git_config_set("maintenance.loose-objects.enabled", "true");
-+ git_config_set("maintenance.loose-objects.schedule", "86000");
++ git_config_set("maintenance.loose-objects.schedule", "daily");
+
+ git_config_set("maintenance.incremental-repack.enabled", "true");
-+ git_config_set("maintenance.incremental-repack.schedule", "86000");
++ git_config_set("maintenance.incremental-repack.schedule", "daily");
+}
+
static int maintenance_register(void)
@@ builtin/gc.c: static int maintenance_register(void)
if (!the_repository || !the_repository->gitdir)
return 0;
-+ if (has_schedule_config())
++ if (!has_schedule_config())
+ set_recommended_schedule();
+
config_get.git_cmd = 1;
@@ builtin/gc.c: static int maintenance_register(void)
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_expect_success 'register and unregister' '
+ git config --global --add maintenance.repo /existing1 &&
git config --global --add maintenance.repo /existing2 &&
git config --global --get-all maintenance.repo >before &&
++
++ # We still have maintenance.<task>.schedule config set,
++ # so this does not update the local schedule
++ git maintenance register &&
++ test_must_fail git config maintenance.auto &&
++
++ # Clear previous maintenance.<task>.schedule values
++ for task in loose-objects commit-graph incremental-repack
++ do
++ git config --unset maintenance.$task.schedule || return 1
++ done &&
git maintenance register &&
+ test_cmp_config false maintenance.auto &&
+ test_cmp_config false maintenance.gc.enabled &&
+ test_cmp_config true maintenance.prefetch.enabled &&
-+ test_cmp_config 3500 maintenance.commit-graph.schedule &&
-+ test_cmp_config 86000 maintenance.incremental-repack.schedule &&
++ test_cmp_config hourly maintenance.commit-graph.schedule &&
++ test_cmp_config daily maintenance.incremental-repack.schedule &&
git config --global --get-all maintenance.repo >actual &&
cp before after &&
pwd >>after &&
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 2/6] maintenance: add --schedule option and config
From: Derrick Stolee via GitGitGadget @ 2020-08-28 15:45 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.680.v3.git.1598629517.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
A user may want to run certain maintenance tasks based on frequency, not
conditions given in the repository. For example, the user may want to
perform a 'prefetch' task every hour, or 'gc' task every day. To assist,
update the 'git maintenance run' command to include a
'--schedule=<frequency>' option. The allowed frequencies are 'hourly',
'daily', and 'weekly'. These values are also allowed in a new config
value 'maintenance.<task>.schedule'.
The 'git maintenance run --schedule=<frequency>' checks the '*.schedule'
config value for each enabled task to see if the configured frequency is
at least as frequent as the frequency from the '--schedule' argument. We
use the following order, for full clarity:
'hourly' > 'daily' > 'weekly'
Use new 'enum schedule_priority' to track these values numerically.
The following cron table would run the scheduled tasks with the correct
frequencies:
0 1-23 * * * git -C <repo> maintenance run --scheduled=hourly
0 0 * * 1-6 git -C <repo> maintenance run --scheduled=daily
0 0 * * 0 git -C <repo> maintenance run --scheduled=weekly
This cron schedule will run --scheduled=hourly every hour except at
midnight. This avoids a concurrent run with the --scheduled=daily that
runs at midnight every day except the first day of the week. This avoids
a concurrent run with the --scheduled=weekly that runs at midnight on
the first day of the week. Since --scheduled=daily also runs the
'hourly' tasks and --scheduled=weekly runs the 'hourly' and 'daily'
tasks, we will still see all tasks run with the proper frequencies.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/config/maintenance.txt | 5 +++
Documentation/git-maintenance.txt | 13 +++++-
builtin/gc.c | 67 +++++++++++++++++++++++++---
t/t7900-maintenance.sh | 40 +++++++++++++++++
4 files changed, 119 insertions(+), 6 deletions(-)
diff --git a/Documentation/config/maintenance.txt b/Documentation/config/maintenance.txt
index 06db758172..70585564fa 100644
--- a/Documentation/config/maintenance.txt
+++ b/Documentation/config/maintenance.txt
@@ -10,6 +10,11 @@ maintenance.<task>.enabled::
`--task` option exists. By default, only `maintenance.gc.enabled`
is true.
+maintenance.<task>.schedule::
+ This config option controls whether or not the given `<task>` runs
+ during a `git maintenance run --schedule=<frequency>` command. The
+ value must be one of "hourly", "daily", or "weekly".
+
maintenance.commit-graph.auto::
This integer config option controls how often the `commit-graph` task
should be run as part of `git maintenance run --auto`. If zero, then
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index b44efb05a3..3af5907b01 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -107,7 +107,18 @@ OPTIONS
only if certain thresholds are met. For example, the `gc` task
runs when the number of loose objects exceeds the number stored
in the `gc.auto` config setting, or when the number of pack-files
- exceeds the `gc.autoPackLimit` config setting.
+ exceeds the `gc.autoPackLimit` config setting. Not compatible with
+ the `--schedule` option.
+
+--schedule::
+ When combined with the `run` subcommand, run maintenance tasks
+ only if certain time conditions are met, as specified by the
+ `maintenance.<task>.schedule` config value for each `<task>`.
+ This config value specifies a number of seconds since the last
+ time that task ran, according to the `maintenance.<task>.lastRun`
+ config value. The tasks that are tested are those provided by
+ the `--task=<task>` option(s) or those with
+ `maintenance.<task>.enabled` set to true.
--quiet::
Do not report progress or other information over `stderr`.
diff --git a/builtin/gc.c b/builtin/gc.c
index f8459df04c..85a3370692 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -704,14 +704,51 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
return 0;
}
-static const char * const builtin_maintenance_run_usage[] = {
- N_("git maintenance run [--auto] [--[no-]quiet] [--task=<task>]"),
+static const char *const builtin_maintenance_run_usage[] = {
+ N_("git maintenance run [--auto] [--[no-]quiet] [--task=<task>] [--schedule]"),
NULL
};
+enum schedule_priority {
+ SCHEDULE_NONE = 0,
+ SCHEDULE_WEEKLY = 1,
+ SCHEDULE_DAILY = 2,
+ SCHEDULE_HOURLY = 3,
+};
+
+static enum schedule_priority parse_schedule(const char *value)
+{
+ if (!value)
+ return SCHEDULE_NONE;
+ if (!strcasecmp(value, "hourly"))
+ return SCHEDULE_HOURLY;
+ if (!strcasecmp(value, "daily"))
+ return SCHEDULE_DAILY;
+ if (!strcasecmp(value, "weekly"))
+ return SCHEDULE_WEEKLY;
+ return SCHEDULE_NONE;
+}
+
+static int maintenance_opt_schedule(const struct option *opt, const char *arg,
+ int unset)
+{
+ enum schedule_priority *priority = opt->value;
+
+ if (unset)
+ die(_("--no-schedule is not allowed"));
+
+ *priority = parse_schedule(arg);
+
+ if (!*priority)
+ die(_("unrecognized --schedule argument '%s'"), arg);
+
+ return 0;
+}
+
struct maintenance_run_opts {
int auto_flag;
int quiet;
+ enum schedule_priority schedule;
};
/* Remember to update object flag allocation in object.h */
@@ -1159,6 +1196,8 @@ struct maintenance_task {
maintenance_auto_fn *auto_condition;
unsigned enabled:1;
+ enum schedule_priority schedule;
+
/* -1 if not selected. */
int selected_order;
};
@@ -1250,8 +1289,10 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts)
continue;
if (opts->auto_flag &&
- (!tasks[i].auto_condition ||
- !tasks[i].auto_condition()))
+ (!tasks[i].auto_condition || !tasks[i].auto_condition()))
+ continue;
+
+ if (opts->schedule && tasks[i].schedule < opts->schedule)
continue;
trace2_region_enter("maintenance", tasks[i].name, r);
@@ -1274,13 +1315,23 @@ static void initialize_task_config(void)
for (i = 0; i < TASK__COUNT; i++) {
int config_value;
+ char *config_str;
- strbuf_setlen(&config_name, 0);
+ strbuf_reset(&config_name);
strbuf_addf(&config_name, "maintenance.%s.enabled",
tasks[i].name);
if (!git_config_get_bool(config_name.buf, &config_value))
tasks[i].enabled = config_value;
+
+ strbuf_reset(&config_name);
+ strbuf_addf(&config_name, "maintenance.%s.schedule",
+ tasks[i].name);
+
+ if (!git_config_get_string(config_name.buf, &config_str)) {
+ tasks[i].schedule = parse_schedule(config_str);
+ free(config_str);
+ }
}
strbuf_release(&config_name);
@@ -1324,6 +1375,9 @@ static int maintenance_run(int argc, const char **argv, const char *prefix)
struct option builtin_maintenance_run_options[] = {
OPT_BOOL(0, "auto", &opts.auto_flag,
N_("run tasks based on the state of the repository")),
+ OPT_CALLBACK(0, "schedule", &opts.schedule, N_("frequency"),
+ N_("run tasks based on frequency"),
+ maintenance_opt_schedule),
OPT_BOOL(0, "quiet", &opts.quiet,
N_("do not report progress or other information over stderr")),
OPT_CALLBACK_F(0, "task", NULL, N_("task"),
@@ -1344,6 +1398,9 @@ static int maintenance_run(int argc, const char **argv, const char *prefix)
builtin_maintenance_run_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (opts.auto_flag && opts.schedule)
+ die(_("use at most one of --auto and --schedule=<frequency>"));
+
if (argc != 0)
usage_with_options(builtin_maintenance_run_usage,
builtin_maintenance_run_options);
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index e0ba19e1ff..328bbaa830 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -264,4 +264,44 @@ test_expect_success 'maintenance.incremental-repack.auto' '
done
'
+test_expect_success '--auto and --schedule incompatible' '
+ test_must_fail git maintenance run --auto --schedule=daily 2>err &&
+ test_i18ngrep "at most one" err
+'
+
+test_expect_success 'invalid --schedule value' '
+ test_must_fail git maintenance run --schedule=annually 2>err &&
+ test_i18ngrep "unrecognized --schedule" err
+'
+
+test_expect_success '--schedule inheritance weekly -> daily -> hourly' '
+ git config maintenance.loose-objects.enabled true &&
+ git config maintenance.loose-objects.schedule hourly &&
+ git config maintenance.commit-graph.enabled true &&
+ git config maintenance.commit-graph.schedule daily &&
+ git config maintenance.incremental-repack.enabled true &&
+ git config maintenance.incremental-repack.schedule weekly &&
+
+ GIT_TRACE2_EVENT="$(pwd)/hourly.txt" \
+ git maintenance run --schedule=hourly 2>/dev/null &&
+ test_subcommand git prune-packed --quiet <hourly.txt &&
+ test_subcommand ! git commit-graph write --split --reachable \
+ --no-progress <hourly.txt &&
+ test_subcommand ! git multi-pack-index write --no-progress <hourly.txt &&
+
+ GIT_TRACE2_EVENT="$(pwd)/daily.txt" \
+ git maintenance run --schedule=daily 2>/dev/null &&
+ test_subcommand git prune-packed --quiet <daily.txt &&
+ test_subcommand git commit-graph write --split --reachable \
+ --no-progress <daily.txt &&
+ test_subcommand ! git multi-pack-index write --no-progress <daily.txt &&
+
+ GIT_TRACE2_EVENT="$(pwd)/weekly.txt" \
+ git maintenance run --schedule=weekly 2>/dev/null &&
+ test_subcommand git prune-packed --quiet <weekly.txt &&
+ test_subcommand git commit-graph write --split --reachable \
+ --no-progress <weekly.txt &&
+ test_subcommand git multi-pack-index write --no-progress <weekly.txt
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 4/6] maintenance: add [un]register subcommands
From: Derrick Stolee via GitGitGadget @ 2020-08-28 15:45 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.680.v3.git.1598629517.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
In preparation for launching background maintenance from the 'git
maintenance' builtin, create register/unregister subcommands. These
commands update the new 'maintenance.repos' config option in the global
config so the background maintenance job knows which repositories to
maintain.
These commands allow users to add a repository to the background
maintenance list without disrupting the actual maintenance mechanism.
For example, a user can run 'git maintenance register' when no
background maintenance is running and it will not start the background
maintenance. A later update to start running background maintenance will
then pick up this repository automatically.
The opposite example is that a user can run 'git maintenance unregister'
to remove the current repository from background maintenance without
halting maintenance for other repositories.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-maintenance.txt | 14 ++++++++
builtin/gc.c | 55 ++++++++++++++++++++++++++++++-
t/t7900-maintenance.sh | 17 +++++++++-
3 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index 3af5907b01..78d0d8df91 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -29,6 +29,15 @@ Git repository.
SUBCOMMANDS
-----------
+register::
+ Initialize Git config values so any scheduled maintenance will
+ start running on this repository. This adds the repository to the
+ `maintenance.repo` config variable in the current user's global
+ config and enables some recommended configuration values for
+ `maintenance.<task>.schedule`. The tasks that are enabled are safe
+ for running in the background without disrupting foreground
+ processes.
+
run::
Run one or more maintenance tasks. If one or more `--task` options
are specified, then those tasks are run in that order. Otherwise,
@@ -36,6 +45,11 @@ run::
config options are true. By default, only `maintenance.gc.enabled`
is true.
+unregister::
+ Remove the current repository from background maintenance. This
+ only removes the repository from the configured list. It does not
+ stop the background maintenance processes from running.
+
TASKS
-----
diff --git a/builtin/gc.c b/builtin/gc.c
index 85a3370692..ec77e8d2fa 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1407,7 +1407,56 @@ static int maintenance_run(int argc, const char **argv, const char *prefix)
return maintenance_run_tasks(&opts);
}
-static const char builtin_maintenance_usage[] = N_("git maintenance run [<options>]");
+static int maintenance_register(void)
+{
+ struct child_process config_set = CHILD_PROCESS_INIT;
+ struct child_process config_get = CHILD_PROCESS_INIT;
+
+ /* There is no current repository, so skip registering it */
+ if (!the_repository || !the_repository->gitdir)
+ return 0;
+
+ config_get.git_cmd = 1;
+ strvec_pushl(&config_get.args, "config", "--global", "--get", "maintenance.repo",
+ the_repository->worktree ? the_repository->worktree
+ : the_repository->gitdir,
+ NULL);
+ config_get.out = -1;
+
+ if (start_command(&config_get))
+ return error(_("failed to run 'git config'"));
+
+ /* We already have this value in our config! */
+ if (!finish_command(&config_get))
+ return 0;
+
+ config_set.git_cmd = 1;
+ strvec_pushl(&config_set.args, "config", "--add", "--global", "maintenance.repo",
+ the_repository->worktree ? the_repository->worktree
+ : the_repository->gitdir,
+ NULL);
+
+ return run_command(&config_set);
+}
+
+static int maintenance_unregister(void)
+{
+ struct child_process config_unset = CHILD_PROCESS_INIT;
+
+ if (!the_repository || !the_repository->gitdir)
+ return error(_("no current repository to unregister"));
+
+ config_unset.git_cmd = 1;
+ strvec_pushl(&config_unset.args, "config", "--global", "--unset",
+ "maintenance.repo",
+ the_repository->worktree ? the_repository->worktree
+ : the_repository->gitdir,
+ NULL);
+
+ return run_command(&config_unset);
+}
+
+static const char builtin_maintenance_usage[] = N_("git maintenance <subcommand> [<options>]");
int cmd_maintenance(int argc, const char **argv, const char *prefix)
{
@@ -1416,6 +1465,10 @@ int cmd_maintenance(int argc, const char **argv, const char *prefix)
if (!strcmp(argv[1], "run"))
return maintenance_run(argc - 1, argv + 1, prefix);
+ if (!strcmp(argv[1], "register"))
+ return maintenance_register();
+ if (!strcmp(argv[1], "unregister"))
+ return maintenance_unregister();
die(_("invalid subcommand: %s"), argv[1]);
}
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 328bbaa830..272d1605d2 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -9,7 +9,7 @@ GIT_TEST_MULTI_PACK_INDEX=0
test_expect_success 'help text' '
test_expect_code 129 git maintenance -h 2>err &&
- test_i18ngrep "usage: git maintenance run" err &&
+ test_i18ngrep "usage: git maintenance <subcommand>" err &&
test_expect_code 128 git maintenance barf 2>err &&
test_i18ngrep "invalid subcommand: barf" err
'
@@ -304,4 +304,19 @@ test_expect_success '--schedule inheritance weekly -> daily -> hourly' '
test_subcommand git multi-pack-index write --no-progress <weekly.txt
'
+test_expect_success 'register and unregister' '
+ test_when_finished git config --global --unset-all maintenance.repo &&
+ git config --global --add maintenance.repo /existing1 &&
+ git config --global --add maintenance.repo /existing2 &&
+ git config --global --get-all maintenance.repo >before &&
+ git maintenance register &&
+ git config --global --get-all maintenance.repo >actual &&
+ cp before after &&
+ pwd >>after &&
+ test_cmp after actual &&
+ git maintenance unregister &&
+ git config --global --get-all maintenance.repo >actual &&
+ test_cmp before actual
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/6] maintenance: optionally skip --auto process
From: Derrick Stolee via GitGitGadget @ 2020-08-28 15:45 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.680.v3.git.1598629517.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
Some commands run 'git maintenance run --auto --[no-]quiet' after doing
their normal work, as a way to keep repositories clean as they are used.
Currently, users who do not want this maintenance to occur would set the
'gc.auto' config option to 0 to avoid the 'gc' task from running.
However, this does not stop the extra process invocation. On Windows,
this extra process invocation can be more expensive than necessary.
Allow users to drop this extra process by setting 'maintenance.auto' to
'false'.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/config/maintenance.txt | 5 +++++
run-command.c | 6 ++++++
t/t7900-maintenance.sh | 13 +++++++++++++
3 files changed, 24 insertions(+)
diff --git a/Documentation/config/maintenance.txt b/Documentation/config/maintenance.txt
index a0706d8f09..06db758172 100644
--- a/Documentation/config/maintenance.txt
+++ b/Documentation/config/maintenance.txt
@@ -1,3 +1,8 @@
+maintenance.auto::
+ This boolean config option controls whether some commands run
+ `git maintenance run --auto` after doing their normal work. Defaults
+ to true.
+
maintenance.<task>.enabled::
This boolean config option controls whether the maintenance task
with name `<task>` is run when no `--task` option is specified to
diff --git a/run-command.c b/run-command.c
index 2ee59acdc8..ea4d0fb4b1 100644
--- a/run-command.c
+++ b/run-command.c
@@ -7,6 +7,7 @@
#include "strbuf.h"
#include "string-list.h"
#include "quote.h"
+#include "config.h"
void child_process_init(struct child_process *child)
{
@@ -1868,8 +1869,13 @@ int run_processes_parallel_tr2(int n, get_next_task_fn get_next_task,
int run_auto_maintenance(int quiet)
{
+ int enabled;
struct child_process maint = CHILD_PROCESS_INIT;
+ if (!git_config_get_bool("maintenance.auto", &enabled) &&
+ !enabled)
+ return 0;
+
maint.git_cmd = 1;
strvec_pushl(&maint.args, "maintenance", "run", "--auto", NULL);
strvec_push(&maint.args, quiet ? "--quiet" : "--no-quiet");
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 6f878b0141..e0ba19e1ff 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -26,6 +26,19 @@ test_expect_success 'run [--auto|--quiet]' '
test_subcommand git gc --no-quiet <run-no-quiet.txt
'
+test_expect_success 'maintenance.auto config option' '
+ GIT_TRACE2_EVENT="$(pwd)/default" git commit --quiet --allow-empty -m 1 &&
+ test_subcommand git maintenance run --auto --quiet <default &&
+ GIT_TRACE2_EVENT="$(pwd)/true" \
+ git -c maintenance.auto=true \
+ commit --quiet --allow-empty -m 2 &&
+ test_subcommand git maintenance run --auto --quiet <true &&
+ GIT_TRACE2_EVENT="$(pwd)/false" \
+ git -c maintenance.auto=false \
+ commit --quiet --allow-empty -m 3 &&
+ test_subcommand ! git maintenance run --auto --quiet <false
+'
+
test_expect_success 'maintenance.<task>.enabled' '
git config maintenance.gc.enabled false &&
git config maintenance.commit-graph.enabled true &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 5/6] maintenance: add start/stop subcommands
From: Derrick Stolee via GitGitGadget @ 2020-08-28 15:45 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.680.v3.git.1598629517.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
Add new subcommands to 'git maintenance' that start or stop background
maintenance using 'cron', when available. This integration is as simple
as I could make it, barring some implementation complications.
The schedule is laid out as follows:
0 1-23 * * * $cmd maintenance run --schedule=hourly
0 0 * * 1-6 $cmd maintenance run --schedule=daily
0 0 * * 0 $cmd maintenance run --schedule=weekly
where $cmd is a properly-qualified 'git for-each-repo' execution:
$cmd=$path/git --exec-path=$path for-each-repo --config=maintenance.repo
where $path points to the location of the Git executable running 'git
maintenance start'. This is critical for systems with multiple versions
of Git. Specifically, macOS has a system version at '/usr/bin/git' while
the version that users can install resides at '/usr/local/bin/git'
(symlinked to '/usr/local/libexec/git-core/git'). This will also use
your locally-built version if you build and run this in your development
environment without installing first.
This conditional schedule avoids having cron launch multiple 'git
for-each-repo' commands in parallel. Such parallel commands would likely
lead to the 'hourly' and 'daily' tasks competing over the object
database lock. This could lead to to some tasks never being run! Since
the --schedule=<frequency> argument will run all tasks with _at least_
the given frequency, the daily runs will also run the hourly tasks.
Similarly, the weekly runs will also run the daily and hourly tasks.
The GIT_TEST_CRONTAB environment variable is not intended for users to
edit, but instead as a way to mock the 'crontab [-l]' command. This
variable is set in test-lib.sh to avoid a future test from accidentally
running anything with the cron integration from modifying the user's
schedule. We use GIT_TEST_CRONTAB='test-tool crontab <file>' in our
tests to check how the schedule is modified in 'git maintenance
(start|stop)' commands.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-maintenance.txt | 11 +++
Makefile | 1 +
builtin/gc.c | 124 ++++++++++++++++++++++++++++++
t/helper/test-crontab.c | 35 +++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t7900-maintenance.sh | 28 +++++++
t/test-lib.sh | 6 ++
8 files changed, 207 insertions(+)
create mode 100644 t/helper/test-crontab.c
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index 78d0d8df91..7f8c279fe8 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -45,6 +45,17 @@ run::
config options are true. By default, only `maintenance.gc.enabled`
is true.
+start::
+ Start running maintenance on the current repository. This performs
+ the same config updates as the `register` subcommand, then updates
+ the background scheduler to run `git maintenance run --scheduled`
+ on an hourly basis.
+
+stop::
+ Halt the background maintenance schedule. The current repository
+ is not removed from the list of maintained repositories, in case
+ the background maintenance is restarted later.
+
unregister::
Remove the current repository from background maintenance. This
only removes the repository from the configured list. It does not
diff --git a/Makefile b/Makefile
index 7c588ff036..c39b39bd7d 100644
--- a/Makefile
+++ b/Makefile
@@ -690,6 +690,7 @@ TEST_BUILTINS_OBJS += test-advise.o
TEST_BUILTINS_OBJS += test-bloom.o
TEST_BUILTINS_OBJS += test-chmtime.o
TEST_BUILTINS_OBJS += test-config.o
+TEST_BUILTINS_OBJS += test-crontab.o
TEST_BUILTINS_OBJS += test-ctype.o
TEST_BUILTINS_OBJS += test-date.o
TEST_BUILTINS_OBJS += test-delta.o
diff --git a/builtin/gc.c b/builtin/gc.c
index ec77e8d2fa..9914417e25 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -32,6 +32,7 @@
#include "remote.h"
#include "midx.h"
#include "object-store.h"
+#include "exec-cmd.h"
#define FAILED_RUN "failed to run %s"
@@ -1456,6 +1457,125 @@ static int maintenance_unregister(void)
return run_command(&config_unset);
}
+#define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE"
+#define END_LINE "# END GIT MAINTENANCE SCHEDULE"
+
+static int update_background_schedule(int run_maintenance)
+{
+ int result = 0;
+ int in_old_region = 0;
+ struct child_process crontab_list = CHILD_PROCESS_INIT;
+ struct child_process crontab_edit = CHILD_PROCESS_INIT;
+ FILE *cron_list, *cron_in;
+ const char *crontab_name;
+ struct strbuf line = STRBUF_INIT;
+ struct lock_file lk;
+ char *lock_path = xstrfmt("%s/schedule", the_repository->objects->odb->path);
+
+ if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0)
+ return error(_("another process is scheduling background maintenance"));
+
+ crontab_name = getenv("GIT_TEST_CRONTAB");
+ if (!crontab_name)
+ crontab_name = "crontab";
+
+ strvec_split(&crontab_list.args, crontab_name);
+ strvec_push(&crontab_list.args, "-l");
+ crontab_list.in = -1;
+ crontab_list.out = dup(lk.tempfile->fd);
+ crontab_list.git_cmd = 0;
+
+ if (start_command(&crontab_list)) {
+ result = error(_("failed to run 'crontab -l'; your system might not support 'cron'"));
+ goto cleanup;
+ }
+
+ /* Ignore exit code, as an empty crontab will return error. */
+ finish_command(&crontab_list);
+
+ /*
+ * Read from the .lock file, filtering out the old
+ * schedule while appending the new schedule.
+ */
+ cron_list = fdopen(lk.tempfile->fd, "r");
+ rewind(cron_list);
+
+ strvec_split(&crontab_edit.args, crontab_name);
+ crontab_edit.in = -1;
+ crontab_edit.git_cmd = 0;
+
+ if (start_command(&crontab_edit)) {
+ result = error(_("failed to run 'crontab'; your system might not support 'cron'"));
+ goto cleanup;
+ }
+
+ cron_in = fdopen(crontab_edit.in, "w");
+ if (!cron_in) {
+ result = error(_("failed to open stdin of 'crontab'"));
+ goto done_editing;
+ }
+
+ while (!strbuf_getline_lf(&line, cron_list)) {
+ if (!in_old_region && !strcmp(line.buf, BEGIN_LINE))
+ in_old_region = 1;
+ if (in_old_region)
+ continue;
+ fprintf(cron_in, "%s\n", line.buf);
+ if (in_old_region && !strcmp(line.buf, END_LINE))
+ in_old_region = 0;
+ }
+
+ if (run_maintenance) {
+ struct strbuf line_format = STRBUF_INIT;
+ const char *exec_path = git_exec_path();
+
+ fprintf(cron_in, "%s\n", BEGIN_LINE);
+ fprintf(cron_in,
+ "# The following schedule was created by Git\n");
+ fprintf(cron_in, "# Any edits made in this region might be\n");
+ fprintf(cron_in,
+ "# replaced in the future by a Git command.\n\n");
+
+ strbuf_addf(&line_format,
+ "%%s %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\n",
+ exec_path, exec_path);
+ fprintf(cron_in, line_format.buf, "0", "1-23", "*", "hourly");
+ fprintf(cron_in, line_format.buf, "0", "0", "1-6", "daily");
+ fprintf(cron_in, line_format.buf, "0", "0", "0", "weekly");
+ strbuf_release(&line_format);
+
+ fprintf(cron_in, "\n%s\n", END_LINE);
+ }
+
+ fflush(cron_in);
+ fclose(cron_in);
+ close(crontab_edit.in);
+
+done_editing:
+ if (finish_command(&crontab_edit)) {
+ result = error(_("'crontab' died"));
+ goto cleanup;
+ }
+ fclose(cron_list);
+
+cleanup:
+ rollback_lock_file(&lk);
+ return result;
+}
+
+static int maintenance_start(void)
+{
+ if (maintenance_register())
+ warning(_("failed to add repo to global config"));
+
+ return update_background_schedule(1);
+}
+
+static int maintenance_stop(void)
+{
+ return update_background_schedule(0);
+}
+
static const char builtin_maintenance_usage[] = N_("git maintenance <subcommand> [<options>]");
int cmd_maintenance(int argc, const char **argv, const char *prefix)
@@ -1465,6 +1585,10 @@ int cmd_maintenance(int argc, const char **argv, const char *prefix)
if (!strcmp(argv[1], "run"))
return maintenance_run(argc - 1, argv + 1, prefix);
+ if (!strcmp(argv[1], "start"))
+ return maintenance_start();
+ if (!strcmp(argv[1], "stop"))
+ return maintenance_stop();
if (!strcmp(argv[1], "register"))
return maintenance_register();
if (!strcmp(argv[1], "unregister"))
diff --git a/t/helper/test-crontab.c b/t/helper/test-crontab.c
new file mode 100644
index 0000000000..f5db6319c6
--- /dev/null
+++ b/t/helper/test-crontab.c
@@ -0,0 +1,35 @@
+#include "test-tool.h"
+#include "cache.h"
+
+/*
+ * Usage: test-tool cron <file> [-l]
+ *
+ * If -l is specified, then write the contents of <file> to stdou.
+ * Otherwise, write from stdin into <file>.
+ */
+int cmd__crontab(int argc, const char **argv)
+{
+ char a;
+ FILE *from, *to;
+
+ if (argc == 3 && !strcmp(argv[2], "-l")) {
+ from = fopen(argv[1], "r");
+ if (!from)
+ return 0;
+ to = stdout;
+ } else if (argc == 2) {
+ from = stdin;
+ to = fopen(argv[1], "w");
+ } else
+ return error("unknown arguments");
+
+ while ((a = fgetc(from)) != EOF)
+ fputc(a, to);
+
+ if (argc == 3)
+ fclose(from);
+ else
+ fclose(to);
+
+ return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 590b2efca7..432b49d948 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -18,6 +18,7 @@ static struct test_cmd cmds[] = {
{ "bloom", cmd__bloom },
{ "chmtime", cmd__chmtime },
{ "config", cmd__config },
+ { "crontab", cmd__crontab },
{ "ctype", cmd__ctype },
{ "date", cmd__date },
{ "delta", cmd__delta },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index ddc8e990e9..7c3281e071 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -8,6 +8,7 @@ int cmd__advise_if_enabled(int argc, const char **argv);
int cmd__bloom(int argc, const char **argv);
int cmd__chmtime(int argc, const char **argv);
int cmd__config(int argc, const char **argv);
+int cmd__crontab(int argc, const char **argv);
int cmd__ctype(int argc, const char **argv);
int cmd__date(int argc, const char **argv);
int cmd__delta(int argc, const char **argv);
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 272d1605d2..8803fcf621 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -319,4 +319,32 @@ test_expect_success 'register and unregister' '
test_cmp before actual
'
+test_expect_success 'start from empty cron table' '
+ GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
+
+ # start registers the repo
+ git config --get --global maintenance.repo "$(pwd)" &&
+
+ grep "for-each-repo --config=maintenance.repo maintenance run --schedule=daily" cron.txt &&
+ grep "for-each-repo --config=maintenance.repo maintenance run --schedule=hourly" cron.txt &&
+ grep "for-each-repo --config=maintenance.repo maintenance run --schedule=weekly" cron.txt
+'
+
+test_expect_success 'stop from existing schedule' '
+ GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
+
+ # stop does not unregister the repo
+ git config --get --global maintenance.repo "$(pwd)" &&
+
+ # Operation is idempotent
+ GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
+ test_must_be_empty cron.txt
+'
+
+test_expect_success 'start preserves existing schedule' '
+ echo "Important information!" >cron.txt &&
+ GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
+ grep "Important information!" cron.txt
+'
+
test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ef31f40037..4a60d1ed76 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1702,3 +1702,9 @@ test_lazy_prereq SHA1 '
test_lazy_prereq REBASE_P '
test -z "$GIT_TEST_SKIP_REBASE_P"
'
+
+# Ensure that no test accidentally triggers a Git command
+# that runs 'crontab', affecting a user's cron schedule.
+# Tests that verify the cron integration must set this locally
+# to avoid errors.
+GIT_TEST_CRONTAB="exit 1"
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 6/6] maintenance: recommended schedule in register/start
From: Derrick Stolee via GitGitGadget @ 2020-08-28 15:45 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.680.v3.git.1598629517.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
The 'git maintenance (register|start)' subcommands add the current
repository to the global Git config so maintenance will operate on that
repository. It does not specify what maintenance should occur or how
often.
If a user sets any 'maintenance.<task>.schedule' config value, then
they have chosen a specific schedule for themselves and Git should
respect that.
However, in an effort to recommend a good schedule for repositories of
all sizes, set new config values for recommended tasks that are safe to
run in the background while users run foreground Git commands. These
commands are generally everything but the 'gc' task.
Author's Note: I feel we should do _something_ to recommend a good
schedule to users, but I'm not 100% set on this schedule. This is the
schedule we use in Scalar and VFS for Git for very large repositories
using the GVFS protocol. While the schedule works in that environment,
it is possible that "normal" Git repositories could benefit from
something more obvious (such as running 'gc' weekly). However, this
patch gives us a place to start a conversation on what we should
recommend. For my purposes, Scalar will set these config values so we
can always differ from core Git's recommendations.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-maintenance.txt | 6 ++++
builtin/gc.c | 46 +++++++++++++++++++++++++++++++
t/t7900-maintenance.sh | 16 +++++++++++
3 files changed, 68 insertions(+)
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index 7f8c279fe8..364b3e32bf 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -37,6 +37,12 @@ register::
`maintenance.<task>.schedule`. The tasks that are enabled are safe
for running in the background without disrupting foreground
processes.
++
+If your repository has no 'maintenance.<task>.schedule' configuration
+values set, then Git will set configuration values to some recommended
+settings. These settings disable foreground maintenance while performing
+maintenance tasks in the background that will not interrupt foreground Git
+operations.
run::
Run one or more maintenance tasks. If one or more `--task` options
diff --git a/builtin/gc.c b/builtin/gc.c
index 9914417e25..5f253d3458 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1408,6 +1408,49 @@ static int maintenance_run(int argc, const char **argv, const char *prefix)
return maintenance_run_tasks(&opts);
}
+static int has_schedule_config(void)
+{
+ int i, found = 0;
+ struct strbuf config_name = STRBUF_INIT;
+ size_t prefix;
+
+ strbuf_addstr(&config_name, "maintenance.");
+ prefix = config_name.len;
+
+ for (i = 0; !found && i < TASK__COUNT; i++) {
+ char *value;
+
+ strbuf_setlen(&config_name, prefix);
+ strbuf_addf(&config_name, "%s.schedule", tasks[i].name);
+
+ if (!git_config_get_string(config_name.buf, &value)) {
+ found = 1;
+ FREE_AND_NULL(value);
+ }
+ }
+
+ strbuf_release(&config_name);
+ return found;
+}
+
+static void set_recommended_schedule(void)
+{
+ git_config_set("maintenance.auto", "false");
+ git_config_set("maintenance.gc.enabled", "false");
+
+ git_config_set("maintenance.prefetch.enabled", "true");
+ git_config_set("maintenance.prefetch.schedule", "hourly");
+
+ git_config_set("maintenance.commit-graph.enabled", "true");
+ git_config_set("maintenance.commit-graph.schedule", "hourly");
+
+ git_config_set("maintenance.loose-objects.enabled", "true");
+ git_config_set("maintenance.loose-objects.schedule", "daily");
+
+ git_config_set("maintenance.incremental-repack.enabled", "true");
+ git_config_set("maintenance.incremental-repack.schedule", "daily");
+}
+
static int maintenance_register(void)
{
struct child_process config_set = CHILD_PROCESS_INIT;
@@ -1417,6 +1460,9 @@ static int maintenance_register(void)
if (!the_repository || !the_repository->gitdir)
return 0;
+ if (!has_schedule_config())
+ set_recommended_schedule();
+
config_get.git_cmd = 1;
strvec_pushl(&config_get.args, "config", "--global", "--get", "maintenance.repo",
the_repository->worktree ? the_repository->worktree
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 8803fcf621..5a31f3925b 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -309,7 +309,23 @@ test_expect_success 'register and unregister' '
git config --global --add maintenance.repo /existing1 &&
git config --global --add maintenance.repo /existing2 &&
git config --global --get-all maintenance.repo >before &&
+
+ # We still have maintenance.<task>.schedule config set,
+ # so this does not update the local schedule
+ git maintenance register &&
+ test_must_fail git config maintenance.auto &&
+
+ # Clear previous maintenance.<task>.schedule values
+ for task in loose-objects commit-graph incremental-repack
+ do
+ git config --unset maintenance.$task.schedule || return 1
+ done &&
git maintenance register &&
+ test_cmp_config false maintenance.auto &&
+ test_cmp_config false maintenance.gc.enabled &&
+ test_cmp_config true maintenance.prefetch.enabled &&
+ test_cmp_config hourly maintenance.commit-graph.schedule &&
+ test_cmp_config daily maintenance.incremental-repack.schedule &&
git config --global --get-all maintenance.repo >actual &&
cp before after &&
pwd >>after &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 3/6] for-each-repo: run subcommands on configured repos
From: Derrick Stolee via GitGitGadget @ 2020-08-28 15:45 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.680.v3.git.1598629517.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
It can be helpful to store a list of repositories in global or system
config and then iterate Git commands on that list. Create a new builtin
that makes this process simple for experts. We will use this builtin to
run scheduled maintenance on all configured repositories in a future
change.
The test is very simple, but does highlight that the "--" argument is
optional.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
.gitignore | 1 +
Documentation/git-for-each-repo.txt | 59 +++++++++++++++++++++++++++++
Makefile | 1 +
builtin.h | 1 +
builtin/for-each-repo.c | 58 ++++++++++++++++++++++++++++
command-list.txt | 1 +
git.c | 1 +
t/t0068-for-each-repo.sh | 30 +++++++++++++++
8 files changed, 152 insertions(+)
create mode 100644 Documentation/git-for-each-repo.txt
create mode 100644 builtin/for-each-repo.c
create mode 100755 t/t0068-for-each-repo.sh
diff --git a/.gitignore b/.gitignore
index a5808fa30d..5eb2a2be71 100644
--- a/.gitignore
+++ b/.gitignore
@@ -67,6 +67,7 @@
/git-filter-branch
/git-fmt-merge-msg
/git-for-each-ref
+/git-for-each-repo
/git-format-patch
/git-fsck
/git-fsck-objects
diff --git a/Documentation/git-for-each-repo.txt b/Documentation/git-for-each-repo.txt
new file mode 100644
index 0000000000..94bd19da26
--- /dev/null
+++ b/Documentation/git-for-each-repo.txt
@@ -0,0 +1,59 @@
+git-for-each-repo(1)
+====================
+
+NAME
+----
+git-for-each-repo - Run a Git command on a list of repositories
+
+
+SYNOPSIS
+--------
+[verse]
+'git for-each-repo' --config=<config> [--] <arguments>
+
+
+DESCRIPTION
+-----------
+Run a Git command on a list of repositories. The arguments after the
+known options or `--` indicator are used as the arguments for the Git
+subprocess.
+
+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
+
+For example, we could run maintenance on each of a list of repositories
+stored in a `maintenance.repo` config variable using
+
+-------------
+git for-each-repo --config=maintenance.repo maintenance run
+-------------
+
+This will run `git -C <repo> maintenance run` for each value `<repo>`
+in the multi-valued config variable `maintenance.repo`.
+
+
+OPTIONS
+-------
+--config=<config>::
+ Use the given config variable as a multi-valued list storing
+ absolute path names. Iterate on that list of paths to run
+ the given arguments.
++
+These config values are loaded from system, global, and local Git config,
+as available. If `git for-each-repo` is run in a directory that is not a
+Git repository, then only the system and global config is used.
+
+
+SUBPROCESS BEHAVIOR
+-------------------
+
+If any `git -C <repo> <arguments>` subprocess returns a non-zero exit code,
+then the `git for-each-repo` process returns that exit code without running
+more subprocesses.
+
+Each `git -C <repo> <arguments>` subprocess inherits the standard file
+descriptors `stdin`, `stdout`, and `stderr`.
+
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 65f8cfb236..7c588ff036 100644
--- a/Makefile
+++ b/Makefile
@@ -1071,6 +1071,7 @@ BUILTIN_OBJS += builtin/fetch-pack.o
BUILTIN_OBJS += builtin/fetch.o
BUILTIN_OBJS += builtin/fmt-merge-msg.o
BUILTIN_OBJS += builtin/for-each-ref.o
+BUILTIN_OBJS += builtin/for-each-repo.o
BUILTIN_OBJS += builtin/fsck.o
BUILTIN_OBJS += builtin/gc.o
BUILTIN_OBJS += builtin/get-tar-commit-id.o
diff --git a/builtin.h b/builtin.h
index 17c1c0ce49..ff7c6e5aa9 100644
--- a/builtin.h
+++ b/builtin.h
@@ -150,6 +150,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix);
int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix);
int cmd_for_each_ref(int argc, const char **argv, const char *prefix);
+int cmd_for_each_repo(int argc, const char **argv, const char *prefix);
int cmd_format_patch(int argc, const char **argv, const char *prefix);
int cmd_fsck(int argc, const char **argv, const char *prefix);
int cmd_gc(int argc, const char **argv, const char *prefix);
diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
new file mode 100644
index 0000000000..5bba623ff1
--- /dev/null
+++ b/builtin/for-each-repo.c
@@ -0,0 +1,58 @@
+#include "cache.h"
+#include "config.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "run-command.h"
+#include "string-list.h"
+
+static const char * const for_each_repo_usage[] = {
+ N_("git for-each-repo --config=<config> <command-args>"),
+ NULL
+};
+
+static int run_command_on_repo(const char *path,
+ void *cbdata)
+{
+ int i;
+ struct child_process child = CHILD_PROCESS_INIT;
+ struct strvec *args = (struct strvec *)cbdata;
+
+ child.git_cmd = 1;
+ strvec_pushl(&child.args, "-C", path, NULL);
+
+ for (i = 0; i < args->nr; i++)
+ strvec_push(&child.args, args->v[i]);
+
+ return run_command(&child);
+}
+
+int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
+{
+ static const char *config_key = NULL;
+ int i, result = 0;
+ const struct string_list *values;
+ struct strvec args = STRVEC_INIT;
+
+ const struct option options[] = {
+ OPT_STRING(0, "config", &config_key, N_("config"),
+ N_("config key storing a list of repository paths")),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options, for_each_repo_usage,
+ PARSE_OPT_STOP_AT_NON_OPTION);
+
+ if (!config_key)
+ die(_("missing --config=<config>"));
+
+ for (i = 0; i < argc; i++)
+ strvec_push(&args, argv[i]);
+
+ values = repo_config_get_value_multi(the_repository,
+ config_key);
+
+ for (i = 0; !result && i < values->nr; i++)
+ result = run_command_on_repo(values->items[i].string, &args);
+
+ return result;
+}
diff --git a/command-list.txt b/command-list.txt
index 0e3204e7d1..581499be82 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -94,6 +94,7 @@ git-fetch-pack synchingrepositories
git-filter-branch ancillarymanipulators
git-fmt-merge-msg purehelpers
git-for-each-ref plumbinginterrogators
+git-for-each-repo plumbinginterrogators
git-format-patch mainporcelain
git-fsck ancillaryinterrogators complete
git-gc mainporcelain
diff --git a/git.c b/git.c
index 24f250d29a..1cab64b5d1 100644
--- a/git.c
+++ b/git.c
@@ -511,6 +511,7 @@ static struct cmd_struct commands[] = {
{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
+ { "for-each-repo", cmd_for_each_repo, RUN_SETUP_GENTLY },
{ "format-patch", cmd_format_patch, RUN_SETUP },
{ "fsck", cmd_fsck, RUN_SETUP },
{ "fsck-objects", cmd_fsck, RUN_SETUP },
diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
new file mode 100755
index 0000000000..136b4ec839
--- /dev/null
+++ b/t/t0068-for-each-repo.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+test_description='git for-each-repo builtin'
+
+. ./test-lib.sh
+
+test_expect_success 'run based on configured value' '
+ git init one &&
+ git init two &&
+ git init three &&
+ git -C two commit --allow-empty -m "DID NOT RUN" &&
+ git config run.key "$TRASH_DIRECTORY/one" &&
+ git config --add run.key "$TRASH_DIRECTORY/three" &&
+ git for-each-repo --config=run.key commit --allow-empty -m "ran" &&
+ git -C one log -1 --pretty=format:%s >message &&
+ grep ran message &&
+ git -C two log -1 --pretty=format:%s >message &&
+ ! grep ran message &&
+ git -C three log -1 --pretty=format:%s >message &&
+ grep ran message &&
+ git for-each-repo --config=run.key -- commit --allow-empty -m "ran again" &&
+ git -C one log -1 --pretty=format:%s >message &&
+ grep again message &&
+ git -C two log -1 --pretty=format:%s >message &&
+ ! grep again message &&
+ git -C three log -1 --pretty=format:%s >message &&
+ grep again message
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* ds/maintenance-part-[1-2] (was: What's cooking in git.git (Aug 2020, #07; Thu, 27))
From: Derrick Stolee @ 2020-08-28 15:48 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqh7snpxy1.fsf@gitster.c.googlers.com>
On 8/27/2020 5:43 PM, Junio C Hamano wrote:
> * ds/maintenance-part-2 (2020-08-25) 8 commits
> - maintenance: add incremental-repack auto condition
> - maintenance: auto-size incremental-repack batch
> - maintenance: add incremental-repack task
> - midx: use start_delayed_progress()
> - midx: enable core.multiPackIndex by default
> - maintenance: create auto condition for loose-objects
> - maintenance: add loose-objects task
> - maintenance: add prefetch task
> (this branch uses ds/maintenance-part-1.)
>
> "git maintenance", an extended big brother of "git gc", continues
> to evolve.
> * ds/maintenance-part-1 (2020-08-25) 11 commits
> - maintenance: add trace2 regions for task execution
> - maintenance: add auto condition for commit-graph task
> - maintenance: use pointers to check --auto
> - maintenance: create maintenance.<task>.enabled config
> - maintenance: take a lock on the objects directory
> - maintenance: add --task option
> - maintenance: add commit-graph task
> - maintenance: initialize task array
> - maintenance: replace run_auto_gc()
> - maintenance: add --quiet option
> - maintenance: create basic maintenance runner
> (this branch is used by ds/maintenance-part-2.)
>
> A "git gc"'s big brother has been introduced to take care of more
> repository maintenance tasks, not limited to the object database
> cleaning.
>
> Comments?
Thanks for calling out a call for comments here.
I appreciate all of the comments on these series. It has
really improved the feature.
I'm interested to hear who still has this on their list
to review or if there are some remaining unresolved
issues.
I sent my "RFC v3" [1] for the background portion of
maintenance, and hope to send a non-RFC version next week
if there are no more radical shifts in the strategy. Please
take a look if you are interested.
[1] https://lore.kernel.org/git/pull.680.v3.git.1598629517.gitgitgadget@gmail.com/
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH] po: add missing letter for French message
From: Jean-Noël AVILA @ 2020-08-28 15:53 UTC (permalink / raw)
To: brian m. carlson, Junio C Hamano; +Cc: git
In-Reply-To: <xmqq8sdzpu71.fsf@gitster.c.googlers.com>
On Friday, 28 August 2020 01:04:02 CEST Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > Add the missing "e" in "de". While it is possible in French to omit it,
> > that only occurs with an apostrophe and only when the next word starts
> > with a vowel or mute h, which is not the case here.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> > I noticed this the other day when trying to delete a remote branch that
> > I'd already deleted. I'm not sure what the preferred approach is for
> > this, whether Junio should pick it up or whether Jean-Noël will want to
> > incorporate it first, but I've CC'd both so y'all can fight it out.
>
> Unless it is in the pre-release period (in which case I'd prefer not
> to touch po/ myself at all, to give i18n/l10n teams a stable base to
> work from), I can take it dircetly as long as somebody from the l10n
> team for the language gives an Ack, as I cannot read most of the
> files under po/ directory.
>
> Thanks.
>
Acked-by: Jean-Noël Avila <jn.avila@free.fr>
>
> > po/fr.po | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/po/fr.po b/po/fr.po
> > index d20fc440ab..75b1e75f6a 100644
> > --- a/po/fr.po
> > +++ b/po/fr.po
> > @@ -6503,7 +6503,7 @@ msgstr "'%s' ne peut pas être résolue comme une branche"
> > #: remote.c:1088
> > #, c-format
> > msgid "unable to delete '%s': remote ref does not exist"
> > -msgstr "suppression d '%s' impossible : la référence distante n'existe pas"
> > +msgstr "suppression de '%s' impossible : la référence distante n'existe pas"
> >
> > #: remote.c:1100
> > #, c-format
^ permalink raw reply
* Re: [PATCH 3/5] worktree: teach "repair" to fix outgoing links to worktrees
From: Eric Sunshine @ 2020-08-28 16:27 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git List, Henré Botha, Jeff King
In-Reply-To: <nycvar.QRO.7.76.6.2008280413450.56@tvgsbejvaqbjf.bet>
On Fri, Aug 28, 2020 at 8:55 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Thu, 27 Aug 2020, Eric Sunshine wrote:
> > + struct strbuf main = STRBUF_INIT;
>
> This is needed to shut up GCC's "‘main’ is usually a function
> [-Werror=main]" error.
D'oh, thanks. I got hit by this just a couple months ago[1] and (for
some reason) was even looking at [1] the same day I submitted this
patch. Will fix it in the re-roll.
[1]: https://lore.kernel.org/git/20200610063049.74666-1-sunshine@sunshineco.com/
^ permalink raw reply
* Re: [PATCH 4/5] strmap: add strdup_strings option
From: Elijah Newren @ 2020-08-28 17:20 UTC (permalink / raw)
To: Jeff King; +Cc: Elijah Newren via GitGitGadget, Git Mailing List
In-Reply-To: <20200828070802.GC2105050@coredump.intra.peff.net>
On Fri, Aug 28, 2020 at 12:08 AM Jeff King <peff@peff.net> wrote:
>
> On Fri, Aug 21, 2020 at 03:25:44PM -0700, Elijah Newren wrote:
>
> > > - That sounds like a lot of maps. :) I guess you've looked at
> > > compacting some of them into a single map-to-struct?
> >
> > Oh, map-to-struct is the primary use. But compacting them won't work,
> > because the reason for the additional maps is that they have different
> > sets of keys (this set of paths meet a certain condition...). Only
> > one map contains all the paths involved in the merge.
>
> OK, I guess I'm not surprised that you would not have missed such an
> obvious optimization. :)
>
> > Also, several of those maps don't even store a value; and are really
> > just a set implemented via strmap (thus meaning the only bit of data I
> > need for some conditions is whether any given path meets it). It
> > seems slightly ugly to have to call strmap_put(map, string, NULL) for
> > those. I wonder if I should have another strset type much like your
> > suggesting for strintmap. Hmm...
>
> FWIW, khash does have a "set" mode where it avoids allocating the value
> array at all.
Cool.
> What's the easiest way to benchmark merge-ort?
Note that I discovered another optimization that I'm working on
implementing; when finished, it should cut down a little more on the
time spent on inexact rename detection. That should have the side
effect of having the time spent on strmaps stick out some more in the
overall timings (as a percentage of overall time anyway). So, I'm
focused on that before I do other benchmarking work (which is part of
the reason I mentioned my strmap/hashmap benchmarking last week might
take a while).
Anyway, on to your question:
=== If you just want to be able to run the ort merge algorithm ===
Clone git@github.com:newren/git and checkout the 'ort' branch and
build it. It currently changes the default merge algorithm to 'ort'
and even ignores '-s recursive' by remapping it to '-s ort' (because I
wanted to see how regression tests fared with ort as a replacement for
recrusive). It should pass the regression tests if you want to run
those first. But note that if you want to compare 'ort' to
'recursive', then currently you need to have two different git builds,
one of my branch and one with a different checkout of something else
(e.g. 2.28.0 or 'master' or whatever).
=== Decide the granularity of your timing ===
I suspect you know more than me here, but maybe my pointers are useful anyway...
Decide if you want to measure overall program runtime, or dive into
details. I used both a simple 'time' and the better 'hyperfine' for
the former, and used both 'perf' and GIT_TRACE2_PERF for the latter.
One nice thing about GIT_TRACE2_PERF was I wrote a simple program to
aggregate the times per region and provide percentages, in a script at
the toplevel named 'summarize-perf' that I can use to prefix commands.
Thus, I could for example run from my linux clone:
$ ../git/summarize-perf git fast-rebase --onto HEAD base hwmon-updates
and I'd get output that looks something like (note that this is a
subset of the real output):
1.400 : 35 : label:inmemory_nonrecursive
0.827 : 41 : ..label:renames
0.019 : <unmeasured> ( 2.2%)
0.803 : 37 : ....label:regular renames
0.004 : 31 : ....label:directory renames
0.001 : 31 : ....label:process renames
0.513 : 41 : ..label:collect_merge_info
0.048 : 35 : ..label:process_entries
0.117 : 1 : label:checkout
0.000 : 1 : label:record_unmerged
and where those fields are <time> : <count> : <region label>.
=== If you want to time the testcases I used heavily while developing ===
The rebase-testcase/redo-timings script (in the ort branch) has
details on what I actually ran, though it has some paranoia around
attempting to make my laptop run semi-quietly and try to avoid all the
variance that I wished I could control a bit better. And it assumes
you are running in a linux clone with a few branches set up a certain
way. Let me explain those tests without using that script, as simply
as I can:
The setup for the particular cases I was testing is as follows:
* Clone the linux kernel, and run the following:
$ git branch hwmon-updates fd8bdb23b91876ac1e624337bb88dc1dcc21d67e
$ git branch hwmon-just-one fd8bdb23b91876ac1e624337bb88dc1dcc21d67e~34
$ git branch base 4703d9119972bf586d2cca76ec6438f819ffa30e
$ git switch -c 5.4-renames v5.4
$ git mv drivers pilots
$ git commit -m "Rename drivers/ to pilots/"
And from there, there were three primary tests I was comparing:
* Rename testcase, 35 patches:
$ git checkout 5.4-renames^0
$ git fast-rebase --onto HEAD base hwmon-updates
* Rename testcase, just 1 patch:
$ git switch 5.4-renames^0
$ git fast-rebase --onto HEAD base hwmon-just-one
* No renames (or at least very few renames) testcase, 35 patches:
$ git checkout v5.4^0
$ git branch -f hwmon-updates
fd8bdb23b91876ac1e624337bb88dc1dcc21d67e # Need to reset
hwmon-updates, due to fast-rebase done above
$ git fast-rebase --onto HEAD base hwmon-updates
(If you want to compare with 'recursive' from a different build of
git, just replace 'fast-rebase' with 'rebase'. You can also use
'rebase' instead of 'fast-rebase' on the ort branch and it'll use the
ort merge algorithm, but you get all the annoying
working-tree-updates-while-rebasing rather than just having the
working tree updated at the end of the rebase. You also get all the
annoying forks of 'git checkout' and 'git commit' that sequencer is
guilty of spawning. But it certainly supports a lot more options and
can save state to allow resuming after conflicts, unlike
'fast-rebase'.)
> I suspect I could swap out hashmap for khash (messily) in an hour or less.
Well, you might be assuming I used sane strmaps, with each strmap
having a fixed type for the stored value. That's mostly true, but
there were two counterexamples I can think of: "paths" (the biggest
strmap) is a map of string -> {merged_info OR conflict_info}, because
merged_info is a smaller subset of conflict_info and saves space for
each path that can be trivially merged. Also, in diffcore-rename,
"dir_rename" starts life as a map of string -> strmap, but later
transitions to string -> string, because I'm evil and didn't set up a
temporary strmap like I probably should have.
Also, the code is littered with FIXME comments, unnecessary #ifdefs,
and is generally in need of lots of cleanup. Sorry.
^ permalink raw reply
* [PATCH] midx: traverse the local MIDX first
From: Taylor Blau @ 2020-08-28 18:06 UTC (permalink / raw)
To: git; +Cc: dstolee, gitster, peff
When a repository has an alternate object directory configured, callers
can traverse through each alternate's MIDX by walking the '->next'
pointer.
But, when 'prepare_multi_pack_index_one()' loads multiple MIDXs, it
places the new ones at the front of this pointer chain, not at the end.
This can be confusing for callers such as 'git repack -ad', causing test
failures like in t7700.6 with 'GIT_TEST_MULTI_PACK_INDEX=1'.
The occurs when dropping a pack known to the local MIDX with alternates
configured that have their own MIDX. Since the alternate's MIDX is
returned via 'get_multi_pack_index()', 'midx_contains_pack()' returns
true (which is correct, since it traverses through the '->next' pointer
to find the MIDX in the chain that does contain the requested object).
But, we call 'clear_midx_file()' on 'the_repository', which drops the
MIDX at the path of the first MIDX in the chain, which (in the case of
t7700.6 is the one in the alternate).
This patch bandaids that situation by trying to place the local MIDX
first in the chain when calling 'prepare_multi_pack_index_one()'.
Specifically, it always inserts all MIDXs loads subsequent to the local
one as the head of the tail of the MIDX chain. This makes it so that we
don't have a quadratic insertion while still keeping the local MIDX at
the front of the list. Likewise, it avoids an 'O(m*n)' lookup in
'remove_redundant_pack()' where 'm' is the number of redundant packs and
'n' is the number of alternates.
Protect 'remove_redundant_pack()' against a case where alternates with
MIDXs exist, but no local MIDX exists by first checking that 'm->local'
before asking whether or not it contains the requested pack.
This invariant is only preserved by the insertion order in
'prepare_packed_git()', which traverses through the ODB's '->next'
pointer, meaning we visit the local object store first. This fragility
makes this an undesirable long-term solution, but it is acceptable for
now since this is the only caller.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
This is kind of a hack, but the order that we call
'prepare_multi_pack_index_one()' from 'prepare_packed_git()' makes it
acceptable, at least in my own assessment.
It is fixing a breakage in 'next', so I'd be inclined to merge this up.
But, if this is too gross, I'd just as soon revert what's in 'next' and
try again later.
builtin/repack.c | 2 +-
midx.c | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 98fac03946..5661d69c16 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -135,7 +135,7 @@ static void remove_redundant_pack(const char *dir_name, const char *base_name)
struct strbuf buf = STRBUF_INIT;
struct multi_pack_index *m = get_multi_pack_index(the_repository);
strbuf_addf(&buf, "%s.pack", base_name);
- if (m && midx_contains_pack(m, buf.buf))
+ if (m && m->local && midx_contains_pack(m, buf.buf))
clear_midx_file(the_repository);
strbuf_insertf(&buf, 0, "%s/", dir_name);
unlink_pack_path(buf.buf, 1);
diff --git a/midx.c b/midx.c
index e9b2e1253a..cc19b66152 100644
--- a/midx.c
+++ b/midx.c
@@ -416,8 +416,12 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i
m = load_multi_pack_index(object_dir, local);
if (m) {
- m->next = r->objects->multi_pack_index;
- r->objects->multi_pack_index = m;
+ struct multi_pack_index *mp = r->objects->multi_pack_index;
+ if (mp) {
+ m->next = mp->next;
+ mp->next = m;
+ } else
+ r->objects->multi_pack_index = m;
return 1;
}
--
2.28.0.338.g87a3b7a5a2.dirty
^ permalink raw reply related
* Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)
From: Taylor Blau @ 2020-08-28 18:09 UTC (permalink / raw)
To: Jeff King; +Cc: Taylor Blau, Derrick Stolee, Junio C Hamano, git
In-Reply-To: <20200828083125.GA2139751@coredump.intra.peff.net>
On Fri, Aug 28, 2020 at 04:31:25AM -0400, Jeff King wrote:
> On Fri, Aug 28, 2020 at 02:26:19AM -0400, Jeff King wrote:
>
> > On Thu, Aug 27, 2020 at 08:39:40PM -0400, Taylor Blau wrote:
> >
> > If I'm understanding midx_contains_pack() correctly, then the code
> > looking for ".pack" could never have matched, and we would never have
> > deleted a midx here. Which makes me wonder why the "repack removes
> > multi-pack-index when deleting packs" test ever succeeded.
>
> Sorry, this is all nonsense.
>
> I forgot about the hackery added in 013fd7ada3 (midx: check both pack
> and index names for containment, 2019-04-05). And anyway, the patch
> above is totally bogus because we need the ".pack" form in the buffer
> when we call unlink_pack_path().
Yeah, 013fd7ada3 is definitely a hack, but what you have is definitely
incorrect.
> So there's definitely something more odd going on in that failing test.
It's the funky order that we load the MIDX chain in (which is that the
MIDX belonging to the deepest alternate shows up *first* as
'r->objects->multi_pack_index', and that the local MIDX shows up as the
last thing when traversing '->next').
I sent what I consider to be a little bit of a hack in [1], but it's
definitely enough to fix t7700.6.
> -Peff
Thanks for reporting it.
Taylor
[1]: https://lore.kernel.org/git/20200828180621.GA9036@nand.nand.local/T/#u
^ permalink raw reply
* Re: [PATCH] midx: traverse the local MIDX first
From: Derrick Stolee @ 2020-08-28 18:27 UTC (permalink / raw)
To: Taylor Blau, git; +Cc: dstolee, gitster, peff
In-Reply-To: <20200828180621.GA9036@nand.nand.local>
On 8/28/2020 2:06 PM, Taylor Blau wrote:
> This invariant is only preserved by the insertion order in
> 'prepare_packed_git()', which traverses through the ODB's '->next'
> pointer, meaning we visit the local object store first. This fragility
> makes this an undesirable long-term solution, but it is acceptable for
> now since this is the only caller.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
> This is kind of a hack, but the order that we call
> 'prepare_multi_pack_index_one()' from 'prepare_packed_git()' makes it
> acceptable, at least in my own assessment.
The natural alternative would be to scan the list _after_ all are
inserted and pull any MIDX marked "local" to the front of the list.
Such a check would need to happen in the same method that iterates
over all alternates, so that seems a bit redundant.
While perhaps a bit hack-ish, I think this is a sound approach.
And, we have a test that will detect change in behavior here!
Thanks,
-Stolee
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox