* Re: Mismatched HEAD default behavior from git log
From: Junio C Hamano @ 2020-08-25 19:51 UTC (permalink / raw)
To: Jeff King; +Cc: Bryan Turner, Git Users
In-Reply-To: <20200825194619.GB1419759@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> As the intent for adding the "--stdin" option to any subcommand has
>> always been "we may need to feed many many things, that may bust the
>> command line length limit, hence we let you feed these things from
>> the standard input, but otherwise there should be no change in
>> behaviour or semantics", when the behaviour of command line and
>> "--stdin" differ, it is a bug in the latter.
>
> Agreed. It also helps in this case that the command-line behavior is
> sensible and the --stdin one is not. :)
>
> I think the solution is probably something like:
You beat me to it while I was wondering what to do between the local
got_rev_arg variable and the revs->rev_input_given field.
> diff --git a/revision.c b/revision.c
> index 96630e3186..f5bbefa091 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2099,12 +2099,13 @@ static void read_pathspec_from_stdin(struct strbuf *sb,
> strvec_push(prune, sb->buf);
> }
>
> -static void read_revisions_from_stdin(struct rev_info *revs,
> - struct strvec *prune)
> +static int read_revisions_from_stdin(struct rev_info *revs,
> + struct strvec *prune)
> {
> struct strbuf sb;
> int seen_dashdash = 0;
> int save_warning;
> + int got_rev_arg = 0;
>
> save_warning = warn_on_object_refname_ambiguity;
> warn_on_object_refname_ambiguity = 0;
> @@ -2124,12 +2125,14 @@ static void read_revisions_from_stdin(struct rev_info *revs,
> if (handle_revision_arg(sb.buf, revs, 0,
> REVARG_CANNOT_BE_FILENAME))
> die("bad revision '%s'", sb.buf);
> + got_rev_arg = 1;
> }
> if (seen_dashdash)
> read_pathspec_from_stdin(&sb, prune);
>
> strbuf_release(&sb);
> warn_on_object_refname_ambiguity = save_warning;
> + return got_rev_arg;
> }
>
> static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
> @@ -2754,7 +2757,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
> }
> if (revs->read_from_stdin++)
> die("--stdin given twice?");
> - read_revisions_from_stdin(revs, &prune_data);
> + if (read_revisions_from_stdin(revs, &prune_data))
> + got_rev_arg = 1;
> continue;
> }
>
>
> Possibly it would make sense to push that flag into rev_info, though,
> and let handle_revision_arg() set it. That would fix this bug and
> prevent similar ones in other code paths (though we're not likely to get
> revisions from anywhere else, I suppose).
>
> -Peff
^ permalink raw reply
* Re: git clone --shallow-since can result in inconsistent shallow clones
From: Jeff King @ 2020-08-25 19:51 UTC (permalink / raw)
To: Nelson Elhage; +Cc: git
In-Reply-To: <CAPSG9dZV2EPpVKkOMcjv5z+NF7rUu=V-ZkZNx47rCv122HsiKg@mail.gmail.com>
On Tue, Aug 25, 2020 at 11:38:04AM -0700, Nelson Elhage wrote:
> It's unfortunately a bit unclear to me what _should_ happen here. We
> really want a way to mark `89ea0c5` as "partially-shallow", and send
> its second parent, but not its first parent, but shallowness is a
> property of an entire commit, not of a specific commit/parent
> relationship. However, it'd be nice if we at least ended up with a
> consistent state, instead of with a repository with invalid `shallow`
> marks.
I think this is the same issue I reported recently here:
https://lore.kernel.org/git/20200721160643.GA3288097@coredump.intra.peff.net/
AFAICT the shallow feature is just defective and can't accurately
represent this situation. Unfortunately nobody seemed to have any bright
ideas, and the developer who implemented most of the shallow features
(including shallow-since) is no longer active.
So I suspect it is fixable, but probably requires somebody to get pretty
familiar with the shallow code, and propose a fix that involves both
code and a protocol change.
-Peff
^ permalink raw reply
* Re: Mismatched HEAD default behavior from git log
From: Jeff King @ 2020-08-25 19:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Bryan Turner, Git Users
In-Reply-To: <xmqq8se21pl1.fsf@gitster.c.googlers.com>
On Tue, Aug 25, 2020 at 12:40:42PM -0700, Junio C Hamano wrote:
> Bryan Turner <bturner@atlassian.com> writes:
>
> > It appears the way --stdin processes input discards nonexistent
> > commits before the machinery that decides whether you provided any
> > revs or not runs, and so if every --stdin rev is discarded then you
> > get the default HEAD. If you provide them via the command line,
> > though, then it seems like they're discarded later and you don't get a
> > default.
> >
> > I'm not sure whether this is intentional or not (certainly I don't see
> > it anywhere in the git log documentation for --ignore-missing or
> > --stdin), but it results in a behavior mismatch that's impossible to
> > reconcile without requiring extra git processes. I can't always
> > provide HEAD since, if multiple revs are supplied, if any revs exist
> > then HEAD would not be included regardless of whether the revs were
> > supplied via the command line or --stdin.
>
> As the intent for adding the "--stdin" option to any subcommand has
> always been "we may need to feed many many things, that may bust the
> command line length limit, hence we let you feed these things from
> the standard input, but otherwise there should be no change in
> behaviour or semantics", when the behaviour of command line and
> "--stdin" differ, it is a bug in the latter.
Agreed. It also helps in this case that the command-line behavior is
sensible and the --stdin one is not. :)
I think the solution is probably something like:
diff --git a/revision.c b/revision.c
index 96630e3186..f5bbefa091 100644
--- a/revision.c
+++ b/revision.c
@@ -2099,12 +2099,13 @@ static void read_pathspec_from_stdin(struct strbuf *sb,
strvec_push(prune, sb->buf);
}
-static void read_revisions_from_stdin(struct rev_info *revs,
- struct strvec *prune)
+static int read_revisions_from_stdin(struct rev_info *revs,
+ struct strvec *prune)
{
struct strbuf sb;
int seen_dashdash = 0;
int save_warning;
+ int got_rev_arg = 0;
save_warning = warn_on_object_refname_ambiguity;
warn_on_object_refname_ambiguity = 0;
@@ -2124,12 +2125,14 @@ static void read_revisions_from_stdin(struct rev_info *revs,
if (handle_revision_arg(sb.buf, revs, 0,
REVARG_CANNOT_BE_FILENAME))
die("bad revision '%s'", sb.buf);
+ got_rev_arg = 1;
}
if (seen_dashdash)
read_pathspec_from_stdin(&sb, prune);
strbuf_release(&sb);
warn_on_object_refname_ambiguity = save_warning;
+ return got_rev_arg;
}
static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
@@ -2754,7 +2757,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
}
if (revs->read_from_stdin++)
die("--stdin given twice?");
- read_revisions_from_stdin(revs, &prune_data);
+ if (read_revisions_from_stdin(revs, &prune_data))
+ got_rev_arg = 1;
continue;
}
Possibly it would make sense to push that flag into rev_info, though,
and let handle_revision_arg() set it. That would fix this bug and
prevent similar ones in other code paths (though we're not likely to get
revisions from anywhere else, I suppose).
-Peff
^ permalink raw reply related
* Re: What's cooking in git.git (Aug 2020, #06; Mon, 24)
From: Emily Shaffer @ 2020-08-25 19:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq7dtn3785.fsf@gitster.c.googlers.com>
> * es/config-hooks (2020-07-30) 6 commits
> - hook: add 'run' subcommand
> - parse-options: parse into argv_array
> - hook: add --porcelain to list command
> - hook: add list command
> - hook: scaffolding for git-hook subcommand
> - doc: propose hooks managed by the config
>
> The "hooks defined in config" topic.
>
> Expecting a reroll.
> Now jk/strvec is in 'master', we may want to see the topic reworked
> on top of it. Are there unresolved issues, or does the topic need
> a round of detailed review?
I have it reworked locally to use strvec instead. I see in
https://lore.kernel.org/git/xmqqsgd8606c.fsf@gitster.c.googlers.com that
you were waiting for another jk/strvec reroll and it looks like that's
happened now, so I'll rebase this series against 'master' once more.
I'm ready for the first four (five?) of the series to receive a detailed
review, but I think it's premature for the 6th to be considered before
I've seen what the conversion process looks like for existing hook
callers. That's my main focus right now, although it tends to be bumped
by concerns for the summit next month as well as pesky life events.
- Emily
^ permalink raw reply
* Re: Mismatched HEAD default behavior from git log
From: Junio C Hamano @ 2020-08-25 19:40 UTC (permalink / raw)
To: Bryan Turner; +Cc: Git Users
In-Reply-To: <CAGyf7-G_ciVpgvvOiH1Fq9kNuWunCpM1fhv3ao_RMXBB0K=HMA@mail.gmail.com>
Bryan Turner <bturner@atlassian.com> writes:
> It appears the way --stdin processes input discards nonexistent
> commits before the machinery that decides whether you provided any
> revs or not runs, and so if every --stdin rev is discarded then you
> get the default HEAD. If you provide them via the command line,
> though, then it seems like they're discarded later and you don't get a
> default.
>
> I'm not sure whether this is intentional or not (certainly I don't see
> it anywhere in the git log documentation for --ignore-missing or
> --stdin), but it results in a behavior mismatch that's impossible to
> reconcile without requiring extra git processes. I can't always
> provide HEAD since, if multiple revs are supplied, if any revs exist
> then HEAD would not be included regardless of whether the revs were
> supplied via the command line or --stdin.
As the intent for adding the "--stdin" option to any subcommand has
always been "we may need to feed many many things, that may bust the
command line length limit, hence we let you feed these things from
the standard input, but otherwise there should be no change in
behaviour or semantics", when the behaviour of command line and
"--stdin" differ, it is a bug in the latter.
^ permalink raw reply
* Mismatched HEAD default behavior from git log
From: Bryan Turner @ 2020-08-25 19:16 UTC (permalink / raw)
To: Git Users
When git log is run without revs, it defaults to HEAD. That's
well-documented. However, when paired with --ignore-missing, there's a
behavior mismatch if all the provided revs are nonexistent.
If you provide the revs on the command line, like "git log
--ignore-missing nonexistent --" (the trailing -- is just to help git
log know "nonexistent" isn't intended to be a path), you get no output
and no error (exit code is 0).
If you provide the revs via stdin, like "echo 'nonexistent' | git log
--ignore-missing --stdin --", you get every commit reachable from
HEAD.
It appears the way --stdin processes input discards nonexistent
commits before the machinery that decides whether you provided any
revs or not runs, and so if every --stdin rev is discarded then you
get the default HEAD. If you provide them via the command line,
though, then it seems like they're discarded later and you don't get a
default.
I'm not sure whether this is intentional or not (certainly I don't see
it anywhere in the git log documentation for --ignore-missing or
--stdin), but it results in a behavior mismatch that's impossible to
reconcile without requiring extra git processes. I can't always
provide HEAD since, if multiple revs are supplied, if any revs exist
then HEAD would not be included regardless of whether the revs were
supplied via the command line or --stdin.
I've confirmed this behavior as far back as Git 1.8.0 all the way up
to 2.28.0, so it's by no means new. I can't recall seeing it come up
on the list before, but that doesn't mean it hasn't. Is this a bug, or
a feature?
Best regards,
Bryan Turner
^ permalink raw reply
* Re: [PATCH] git-gui: Basic dark mode support
From: Matthias Aßhauer @ 2020-08-25 19:01 UTC (permalink / raw)
To: serg.partizan; +Cc: git, Pratyush Yadav
In-Reply-To: <20200824154835.160749-1-serg.partizan@gmail.com>
>One problem that i can't yet fix: gray background for files
inchangelists. Any advice on this?
If I understand the issue and the tcl/tk docs correctly, you can change
that color using -highlightcolor on the ttext widget.
I've cc'ed in Pratyush, the current git-gui maintainer. He's probably
best suited to review this patch.
Best regards
Matthias
^ permalink raw reply
* [PATCH v2 5/7] maintenance: add [un]register subcommands
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:40 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.680.v2.git.1598380805.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 2bc02c65e4..c42a176a95 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 5726a9a3b3..5218d52cb7 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1414,7 +1414,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)
{
@@ -1423,6 +1472,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 3e0c5f1ca8..b20ee2d542 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
'
@@ -294,4 +294,19 @@ test_expect_success '--scheduled with specific time' '
test_cmp_config 1595000100 maintenance.commit-graph.lastrun
'
+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 v2 7/7] maintenance: recommended schedule in register/start
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:40 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.680.v2.git.1598380805.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>.scheduled' 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' once a day). 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 | 44 +++++++++++++++++++++++++++++++
t/t7900-maintenance.sh | 5 ++++
3 files changed, 55 insertions(+)
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index d0316db5ae..bba76f0b0d 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 d97af4e546..037402e47f 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1415,6 +1415,47 @@ 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++) {
+ int value;
+
+ strbuf_setlen(&config_name, prefix);
+ strbuf_addf(&config_name, "%s.schedule", tasks[i].name);
+
+ if (!git_config_get_int(config_name.buf, &value))
+ found = 1;
+ }
+
+ 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", "3500");
+
+ git_config_set("maintenance.commit-graph.enabled", "true");
+ git_config_set("maintenance.commit-graph.schedule", "3500");
+
+ git_config_set("maintenance.loose-objects.enabled", "true");
+ git_config_set("maintenance.loose-objects.schedule", "86000");
+
+ git_config_set("maintenance.incremental-repack.enabled", "true");
+ git_config_set("maintenance.incremental-repack.schedule", "86000");
+}
+
static int maintenance_register(void)
{
struct child_process config_set = CHILD_PROCESS_INIT;
@@ -1424,6 +1465,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 6491031be8..7417e5858a 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -300,6 +300,11 @@ test_expect_success 'register and unregister' '
git config --global --add maintenance.repo /existing2 &&
git config --global --get-all maintenance.repo >before &&
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 &&
git config --global --get-all maintenance.repo >actual &&
cp before after &&
pwd >>after &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 6/7] maintenance: add start/stop subcommands
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:40 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.680.v2.git.1598380805.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.
For now, the background maintenance is scheduled to run hourly via the
following cron table row (ignore line breaks):
0 * * * * $p/git --exec-path=$p
for-each-repo --config=maintenance.repo
maintenance run --scheduled
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.
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
environment without installing first.
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 | 117 ++++++++++++++++++++++++++++++
t/helper/test-crontab.c | 35 +++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t7900-maintenance.sh | 30 ++++++++
t/test-lib.sh | 6 ++
8 files changed, 202 insertions(+)
create mode 100644 t/helper/test-crontab.c
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index c42a176a95..d0316db5ae 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 5218d52cb7..d97af4e546 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"
@@ -1463,6 +1464,118 @@ 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) {
+ 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, "# 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);
+
+ 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)
@@ -1472,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 b20ee2d542..6491031be8 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -309,4 +309,34 @@ 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 --scheduled" 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)" &&
+
+ # 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_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 v2 4/7] for-each-repo: run subcommands on configured repos
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:40 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.680.v2.git.1598380805.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
* [PATCH v2 3/7] maintenance: add --scheduled option and config
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:40 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.680.v2.git.1598380805.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 --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.
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.
A user could then set up an hourly maintenance run with the following
cron table:
0 * * * * git -C <repo> maintenance run --scheduled
Then, the user could configure the repository with the following config
values:
maintenance.prefetch.schedule 3000
maintenance.gc.schedule 86000
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.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/config/maintenance.txt | 9 +++++
Documentation/git-maintenance.txt | 13 ++++++-
builtin/gc.c | 54 ++++++++++++++++++++++++++--
t/t7900-maintenance.sh | 20 +++++++++++
4 files changed, 92 insertions(+), 4 deletions(-)
diff --git a/Documentation/config/maintenance.txt b/Documentation/config/maintenance.txt
index 8dd34169da..caacacd322 100644
--- a/Documentation/config/maintenance.txt
+++ b/Documentation/config/maintenance.txt
@@ -15,6 +15,15 @@ maintenance.<task>.lastRun::
`<task>` is run. It stores a timestamp representing the most-recent
run of the `<task>`.
+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.
+
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..2bc02c65e4 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 `--scheduled` option.
+
+--scheduled::
+ 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 fb6f231a5c..5726a9a3b3 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -705,12 +705,13 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
}
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]"),
NULL
};
struct maintenance_run_opts {
int auto_flag;
+ int scheduled;
int quiet;
};
@@ -1157,7 +1158,8 @@ struct maintenance_task {
const char *name;
maintenance_task_fn *fn;
maintenance_auto_fn *auto_condition;
- unsigned enabled:1;
+ unsigned enabled:1,
+ scheduled:1;
/* -1 if not selected. */
int selected_order;
@@ -1268,6 +1270,9 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts)
!tasks[i].auto_condition()))
continue;
+ if (opts->scheduled && !tasks[i].scheduled)
+ continue;
+
update_last_run(&tasks[i]);
trace2_region_enter("maintenance", tasks[i].name, r);
@@ -1282,6 +1287,29 @@ 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;
@@ -1290,13 +1318,28 @@ 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)) {
+ timestamp_t schedule_delay = git_config_int64(
+ config_name.buf,
+ config_str);
+ fill_schedule_info(&tasks[i],
+ config_name.buf,
+ schedule_delay);
+ free(config_str);
+ }
}
strbuf_release(&config_name);
@@ -1340,6 +1383,8 @@ 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_BOOL(0, "scheduled", &opts.scheduled,
+ N_("run tasks based on time intervals")),
OPT_BOOL(0, "quiet", &opts.quiet,
N_("do not report progress or other information over stderr")),
OPT_CALLBACK_F(0, "task", NULL, N_("task"),
@@ -1360,6 +1405,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.scheduled > 1)
+ die(_("use at most one of the --auto and --scheduled options"));
+
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 a985ce3674..3e0c5f1ca8 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -264,6 +264,11 @@ 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_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" \
@@ -274,4 +279,19 @@ 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_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 &&
+ test_subcommand git commit-graph write --split --reachable \
+ --no-progress <long-enough.txt &&
+ test_cmp_config 1595000100 maintenance.commit-graph.lastrun
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/7] maintenance: store the "last run" time in config
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:39 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.680.v2.git.1598380805.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
Users may want to run certain maintenance tasks only so often. Update
the local config with a new 'maintenance.<task>.lastRun' config option
that stores the timestamp just before running the maintenance task.
I selected the timestamp before the task, as opposed to after the task,
for a couple reasons:
1. The time the task takes to execute should not contribute to the
interval between running the tasks. If a daily task takes 10 minutes
to run, then every day the execution will drift by at least 10
minutes.
2. If the task fails for some unforseen reason, it would be good to
indicate that we _attempted_ the task at a certain timestamp. This
will avoid spamming a repository that is in a bad state.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/config/maintenance.txt | 5 +++++
builtin/gc.c | 16 ++++++++++++++++
t/t7900-maintenance.sh | 10 ++++++++++
3 files changed, 31 insertions(+)
diff --git a/Documentation/config/maintenance.txt b/Documentation/config/maintenance.txt
index 06db758172..8dd34169da 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>.lastRun::
+ This config value is automatically updated by Git when the task
+ `<task>` is run. It stores a timestamp representing the most-recent
+ run of the `<task>`.
+
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/builtin/gc.c b/builtin/gc.c
index f8459df04c..fb6f231a5c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1212,6 +1212,20 @@ static int compare_tasks_by_selection(const void *a_, const void *b_)
return b->selected_order - a->selected_order;
}
+static void update_last_run(struct maintenance_task *task)
+{
+ timestamp_t now = approxidate("now");
+ struct strbuf config = STRBUF_INIT;
+ struct strbuf value = STRBUF_INIT;
+ strbuf_addf(&config, "maintenance.%s.lastrun", task->name);
+ strbuf_addf(&value, "%"PRItime"", now);
+
+ git_config_set(config.buf, value.buf);
+
+ strbuf_release(&config);
+ strbuf_release(&value);
+}
+
static int maintenance_run_tasks(struct maintenance_run_opts *opts)
{
int i, found_selected = 0;
@@ -1254,6 +1268,8 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts)
!tasks[i].auto_condition()))
continue;
+ update_last_run(&tasks[i]);
+
trace2_region_enter("maintenance", tasks[i].name, r);
if (tasks[i].fn(opts)) {
error(_("task '%s' failed"), tasks[i].name);
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index e0ba19e1ff..a985ce3674 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -264,4 +264,14 @@ test_expect_success 'maintenance.incremental-repack.auto' '
done
'
+test_expect_success 'tasks update maintenance.<task>.lastRun' '
+ git config --unset maintenance.commit-graph.lastrun &&
+ GIT_TRACE2_EVENT="$(pwd)/run.txt" \
+ GIT_TEST_DATE_NOW=1595000000 \
+ git maintenance run --task=commit-graph 2>/dev/null &&
+ test_subcommand git commit-graph write --split --reachable \
+ --no-progress <run.txt &&
+ test_cmp_config 1595000000 maintenance.commit-graph.lastrun
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 1/7] maintenance: optionally skip --auto process
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:39 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.680.v2.git.1598380805.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 v2 0/7] [RFC] Maintenance III: background maintenance
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:39 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee
In-Reply-To: <pull.680.git.1597857408.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 --scheduled option that
triggers enabled tasks only if they have not been run in some amount of
time. The timestamp of the previous run is stored in the
maintenance.<task>.lastRun config value while the interval is stored in the
maintenance.<task>.schedule config value.
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.
It could be helpful for contributors to suggest ways to configure certain
jobs to run "nightly" or "overnight on a weekend" instead of just "whenever
the last run was long enough ago." One way to do this would be to set the
lastRun config to be at the time of day that the job should run. Another
option would be to make the cron table more complicated with multiple rows,
but I would prefer to avoid that option if there is a simpler mechanism.
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 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
Derrick Stolee (7):
maintenance: optionally skip --auto process
maintenance: store the "last run" time in config
maintenance: add --scheduled 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 | 19 ++
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 | 286 ++++++++++++++++++++++++++-
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 | 95 ++++++++-
t/test-lib.sh | 6 +
17 files changed, 640 insertions(+), 6 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-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-680/derrickstolee/maintenance/scheduled-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/680
Range-diff vs v1:
1: 90de25d128 ! 1: 5fdd8188b1 maintenance: optionally skip --auto process
@@ run-command.c: int run_processes_parallel_tr2(int n, get_next_task_fn get_next_t
struct child_process maint = CHILD_PROCESS_INIT;
+ if (!git_config_get_bool("maintenance.auto", &enabled) &&
-+ !enabled) {
-+ fprintf(stderr, "enabled: %d\n", enabled);
++ !enabled)
+ return 0;
-+ }
+
maint.git_cmd = 1;
strvec_pushl(&maint.args, "maintenance", "run", "--auto", NULL);
2: bdc27fa28e ! 2: e3ef0b9bea maintenance: store the "last run" time in config
@@ builtin/gc.c: static int compare_tasks_by_selection(const void *a_, const void *
+ strbuf_release(&value);
+}
+
- static int maintenance_run(struct maintenance_opts *opts)
+ static int maintenance_run_tasks(struct maintenance_run_opts *opts)
{
int i, found_selected = 0;
-@@ builtin/gc.c: static int maintenance_run(struct maintenance_opts *opts)
+@@ builtin/gc.c: static int maintenance_run_tasks(struct maintenance_run_opts *opts)
!tasks[i].auto_condition()))
continue;
@@ builtin/gc.c: static int maintenance_run(struct maintenance_opts *opts)
if (tasks[i].fn(opts)) {
error(_("task '%s' failed"), tasks[i].name);
- ## git-gvfs-helper (new) ##
- Binary files /dev/null and git-gvfs-helper differ
-
- ## t/helper/test-gvfs-protocol (new) ##
- Binary files /dev/null and t/helper/test-gvfs-protocol differ
-
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_expect_success 'maintenance.incremental-repack.auto' '
done
3: 4473c93b11 ! 3: c728c57d85 maintenance: add --scheduled option and config
@@ Documentation/git-maintenance.txt: OPTIONS
Do not report progress or other information over `stderr`.
## builtin/gc.c ##
-@@ builtin/gc.c: static const char * const builtin_maintenance_usage[] = {
+@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
+ }
+
+ 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]"),
+ NULL
+ };
- struct maintenance_opts {
+ struct maintenance_run_opts {
int auto_flag;
+ int scheduled;
int quiet;
@@ builtin/gc.c: struct maintenance_task {
/* -1 if not selected. */
int selected_order;
-@@ builtin/gc.c: static int maintenance_run(struct maintenance_opts *opts)
+@@ builtin/gc.c: static int maintenance_run_tasks(struct maintenance_run_opts *opts)
!tasks[i].auto_condition()))
continue;
@@ builtin/gc.c: static int maintenance_run(struct maintenance_opts *opts)
update_last_run(&tasks[i]);
trace2_region_enter("maintenance", tasks[i].name, r);
-@@ builtin/gc.c: static int maintenance_run(struct maintenance_opts *opts)
+@@ builtin/gc.c: static int maintenance_run_tasks(struct maintenance_run_opts *opts)
return result;
}
@@ builtin/gc.c: static void initialize_task_config(void)
int config_value;
+ char *config_str;
- strbuf_setlen(&config_name, 0);
+- strbuf_setlen(&config_name, 0);
++ strbuf_reset(&config_name);
strbuf_addf(&config_name, "maintenance.%s.enabled",
-@@ builtin/gc.c: static void initialize_task_config(void)
+ tasks[i].name);
if (!git_config_get_bool(config_name.buf, &config_value))
tasks[i].enabled = config_value;
+
-+ strbuf_setlen(&config_name, 0);
++ strbuf_reset(&config_name);
+ strbuf_addf(&config_name, "maintenance.%s.schedule",
+ tasks[i].name);
+
@@ builtin/gc.c: static void initialize_task_config(void)
}
strbuf_release(&config_name);
-@@ builtin/gc.c: int cmd_maintenance(int argc, const char **argv, const char *prefix)
- struct option builtin_maintenance_options[] = {
+@@ builtin/gc.c: 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_BOOL(0, "scheduled", &opts.scheduled,
@@ builtin/gc.c: int cmd_maintenance(int argc, const char **argv, const char *prefi
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: int cmd_maintenance(int argc, const char **argv, const char *prefix)
- builtin_maintenance_usage,
- PARSE_OPT_KEEP_UNKNOWN);
+@@ builtin/gc.c: 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.scheduled > 1)
+ die(_("use at most one of the --auto and --scheduled options"));
+
- if (argc != 1)
- usage_with_options(builtin_maintenance_usage,
- builtin_maintenance_options);
+ if (argc != 0)
+ usage_with_options(builtin_maintenance_run_usage,
+ builtin_maintenance_run_options);
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_expect_success 'maintenance.incremental-repack.auto' '
4: ccb667dc6f ! 4: 0314258c5c for-each-repo: run subcommands on configured repos
@@ Documentation/git-for-each-repo.txt (new)
+
+DESCRIPTION
+-----------
-+Run a Git commands on a list of repositories. The arguments after the
++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
-+command.
++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
@@ Documentation/git-for-each-repo.txt (new)
+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
@@ builtin/for-each-repo.c (new)
+ return result;
+}
+ ## command-list.txt ##
+@@ command-list.txt: 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
+
## git.c ##
@@ git.c: static struct cmd_struct commands[] = {
{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
5: f44c6a0f20 ! 5: c0ce1267a9 maintenance: add [un]register subcommands
@@ Documentation/git-maintenance.txt: run::
## builtin/gc.c ##
-@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
- }
-
- static const char * const builtin_maintenance_usage[] = {
-- N_("git maintenance run [<options>]"),
-+ N_("git maintenance <subcommand> [<options>]"),
- NULL
- };
-
-@@ builtin/gc.c: static int task_option_parse(const struct option *opt,
- return 0;
+@@ builtin/gc.c: 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;
@@ builtin/gc.c: static int task_option_parse(const struct option *opt,
+ 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)
{
- int i;
@@ builtin/gc.c: int cmd_maintenance(int argc, const char **argv, const char *prefix)
- usage_with_options(builtin_maintenance_usage,
- builtin_maintenance_options);
-+ if (!strcmp(argv[0], "register"))
+ if (!strcmp(argv[1], "run"))
+ return maintenance_run(argc - 1, argv + 1, prefix);
++ if (!strcmp(argv[1], "register"))
+ return maintenance_register();
- if (!strcmp(argv[0], "run"))
- return maintenance_run(&opts);
-+ if (!strcmp(argv[0], "unregister"))
++ if (!strcmp(argv[1], "unregister"))
+ return maintenance_unregister();
- die(_("invalid subcommand: %s"), argv[0]);
+ die(_("invalid subcommand: %s"), argv[1]);
}
## t/t7900-maintenance.sh ##
6: 2442071fd0 ! 6: 8a7c34035a maintenance: add start/stop subcommands
@@ builtin/gc.c: static int maintenance_unregister(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)
- {
- int i;
@@ builtin/gc.c: int cmd_maintenance(int argc, const char **argv, const char *prefix)
- return maintenance_register();
- if (!strcmp(argv[0], "run"))
- return maintenance_run(&opts);
-+ if (!strcmp(argv[0], "start"))
+
+ if (!strcmp(argv[1], "run"))
+ return maintenance_run(argc - 1, argv + 1, prefix);
++ if (!strcmp(argv[1], "start"))
+ return maintenance_start();
-+ if (!strcmp(argv[0], "stop"))
++ if (!strcmp(argv[1], "stop"))
+ return maintenance_stop();
- if (!strcmp(argv[0], "unregister"))
- return maintenance_unregister();
-
+ if (!strcmp(argv[1], "register"))
+ return maintenance_register();
+ if (!strcmp(argv[1], "unregister"))
## t/helper/test-crontab.c (new) ##
@@
7: 40b1a0546c ! 7: 9ecabeb055 maintenance: recommended schedule in register/start
@@ Documentation/git-maintenance.txt: register::
Run one or more maintenance tasks. If one or more `--task` options
## builtin/gc.c ##
-@@ builtin/gc.c: static int task_option_parse(const struct option *opt,
- return 0;
+@@ builtin/gc.c: static int maintenance_run(int argc, const char **argv, const char *prefix)
+ return maintenance_run_tasks(&opts);
}
+static int has_schedule_config(void)
--
gitgitgadget
^ permalink raw reply
* git clone --shallow-since can result in inconsistent shallow clones
From: Nelson Elhage @ 2020-08-25 18:38 UTC (permalink / raw)
To: git
Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.
What did you do before the bug happened? (Steps to reproduce your issue)
I ran
git clone --shallow-since="1548454011" "https://github.com/abseil/abseil-cpp"
to produce a shallow clone of abseil-cpp.git, with the aim of going
deep enough to grab commit `5e0dcf72c64fae912184d2e0de87195fe8f0a425`,
which I know to have a commit date of `1548454011`.
What did you expect to happen? (Expected behavior)
- I expected the command to produce a valid shallow git clone.
- I further expected the repository to include commit
5e0dcf72c64fae912184d2e0de87195fe8f0a425, which has a commit date <=
the provided `--shallow`, as do all of its descendants up to the
`master` branch
What happened instead? (Actual behavior)
- The clone command produced an inconsistent shallow clone. In the
repository I see:
$ cat .git/shallow
5e0dcf72c64fae912184d2e0de87195fe8f0a425
89ea0c5ff34aaa5855cfc7aa41f323b8a0ef0ede
But commit `5e0dcf72c64fae912184d2e0de87195fe8f0a425` is missing. An
attempt to `git fetch --unshallow` errors out, because the server
sends an `unshallow 5e0dcf72c64fae912184d2e0de87195fe8f0a425`, which
we are unable to execute since we're missing that object.
That object is also the specific one I mentioned above that I wanted.
What's different between what you expected and what actually happened?
Anything else you want to add:
The problem here is triggered by passing a `shallow-since` that lies
*between* the first and and second parents of a merge commit that
itself is on the first-parent spine. If we examine the relevant
portion of `abseil-cpp.git`'s history, we find:
$ git --no-pager log --format='%h %ct' --graph
89ea0c5ff34aaa5855cfc7aa41f323b8a0ef0ede~6..89ea0c5ff34aaa5855cfc7aa41f323b8a0ef0ede
* 89ea0c5 1548698816 # WANT
|\
| * 7ec3270 1548194022 # WANT
* | 5e0dcf7 1548454011 # WANT
* | 0dffca4 1548346230 # DON'T WANT
* | 6b4201f 1548261751 # DON'T WANT
|/
* 0b1e6d4 1547838308 # DON'T WANT
* efccc50 1547753737 # DON'T WANT
I've annotated the commits with WANT or DON'T WONT based on whether or
not their commit time is included by the `--shallow-since` filter.
What is happening, I believe, is that we are marking 89ea0c5 as
shallow, since its first parent is unwanted. However, marking it
shallow causes pack generation to ignore _all_ of its parents,
including 7ec3270, which we _do_ want. This results in the
inconsistent state where we mark `5e0dcf7` as shallow (and send the
`shallow` line), but don't send the actual object.
It's unfortunately a bit unclear to me what _should_ happen here. We
really want a way to mark `89ea0c5` as "partially-shallow", and send
its second parent, but not its first parent, but shallowness is a
property of an entire commit, not of a specific commit/parent
relationship. However, it'd be nice if we at least ended up with a
consistent state, instead of with a repository with invalid `shallow`
marks.
Please review the rest of the bug report below.
You can delete any lines you don't wish to share.
[System Info]
git version:
git version 2.28.0.461.g40977abb40
cpu: x86_64
built from commit: 40977abb4059c11004726852a79df64f4553944d
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.4.0-42-generic #46-Ubuntu SMP Fri Jul 10 00:24:02 UTC 2020 x86_64
compiler info: gnuc: 9.3
libc info: glibc: 2.31
$SHELL (typically, interactive shell): /bin/bash
[Enabled Hooks]
- Nelson Elhage
^ permalink raw reply
* [PATCH v3 7/8] maintenance: auto-size incremental-repack batch
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:36 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.696.v3.git.1598380599.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
When repacking during the 'incremental-repack' task, we use the
--batch-size option in 'git multi-pack-index repack'. The initial setting
used --batch-size=0 to repack everything into a single pack-file. This is
not sustainable for a large repository. The amount of work required is
also likely to use too many system resources for a background job.
Update the 'incremental-repack' task by dynamically computing a
--batch-size option based on the current pack-file structure.
The dynamic default size is computed with this idea in mind for a client
repository that was cloned from a very large remote: there is likely one
"big" pack-file that was created at clone time. Thus, do not try
repacking it as it is likely packed efficiently by the server.
Instead, we select the second-largest pack-file, and create a batch size
that is one larger than that pack-file. If there are three or more
pack-files, then this guarantees that at least two will be combined into
a new pack-file.
Of course, this means that the second-largest pack-file size is likely
to grow over time and may eventually surpass the initially-cloned
pack-file. Recall that the pack-file batch is selected in a greedy
manner: the packs are considered from oldest to newest and are selected
if they have size smaller than the batch size until the total selected
size is larger than the batch size. Thus, that oldest "clone" pack will
be first to repack after the new data creates a pack larger than that.
We also want to place some limits on how large these pack-files become,
in order to bound the amount of time spent repacking. A maximum
batch-size of two gigabytes means that large repositories will never be
packed into a single pack-file using this job, but also that repack is
rather expensive. This is a trade-off that is valuable to have if the
maintenance is being run automatically or in the background. Users who
truly want to optimize for space and performance (and are willing to pay
the upfront cost of a full repack) can use the 'gc' task to do so.
Create a test for this two gigabyte limit by creating an EXPENSIVE test
that generates two pack-files of roughly 2.5 gigabytes in size, then
performs an incremental repack. Check that the --batch-size argument in
the subcommand uses the hard-coded maximum.
Helped-by: Chris Torek <chris.torek@gmail.com>
Reported-by: Son Luong Ngoc <sluongng@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
builtin/gc.c | 43 +++++++++++++++++++++++++++++++++++++++++-
t/t7900-maintenance.sh | 36 +++++++++++++++++++++++++++++++++--
2 files changed, 76 insertions(+), 3 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index fbf84996fa..e043403400 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1036,6 +1036,46 @@ static int multi_pack_index_expire(struct maintenance_run_opts *opts)
return 0;
}
+#define TWO_GIGABYTES (INT32_MAX)
+
+static off_t get_auto_pack_size(void)
+{
+ /*
+ * The "auto" value is special: we optimize for
+ * one large pack-file (i.e. from a clone) and
+ * expect the rest to be small and they can be
+ * repacked quickly.
+ *
+ * The strategy we select here is to select a
+ * size that is one more than the second largest
+ * pack-file. This ensures that we will repack
+ * at least two packs if there are three or more
+ * packs.
+ */
+ off_t max_size = 0;
+ off_t second_largest_size = 0;
+ off_t result_size;
+ struct packed_git *p;
+ struct repository *r = the_repository;
+
+ reprepare_packed_git(r);
+ for (p = get_all_packs(r); p; p = p->next) {
+ if (p->pack_size > max_size) {
+ second_largest_size = max_size;
+ max_size = p->pack_size;
+ } else if (p->pack_size > second_largest_size)
+ second_largest_size = p->pack_size;
+ }
+
+ result_size = second_largest_size + 1;
+
+ /* But limit ourselves to a batch size of 2g */
+ if (result_size > TWO_GIGABYTES)
+ result_size = TWO_GIGABYTES;
+
+ return result_size;
+}
+
static int multi_pack_index_repack(struct maintenance_run_opts *opts)
{
struct child_process child = CHILD_PROCESS_INIT;
@@ -1046,7 +1086,8 @@ static int multi_pack_index_repack(struct maintenance_run_opts *opts)
if (opts->quiet)
strvec_push(&child.args, "--no-progress");
- strvec_push(&child.args, "--batch-size=0");
+ strvec_pushf(&child.args, "--batch-size=%"PRIuMAX,
+ (uintmax_t)get_auto_pack_size());
close_object_store(the_repository->objects);
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index dde28cf837..5c08afc19a 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -182,10 +182,42 @@ test_expect_success 'incremental-repack task' '
test_line_count = 4 packs-between &&
# the job deletes the two old packs, and does not write
- # a new one because only one pack remains.
+ # a new one because the batch size is not high enough to
+ # pack the largest pack-file.
git maintenance run --task=incremental-repack &&
ls .git/objects/pack/*.pack >packs-after &&
- test_line_count = 1 packs-after
+ test_line_count = 2 packs-after
+'
+
+test_expect_success EXPENSIVE 'incremental-repack 2g limit' '
+ for i in $(test_seq 1 5)
+ do
+ test-tool genrandom foo$i $((512 * 1024 * 1024 + 1)) >>big ||
+ return 1
+ done &&
+ git add big &&
+ git commit -m "Add big file (1)" &&
+
+ # ensure any possible loose objects are in a pack-file
+ git maintenance run --task=loose-objects &&
+
+ rm big &&
+ for i in $(test_seq 6 10)
+ do
+ test-tool genrandom foo$i $((512 * 1024 * 1024 + 1)) >>big ||
+ return 1
+ done &&
+ git add big &&
+ git commit -m "Add big file (2)" &&
+
+ # ensure any possible loose objects are in a pack-file
+ git maintenance run --task=loose-objects &&
+
+ # Now run the incremental-repack task and check the batch-size
+ GIT_TRACE2_EVENT="$(pwd)/run-2g.txt" git maintenance run \
+ --task=incremental-repack 2>/dev/null &&
+ test_subcommand git multi-pack-index repack \
+ --no-progress --batch-size=2147483647 <run-2g.txt
'
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 8/8] maintenance: add incremental-repack auto condition
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:36 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.696.v3.git.1598380599.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
The incremental-repack task updates the multi-pack-index by deleting pack-
files that have been replaced with new packs, then repacking a batch of
small pack-files into a larger pack-file. This incremental repack is faster
than rewriting all object data, but is slower than some other
maintenance activities.
The 'maintenance.incremental-repack.auto' config option specifies how many
pack-files should exist outside of the multi-pack-index before running
the step. These pack-files could be created by 'git fetch' commands or
by the loose-objects task. The default value is 10.
Setting the option to zero disables the task with the '--auto' option,
and a negative value makes the task run every time.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/config/maintenance.txt | 9 ++++++++
builtin/gc.c | 31 ++++++++++++++++++++++++++++
t/t7900-maintenance.sh | 31 ++++++++++++++++++++++++++++
3 files changed, 71 insertions(+)
diff --git a/Documentation/config/maintenance.txt b/Documentation/config/maintenance.txt
index c31613be62..a0706d8f09 100644
--- a/Documentation/config/maintenance.txt
+++ b/Documentation/config/maintenance.txt
@@ -23,3 +23,12 @@ maintenance.loose-objects.auto::
positive value implies the command should run when the number of
loose objects is at least the value of `maintenance.loose-objects.auto`.
The default value is 100.
+
+maintenance.incremental-repack.auto::
+ This integer config option controls how often the `incremental-repack`
+ task should be run as part of `git maintenance run --auto`. If zero,
+ then the `incremental-repack` task will not run with the `--auto`
+ option. A negative value will force the task to run every time.
+ Otherwise, a positive value implies the command should run when the
+ number of pack-files not in the multi-pack-index is at least the value
+ of `maintenance.incremental-repack.auto`. The default value is 10.
diff --git a/builtin/gc.c b/builtin/gc.c
index e043403400..f8459df04c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -31,6 +31,7 @@
#include "refs.h"
#include "remote.h"
#include "midx.h"
+#include "object-store.h"
#define FAILED_RUN "failed to run %s"
@@ -1002,6 +1003,35 @@ static int maintenance_task_loose_objects(struct maintenance_run_opts *opts)
return prune_packed(opts) || pack_loose(opts);
}
+static int incremental_repack_auto_condition(void)
+{
+ struct packed_git *p;
+ int enabled;
+ int incremental_repack_auto_limit = 10;
+ int count = 0;
+
+ if (git_config_get_bool("core.multiPackIndex", &enabled) ||
+ !enabled)
+ return 0;
+
+ git_config_get_int("maintenance.incremental-repack.auto",
+ &incremental_repack_auto_limit);
+
+ if (!incremental_repack_auto_limit)
+ return 0;
+ if (incremental_repack_auto_limit < 0)
+ return 1;
+
+ for (p = get_packed_git(the_repository);
+ count < incremental_repack_auto_limit && p;
+ p = p->next) {
+ if (!p->multi_pack_index)
+ count++;
+ }
+
+ return count >= incremental_repack_auto_limit;
+}
+
static int multi_pack_index_write(struct maintenance_run_opts *opts)
{
struct child_process child = CHILD_PROCESS_INIT;
@@ -1157,6 +1187,7 @@ static struct maintenance_task tasks[] = {
[TASK_INCREMENTAL_REPACK] = {
"incremental-repack",
maintenance_task_incremental_repack,
+ incremental_repack_auto_condition,
},
[TASK_GC] = {
"gc",
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 5c08afc19a..6f878b0141 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -220,4 +220,35 @@ test_expect_success EXPENSIVE 'incremental-repack 2g limit' '
--no-progress --batch-size=2147483647 <run-2g.txt
'
+test_expect_success 'maintenance.incremental-repack.auto' '
+ git repack -adk &&
+ git config core.multiPackIndex true &&
+ git multi-pack-index write &&
+ GIT_TRACE2_EVENT="$(pwd)/midx-init.txt" git \
+ -c maintenance.incremental-repack.auto=1 \
+ maintenance run --auto --task=incremental-repack 2>/dev/null &&
+ test_subcommand ! git multi-pack-index write --no-progress <midx-init.txt &&
+ for i in 1 2
+ do
+ test_commit A-$i &&
+ git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
+ HEAD
+ ^HEAD~1
+ EOF
+ GIT_TRACE2_EVENT=$(pwd)/trace-A-$i git \
+ -c maintenance.incremental-repack.auto=2 \
+ maintenance run --auto --task=incremental-repack 2>/dev/null &&
+ test_subcommand ! git multi-pack-index write --no-progress <trace-A-$i &&
+ test_commit B-$i &&
+ git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
+ HEAD
+ ^HEAD~1
+ EOF
+ GIT_TRACE2_EVENT=$(pwd)/trace-B-$i git \
+ -c maintenance.incremental-repack.auto=2 \
+ maintenance run --auto --task=incremental-repack 2>/dev/null &&
+ test_subcommand git multi-pack-index write --no-progress <trace-B-$i || return 1
+ done
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 6/8] maintenance: add incremental-repack task
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:36 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.696.v3.git.1598380599.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
The previous change cleaned up loose objects using the
'loose-objects' that can be run safely in the background. Add a
similar job that performs similar cleanups for pack-files.
One issue with running 'git repack' is that it is designed to
repack all pack-files into a single pack-file. While this is the
most space-efficient way to store object data, it is not time or
memory efficient. This becomes extremely important if the repo is
so large that a user struggles to store two copies of the pack on
their disk.
Instead, perform an "incremental" repack by collecting a few small
pack-files into a new pack-file. The multi-pack-index facilitates
this process ever since 'git multi-pack-index expire' was added in
19575c7 (multi-pack-index: implement 'expire' subcommand,
2019-06-10) and 'git multi-pack-index repack' was added in ce1e4a1
(midx: implement midx_repack(), 2019-06-10).
The 'incremental-repack' task runs the following steps:
1. 'git multi-pack-index write' creates a multi-pack-index file if
one did not exist, and otherwise will update the multi-pack-index
with any new pack-files that appeared since the last write. This
is particularly relevant with the background fetch job.
When the multi-pack-index sees two copies of the same object, it
stores the offset data into the newer pack-file. This means that
some old pack-files could become "unreferenced" which I will use
to mean "a pack-file that is in the pack-file list of the
multi-pack-index but none of the objects in the multi-pack-index
reference a location inside that pack-file."
2. 'git multi-pack-index expire' deletes any unreferenced pack-files
and updaes the multi-pack-index to drop those pack-files from the
list. This is safe to do as concurrent Git processes will see the
multi-pack-index and not open those packs when looking for object
contents. (Similar to the 'loose-objects' job, there are some Git
commands that open pack-files regardless of the multi-pack-index,
but they are rarely used. Further, a user that self-selects to
use background operations would likely refrain from using those
commands.)
3. 'git multi-pack-index repack --bacth-size=<size>' collects a set
of pack-files that are listed in the multi-pack-index and creates
a new pack-file containing the objects whose offsets are listed
by the multi-pack-index to be in those objects. The set of pack-
files is selected greedily by sorting the pack-files by modified
time and adding a pack-file to the set if its "expected size" is
smaller than the batch size until the total expected size of the
selected pack-files is at least the batch size. The "expected
size" is calculated by taking the size of the pack-file divided
by the number of objects in the pack-file and multiplied by the
number of objects from the multi-pack-index with offset in that
pack-file. The expected size approximates how much data from that
pack-file will contribute to the resulting pack-file size. The
intention is that the resulting pack-file will be close in size
to the provided batch size.
The next run of the incremental-repack task will delete these
repacked pack-files during the 'expire' step.
In this version, the batch size is set to "0" which ignores the
size restrictions when selecting the pack-files. It instead
selects all pack-files and repacks all packed objects into a
single pack-file. This will be updated in the next change, but
it requires doing some calculations that are better isolated to
a separate change.
These steps are based on a similar background maintenance step in
Scalar (and VFS for Git) [1]. This was incredibly effective for
users of the Windows OS repository. After using the same VFS for Git
repository for over a year, some users had _thousands_ of pack-files
that combined to up to 250 GB of data. We noticed a few users were
running into the open file descriptor limits (due in part to a bug
in the multi-pack-index fixed by af96fe3 (midx: add packs to
packed_git linked list, 2019-04-29).
These pack-files were mostly small since they contained the commits
and trees that were pushed to the origin in a given hour. The GVFS
protocol includes a "prefetch" step that asks for pre-computed pack-
files containing commits and trees by timestamp. These pack-files
were grouped into "daily" pack-files once a day for up to 30 days.
If a user did not request prefetch packs for over 30 days, then they
would get the entire history of commits and trees in a new, large
pack-file. This led to a large number of pack-files that had poor
delta compression.
By running this pack-file maintenance step once per day, these repos
with thousands of packs spanning 200+ GB dropped to dozens of pack-
files spanning 30-50 GB. This was done all without removing objects
from the system and using a constant batch size of two gigabytes.
Once the work was done to reduce the pack-files to small sizes, the
batch size of two gigabytes means that not every run triggers a
repack operation, so the following run will not expire a pack-file.
This has kept these repos in a "clean" state.
[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/PackfileMaintenanceStep.cs
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-maintenance.txt | 15 ++++++
builtin/gc.c | 77 +++++++++++++++++++++++++++++++
midx.c | 2 +-
midx.h | 1 +
t/t5319-multi-pack-index.sh | 1 +
t/t7900-maintenance.sh | 38 +++++++++++++++
6 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index fc95eb594f..b44efb05a3 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -85,6 +85,21 @@ loose-objects::
advisable to enable both the `loose-objects` and `gc` tasks at the
same time.
+incremental-repack::
+ The `incremental-repack` job repacks the object directory
+ using the `multi-pack-index` feature. In order to prevent race
+ conditions with concurrent Git commands, it follows a two-step
+ process. First, it deletes any pack-files included in the
+ `multi-pack-index` where none of the objects in the
+ `multi-pack-index` reference those pack-files; this only happens
+ if all objects in the pack-file are also stored in a newer
+ pack-file. Second, it selects a group of pack-files whose "expected
+ size" is below the batch size until the group has total expected
+ size at least the batch size; see the `--batch-size` option for
+ the `repack` subcommand in linkgit:git-multi-pack-index[1]. The
+ default batch-size is zero, which is a special case that attempts
+ to repack all pack-files into a single pack-file.
+
OPTIONS
-------
--auto::
diff --git a/builtin/gc.c b/builtin/gc.c
index 25245bcc10..fbf84996fa 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -30,6 +30,7 @@
#include "promisor-remote.h"
#include "refs.h"
#include "remote.h"
+#include "midx.h"
#define FAILED_RUN "failed to run %s"
@@ -1001,6 +1002,77 @@ static int maintenance_task_loose_objects(struct maintenance_run_opts *opts)
return prune_packed(opts) || pack_loose(opts);
}
+static int multi_pack_index_write(struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
+ child.git_cmd = 1;
+ strvec_pushl(&child.args, "multi-pack-index", "write", NULL);
+
+ if (opts->quiet)
+ strvec_push(&child.args, "--no-progress");
+
+ if (run_command(&child))
+ return error(_("failed to write multi-pack-index"));
+
+ return 0;
+}
+
+static int multi_pack_index_expire(struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
+ child.git_cmd = 1;
+ strvec_pushl(&child.args, "multi-pack-index", "expire", NULL);
+
+ if (opts->quiet)
+ strvec_push(&child.args, "--no-progress");
+
+ close_object_store(the_repository->objects);
+
+ if (run_command(&child))
+ return error(_("'git multi-pack-index expire' failed"));
+
+ return 0;
+}
+
+static int multi_pack_index_repack(struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
+ child.git_cmd = 1;
+ strvec_pushl(&child.args, "multi-pack-index", "repack", NULL);
+
+ if (opts->quiet)
+ strvec_push(&child.args, "--no-progress");
+
+ strvec_push(&child.args, "--batch-size=0");
+
+ close_object_store(the_repository->objects);
+
+ if (run_command(&child))
+ return error(_("'git multi-pack-index repack' failed"));
+
+ return 0;
+}
+
+static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts)
+{
+ prepare_repo_settings(the_repository);
+ if (!the_repository->settings.core_multi_pack_index) {
+ warning(_("skipping incremental-repack task because core.multiPackIndex is disabled"));
+ return 0;
+ }
+
+ if (multi_pack_index_write(opts))
+ return 1;
+ if (multi_pack_index_expire(opts))
+ return 1;
+ if (multi_pack_index_repack(opts))
+ return 1;
+ return 0;
+}
+
typedef int maintenance_task_fn(struct maintenance_run_opts *opts);
/*
@@ -1023,6 +1095,7 @@ struct maintenance_task {
enum maintenance_task_label {
TASK_PREFETCH,
TASK_LOOSE_OBJECTS,
+ TASK_INCREMENTAL_REPACK,
TASK_GC,
TASK_COMMIT_GRAPH,
@@ -1040,6 +1113,10 @@ static struct maintenance_task tasks[] = {
maintenance_task_loose_objects,
loose_object_auto_condition,
},
+ [TASK_INCREMENTAL_REPACK] = {
+ "incremental-repack",
+ maintenance_task_incremental_repack,
+ },
[TASK_GC] = {
"gc",
maintenance_task_gc,
diff --git a/midx.c b/midx.c
index aa37d5da86..66d7053d83 100644
--- a/midx.c
+++ b/midx.c
@@ -37,7 +37,7 @@
#define PACK_EXPIRED UINT_MAX
-static char *get_midx_filename(const char *object_dir)
+char *get_midx_filename(const char *object_dir)
{
return xstrfmt("%s/pack/multi-pack-index", object_dir);
}
diff --git a/midx.h b/midx.h
index b18cf53bc4..baeecc70c9 100644
--- a/midx.h
+++ b/midx.h
@@ -37,6 +37,7 @@ struct multi_pack_index {
#define MIDX_PROGRESS (1 << 0)
+char *get_midx_filename(const char *object_dir);
struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local);
int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id);
int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result);
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index ec87f616c6..2f942ee1fa 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -3,6 +3,7 @@
test_description='multi-pack-indexes'
. ./test-lib.sh
+GIT_TEST_MULTI_PACK_INDEX=0
objdir=.git/objects
midx_read_expect () {
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index efda1cf69b..dde28cf837 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -5,6 +5,7 @@ test_description='git maintenance builtin'
. ./test-lib.sh
GIT_TEST_COMMIT_GRAPH=0
+GIT_TEST_MULTI_PACK_INDEX=0
test_expect_success 'help text' '
test_expect_code 129 git maintenance -h 2>err &&
@@ -150,4 +151,41 @@ test_expect_success 'maintenance.loose-objects.auto' '
done
'
+test_expect_success 'incremental-repack task' '
+ packDir=.git/objects/pack &&
+ for i in $(test_seq 1 5)
+ do
+ test_commit $i || return 1
+ done &&
+
+ # Create three disjoint pack-files with size BIG, small, small.
+ echo HEAD~2 | git pack-objects --revs $packDir/test-1 &&
+ test_tick &&
+ git pack-objects --revs $packDir/test-2 <<-\EOF &&
+ HEAD~1
+ ^HEAD~2
+ EOF
+ test_tick &&
+ git pack-objects --revs $packDir/test-3 <<-\EOF &&
+ HEAD
+ ^HEAD~1
+ EOF
+ rm -f $packDir/pack-* &&
+ rm -f $packDir/loose-* &&
+ ls $packDir/*.pack >packs-before &&
+ test_line_count = 3 packs-before &&
+
+ # the job repacks the two into a new pack, but does not
+ # delete the old ones.
+ git maintenance run --task=incremental-repack &&
+ ls $packDir/*.pack >packs-between &&
+ test_line_count = 4 packs-between &&
+
+ # the job deletes the two old packs, and does not write
+ # a new one because only one pack remains.
+ git maintenance run --task=incremental-repack &&
+ ls .git/objects/pack/*.pack >packs-after &&
+ test_line_count = 1 packs-after
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 5/8] midx: use start_delayed_progress()
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:36 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.696.v3.git.1598380599.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
Now that the multi-pack-index may be written as part of auto maintenance
at the end of a command, reduce the progress output when the operations
are quick. Use start_delayed_progress() instead of start_progress().
Update t5319-multi-pack-index.sh to use GIT_PROGRESS_DELAY=0 now that
the progress indicators are conditional.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
midx.c | 10 +++++-----
t/t5319-multi-pack-index.sh | 14 +++++++-------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/midx.c b/midx.c
index ef499cf504..aa37d5da86 100644
--- a/midx.c
+++ b/midx.c
@@ -832,7 +832,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
packs.pack_paths_checked = 0;
if (flags & MIDX_PROGRESS)
- packs.progress = start_progress(_("Adding packfiles to multi-pack-index"), 0);
+ packs.progress = start_delayed_progress(_("Adding packfiles to multi-pack-index"), 0);
else
packs.progress = NULL;
@@ -969,7 +969,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
}
if (flags & MIDX_PROGRESS)
- progress = start_progress(_("Writing chunks to multi-pack-index"),
+ progress = start_delayed_progress(_("Writing chunks to multi-pack-index"),
num_chunks);
for (i = 0; i < num_chunks; i++) {
if (written != chunk_offsets[i])
@@ -1104,7 +1104,7 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
return 0;
if (flags & MIDX_PROGRESS)
- progress = start_progress(_("Looking for referenced packfiles"),
+ progress = start_delayed_progress(_("Looking for referenced packfiles"),
m->num_packs);
for (i = 0; i < m->num_packs; i++) {
if (prepare_midx_pack(r, m, i))
@@ -1225,7 +1225,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
count = xcalloc(m->num_packs, sizeof(uint32_t));
if (flags & MIDX_PROGRESS)
- progress = start_progress(_("Counting referenced objects"),
+ progress = start_delayed_progress(_("Counting referenced objects"),
m->num_objects);
for (i = 0; i < m->num_objects; i++) {
int pack_int_id = nth_midxed_pack_int_id(m, i);
@@ -1235,7 +1235,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
stop_progress(&progress);
if (flags & MIDX_PROGRESS)
- progress = start_progress(_("Finding and deleting unreferenced packfiles"),
+ progress = start_delayed_progress(_("Finding and deleting unreferenced packfiles"),
m->num_packs);
for (i = 0; i < m->num_packs; i++) {
char *pack_name;
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 7dfff0f8f4..ec87f616c6 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -171,12 +171,12 @@ test_expect_success 'write progress off for redirected stderr' '
'
test_expect_success 'write force progress on for stderr' '
- git multi-pack-index --object-dir=$objdir --progress write 2>err &&
+ GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --progress write 2>err &&
test_file_not_empty err
'
test_expect_success 'write with the --no-progress option' '
- git multi-pack-index --object-dir=$objdir --no-progress write 2>err &&
+ GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --no-progress write 2>err &&
test_line_count = 0 err
'
@@ -333,17 +333,17 @@ test_expect_success 'git-fsck incorrect offset' '
'
test_expect_success 'repack progress off for redirected stderr' '
- git multi-pack-index --object-dir=$objdir repack 2>err &&
+ GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir repack 2>err &&
test_line_count = 0 err
'
test_expect_success 'repack force progress on for stderr' '
- git multi-pack-index --object-dir=$objdir --progress repack 2>err &&
+ GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --progress repack 2>err &&
test_file_not_empty err
'
test_expect_success 'repack with the --no-progress option' '
- git multi-pack-index --object-dir=$objdir --no-progress repack 2>err &&
+ GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --no-progress repack 2>err &&
test_line_count = 0 err
'
@@ -487,7 +487,7 @@ test_expect_success 'expire progress off for redirected stderr' '
test_expect_success 'expire force progress on for stderr' '
(
cd dup &&
- git multi-pack-index --progress expire 2>err &&
+ GIT_PROGRESS_DELAY=0 git multi-pack-index --progress expire 2>err &&
test_file_not_empty err
)
'
@@ -495,7 +495,7 @@ test_expect_success 'expire force progress on for stderr' '
test_expect_success 'expire with the --no-progress option' '
(
cd dup &&
- git multi-pack-index --no-progress expire 2>err &&
+ GIT_PROGRESS_DELAY=0 git multi-pack-index --no-progress expire 2>err &&
test_line_count = 0 err
)
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 4/8] midx: enable core.multiPackIndex by default
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:36 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.696.v3.git.1598380599.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
The core.multiPackIndex setting has been around since c4d25228ebb
(config: create core.multiPackIndex setting, 2018-07-12), but has been
disabled by default. If a user wishes to use the multi-pack-index
feature, then they must enable this config and run 'git multi-pack-index
write'.
The multi-pack-index feature is relatively stable now, so make the
config option true by default. For users that do not use a
multi-pack-index, the only extra cost will be a file lookup to see if a
multi-pack-index file exists (once per process, per object directory).
Also, this config option will be referenced by an upcoming
"incremental-repack" task in the maintenance builtin, so move the config
option into the repository settings struct. Note that if
GIT_TEST_MULTI_PACK_INDEX=1, then we want to ignore the config option
and treat core.multiPackIndex as enabled.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/config/core.txt | 4 ++--
midx.c | 11 +++--------
repo-settings.c | 6 ++++++
repository.h | 2 ++
4 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 74619a9c03..86c91d5381 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -606,8 +606,8 @@ core.useReplaceRefs::
core.multiPackIndex::
Use the multi-pack-index file to track multiple packfiles using a
- single index. See link:technical/multi-pack-index.html[the
- multi-pack-index design document].
+ single index. See linkgit:git-multi-pack-index[1] for more
+ information. Defaults to true.
core.sparseCheckout::
Enable "sparse checkout" feature. See linkgit:git-sparse-checkout[1]
diff --git a/midx.c b/midx.c
index a5fb797ede..ef499cf504 100644
--- a/midx.c
+++ b/midx.c
@@ -10,6 +10,7 @@
#include "progress.h"
#include "trace2.h"
#include "run-command.h"
+#include "repository.h"
#define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
#define MIDX_VERSION 1
@@ -384,15 +385,9 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i
{
struct multi_pack_index *m;
struct multi_pack_index *m_search;
- int config_value;
- static int env_value = -1;
- if (env_value < 0)
- env_value = git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0);
-
- if (!env_value &&
- (repo_config_get_bool(r, "core.multipackindex", &config_value) ||
- !config_value))
+ prepare_repo_settings(r);
+ if (!r->settings.core_multi_pack_index)
return 0;
for (m_search = r->objects->multi_pack_index; m_search; m_search = m_search->next)
diff --git a/repo-settings.c b/repo-settings.c
index 0918408b34..5bd2c22726 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "config.h"
#include "repository.h"
+#include "midx.h"
#define UPDATE_DEFAULT_BOOL(s,v) do { if (s == -1) { s = v; } } while(0)
@@ -47,6 +48,11 @@ void prepare_repo_settings(struct repository *r)
r->settings.pack_use_sparse = value;
UPDATE_DEFAULT_BOOL(r->settings.pack_use_sparse, 1);
+ value = git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0);
+ if (value || !repo_config_get_bool(r, "core.multipackindex", &value))
+ r->settings.core_multi_pack_index = value;
+ UPDATE_DEFAULT_BOOL(r->settings.core_multi_pack_index, 1);
+
if (!repo_config_get_bool(r, "feature.manyfiles", &value) && value) {
UPDATE_DEFAULT_BOOL(r->settings.index_version, 4);
UPDATE_DEFAULT_BOOL(r->settings.core_untracked_cache, UNTRACKED_CACHE_WRITE);
diff --git a/repository.h b/repository.h
index 3c1f7d54bd..3901ce0b65 100644
--- a/repository.h
+++ b/repository.h
@@ -37,6 +37,8 @@ struct repo_settings {
int pack_use_sparse;
enum fetch_negotiation_setting fetch_negotiation_algorithm;
+
+ int core_multi_pack_index;
};
struct repository {
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 3/8] maintenance: create auto condition for loose-objects
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:36 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.696.v3.git.1598380599.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
The loose-objects task deletes loose objects that already exist in a
pack-file, then place the remaining loose objects into a new pack-file.
If this step runs all the time, then we risk creating pack-files with
very few objects with every 'git commit' process. To prevent
overwhelming the packs directory with small pack-files, place a minimum
number of objects to justify the task.
The 'maintenance.loose-objects.auto' config option specifies a minimum
number of loose objects to justify the task to run under the '--auto'
option. This defaults to 100 loose objects. Setting the value to zero
will prevent the step from running under '--auto' while a negative value
will force it to run every time.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/config/maintenance.txt | 9 +++++++++
builtin/gc.c | 30 ++++++++++++++++++++++++++++
t/t7900-maintenance.sh | 25 +++++++++++++++++++++++
3 files changed, 64 insertions(+)
diff --git a/Documentation/config/maintenance.txt b/Documentation/config/maintenance.txt
index 7cc6700d57..c31613be62 100644
--- a/Documentation/config/maintenance.txt
+++ b/Documentation/config/maintenance.txt
@@ -14,3 +14,12 @@ maintenance.commit-graph.auto::
reachable commits that are not in the commit-graph file is at least
the value of `maintenance.commit-graph.auto`. The default value is
100.
+
+maintenance.loose-objects.auto::
+ This integer config option controls how often the `loose-objects` task
+ should be run as part of `git maintenance run --auto`. If zero, then
+ the `loose-objects` task will not run with the `--auto` option. A
+ negative value will force the task to run every time. Otherwise, a
+ positive value implies the command should run when the number of
+ loose objects is at least the value of `maintenance.loose-objects.auto`.
+ The default value is 100.
diff --git a/builtin/gc.c b/builtin/gc.c
index 248ccde3c3..25245bcc10 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -899,6 +899,35 @@ struct write_loose_object_data {
int batch_size;
};
+static int loose_object_auto_limit = 100;
+
+static int loose_object_count(const struct object_id *oid,
+ const char *path,
+ void *data)
+{
+ int *count = (int*)data;
+ if (++(*count) >= loose_object_auto_limit)
+ return 1;
+ return 0;
+}
+
+static int loose_object_auto_condition(void)
+{
+ int count = 0;
+
+ git_config_get_int("maintenance.loose-objects.auto",
+ &loose_object_auto_limit);
+
+ if (!loose_object_auto_limit)
+ return 0;
+ if (loose_object_auto_limit < 0)
+ return 1;
+
+ return for_each_loose_file_in_objdir(the_repository->objects->odb->path,
+ loose_object_count,
+ NULL, NULL, &count);
+}
+
static int bail_on_loose(const struct object_id *oid,
const char *path,
void *data)
@@ -1009,6 +1038,7 @@ static struct maintenance_task tasks[] = {
[TASK_LOOSE_OBJECTS] = {
"loose-objects",
maintenance_task_loose_objects,
+ loose_object_auto_condition,
},
[TASK_GC] = {
"gc",
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 2e9e369786..efda1cf69b 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -125,4 +125,29 @@ test_expect_success 'loose-objects task' '
test_cmp packs-between packs-after
'
+test_expect_success 'maintenance.loose-objects.auto' '
+ git repack -adk &&
+ GIT_TRACE2_EVENT="$(pwd)/trace-lo1.txt" \
+ git -c maintenance.loose-objects.auto=1 maintenance \
+ run --auto --task=loose-objects 2>/dev/null &&
+ test_subcommand ! git prune-packed --quiet <trace-lo1.txt &&
+ for i in 1 2
+ do
+ printf data-A-$i | git hash-object -t blob --stdin -w &&
+ GIT_TRACE2_EVENT="$(pwd)/trace-loA-$i" \
+ git -c maintenance.loose-objects.auto=2 \
+ maintenance run --auto --task=loose-objects 2>/dev/null &&
+ test_subcommand ! git prune-packed --quiet <trace-loA-$i &&
+ printf data-B-$i | git hash-object -t blob --stdin -w &&
+ GIT_TRACE2_EVENT="$(pwd)/trace-loB-$i" \
+ git -c maintenance.loose-objects.auto=2 \
+ maintenance run --auto --task=loose-objects 2>/dev/null &&
+ test_subcommand git prune-packed --quiet <trace-loB-$i &&
+ GIT_TRACE2_EVENT="$(pwd)/trace-loC-$i" \
+ git -c maintenance.loose-objects.auto=2 \
+ maintenance run --auto --task=loose-objects 2>/dev/null &&
+ test_subcommand git prune-packed --quiet <trace-loC-$i || return 1
+ done
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/8] maintenance: add loose-objects task
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:36 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.696.v3.git.1598380599.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
One goal of background maintenance jobs is to allow a user to
disable auto-gc (gc.auto=0) but keep their repository in a clean
state. Without any cleanup, loose objects will clutter the object
database and slow operations. In addition, the loose objects will
take up extra space because they are not stored with deltas against
similar objects.
Create a 'loose-objects' task for the 'git maintenance run' command.
This helps clean up loose objects without disrupting concurrent Git
commands using the following sequence of events:
1. Run 'git prune-packed' to delete any loose objects that exist
in a pack-file. Concurrent commands will prefer the packed
version of the object to the loose version. (Of course, there
are exceptions for commands that specifically care about the
location of an object. These are rare for a user to run on
purpose, and we hope a user that has selected background
maintenance will not be trying to do foreground maintenance.)
2. Run 'git pack-objects' on a batch of loose objects. These
objects are grouped by scanning the loose object directories in
lexicographic order until listing all loose objects -or-
reaching 50,000 objects. This is more than enough if the loose
objects are created only by a user doing normal development.
We noticed users with _millions_ of loose objects because VFS
for Git downloads blobs on-demand when a file read operation
requires populating a virtual file. This has potential of
happening in partial clones if someone runs 'git grep' or
otherwise evades the batch-download feature for requesting
promisor objects.
This step is based on a similar step in Scalar [1] and VFS for Git.
[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/LooseObjectsStep.cs
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-maintenance.txt | 15 +++++
builtin/gc.c | 97 +++++++++++++++++++++++++++++++
t/t7900-maintenance.sh | 39 +++++++++++++
3 files changed, 151 insertions(+)
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index 12668fccf7..fc95eb594f 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -70,6 +70,21 @@ gc::
be disruptive in some situations, as it deletes stale data. See
linkgit:git-gc[1] for more details on garbage collection in Git.
+loose-objects::
+ The `loose-objects` job cleans up loose objects and places them into
+ pack-files. In order to prevent race conditions with concurrent Git
+ commands, it follows a two-step process. First, it deletes any loose
+ objects that already exist in a pack-file; concurrent Git processes
+ will examine the pack-file for the object data instead of the loose
+ object. Second, it creates a new pack-file (starting with "loose-")
+ containing a batch of loose objects. The batch size is limited to 50
+ thousand objects to prevent the job from taking too long on a
+ repository with many loose objects. The `gc` task writes unreachable
+ objects as loose objects to be cleaned up by a later step only if
+ they are not re-added to a pack-file; for this reason it is not
+ advisable to enable both the `loose-objects` and `gc` tasks at the
+ same time.
+
OPTIONS
-------
--auto::
diff --git a/builtin/gc.c b/builtin/gc.c
index eb9c4d6147..248ccde3c3 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -880,6 +880,98 @@ static int maintenance_task_gc(struct maintenance_run_opts *opts)
return run_command(&child);
}
+static int prune_packed(struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
+ child.git_cmd = 1;
+ strvec_push(&child.args, "prune-packed");
+
+ if (opts->quiet)
+ strvec_push(&child.args, "--quiet");
+
+ return !!run_command(&child);
+}
+
+struct write_loose_object_data {
+ FILE *in;
+ int count;
+ int batch_size;
+};
+
+static int bail_on_loose(const struct object_id *oid,
+ const char *path,
+ void *data)
+{
+ return 1;
+}
+
+static int write_loose_object_to_stdin(const struct object_id *oid,
+ const char *path,
+ void *data)
+{
+ struct write_loose_object_data *d = (struct write_loose_object_data *)data;
+
+ fprintf(d->in, "%s\n", oid_to_hex(oid));
+
+ return ++(d->count) > d->batch_size;
+}
+
+static int pack_loose(struct maintenance_run_opts *opts)
+{
+ struct repository *r = the_repository;
+ int result = 0;
+ struct write_loose_object_data data;
+ struct child_process pack_proc = CHILD_PROCESS_INIT;
+
+ /*
+ * Do not start pack-objects process
+ * if there are no loose objects.
+ */
+ if (!for_each_loose_file_in_objdir(r->objects->odb->path,
+ bail_on_loose,
+ NULL, NULL, NULL))
+ return 0;
+
+ pack_proc.git_cmd = 1;
+
+ strvec_push(&pack_proc.args, "pack-objects");
+ if (opts->quiet)
+ strvec_push(&pack_proc.args, "--quiet");
+ strvec_pushf(&pack_proc.args, "%s/pack/loose", r->objects->odb->path);
+
+ pack_proc.in = -1;
+
+ if (start_command(&pack_proc)) {
+ error(_("failed to start 'git pack-objects' process"));
+ return 1;
+ }
+
+ data.in = xfdopen(pack_proc.in, "w");
+ data.count = 0;
+ data.batch_size = 50000;
+
+ for_each_loose_file_in_objdir(r->objects->odb->path,
+ write_loose_object_to_stdin,
+ NULL,
+ NULL,
+ &data);
+
+ fclose(data.in);
+
+ if (finish_command(&pack_proc)) {
+ error(_("failed to finish 'git pack-objects' process"));
+ result = 1;
+ }
+
+ return result;
+}
+
+static int maintenance_task_loose_objects(struct maintenance_run_opts *opts)
+{
+ return prune_packed(opts) || pack_loose(opts);
+}
+
typedef int maintenance_task_fn(struct maintenance_run_opts *opts);
/*
@@ -901,6 +993,7 @@ struct maintenance_task {
enum maintenance_task_label {
TASK_PREFETCH,
+ TASK_LOOSE_OBJECTS,
TASK_GC,
TASK_COMMIT_GRAPH,
@@ -913,6 +1006,10 @@ static struct maintenance_task tasks[] = {
"prefetch",
maintenance_task_prefetch,
},
+ [TASK_LOOSE_OBJECTS] = {
+ "loose-objects",
+ maintenance_task_loose_objects,
+ },
[TASK_GC] = {
"gc",
maintenance_task_gc,
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 0bade09c43..2e9e369786 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -86,4 +86,43 @@ test_expect_success 'prefetch multiple remotes' '
test_cmp_rev refs/remotes/remote2/two refs/prefetch/remote2/two
'
+test_expect_success 'loose-objects task' '
+ # Repack everything so we know the state of the object dir
+ git repack -adk &&
+
+ # Hack to stop maintenance from running during "git commit"
+ echo in use >.git/objects/maintenance.lock &&
+
+ # Assuming that "git commit" creates at least one loose object
+ test_commit create-loose-object &&
+ rm .git/objects/maintenance.lock &&
+
+ ls .git/objects >obj-dir-before &&
+ test_file_not_empty obj-dir-before &&
+ ls .git/objects/pack/*.pack >packs-before &&
+ test_line_count = 1 packs-before &&
+
+ # The first run creates a pack-file
+ # but does not delete loose objects.
+ git maintenance run --task=loose-objects &&
+ ls .git/objects >obj-dir-between &&
+ test_cmp obj-dir-before obj-dir-between &&
+ ls .git/objects/pack/*.pack >packs-between &&
+ test_line_count = 2 packs-between &&
+ ls .git/objects/pack/loose-*.pack >loose-packs &&
+ test_line_count = 1 loose-packs &&
+
+ # The second run deletes loose objects
+ # but does not create a pack-file.
+ git maintenance run --task=loose-objects &&
+ ls .git/objects >obj-dir-after &&
+ cat >expect <<-\EOF &&
+ info
+ pack
+ EOF
+ test_cmp expect obj-dir-after &&
+ ls .git/objects/pack/*.pack >packs-after &&
+ test_cmp packs-between packs-after
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/8] maintenance: add prefetch task
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:36 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.696.v3.git.1598380599.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
When working with very large repositories, an incremental 'git fetch'
command can download a large amount of data. If there are many other
users pushing to a common repo, then this data can rival the initial
pack-file size of a 'git clone' of a medium-size repo.
Users may want to keep the data on their local repos as close as
possible to the data on the remote repos by fetching periodically in
the background. This can break up a large daily fetch into several
smaller hourly fetches.
The task is called "prefetch" because it is work done in advance
of a foreground fetch to make that 'git fetch' command much faster.
However, if we simply ran 'git fetch <remote>' in the background,
then the user running a foregroudn 'git fetch <remote>' would lose
some important feedback when a new branch appears or an existing
branch updates. This is especially true if a remote branch is
force-updated and this isn't noticed by the user because it occurred
in the background. Further, the functionality of 'git push
--force-with-lease' becomes suspect.
When running 'git fetch <remote> <options>' in the background, use
the following options for careful updating:
1. --no-tags prevents getting a new tag when a user wants to see
the new tags appear in their foreground fetches.
2. --refmap= removes the configured refspec which usually updates
refs/remotes/<remote>/* with the refs advertised by the remote.
While this looks confusing, this was documented and tested by
b40a50264ac (fetch: document and test --refmap="", 2020-01-21),
including this sentence in the documentation:
Providing an empty `<refspec>` to the `--refmap` option
causes Git to ignore the configured refspecs and rely
entirely on the refspecs supplied as command-line arguments.
3. By adding a new refspec "+refs/heads/*:refs/prefetch/<remote>/*"
we can ensure that we actually load the new values somewhere in
our refspace while not updating refs/heads or refs/remotes. By
storing these refs here, the commit-graph job will update the
commit-graph with the commits from these hidden refs.
4. --prune will delete the refs/prefetch/<remote> refs that no
longer appear on the remote.
5. --no-write-fetch-head prevents updating FETCH_HEAD.
We've been using this step as a critical background job in Scalar
[1] (and VFS for Git). This solved a pain point that was showing up
in user reports: fetching was a pain! Users do not like waiting to
download the data that was created while they were away from their
machines. After implementing background fetch, the foreground fetch
commands sped up significantly because they mostly just update refs
and download a small amount of new data. The effect is especially
dramatic when paried with --no-show-forced-udpates (through
fetch.showForcedUpdates=false).
[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/FetchStep.cs
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-maintenance.txt | 15 +++++++++
builtin/gc.c | 51 +++++++++++++++++++++++++++++++
t/t7900-maintenance.sh | 26 ++++++++++++++++
3 files changed, 92 insertions(+)
diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
index 6abcb8255a..12668fccf7 100644
--- a/Documentation/git-maintenance.txt
+++ b/Documentation/git-maintenance.txt
@@ -47,6 +47,21 @@ commit-graph::
`commit-graph-chain` file. They will be deleted by a later run based
on the expiration delay.
+prefetch::
+ The `prefetch` task updates the object directory with the latest
+ objects from all registered remotes. For each remote, a `git fetch`
+ command is run. The refmap is custom to avoid updating local or remote
+ branches (those in `refs/heads` or `refs/remotes`). Instead, the
+ remote refs are stored in `refs/prefetch/<remote>/`. Also, tags are
+ not updated.
++
+This is done to avoid disrupting the remote-tracking branches. The end users
+expect these refs to stay unmoved unless they initiate a fetch. With prefetch
+task, however, the objects necessary to complete a later real fetch would
+already be obtained, so the real fetch would go faster. In the ideal case,
+it will just become an update to bunch of remote-tracking branches without
+any object transfer.
+
gc::
Clean up unnecessary files and optimize the local repository. "GC"
stands for "garbage collection," but this task performs many
diff --git a/builtin/gc.c b/builtin/gc.c
index c3bcdc1167..eb9c4d6147 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -29,6 +29,7 @@
#include "tree.h"
#include "promisor-remote.h"
#include "refs.h"
+#include "remote.h"
#define FAILED_RUN "failed to run %s"
@@ -816,6 +817,51 @@ static int maintenance_task_commit_graph(struct maintenance_run_opts *opts)
return 0;
}
+static int fetch_remote(const char *remote, struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
+ child.git_cmd = 1;
+ strvec_pushl(&child.args, "fetch", remote, "--prune", "--no-tags",
+ "--no-write-fetch-head", "--recurse-submodules=no",
+ "--refmap=", NULL);
+
+ if (opts->quiet)
+ strvec_push(&child.args, "--quiet");
+
+ strvec_pushf(&child.args, "+refs/heads/*:refs/prefetch/%s/*", remote);
+
+ return !!run_command(&child);
+}
+
+static int append_remote(struct remote *remote, void *cbdata)
+{
+ struct string_list *remotes = (struct string_list *)cbdata;
+
+ string_list_append(remotes, remote->name);
+ return 0;
+}
+
+static int maintenance_task_prefetch(struct maintenance_run_opts *opts)
+{
+ int result = 0;
+ struct string_list_item *item;
+ struct string_list remotes = STRING_LIST_INIT_DUP;
+
+ if (for_each_remote(append_remote, &remotes)) {
+ error(_("failed to fill remotes"));
+ result = 1;
+ goto cleanup;
+ }
+
+ for_each_string_list_item(item, &remotes)
+ result |= fetch_remote(item->string, opts);
+
+cleanup:
+ string_list_clear(&remotes, 0);
+ return result;
+}
+
static int maintenance_task_gc(struct maintenance_run_opts *opts)
{
struct child_process child = CHILD_PROCESS_INIT;
@@ -854,6 +900,7 @@ struct maintenance_task {
};
enum maintenance_task_label {
+ TASK_PREFETCH,
TASK_GC,
TASK_COMMIT_GRAPH,
@@ -862,6 +909,10 @@ enum maintenance_task_label {
};
static struct maintenance_task tasks[] = {
+ [TASK_PREFETCH] = {
+ "prefetch",
+ maintenance_task_prefetch,
+ },
[TASK_GC] = {
"gc",
maintenance_task_gc,
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 4f6a04ddb1..0bade09c43 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -60,4 +60,30 @@ test_expect_success 'run --task duplicate' '
test_i18ngrep "cannot be selected multiple times" err
'
+test_expect_success 'run --task=prefetch with no remotes' '
+ git maintenance run --task=prefetch 2>err &&
+ test_must_be_empty err
+'
+
+test_expect_success 'prefetch multiple remotes' '
+ git clone . clone1 &&
+ git clone . clone2 &&
+ git remote add remote1 "file://$(pwd)/clone1" &&
+ git remote add remote2 "file://$(pwd)/clone2" &&
+ git -C clone1 switch -c one &&
+ git -C clone2 switch -c two &&
+ test_commit -C clone1 one &&
+ test_commit -C clone2 two &&
+ GIT_TRACE2_EVENT="$(pwd)/run-prefetch.txt" git maintenance run --task=prefetch 2>/dev/null &&
+ fetchargs="--prune --no-tags --no-write-fetch-head --recurse-submodules=no --refmap= --quiet" &&
+ test_subcommand git fetch remote1 $fetchargs +refs/heads/\\*:refs/prefetch/remote1/\\* <run-prefetch.txt &&
+ test_subcommand git fetch remote2 $fetchargs +refs/heads/\\*:refs/prefetch/remote2/\\* <run-prefetch.txt &&
+ test_path_is_missing .git/refs/remotes &&
+ git log prefetch/remote1/one &&
+ git log prefetch/remote2/two &&
+ git fetch --all &&
+ test_cmp_rev refs/remotes/remote1/one refs/prefetch/remote1/one &&
+ test_cmp_rev refs/remotes/remote2/two refs/prefetch/remote2/two
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/8] Maintenance II: prefetch, loose-objects, incremental-repack tasks
From: Derrick Stolee via GitGitGadget @ 2020-08-25 18:36 UTC (permalink / raw)
To: git
Cc: sandals, steadmon, jrnieder, peff, congdanhqx, phillip.wood123,
emilyshaffer, sluongng, jonathantanmy, Derrick Stolee
In-Reply-To: <pull.696.v2.git.1597760730.gitgitgadget@gmail.com>
This series is based on v3 of part I (ds/maintenance-part-1) [2].
This patch series contains 9 patches that were going to be part of v4 of
ds/maintenance [1], but the discussion has gotten really long. To help, I'm
splitting out the portions that create and test the 'maintenance' builtin
from the additional tasks (prefetch, loose-objects, incremental-repack) that
can be brought in later.
[1]
https://lore.kernel.org/git/pull.671.git.1594131695.gitgitgadget@gmail.com/
[2]
https://lore.kernel.org/git/pull.695.v3.git.1598380426.gitgitgadget@gmail.com/
As detailed in [2], the 'git maintenance run' subcommand will run certain
tasks based on config options or the --task= arguments. The --auto option
indicates to the task to only run based on some internal check that there
has been "enough" change in that domain to merit the work. In the case of
the 'gc' task, this also reduces the amount of work done.
The new maintenance tasks in this series are:
* 'loose-objects' : prune packed loose objects, then create a new pack from
a batch of loose objects.
* 'pack-files' : expire redundant packs from the multi-pack-index, then
repack using the multi-pack-index's incremental repack strategy.
* 'prefetch' : fetch from each remote, storing the refs in 'refs/prefetch/
/'.
These tasks are all disabled by default, but can be enabled with config
options or run explicitly using "git maintenance run --task=".
Since [2] replaced the 'git gc --auto' calls with 'git maintenance run
--auto' at the end of some Git commands, users could replace the 'gc' task
with these lighter-weight changes for foreground maintenance.
The 'git maintenance' builtin has a 'run' subcommand so it can be extended
later with subcommands that manage background maintenance, such as 'start'
or 'stop'. These are not the subject of this series, as it is important to
focus on the maintenance activities themselves. I have an RFC series for
this available at [3].
[3]
https://lore.kernel.org/git/pull.680.git.1597857408.gitgitgadget@gmail.com/
Updates since v2
================
* Dropped "fetch: optionally allow disabling FETCH_HEAD update"
* A lot of fallout from the change in the option parsing in v3 of
Maintenance II.
* Dropped the "verify, and delete and rewrite on failure" logic from the
incremental-repack task. This might be added again later after it can be
tested more thoroughly.
Updates since v1 (of this series)
=================================
* PATCH 1 ("fetch: optionally allow disabling FETCH_HEAD update") was
rewritten on-list. Getting a version out with this patch is the main
reason for rolling a v2. (That, and Part I is re-rolled with a v2 and I
want to make sure this series applies cleanly.)
* The 'prefetch' and 'loose-objects' tasks had some review, but my proposed
changes were not acked, so they may need another review.
UPDATES since v3 of [1]
=======================
* The biggest change here is the use of "test_subcommand", based on
Jonathan Nieder's approach. This requires having the exact command-line
figured out, which now requires spelling out all --no- [quiet%7Cprogress]
options. I also added a bunch of "2>/dev/null" checks because of the
isatty(2) calls. Without that, the behavior will change depending on
whether the test is run with -x/-v or without.
* The 0x7FFF/0x7FFFFFFF constant problem is fixed with an EXPENSIVE test
that verifies it.
* The option parsing has changed to use a local struct and pass that struct
to the helper methods. This is instead of having a global singleton.
Thanks, -Stolee
Derrick Stolee (8):
maintenance: add prefetch task
maintenance: add loose-objects task
maintenance: create auto condition for loose-objects
midx: enable core.multiPackIndex by default
midx: use start_delayed_progress()
maintenance: add incremental-repack task
maintenance: auto-size incremental-repack batch
maintenance: add incremental-repack auto condition
Documentation/config/core.txt | 4 +-
Documentation/config/maintenance.txt | 18 ++
Documentation/git-maintenance.txt | 45 ++++
builtin/gc.c | 327 +++++++++++++++++++++++++++
midx.c | 23 +-
midx.h | 1 +
repo-settings.c | 6 +
repository.h | 2 +
t/t5319-multi-pack-index.sh | 15 +-
t/t7900-maintenance.sh | 191 ++++++++++++++++
10 files changed, 609 insertions(+), 23 deletions(-)
base-commit: 652a8eac57d04a51820c7a5b45031b50c5188e7b
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-696%2Fderrickstolee%2Fmaintenance%2Fgc-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-696/derrickstolee/maintenance/gc-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/696
Range-diff vs v2:
1: f3bc0b2d92 < -: ---------- fetch: optionally allow disabling FETCH_HEAD update
2: 8779c6c20d ! 1: da64c51a81 maintenance: add prefetch task
@@ Commit message
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
## Documentation/git-maintenance.txt ##
-@@ Documentation/git-maintenance.txt: since it will not expire `.graph` files that were in the previous
- `commit-graph-chain` file. They will be deleted by a later run based on
- the expiration delay.
+@@ Documentation/git-maintenance.txt: commit-graph::
+ `commit-graph-chain` file. They will be deleted by a later run based
+ on the expiration delay.
+prefetch::
+ The `prefetch` task updates the object directory with the latest
@@ builtin/gc.c
#define FAILED_RUN "failed to run %s"
-@@ builtin/gc.c: static int maintenance_task_commit_graph(struct maintenance_opts *opts)
- return 1;
+@@ builtin/gc.c: static int maintenance_task_commit_graph(struct maintenance_run_opts *opts)
+ return 0;
}
-+static int fetch_remote(const char *remote, struct maintenance_opts *opts)
++static int fetch_remote(const char *remote, struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
@@ builtin/gc.c: static int maintenance_task_commit_graph(struct maintenance_opts *
+ return 0;
+}
+
-+static int maintenance_task_prefetch(struct maintenance_opts *opts)
++static int maintenance_task_prefetch(struct maintenance_run_opts *opts)
+{
+ int result = 0;
+ struct string_list_item *item;
@@ builtin/gc.c: static int maintenance_task_commit_graph(struct maintenance_opts *
+ return result;
+}
+
- static int maintenance_task_gc(struct maintenance_opts *opts)
+ static int maintenance_task_gc(struct maintenance_run_opts *opts)
{
struct child_process child = CHILD_PROCESS_INIT;
@@ builtin/gc.c: struct maintenance_task {
3: 4fa9d298b9 ! 2: 75e846456b maintenance: add loose-objects task
@@ Documentation/git-maintenance.txt: gc::
--auto::
## builtin/gc.c ##
-@@ builtin/gc.c: static int maintenance_task_gc(struct maintenance_opts *opts)
+@@ builtin/gc.c: static int maintenance_task_gc(struct maintenance_run_opts *opts)
return run_command(&child);
}
-+static int prune_packed(struct maintenance_opts *opts)
++static int prune_packed(struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
@@ builtin/gc.c: static int maintenance_task_gc(struct maintenance_opts *opts)
+ return ++(d->count) > d->batch_size;
+}
+
-+static int pack_loose(struct maintenance_opts *opts)
++static int pack_loose(struct maintenance_run_opts *opts)
+{
+ struct repository *r = the_repository;
+ int result = 0;
@@ builtin/gc.c: static int maintenance_task_gc(struct maintenance_opts *opts)
+ return result;
+}
+
-+static int maintenance_task_loose_objects(struct maintenance_opts *opts)
++static int maintenance_task_loose_objects(struct maintenance_run_opts *opts)
+{
+ return prune_packed(opts) || pack_loose(opts);
+}
+
- typedef int maintenance_task_fn(struct maintenance_opts *opts);
+ typedef int maintenance_task_fn(struct maintenance_run_opts *opts);
/*
@@ builtin/gc.c: struct maintenance_task {
4: 3432bc3167 = 3: d6e382c43e maintenance: create auto condition for loose-objects
5: 0ee2434bdb = 4: d0f2ec70d9 midx: enable core.multiPackIndex by default
6: e157ea8dd7 = 5: 2cd3c803d9 midx: use start_delayed_progress()
7: a8d956dad6 ! 6: 0dd26bb584 maintenance: add incremental-repack task
@@ Commit message
it requires doing some calculations that are better isolated to
a separate change.
- Each of the above steps update the multi-pack-index file. After
- each step, we verify the new multi-pack-index. If the new
- multi-pack-index is corrupt, then delete the multi-pack-index,
- rewrite it from scratch, and stop doing the later steps of the
- job. This is intended to be an extra-safe check without leaving
- a repo with many pack-files without a multi-pack-index.
-
These steps are based on a similar background maintenance step in
Scalar (and VFS for Git) [1]. This was incredibly effective for
users of the Windows OS repository. After using the same VFS for Git
@@ builtin/gc.c
#define FAILED_RUN "failed to run %s"
-@@ builtin/gc.c: static int maintenance_task_loose_objects(struct maintenance_opts *opts)
+@@ builtin/gc.c: static int maintenance_task_loose_objects(struct maintenance_run_opts *opts)
return prune_packed(opts) || pack_loose(opts);
}
-+static int multi_pack_index_write(struct maintenance_opts *opts)
++static int multi_pack_index_write(struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
@@ builtin/gc.c: static int maintenance_task_loose_objects(struct maintenance_opts
+ return 0;
+}
+
-+static int rewrite_multi_pack_index(struct maintenance_opts *opts)
-+{
-+ struct repository *r = the_repository;
-+ char *midx_name = get_midx_filename(r->objects->odb->path);
-+
-+ unlink(midx_name);
-+ free(midx_name);
-+
-+ return multi_pack_index_write(opts);
-+}
-+
-+static int multi_pack_index_verify(struct maintenance_opts *opts,
-+ const char *message)
-+{
-+ struct child_process child = CHILD_PROCESS_INIT;
-+
-+ child.git_cmd = 1;
-+ strvec_pushl(&child.args, "multi-pack-index", "verify", NULL);
-+
-+ if (opts->quiet)
-+ strvec_push(&child.args, "--no-progress");
-+
-+ if (run_command(&child)) {
-+ warning(_("'git multi-pack-index verify' failed %s"), message);
-+ return 1;
-+ }
-+
-+ return 0;
-+}
-+
-+static int multi_pack_index_expire(struct maintenance_opts *opts)
++static int multi_pack_index_expire(struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
@@ builtin/gc.c: static int maintenance_task_loose_objects(struct maintenance_opts
+ return 0;
+}
+
-+static int multi_pack_index_repack(struct maintenance_opts *opts)
++static int multi_pack_index_repack(struct maintenance_run_opts *opts)
+{
+ struct child_process child = CHILD_PROCESS_INIT;
+
@@ builtin/gc.c: static int maintenance_task_loose_objects(struct maintenance_opts
+ return 0;
+}
+
-+static int maintenance_task_incremental_repack(struct maintenance_opts *opts)
++static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts)
+{
+ prepare_repo_settings(the_repository);
+ if (!the_repository->settings.core_multi_pack_index) {
@@ builtin/gc.c: static int maintenance_task_loose_objects(struct maintenance_opts
+
+ if (multi_pack_index_write(opts))
+ return 1;
-+ if (multi_pack_index_verify(opts, "after initial write"))
-+ return rewrite_multi_pack_index(opts);
+ if (multi_pack_index_expire(opts))
+ return 1;
-+ if (multi_pack_index_verify(opts, "after expire step"))
-+ return !!rewrite_multi_pack_index(opts);
+ if (multi_pack_index_repack(opts))
+ return 1;
-+ if (multi_pack_index_verify(opts, "after repack step"))
-+ return !!rewrite_multi_pack_index(opts);
+ return 0;
+}
+
- typedef int maintenance_task_fn(struct maintenance_opts *opts);
+ typedef int maintenance_task_fn(struct maintenance_run_opts *opts);
/*
@@ builtin/gc.c: struct maintenance_task {
8: f0e7276755 ! 7: f3b25a9927 maintenance: auto-size incremental-repack batch
@@ Commit message
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
## builtin/gc.c ##
-@@ builtin/gc.c: static int multi_pack_index_expire(struct maintenance_opts *opts)
+@@ builtin/gc.c: static int multi_pack_index_expire(struct maintenance_run_opts *opts)
return 0;
}
@@ builtin/gc.c: static int multi_pack_index_expire(struct maintenance_opts *opts)
+ return result_size;
+}
+
- static int multi_pack_index_repack(struct maintenance_opts *opts)
+ static int multi_pack_index_repack(struct maintenance_run_opts *opts)
{
struct child_process child = CHILD_PROCESS_INIT;
-@@ builtin/gc.c: static int multi_pack_index_repack(struct maintenance_opts *opts)
+@@ builtin/gc.c: static int multi_pack_index_repack(struct maintenance_run_opts *opts)
if (opts->quiet)
strvec_push(&child.args, "--no-progress");
9: 5659a23ad5 ! 8: e9bb32f53a maintenance: add incremental-repack auto condition
@@ builtin/gc.c
#define FAILED_RUN "failed to run %s"
-@@ builtin/gc.c: static int maintenance_task_loose_objects(struct maintenance_opts *opts)
+@@ builtin/gc.c: static int maintenance_task_loose_objects(struct maintenance_run_opts *opts)
return prune_packed(opts) || pack_loose(opts);
}
@@ builtin/gc.c: static int maintenance_task_loose_objects(struct maintenance_opts
+ return count >= incremental_repack_auto_limit;
+}
+
- static int multi_pack_index_write(struct maintenance_opts *opts)
+ static int multi_pack_index_write(struct maintenance_run_opts *opts)
{
struct child_process child = CHILD_PROCESS_INIT;
@@ builtin/gc.c: static struct maintenance_task tasks[] = {
--
gitgitgadget
^ 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