* Re: [PATCH 2/2] parse-options: use and require int pointer for OPT_CMDMODE
From: René Scharfe @ 2023-10-03 8:49 UTC (permalink / raw)
To: phillip.wood, Junio C Hamano; +Cc: Jeff King, Oswald Buddenhagen, Git List
In-Reply-To: <000ff1b9-e7a5-4dd6-bc61-4b6761f66997@gmail.com>
Am 18.09.23 um 21:48 schrieb Phillip Wood:
> On 18/09/2023 18:11, Junio C Hamano wrote:
>>> Then in builtin/am.c at the top level we'd add
>>>
>>> MAKE_CMDMODE_SETTER(resume_type)
>>>
>>> and change the option definitions to look like
>>>
>>> OPT_CMDMODE(0, "continue", resume_type, &resume.mode, ...)
>>
>> Yup, that is ergonomic and corrects "The shape of a particular enum
>> may not match 'int'" issue nicely. I do not know how severe the
>> problem is that it is not quite type safe that we cannot enforce
>> resume_type is the same as typeof(resume.mode) here, though.
>
> We could use gcc's __builtin_types_compatible_p() if we're prepared to have two definitions of OPT_CMDMODE_F
>
> #if defined(__GNUC__)
> #define OPT_CMDMODE_F(s, l, n, v, h, i, f) { \
> ...
> .defval (i) + \
> BUILD_ASSERT_OR_ZERO(__builtin_types_compatible_p(enum n, __typeof__(v))), \
> }
> #else
> #define OPT_CMDMODE_F(s, l, n, v, h, i, f) { \
> ...
> .defval (i), \
> }
> #endif
>
> Best Wishes
>
> Phillip
That would work, but we can do a type check without a compiler builtin
by creating a function for that purpose. How about this?
--- >8 ---
Subject: [PATCH] parse-options: make OPT_CMDMODE type-safe
Some uses of OPT_CMDMODE point to an enum as value, but the option
parser dereferences it as an int pointer. Depending on the platform,
pointers to int and enums can be incompatible.
Add value accessor functions to struct option to dereference the pointer
safely, use them for PARSE_OPT_CMDMODE error detection and in the new
OPTION_SET_VALUE option type, add a typed version of OPT_CMDMODE named
OPT_CMDMODE_T, make OPT_CMDMODE only accept int and convert enum users
to declare the appropriate type.
The accessor functions for each type must be defined using
DEFINE_OPTION_VALUE_TYPE. OPT_CMDMODE needs the ones for int, so we
define them centrally in parse-options.h.
builtin/am.c::parse_opt_show_current_patch() uses the value pointer just
to calculate the offset of the enclosing struct resume_mode, making the
pointer type inconsequential. Use the correct type instead of int *
regardless, for consistency.
The enum used with OPT_CMDMODE in builtin/replace.c was anonymous. Give
it a name to allow specifying it in DEFINE_OPTION_VALUE_TYPE and then
OPT_CMDMODE_T.
Suggested-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Documentation/technical/api-parse-options.txt | 7 ++++
builtin/am.c | 32 ++++++++++------
builtin/help.c | 37 +++++++++++--------
builtin/ls-tree.c | 18 +++++----
builtin/rebase.c | 33 ++++++++++-------
builtin/replace.c | 37 ++++++++++++-------
builtin/stripspace.c | 14 ++++---
parse-options.c | 20 +++++++++-
parse-options.h | 36 ++++++++++++++++--
9 files changed, 159 insertions(+), 75 deletions(-)
diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 61fa6ee167..3958ab7c94 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -278,6 +278,13 @@ There are some macros to easily define options:
option has already set its value to the same `int_var`.
In new commands consider using subcommands instead.
+`OPT_CMDMODE_T(short, long, type_name, &enum_var, description, enum_val)`::
+ Same as `OPT_CMDMODE`, but allows specifying an enum variable
+ instead of an int.
+ Requires `DEFINE_OPTION_VALUE_TYPE(type_name, enum_type);` at
+ file scope to declare `type_name`; `enum_type` must be the type
+ of `enum_var`.
+
`OPT_SUBCOMMAND(long, &fn_ptr, subcommand_fn)`::
Define a subcommand. `subcommand_fn` is put into `fn_ptr` when
this subcommand is used.
diff --git a/builtin/am.c b/builtin/am.c
index 6655059a57..4642dc5099 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2268,6 +2268,8 @@ enum resume_type {
RESUME_ALLOW_EMPTY,
};
+DEFINE_OPTION_VALUE_TYPE(resume_type, enum resume_type);
+
struct resume_mode {
enum resume_type mode;
enum show_patch_type sub_mode;
@@ -2275,7 +2277,7 @@ struct resume_mode {
static int parse_opt_show_current_patch(const struct option *opt, const char *arg, int unset)
{
- int *opt_value = opt->value;
+ enum resume_type *opt_value = opt->value;
struct resume_mode *resume = container_of(opt_value, struct resume_mode, mode);
/*
@@ -2388,27 +2390,33 @@ int cmd_am(int argc, const char **argv, const char *prefix)
PARSE_OPT_NOARG),
OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL,
N_("override error message when patch failure occurs")),
- OPT_CMDMODE(0, "continue", &resume.mode,
+ OPT_CMDMODE_T(0, "continue", resume_type, &resume.mode,
N_("continue applying patches after resolving a conflict"),
RESUME_RESOLVED),
- OPT_CMDMODE('r', "resolved", &resume.mode,
+ OPT_CMDMODE_T('r', "resolved", resume_type, &resume.mode,
N_("synonyms for --continue"),
RESUME_RESOLVED),
- OPT_CMDMODE(0, "skip", &resume.mode,
+ OPT_CMDMODE_T(0, "skip", resume_type, &resume.mode,
N_("skip the current patch"),
RESUME_SKIP),
- OPT_CMDMODE(0, "abort", &resume.mode,
+ OPT_CMDMODE_T(0, "abort", resume_type, &resume.mode,
N_("restore the original branch and abort the patching operation"),
RESUME_ABORT),
- OPT_CMDMODE(0, "quit", &resume.mode,
+ OPT_CMDMODE_T(0, "quit", resume_type, &resume.mode,
N_("abort the patching operation but keep HEAD where it is"),
RESUME_QUIT),
- { OPTION_CALLBACK, 0, "show-current-patch", &resume.mode,
- "(diff|raw)",
- N_("show the patch being applied"),
- PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
- parse_opt_show_current_patch, RESUME_SHOW_PATCH },
- OPT_CMDMODE(0, "allow-empty", &resume.mode,
+ {
+ .type = OPTION_CALLBACK,
+ .long_name = "show-current-patch",
+ OPTION_VALUE(resume_type, &resume.mode),
+ .argh = "(diff|raw)",
+ .help = N_("show the patch being applied"),
+ .flags = PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG |
+ PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
+ .callback = parse_opt_show_current_patch,
+ .defval = RESUME_SHOW_PATCH,
+ },
+ OPT_CMDMODE_T(0, "allow-empty", resume_type, &resume.mode,
N_("record the empty patch as an empty commit"),
RESUME_ALLOW_EMPTY),
OPT_BOOL(0, "committer-date-is-author-date",
diff --git a/builtin/help.c b/builtin/help.c
index dc1fbe2b98..48437c8636 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -52,6 +52,8 @@ static enum help_action {
HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION,
} cmd_mode;
+DEFINE_OPTION_VALUE_TYPE(help_action, enum help_action);
+
static const char *html_path;
static int verbose = 1;
static enum help_format help_format = HELP_FORMAT_NONE;
@@ -59,8 +61,8 @@ static int exclude_guides;
static int show_external_commands = -1;
static int show_aliases = -1;
static struct option builtin_help_options[] = {
- OPT_CMDMODE('a', "all", &cmd_mode, N_("print all available commands"),
- HELP_ACTION_ALL),
+ OPT_CMDMODE_T('a', "all", help_action, &cmd_mode,
+ N_("print all available commands"), HELP_ACTION_ALL),
OPT_BOOL(0, "external-commands", &show_external_commands,
N_("show external commands in --all")),
OPT_BOOL(0, "aliases", &show_aliases, N_("show aliases in --all")),
@@ -72,20 +74,23 @@ static struct option builtin_help_options[] = {
HELP_FORMAT_INFO),
OPT__VERBOSE(&verbose, N_("print command description")),
- OPT_CMDMODE('g', "guides", &cmd_mode, N_("print list of useful guides"),
- HELP_ACTION_GUIDES),
- OPT_CMDMODE(0, "user-interfaces", &cmd_mode,
- N_("print list of user-facing repository, command and file interfaces"),
- HELP_ACTION_USER_INTERFACES),
- OPT_CMDMODE(0, "developer-interfaces", &cmd_mode,
- N_("print list of file formats, protocols and other developer interfaces"),
- HELP_ACTION_DEVELOPER_INTERFACES),
- OPT_CMDMODE('c', "config", &cmd_mode, N_("print all configuration variable names"),
- HELP_ACTION_CONFIG),
- OPT_CMDMODE_F(0, "config-for-completion", &cmd_mode, "",
- HELP_ACTION_CONFIG_FOR_COMPLETION, PARSE_OPT_HIDDEN),
- OPT_CMDMODE_F(0, "config-sections-for-completion", &cmd_mode, "",
- HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION, PARSE_OPT_HIDDEN),
+ OPT_CMDMODE_T('g', "guides", help_action, &cmd_mode,
+ N_("print list of useful guides"), HELP_ACTION_GUIDES),
+ OPT_CMDMODE_T(0, "user-interfaces", help_action, &cmd_mode,
+ N_("print list of user-facing repository, command and file interfaces"),
+ HELP_ACTION_USER_INTERFACES),
+ OPT_CMDMODE_T(0, "developer-interfaces", help_action, &cmd_mode,
+ N_("print list of file formats, protocols and other developer interfaces"),
+ HELP_ACTION_DEVELOPER_INTERFACES),
+ OPT_CMDMODE_T('c', "config", help_action, &cmd_mode,
+ N_("print all configuration variable names"),
+ HELP_ACTION_CONFIG),
+ OPT_CMDMODE_T_F(0, "config-for-completion", help_action, &cmd_mode, "",
+ HELP_ACTION_CONFIG_FOR_COMPLETION, PARSE_OPT_HIDDEN),
+ OPT_CMDMODE_T_F(0, "config-sections-for-completion",
+ help_action, &cmd_mode, "",
+ HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION,
+ PARSE_OPT_HIDDEN),
OPT_END(),
};
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 209d2dc0d5..2e7faf510d 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -339,6 +339,8 @@ static struct ls_tree_cmdmode_to_fmt ls_tree_cmdmode_format[] = {
},
};
+DEFINE_OPTION_VALUE_TYPE(cmdmode, enum ls_tree_cmdmode);
+
int cmd_ls_tree(int argc, const char **argv, const char *prefix)
{
struct object_id oid;
@@ -358,14 +360,14 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
LS_SHOW_TREES),
OPT_BOOL('z', NULL, &null_termination,
N_("terminate entries with NUL byte")),
- OPT_CMDMODE('l', "long", &cmdmode, N_("include object size"),
- MODE_LONG),
- OPT_CMDMODE(0, "name-only", &cmdmode, N_("list only filenames"),
- MODE_NAME_ONLY),
- OPT_CMDMODE(0, "name-status", &cmdmode, N_("list only filenames"),
- MODE_NAME_STATUS),
- OPT_CMDMODE(0, "object-only", &cmdmode, N_("list only objects"),
- MODE_OBJECT_ONLY),
+ OPT_CMDMODE_T('l', "long", cmdmode, &cmdmode,
+ N_("include object size"), MODE_LONG),
+ OPT_CMDMODE_T(0, "name-only", cmdmode, &cmdmode,
+ N_("list only filenames"), MODE_NAME_ONLY),
+ OPT_CMDMODE_T(0, "name-status", cmdmode, &cmdmode,
+ N_("list only filenames"), MODE_NAME_STATUS),
+ OPT_CMDMODE_T(0, "object-only", cmdmode, &cmdmode,
+ N_("list only objects"), MODE_OBJECT_ONLY),
OPT_BOOL(0, "full-name", &full_name, N_("use full path names")),
OPT_BOOL(0, "full-tree", &full_tree,
N_("list entire tree; not just current directory "
diff --git a/builtin/rebase.c b/builtin/rebase.c
index ed15accec9..50739327bd 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -75,6 +75,8 @@ enum action {
ACTION_SHOW_CURRENT_PATCH
};
+DEFINE_OPTION_VALUE_TYPE(action, enum action);
+
static const char *action_names[] = {
"undefined",
"continue",
@@ -1116,20 +1118,23 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
OPT_BIT(0, "no-ff", &options.flags,
N_("cherry-pick all commits, even if unchanged"),
REBASE_FORCE),
- OPT_CMDMODE(0, "continue", &options.action, N_("continue"),
- ACTION_CONTINUE),
- OPT_CMDMODE(0, "skip", &options.action,
- N_("skip current patch and continue"), ACTION_SKIP),
- OPT_CMDMODE(0, "abort", &options.action,
- N_("abort and check out the original branch"),
- ACTION_ABORT),
- OPT_CMDMODE(0, "quit", &options.action,
- N_("abort but keep HEAD where it is"), ACTION_QUIT),
- OPT_CMDMODE(0, "edit-todo", &options.action, N_("edit the todo list "
- "during an interactive rebase"), ACTION_EDIT_TODO),
- OPT_CMDMODE(0, "show-current-patch", &options.action,
- N_("show the patch file being applied or merged"),
- ACTION_SHOW_CURRENT_PATCH),
+ OPT_CMDMODE_T(0, "continue", action, &options.action,
+ N_("continue"), ACTION_CONTINUE),
+ OPT_CMDMODE_T(0, "skip", action, &options.action,
+ N_("skip current patch and continue"),
+ ACTION_SKIP),
+ OPT_CMDMODE_T(0, "abort", action, &options.action,
+ N_("abort and check out the original branch"),
+ ACTION_ABORT),
+ OPT_CMDMODE_T(0, "quit", action, &options.action,
+ N_("abort but keep HEAD where it is"),
+ ACTION_QUIT),
+ OPT_CMDMODE_T(0, "edit-todo", action, &options.action,
+ N_("edit the todo list during an interactive rebase"),
+ ACTION_EDIT_TODO),
+ OPT_CMDMODE_T(0, "show-current-patch", action, &options.action,
+ N_("show the patch file being applied or merged"),
+ ACTION_SHOW_CURRENT_PATCH),
OPT_CALLBACK_F(0, "apply", &options, NULL,
N_("use apply strategies to rebase"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG,
diff --git a/builtin/replace.c b/builtin/replace.c
index da59600ad2..09aebc9de2 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -539,27 +539,36 @@ static int convert_graft_file(int force)
return -1;
}
+enum cmdmode {
+ MODE_UNSPECIFIED = 0,
+ MODE_LIST,
+ MODE_DELETE,
+ MODE_EDIT,
+ MODE_GRAFT,
+ MODE_CONVERT_GRAFT_FILE,
+ MODE_REPLACE
+};
+
+DEFINE_OPTION_VALUE_TYPE(cmdmode, enum cmdmode);
int cmd_replace(int argc, const char **argv, const char *prefix)
{
int force = 0;
int raw = 0;
const char *format = NULL;
- enum {
- MODE_UNSPECIFIED = 0,
- MODE_LIST,
- MODE_DELETE,
- MODE_EDIT,
- MODE_GRAFT,
- MODE_CONVERT_GRAFT_FILE,
- MODE_REPLACE
- } cmdmode = MODE_UNSPECIFIED;
+ enum cmdmode cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
- OPT_CMDMODE('l', "list", &cmdmode, N_("list replace refs"), MODE_LIST),
- OPT_CMDMODE('d', "delete", &cmdmode, N_("delete replace refs"), MODE_DELETE),
- OPT_CMDMODE('e', "edit", &cmdmode, N_("edit existing object"), MODE_EDIT),
- OPT_CMDMODE('g', "graft", &cmdmode, N_("change a commit's parents"), MODE_GRAFT),
- OPT_CMDMODE(0, "convert-graft-file", &cmdmode, N_("convert existing graft file"), MODE_CONVERT_GRAFT_FILE),
+ OPT_CMDMODE_T('l', "list", cmdmode, &cmdmode,
+ N_("list replace refs"), MODE_LIST),
+ OPT_CMDMODE_T('d', "delete", cmdmode, &cmdmode,
+ N_("delete replace refs"), MODE_DELETE),
+ OPT_CMDMODE_T('e', "edit", cmdmode, &cmdmode,
+ N_("edit existing object"), MODE_EDIT),
+ OPT_CMDMODE_T('g', "graft", cmdmode, &cmdmode,
+ N_("change a commit's parents"), MODE_GRAFT),
+ OPT_CMDMODE_T(0, "convert-graft-file", cmdmode, &cmdmode,
+ N_("convert existing graft file"),
+ MODE_CONVERT_GRAFT_FILE),
OPT_BOOL_F('f', "force", &force, N_("replace the ref if it exists"),
PARSE_OPT_NOCOMPLETE),
OPT_BOOL(0, "raw", &raw, N_("do not pretty-print contents for --edit")),
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index 7b700a9fb1..82b9a95a4f 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -29,6 +29,8 @@ enum stripspace_mode {
COMMENT_LINES
};
+DEFINE_OPTION_VALUE_TYPE(mode, enum stripspace_mode);
+
int cmd_stripspace(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
@@ -36,12 +38,12 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
int nongit;
const struct option options[] = {
- OPT_CMDMODE('s', "strip-comments", &mode,
- N_("skip and remove all lines starting with comment character"),
- STRIP_COMMENTS),
- OPT_CMDMODE('c', "comment-lines", &mode,
- N_("prepend comment character and space to each line"),
- COMMENT_LINES),
+ OPT_CMDMODE_T('s', "strip-comments", mode, &mode,
+ N_("skip and remove all lines starting with comment character"),
+ STRIP_COMMENTS),
+ OPT_CMDMODE_T('c', "comment-lines", mode, &mode,
+ N_("prepend comment character and space to each line"),
+ COMMENT_LINES),
OPT_END()
};
diff --git a/parse-options.c b/parse-options.c
index e8e076c3a6..63a2247128 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -85,7 +85,7 @@ static enum parse_opt_result opt_command_mode_error(
if (that == opt ||
!(that->flags & PARSE_OPT_CMDMODE) ||
that->value != opt->value ||
- that->defval != *(int *)opt->value)
+ that->defval != opt->get_value(opt->value))
continue;
if (that->long_name)
@@ -122,7 +122,8 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
* is not a grave error, so let it pass.
*/
if ((opt->flags & PARSE_OPT_CMDMODE) &&
- *(int *)opt->value && *(int *)opt->value != opt->defval)
+ opt->get_value(opt->value) &&
+ opt->get_value(opt->value) != opt->defval)
return opt_command_mode_error(opt, all_opts, flags);
switch (opt->type) {
@@ -160,6 +161,10 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
*(int *)opt->value = unset ? 0 : opt->defval;
return 0;
+ case OPTION_SET_VALUE:
+ opt->set_value(opt->value, unset ? 0 : opt->defval);
+ return 0;
+
case OPTION_STRING:
if (unset)
*(const char **)opt->value = NULL;
@@ -483,11 +488,21 @@ static void parse_options_check(const struct option *opts)
if (opts->type == OPTION_SET_INT && !opts->defval &&
opts->long_name && !(opts->flags & PARSE_OPT_NONEG))
optbug(opts, "OPTION_SET_INT 0 should not be negatable");
+ if (opts->type == OPTION_SET_VALUE && !opts->defval &&
+ opts->long_name && !(opts->flags & PARSE_OPT_NONEG))
+ optbug(opts, "OPTION_SET_VALUE 0 should not be negatable");
+ if (opts->type == OPTION_SET_VALUE &&
+ (!opts->get_value || !opts->set_value))
+ optbug(opts, "OPTION_SET_VALUE requires accessors");
+ if ((opts->flags & PARSE_OPT_CMDMODE) &&
+ (!opts->get_value || !opts->set_value))
+ optbug(opts, "PARSE_OPT_CMDMODE requires accessors");
switch (opts->type) {
case OPTION_COUNTUP:
case OPTION_BIT:
case OPTION_NEGBIT:
case OPTION_SET_INT:
+ case OPTION_SET_VALUE:
case OPTION_NUMBER:
if ((opts->flags & PARSE_OPT_OPTARG) ||
!(opts->flags & PARSE_OPT_NOARG))
@@ -611,6 +626,7 @@ static void show_negated_gitcomp(const struct option *opts, int show_all,
case OPTION_NEGBIT:
case OPTION_COUNTUP:
case OPTION_SET_INT:
+ case OPTION_SET_VALUE:
has_unset_form = 1;
break;
default:
diff --git a/parse-options.h b/parse-options.h
index 57a7fe9d91..764e7f7896 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -20,6 +20,7 @@ enum parse_opt_type {
OPTION_BITOP,
OPTION_COUNTUP,
OPTION_SET_INT,
+ OPTION_SET_VALUE,
/* options with arguments (usually) */
OPTION_STRING,
OPTION_INTEGER,
@@ -158,8 +159,34 @@ struct option {
parse_opt_ll_cb *ll_callback;
intptr_t extra;
parse_opt_subcommand_fn *subcommand_fn;
+ intptr_t (*get_value)(void *);
+ void (*set_value)(void *, intptr_t);
};
+#define DEFINE_OPTION_VALUE_TYPE(type_name, type) \
+static inline intptr_t type_name##__get(void *void_ptr) \
+{ \
+ type *ptr = void_ptr; \
+ return (intptr_t)*ptr; \
+} \
+static inline void type_name##__set(void *void_ptr, intptr_t value) \
+{ \
+ type *ptr = void_ptr; \
+ *ptr = (type)value; \
+} \
+static inline void *type_name##__check(type *ptr) \
+{ \
+ return ptr; \
+} \
+static inline void *type_name##__check(type *ptr)
+
+DEFINE_OPTION_VALUE_TYPE(int, int);
+
+#define OPTION_VALUE(type_name, v) \
+ .get_value = type_name##__get, \
+ .set_value = type_name##__set, \
+ .value = (1 ? (v) : type_name##__check(v))
+
#define OPT_BIT_F(s, l, v, h, b, f) { \
.type = OPTION_BIT, \
.short_name = (s), \
@@ -256,15 +283,18 @@ struct option {
.flags = PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, \
.defval = 1, \
}
-#define OPT_CMDMODE_F(s, l, v, h, i, f) { \
- .type = OPTION_SET_INT, \
+
+#define OPT_CMDMODE_T_F(s, l, t, v, h, i, f) { \
+ .type = OPTION_SET_VALUE, \
.short_name = (s), \
.long_name = (l), \
- .value = (v), \
+ OPTION_VALUE(t, (v)), \
.help = (h), \
.flags = PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG | (f), \
.defval = (i), \
}
+#define OPT_CMDMODE_T(s, l, t, v, h, i) OPT_CMDMODE_T_F(s, l, t, v, h, i, 0)
+#define OPT_CMDMODE_F(s, l, v, h, i, f) OPT_CMDMODE_T_F(s, l, int, v, h, i, f)
#define OPT_CMDMODE(s, l, v, h, i) OPT_CMDMODE_F(s, l, v, h, i, 0)
#define OPT_INTEGER(s, l, v, h) OPT_INTEGER_F(s, l, v, h, 0)
--
2.42.0
^ permalink raw reply related
* Re: [PATCH 2/2] parse-options: use and require int pointer for OPT_CMDMODE
From: René Scharfe @ 2023-10-03 8:49 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Git List, Jeff King, Junio C Hamano
In-Reply-To: <ZQwdsfh1GQX0IOQs@ugly>
Am 21.09.23 um 12:40 schrieb Oswald Buddenhagen:
> On Wed, Sep 20, 2023 at 10:18:10AM +0200, René Scharfe wrote:
>> MSVC warns about all combinations.
>>
> yes, though that's not a problem: after we established that the
> underlying type is int, we can just have a cast in the initializer
> macro.
MSVC does some weird things in general; it's tempting to ignore it.
>>> so how about simply adding a (configure) test to ensure that
>>> there is actually no problem, and calling it a day?
>
>> If we base it on type size then we're making assumptions that I
>> find hard to justify.
>>
> the only one i can think of is signedness. i think this can be safely
> ignored as long as we use only small positive integers.
I don't fully understand the pointer-sign warning, so I'm not
confident enough to silence it.
René
^ permalink raw reply
* [PATCH] doc/cat-file: clarify description regarding various command forms
From: Štěpán Němec @ 2023-10-03 8:25 UTC (permalink / raw)
To: git; +Cc: avarab
The DESCRIPTION's "first form" is actually the 1st, 2nd, 3rd and 5th
form in SYNOPSIS, the "second form" is the 4th one.
Interestingly, this state of affairs was introduced in
97fe7250753b (cat-file docs: fix SYNOPSIS and "-h" output, 2021-12-28)
with the claim of "Now the two will match again." ("the two" being
DESCRIPTION and SYNOPSIS)...
Ordinals are hard, let's try talking about batch and non-batch mode
instead.
Signed-off-by: Štěpán Němec <stepnem@smrk.net>
---
Documentation/git-cat-file.txt | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 0e4936d18263..1957f90335a4 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -20,12 +20,15 @@ SYNOPSIS
DESCRIPTION
-----------
-In its first form, the command provides the content or the type of an object in
+This command can operate in two modes, depending on whether an option from
+the `--batch` family is specified.
+
+In non-batch mode, the command provides the content or the type of an object in
the repository. The type is required unless `-t` or `-p` is used to find the
object type, or `-s` is used to find the object size, or `--textconv` or
`--filters` is used (which imply type "blob").
-In the second form, a list of objects (separated by linefeeds) is provided on
+In batch mode, a list of objects (separated by linefeeds) is provided on
stdin, and the SHA-1, type, and size of each object is printed on stdout. The
output format can be overridden using the optional `<format>` argument. If
either `--textconv` or `--filters` was specified, the input is expected to
base-commit: d0e8084c65cbf949038ae4cc344ac2c2efd77415
--
2.42.0
^ permalink raw reply related
* [PATCH 5/5] t/README: fix multi-prerequisite example
From: Štěpán Němec @ 2023-10-03 8:21 UTC (permalink / raw)
To: git
In-Reply-To: <20231003082107.3002173-1-stepnem@smrk.net>
With the broken quoting the test wouldn't even parse correctly, but
there's also the '==' instead of POSIX '=' (of the shells I tested,
busybox ash, bash and ksh (93 and OpenBSD) accept '==', dash and zsh do
not), and 'print 2' from Python 2 days.
(I assume the test failing due to 3 != 4 is intentional or immaterial.)
Fixes: 93a572461386 ("test-lib: Add support for multiple test prerequisites")
Signed-off-by: Štěpán Němec <stepnem@smrk.net>
---
t/README | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/README b/t/README
index a3e87385d065..5b6b8eeb2a5f 100644
--- a/t/README
+++ b/t/README
@@ -886,7 +886,7 @@ see test-lib-functions.sh for the full list and their options.
rare case where your test depends on more than one:
test_expect_success PERL,PYTHON 'yo dawg' \
- ' test $(perl -E 'print eval "1 +" . qx[python -c "print 2"]') == "4" '
+ ' test $(perl -E '\''print eval "1 +" . qx[python -c "print(2)"]'\'') = "4" '
- test_expect_failure [<prereq>] <message> <script>
--
2.42.0
^ permalink raw reply related
* [PATCH 4/5] doc/gitk: s/sticked/stuck/
From: Štěpán Němec @ 2023-10-03 8:21 UTC (permalink / raw)
To: git
In-Reply-To: <20231003082107.3002173-1-stepnem@smrk.net>
The terminology was changed in b0d12fc9b23a (Use the word 'stuck'
instead of 'sticked').
Signed-off-by: Štěpán Němec <stepnem@smrk.net>
---
Documentation/gitk.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index d50e9ed10e04..c2213bb77b38 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -26,7 +26,7 @@ changes each commit introduces are shown. Finally, it supports some
gitk-specific options.
gitk generally only understands options with arguments in the
-'sticked' form (see linkgit:gitcli[7]) due to limitations in the
+'stuck' form (see linkgit:gitcli[7]) due to limitations in the
command-line parser.
rev-list options and arguments
--
2.42.0
^ permalink raw reply related
* [PATCH 1/5] Fix some typos, grammar or wording issues in the documentation
From: Štěpán Němec @ 2023-10-03 8:21 UTC (permalink / raw)
To: git
Signed-off-by: Štěpán Němec <stepnem@smrk.net>
---
Documentation/SubmittingPatches | 10 +++++-----
Documentation/diff-options.txt | 2 +-
Documentation/git-branch.txt | 2 +-
Documentation/git-range-diff.txt | 2 +-
Documentation/git.txt | 2 +-
Documentation/gitattributes.txt | 2 +-
Documentation/giteveryday.txt | 2 +-
contrib/README | 4 ++--
strbuf.h | 8 ++++----
t/README | 30 ++++++++++++++----------------
10 files changed, 31 insertions(+), 33 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 973d7a81d449..1259549cd488 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -393,8 +393,8 @@ mailing list{security-ml}, instead of the public mailing list.
Learn to use format-patch and send-email if possible. These commands
are optimized for the workflow of sending patches, avoiding many ways
-your existing e-mail client that is optimized for "multipart/*" mime
-type e-mails to corrupt and render your patches unusable.
+your existing e-mail client (often optimized for "multipart/*" MIME
+type e-mails) might render your patches unusable.
People on the Git mailing list need to be able to read and
comment on the changes you are submitting. It is important for
@@ -515,8 +515,8 @@ repositories.
git://git.ozlabs.org/~paulus/gitk
- Those who are interested in improve gitk can volunteer to help Paul
- in maintaining it cf. <YntxL/fTplFm8lr6@cleo>.
+ Those who are interested in improving gitk can volunteer to help Paul
+ maintain it, cf. <YntxL/fTplFm8lr6@cleo>.
- `po/` comes from the localization coordinator, Jiang Xin:
@@ -556,7 +556,7 @@ help you find out who they are.
In any time between the (2)-(3) cycle, the maintainer may pick it up
from the list and queue it to `seen`, in order to make it easier for
-people play with it without having to pick up and apply the patch to
+people to play with it without having to pick up and apply the patch to
their trees themselves.
[[patch-status]]
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 35fae7c87c8a..ee256ec077be 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -301,7 +301,7 @@ ifndef::git-format-patch[]
-z::
ifdef::git-log[]
- Separate the commits with NULs instead of with new newlines.
+ Separate the commits with NULs instead of newlines.
+
Also, when `--raw` or `--numstat` has been given, do not munge
pathnames and use NULs as output field terminators.
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index d207da9101a5..4395aa935438 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -324,7 +324,7 @@ superproject's "origin/main", but tracks the submodule's "origin/main".
multiple times, in which case the last key becomes the primary
key. The keys supported are the same as those in `git
for-each-ref`. Sort order defaults to the value configured for the
- `branch.sort` variable if exists, or to sorting based on the
+ `branch.sort` variable if it exists, or to sorting based on the
full refname (including `refs/...` prefix). This lists
detached HEAD (if present) first, then local branches and
finally remote-tracking branches. See linkgit:git-config[1].
diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 0b393715d707..6c728dbe44b2 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt
@@ -166,7 +166,7 @@ A typical output of `git range-diff` would look like this:
In this example, there are 3 old and 3 new commits, where the developer
removed the 3rd, added a new one before the first two, and modified the
-commit message of the 2nd commit as well its diff.
+commit message of the 2nd commit as well as its diff.
When the output goes to a terminal, it is color-coded by default, just
like regular `git diff`'s output. In addition, the first line (adding a
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 11228956cd5e..b246dc86c3f3 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -96,7 +96,7 @@ foo.bar= ...`) sets `foo.bar` to the empty string which `git config
to avoid ambiguity with `<name>` containing one.
+
This is useful for cases where you want to pass transitory
-configuration options to git, but are doing so on OS's where
+configuration options to git, but are doing so on OSes where
other processes might be able to read your cmdline
(e.g. `/proc/self/cmdline`), but not your environ
(e.g. `/proc/self/environ`). That behavior is the default on
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 6deb89a29677..7cdc68f54a5c 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -1151,7 +1151,7 @@ will be stored via placeholder `%P`.
^^^^^^^^^^^^^^^^^^^^^^
This attribute controls the length of conflict markers left in
-the work tree file during a conflicted merge. Only setting to
+the work tree file during a conflicted merge. Only setting
the value to a positive integer has any meaningful effect.
For example, this line in `.gitattributes` can be used to tell the merge
diff --git a/Documentation/giteveryday.txt b/Documentation/giteveryday.txt
index faba2ef0881c..f74a33fb35b9 100644
--- a/Documentation/giteveryday.txt
+++ b/Documentation/giteveryday.txt
@@ -229,7 +229,7 @@ without a formal "merging". Or longhand +
git am -3 -k`
An alternate participant submission mechanism is using the
-`git request-pull` or pull-request mechanisms (e.g as used on
+`git request-pull` or pull-request mechanisms (e.g. as used on
GitHub (www.github.com) to notify your upstream of your
contribution.
diff --git a/contrib/README b/contrib/README
index 05f291c1f1d3..d3a327d04074 100644
--- a/contrib/README
+++ b/contrib/README
@@ -24,14 +24,14 @@ lesser degree various foreign SCM interfaces, so you know the
drill.
I expect that things that start their life in the contrib/ area
-to graduate out of contrib/ once they mature, either by becoming
+graduate out of contrib/ once they mature, either by becoming
projects on their own, or moving to the toplevel directory. On
the other hand, I expect I'll be proposing removal of disused
and inactive ones from time to time.
If you have new things to add to this area, please first propose
it on the git mailing list, and after a list discussion proves
-there are some general interests (it does not have to be a
+there is general interest (it does not have to be a
list-wide consensus for a tool targeted to a relatively narrow
audience -- for example I do not work with projects whose
upstream is svn, so I have no use for git-svn myself, but it is
diff --git a/strbuf.h b/strbuf.h
index fd43c46433a4..e959caca876a 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -12,9 +12,9 @@
struct string_list;
/**
- * strbuf's are meant to be used with all the usual C string and memory
+ * strbufs are meant to be used with all the usual C string and memory
* APIs. Given that the length of the buffer is known, it's often better to
- * use the mem* functions than a str* one (memchr vs. strchr e.g.).
+ * use the mem* functions than a str* one (e.g., memchr vs. strchr).
* Though, one has to be careful about the fact that str* functions often
* stop on NULs and that strbufs may have embedded NULs.
*
@@ -24,7 +24,7 @@ struct string_list;
* strbufs have some invariants that are very important to keep in mind:
*
* - The `buf` member is never NULL, so it can be used in any usual C
- * string operations safely. strbuf's _have_ to be initialized either by
+ * string operations safely. strbufs _have_ to be initialized either by
* `strbuf_init()` or by `= STRBUF_INIT` before the invariants, though.
*
* Do *not* assume anything on what `buf` really is (e.g. if it is
@@ -37,7 +37,7 @@ struct string_list;
*
* - The `buf` member is a byte array that has at least `len + 1` bytes
* allocated. The extra byte is used to store a `'\0'`, allowing the
- * `buf` member to be a valid C-string. Every strbuf function ensure this
+ * `buf` member to be a valid C-string. All strbuf functions ensure this
* invariant is preserved.
*
* NOTE: It is OK to "play" with the buffer directly if you work it this
diff --git a/t/README b/t/README
index 61080859899b..a3e87385d065 100644
--- a/t/README
+++ b/t/README
@@ -262,8 +262,8 @@ The argument for --run, <test-selector>, is a list of description
substrings or globs or individual test numbers or ranges with an
optional negation prefix (of '!') that define what tests in a test
suite to include (or exclude, if negated) in the run. A range is two
-numbers separated with a dash and matches a range of tests with both
-ends been included. You may omit the first or the second number to
+numbers separated with a dash and matches an inclusive range of tests
+to run. You may omit the first or the second number to
mean "from the first test" or "up to the very last test" respectively.
The argument to --run is split on commas into separate strings,
@@ -274,10 +274,10 @@ text that you want to match includes a comma, use the glob character
on all tests that match either the glob *rebase* or the glob
*merge?cherry-pick*.
-If --run starts with an unprefixed number or range the initial
-set of tests to run is empty. If the first item starts with '!'
+If --run starts with an unprefixed number or range, the initial
+set of tests to run is empty. If the first item starts with '!',
all the tests are added to the initial set. After initial set is
-determined every test number or range is added or excluded from
+determined, every test number or range is added or excluded from
the set one by one, from left to right.
For example, to run only tests up to a specific test (21), one
@@ -579,11 +579,10 @@ This test harness library does the following things:
Recommended style
-----------------
-Here are some recommented styles when writing test case.
- - Keep test title the same line with test helper function itself.
+ - Keep test titles and helper function invocations on the same line.
- Take test_expect_success helper for example, write it like:
+ For example, with test_expect_success, write it like:
test_expect_success 'test title' '
... test body ...
@@ -595,10 +594,9 @@ Here are some recommented styles when writing test case.
'test title' \
'... test body ...'
+ - End the line with an opening single quote.
- - End the line with a single quote.
-
- - Indent the body of here-document, and use "<<-" instead of "<<"
+ - Indent here-document bodies, and use "<<-" instead of "<<"
to strip leading TABs used for indentation:
test_expect_success 'test something' '
@@ -624,7 +622,7 @@ Here are some recommented styles when writing test case.
'
- Quote or escape the EOF delimiter that begins a here-document if
- there is no parameter and other expansion in it, to signal readers
+ there is no parameter or other expansion in it, to signal readers
that they can skim it more casually:
cmd <<-\EOF
@@ -638,7 +636,7 @@ Do's & don'ts
Here are a few examples of things you probably should and shouldn't do
when writing tests.
-Here are the "do's:"
+The "do's:"
- Put all code inside test_expect_success and other assertions.
@@ -1237,8 +1235,8 @@ and it knows that the object ID of an empty tree is a certain
because the things the very basic core test tries to achieve is
to serve as a basis for people who are changing the Git internals
drastically. For these people, after making certain changes,
-not seeing failures from the basic test _is_ a failure. And
-such drastic changes to the core Git that even changes these
+not seeing failures from the basic test _is_ a failure. Any
+Git core changes so drastic that they change even these
otherwise supposedly stable object IDs should be accompanied by
an update to t0000-basic.sh.
@@ -1248,7 +1246,7 @@ knowledge of the core Git internals. If all the test scripts
hardcoded the object IDs like t0000-basic.sh does, that defeats
the purpose of t0000-basic.sh, which is to isolate that level of
validation in one place. Your test also ends up needing
-updating when such a change to the internal happens, so do _not_
+an update whenever the internals change, so do _not_
do it and leave the low level of validation to t0000-basic.sh.
Test coverage
--
2.42.0
^ permalink raw reply related
* [PATCH 3/5] git-jump: admit to passing merge mode args to ls-files
From: Štěpán Němec @ 2023-10-03 8:21 UTC (permalink / raw)
To: git
In-Reply-To: <20231003082107.3002173-1-stepnem@smrk.net>
There's even an example of such usage in the README.
Fixes: 67ba13e5a4b2 ("git-jump: pass "merge" arguments to ls-files")
Signed-off-by: Štěpán Němec <stepnem@smrk.net>
---
contrib/git-jump/git-jump | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 40c4b0d11107..47e0c557e63f 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -9,7 +9,7 @@ The <mode> parameter is one of:
diff: elements are diff hunks. Arguments are given to diff.
-merge: elements are merge conflicts. Arguments are ignored.
+merge: elements are merge conflicts. Arguments are given to ls-files -u.
grep: elements are grep hits. Arguments are given to git grep or, if
configured, to the command in `jump.grepCmd`.
--
2.42.0
^ permalink raw reply related
* [PATCH 2/5] doc/diff-options: improve wording of the log.diffMerges mention
From: Štěpán Němec @ 2023-10-03 8:21 UTC (permalink / raw)
To: git
In-Reply-To: <20231003082107.3002173-1-stepnem@smrk.net>
Fix the grammar ("which default value is") and reword to match other
similar descriptions (say "configuration variable" instead of
"parameter", link to git-config(1)).
Signed-off-by: Štěpán Němec <stepnem@smrk.net>
---
Documentation/diff-options.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index ee256ec077be..48a5012748dd 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -53,9 +53,9 @@ ifdef::git-log[]
-m:::
This option makes diff output for merge commits to be shown in
the default format. `-m` will produce the output only if `-p`
- is given as well. The default format could be changed using
- `log.diffMerges` configuration parameter, which default value
- is `separate`.
+ is given as well. The default format can be specified using
+ the configuration variable `log.diffMerges` (see
+ linkgit:git-config[1]). It defaults to `separate`.
+
--diff-merges=first-parent:::
--diff-merges=1:::
--
2.42.0
^ permalink raw reply related
* [PATCH] test-lib: make sure TEST_DIRECTORY has no trailing slash
From: Štěpán Němec @ 2023-10-03 8:23 UTC (permalink / raw)
To: git
Turns out having `pwd` (which TEST_DIRECTORY defaults to) print $PWD
with a trailing slash isn't very difficult, in my case it went something
like
; tmux new-window -c ~/src/git/t/
[in the new window]
; sh ./t0000-basic.sh
PANIC: Running in a /home/stepnem/src/git/t/ that doesn't end in '/t'?
; pwd
/home/stepnem/src/git/t/
(tmux(1) apparently sets PWD in the environment in addition to calling
chdir(2), which seems enough to make at least some shells preserve the
trailing slash in `pwd` output.)
Strip the trailing slash, if present, to prevent bailing out with the
PANIC message in such cases.
Signed-off-by: Štěpán Němec <stepnem@smrk.net>
---
t/test-lib.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1656c9eed006..3b6f1a17e349 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -35,6 +35,7 @@ else
# needing to exist.
TEST_DIRECTORY=$(cd "$TEST_DIRECTORY" && pwd) || exit 1
fi
+TEST_DIRECTORY="${TEST_DIRECTORY%/}"
if test -z "$TEST_OUTPUT_DIRECTORY"
then
# Similarly, override this to store the test-results subdir
base-commit: d0e8084c65cbf949038ae4cc344ac2c2efd77415
--
2.42.0
^ permalink raw reply related
* Is git-p4 maintained?
From: Yuri @ 2023-10-03 7:41 UTC (permalink / raw)
To: Git Mailing List
There are problems with the 'git p4 submit' command.
The 'git p4 submit' command first asks many questions like:
Unfortunately applying the change failed!
//xx/xx/xx/xx/xx/xx/xx#92 - was edit, reverted
What do you want to do?
[s]kip this commit but apply the rest, or [q]uit?
The files that it complains about aren't even involved in the commit
that is being pushed.
After all the questions are answered with "s" - git-p4 often fails to
push commits.
Deleted files and added files cause problems during 'git p4 submit' and
when such files are in the commit - 'git p4 submit' prints some error
messages and fails to push commits.
git-p4 says that it can't apply some patches and fails when it is
pushing commits that delete or add files.
The workaround is to place the deleted file back and remove the added
files before the 'git p4 submit' operation.
Is git-p4 maintained?
Is there any chance for these problems to be fixed?
Thanks,
Yuri
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2023, #01; Mon, 2)
From: Sergey Organov @ 2023-10-03 7:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqedic35u4.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
[...]
> * so/diff-merges-d (2023-09-11) 2 commits
> - diff-merges: introduce '-d' option
> - diff-merges: improve --diff-merges documentation
>
> Teach a new "-d" option that shows the patch against the first
> parent for merge commits (which is "--diff-merges=first-parent -p").
Which happens to naturally mean "show diff for all commits" for the
user.
>
> Letting a less useful combination of options squat on short-and-sweet
> "-d" feels dubious. source:
> <20230909125446.142715-1-sorganov@gmail.com>
I believe I've addressed this in details in my reply here:
<87o7hok8dx.fsf@osv.gnss.ru>, and got no further objections from you
since then, so I figure I'd ask to finally let the patch in.
To summarize my position here, "-d" meaning "show me *d*iff for all
commits", as implemented in the patch, is very mnemonic, has natural
semantics for "-d" in the context of "git log", and is straight to the
point. Therefore it is indeed short-and-sweet compared to the only
alternative proposed: "follow first parent only while traversing history
and show me diffs for all commits", that would indeed need a different
short-cut, if any.
Thanks,
-- Sergey Organov
^ permalink raw reply
* VENDORS EOI
From: Abu Dhabi National Oil Company @ 2023-10-02 22:07 UTC (permalink / raw)
To: git
Greetings of the day,
We are inviting your esteemed company for vendor registration and
intending partners for Abu Dhabi National Oil Company (ADNOC)
2023/2024 projects.
These projects are open for all companies around the world, if
you have intention to participate in the process, please confirm
your interest by asking for Vendor Questionnaire and EOI.
We appreciate your interest in this invitation, and look forward
to your early response.
Kind Regards,
Mr. Mohamed Ghazi B.
Senior Project Manager
Projects@adnoc-purchase.com
^ permalink raw reply
* batch-command wishlist [was: [TOPIC 02/12] Libification Goals and Progress]
From: Eric Wong @ 2023-10-03 0:52 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Taylor Blau
Taylor Blau <me@ttaylorr.com> wrote:
> * (Jonathan Tan) back to process isolation: is the short lifetime of the process
> important?
> * (Taylor Blau) seems like an impossible goal to be able to do multi-command
> executions in a single process, the code is just not designed for it.
-- Split out from https://lore.kernel.org/git/ZRrfN2lbg14IOLiK@nand.local/
Thanks for posting in an accessible format for non-JS/video users.
> * (Junio) is anybody using the `git cat-file --batch-command` mode that switches
> between batch and batch-check.
I've started using --batch-command, but still have to test
backwards compatibility with git 2.35 to ensure users of older
git aren't left out. I still try to support git 1.8.x, even...
But it would be nice if --batch-command grew more functionality:
* ability to add/remove alternates
* ability to specify a preferred alternate for a lookup[1]
* detect unlinked packs/removed repos
Not sure if cat-file is the place for it, but a persistent
process to deal with:
* `git config -f FILENAME ...' (especially --get-urlmatch --type=FOO)
* approxidate parsing for other tools[2]
Would also be nice...
Maybe I'll find the time and patience to implement this... *shrug*
Building C projects is pretty slow for me, even with -O0.
[1] I'm potentially dealing with 50-100k alternates, after all;
and potentially config files in the 300-500k line range...
> * (Patrick Steinhardt) they are longer lived, but only "middle" long-lived.
> GitLab limits the maximum runtime, on the order of ~minutes, at which point
> they are reaped.
> * (Taylor Blau) lots of issues besides memory leaks that would become an issue
> * (Jeff Hostetler) would be nice to keep memory-hungry components pinned across
> multiple command-equivalents.
> * (Taylor Blau): same issue as reading configuration.
sidenote: __attribute__((cleanup)) found in gcc + clang + tinycc
has greatly improved my life. Perhaps only supporting Free software
compilers and cross-compiling for everything else is in order :P
The only gotcha I've noticed with ((cleanup)) is it doesn't fire
on longjmp, but that's not a problem for git.
I've also been abusing more Perl5 over the years as a code
generator and better CPP to make dealing with tedious tasks
more acceptable.
[2] I did recently license the code of a standalone C++ executable
as GPL-2+ so I can copy approxidate parsing from git and
perhaps figure out enough C++ to use Xapian query parser
bindings instead of the hacky `git rev-parse --since=' thing
I do.
^ permalink raw reply
* [PATCH v2 3/3] builtin/repack.c: implement support for `--max-cruft-size`
From: Taylor Blau @ 2023-10-03 0:44 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt, Eric Sunshine
In-Reply-To: <cover.1696293862.git.me@ttaylorr.com>
Cruft packs are an alternative mechanism for storing a collection of
unreachable objects whose mtimes are recent enough to avoid being
pruned out of the repository.
When cruft packs were first introduced back in b757353676
(builtin/pack-objects.c: --cruft without expiration, 2022-05-20) and
a7d493833f (builtin/pack-objects.c: --cruft with expiration,
2022-05-20), the recommended workflow consisted of:
- Repacking periodically, either by packing anything loose in the
repository (via `git repack -d`) or producing a geometric sequence
of packs (via `git repack --geometric=<d> -d`).
- Every so often, splitting the repository into two packs, one cruft
to store the unreachable objects, and another non-cruft pack to
store the reachable objects.
Repositories may (out of band with the above) choose periodically to
prune out some unreachable objects which have aged out of the grace
period by generating a pack with `--cruft-expiration=<approxidate>`.
This allowed repositories to maintain relatively few packs on average,
and quarantine unreachable objects together in a cruft pack, avoiding
the pitfalls of holding unreachable objects as loose while they age out
(for more, see some of the details in 3d89a8c118
(Documentation/technical: add cruft-packs.txt, 2022-05-20)).
This all works, but can be costly from an I/O-perspective when
frequently repacking a repository that has many unreachable objects.
This problem is exacerbated when those unreachable objects are rarely
(if every) pruned.
Since there is at most one cruft pack in the above scheme, each time we
update the cruft pack it must be rewritten from scratch. Because much of
the pack is reused, this is a relatively inexpensive operation from a
CPU-perspective, but is very costly in terms of I/O since we end up
rewriting basically the same pack (plus any new unreachable objects that
have entered the repository since the last time a cruft pack was
generated).
At the time, we decided against implementing more robust support for
multiple cruft packs. This patch implements that support which we were
lacking.
Introduce a new option `--max-cruft-size` which allows repositories to
accumulate cruft packs up to a given size, after which point a new
generation of cruft packs can accumulate until it reaches the maximum
size, and so on. To generate a new cruft pack, the process works like
so:
- Sort a list of any existing cruft packs in ascending order of pack
size.
- Starting from the beginning of the list, group cruft packs together
while the accumulated size is smaller than the maximum specified
pack size.
- Combine the objects in these cruft packs together into a new cruft
pack, along with any other unreachable objects which have since
entered the repository.
Once a cruft pack grows beyond the size specified via `--max-cruft-size`
the pack is effectively frozen. This limits the I/O churn up to a
quadratic function of the value specified by the `--max-cruft-size`
option, instead of behaving quadratically in the number of total
unreachable objects.
When pruning unreachable objects, we bypass the new code paths which
combine small cruft packs together, and instead start from scratch,
passing in the appropriate `--max-pack-size` down to `pack-objects`,
putting it in charge of keeping the resulting set of cruft packs sized
correctly.
This may seem like further I/O churn, but in practice it isn't so bad.
We could prune old cruft packs for whom all or most objects are removed,
and then generate a new cruft pack with just the remaining set of
objects. But this additional complexity buys us relatively little,
because most objects end up being pruned anyway, so the I/O churn is
well contained.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/config/gc.txt | 6 +
Documentation/git-gc.txt | 7 +
Documentation/git-repack.txt | 11 ++
builtin/gc.c | 7 +
builtin/repack.c | 134 +++++++++++++++++--
t/t6500-gc.sh | 27 ++++
t/t7704-repack-cruft.sh | 245 +++++++++++++++++++++++++++++++++++
7 files changed, 426 insertions(+), 11 deletions(-)
diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index ca47eb2008..83ebc2de55 100644
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -86,6 +86,12 @@ gc.cruftPacks::
linkgit:git-repack[1]) instead of as loose objects. The default
is `true`.
+gc.maxCruftSize::
+ Limit the size of new cruft packs when repacking. When
+ specified in addition to `--max-cruft-size`, the command line
+ option takes priority. See the `--max-cruft-size` option of
+ linkgit:git-repack[1].
+
gc.pruneExpire::
When 'git gc' is run, it will call 'prune --expire 2.weeks.ago'
(and 'repack --cruft --cruft-expiration 2.weeks.ago' if using
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 90806fd26a..fa0541b416 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -59,6 +59,13 @@ be performed as well.
cruft pack instead of storing them as loose objects. `--cruft`
is on by default.
+--max-cruft-size=<n>::
+ When packing unreachable objects into a cruft pack, limit the
+ size of new cruft packs to be at most `<n>`. Overrides any
+ value specified via the `gc.maxCruftSize` configuration. See
+ the `--max-cruft-size` option of linkgit:git-repack[1] for
+ more.
+
--prune=<date>::
Prune loose objects older than date (default is 2 weeks ago,
overridable by the config variable `gc.pruneExpire`).
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 4017157949..fbfc72e1b2 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -74,6 +74,17 @@ to the new separate pack will be written.
immediately instead of waiting for the next `git gc` invocation.
Only useful with `--cruft -d`.
+--max-cruft-size=<n>::
+ Repack cruft objects into packs as large as `<n>` bytes before
+ creating new packs. As long as there are enough cruft packs
+ smaller than `<n>`, repacking will cause a new cruft pack to
+ be created containing objects from any combined cruft packs,
+ along with any new unreachable objects. Cruft packs larger than
+ `<n>` will not be modified. When the new cruft pack is larger
+ than `<n>` bytes, it will be split into multiple packs, all of
+ which are guaranteed to be at most `<n>` bytes in size. Only
+ useful with `--cruft -d`.
+
--expire-to=<dir>::
Write a cruft pack containing pruned objects (if any) to the
directory `<dir>`. This option is useful for keeping a copy of
diff --git a/builtin/gc.c b/builtin/gc.c
index 00192ae5d3..c97c9fb046 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -52,6 +52,7 @@ static const char * const builtin_gc_usage[] = {
static int pack_refs = 1;
static int prune_reflogs = 1;
static int cruft_packs = 1;
+static unsigned long max_cruft_size;
static int aggressive_depth = 50;
static int aggressive_window = 250;
static int gc_auto_threshold = 6700;
@@ -163,6 +164,7 @@ static void gc_config(void)
git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
git_config_get_bool("gc.autodetach", &detach_auto);
git_config_get_bool("gc.cruftpacks", &cruft_packs);
+ git_config_get_ulong("gc.maxcruftsize", &max_cruft_size);
git_config_get_expiry("gc.pruneexpire", &prune_expire);
git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
git_config_get_expiry("gc.logexpiry", &gc_log_expire);
@@ -347,6 +349,9 @@ static void add_repack_all_option(struct string_list *keep_pack)
strvec_push(&repack, "--cruft");
if (prune_expire)
strvec_pushf(&repack, "--cruft-expiration=%s", prune_expire);
+ if (max_cruft_size)
+ strvec_pushf(&repack, "--max-cruft-size=%lu",
+ max_cruft_size);
} else {
strvec_push(&repack, "-A");
if (prune_expire)
@@ -575,6 +580,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
N_("prune unreferenced objects"),
PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
OPT_BOOL(0, "cruft", &cruft_packs, N_("pack unreferenced objects separately")),
+ OPT_MAGNITUDE(0, "max-cruft-size", &max_cruft_size,
+ N_("with --cruft, limit the size of new cruft packs")),
OPT_BOOL(0, "aggressive", &aggressive, N_("be more thorough (increased runtime)")),
OPT_BOOL_F(0, "auto", &auto_gc, N_("enable auto-gc mode"),
PARSE_OPT_NOCOMPLETE),
diff --git a/builtin/repack.c b/builtin/repack.c
index 8a5bbb9cba..04770b15fe 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -27,6 +27,7 @@
#define PACK_CRUFT 4
#define DELETE_PACK 1
+#define RETAIN_PACK 2
static int pack_everything;
static int delta_base_offset = 1;
@@ -116,11 +117,26 @@ static void pack_mark_for_deletion(struct string_list_item *item)
item->util = (void*)((uintptr_t)item->util | DELETE_PACK);
}
+static void pack_unmark_for_deletion(struct string_list_item *item)
+{
+ item->util = (void*)((uintptr_t)item->util & ~DELETE_PACK);
+}
+
static int pack_is_marked_for_deletion(struct string_list_item *item)
{
return (uintptr_t)item->util & DELETE_PACK;
}
+static void pack_mark_retained(struct string_list_item *item)
+{
+ item->util = (void*)((uintptr_t)item->util | RETAIN_PACK);
+}
+
+static int pack_is_retained(struct string_list_item *item)
+{
+ return (uintptr_t)item->util & RETAIN_PACK;
+}
+
static void mark_packs_for_deletion_1(struct string_list *names,
struct string_list *list)
{
@@ -133,17 +149,39 @@ static void mark_packs_for_deletion_1(struct string_list *names,
if (len < hexsz)
continue;
sha1 = item->string + len - hexsz;
- /*
- * Mark this pack for deletion, which ensures that this
- * pack won't be included in a MIDX (if `--write-midx`
- * was given) and that we will actually delete this pack
- * (if `-d` was given).
- */
- if (!string_list_has_string(names, sha1))
+
+ if (pack_is_retained(item)) {
+ pack_unmark_for_deletion(item);
+ } else if (!string_list_has_string(names, sha1)) {
+ /*
+ * Mark this pack for deletion, which ensures
+ * that this pack won't be included in a MIDX
+ * (if `--write-midx` was given) and that we
+ * will actually delete this pack (if `-d` was
+ * given).
+ */
pack_mark_for_deletion(item);
+ }
}
}
+static void retain_cruft_pack(struct existing_packs *existing,
+ struct packed_git *cruft)
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct string_list_item *item;
+
+ strbuf_addstr(&buf, pack_basename(cruft));
+ strbuf_strip_suffix(&buf, ".pack");
+
+ item = string_list_lookup(&existing->cruft_packs, buf.buf);
+ if (!item)
+ BUG("could not find cruft pack '%s'", pack_basename(cruft));
+
+ pack_mark_retained(item);
+ strbuf_release(&buf);
+}
+
static void mark_packs_for_deletion(struct existing_packs *existing,
struct string_list *names)
@@ -225,6 +263,8 @@ static void collect_pack_filenames(struct existing_packs *existing,
}
string_list_sort(&existing->kept_packs);
+ string_list_sort(&existing->non_kept_packs);
+ string_list_sort(&existing->cruft_packs);
strbuf_release(&buf);
}
@@ -806,6 +846,72 @@ static void remove_redundant_bitmaps(struct string_list *include,
strbuf_release(&path);
}
+static int existing_cruft_pack_cmp(const void *va, const void *vb)
+{
+ struct packed_git *a = *(struct packed_git **)va;
+ struct packed_git *b = *(struct packed_git **)vb;
+
+ if (a->pack_size < b->pack_size)
+ return -1;
+ if (a->pack_size > b->pack_size)
+ return 1;
+ return 0;
+}
+
+static void collapse_small_cruft_packs(FILE *in, size_t max_size,
+ struct existing_packs *existing)
+{
+ struct packed_git **existing_cruft, *p;
+ struct strbuf buf = STRBUF_INIT;
+ size_t total_size = 0;
+ size_t existing_cruft_nr = 0;
+ size_t i;
+
+ ALLOC_ARRAY(existing_cruft, existing->cruft_packs.nr);
+
+ for (p = get_all_packs(the_repository); p; p = p->next) {
+ if (!(p->is_cruft && p->pack_local))
+ continue;
+
+ strbuf_reset(&buf);
+ strbuf_addstr(&buf, pack_basename(p));
+ strbuf_strip_suffix(&buf, ".pack");
+
+ if (!string_list_has_string(&existing->cruft_packs, buf.buf))
+ continue;
+
+ if (existing_cruft_nr >= existing->cruft_packs.nr)
+ BUG("too many cruft packs (found %"PRIuMAX", but knew "
+ "of %"PRIuMAX")",
+ (uintmax_t)existing_cruft_nr + 1,
+ (uintmax_t)existing->cruft_packs.nr);
+ existing_cruft[existing_cruft_nr++] = p;
+ }
+
+ QSORT(existing_cruft, existing_cruft_nr, existing_cruft_pack_cmp);
+
+ for (i = 0; i < existing_cruft_nr; i++) {
+ size_t proposed;
+
+ p = existing_cruft[i];
+ proposed = st_add(total_size, p->pack_size);
+
+ if (proposed <= max_size) {
+ total_size = proposed;
+ fprintf(in, "-%s\n", pack_basename(p));
+ } else {
+ retain_cruft_pack(existing, p);
+ fprintf(in, "%s\n", pack_basename(p));
+ }
+ }
+
+ for (i = 0; i < existing->non_kept_packs.nr; i++)
+ fprintf(in, "-%s.pack\n",
+ existing->non_kept_packs.items[i].string);
+
+ strbuf_release(&buf);
+}
+
static int write_cruft_pack(const struct pack_objects_args *args,
const char *destination,
const char *pack_prefix,
@@ -853,10 +959,14 @@ static int write_cruft_pack(const struct pack_objects_args *args,
in = xfdopen(cmd.in, "w");
for_each_string_list_item(item, names)
fprintf(in, "%s-%s.pack\n", pack_prefix, item->string);
- for_each_string_list_item(item, &existing->non_kept_packs)
- fprintf(in, "-%s.pack\n", item->string);
- for_each_string_list_item(item, &existing->cruft_packs)
- fprintf(in, "-%s.pack\n", item->string);
+ if (args->max_pack_size && !cruft_expiration) {
+ collapse_small_cruft_packs(in, args->max_pack_size, existing);
+ } else {
+ for_each_string_list_item(item, &existing->non_kept_packs)
+ fprintf(in, "-%s.pack\n", item->string);
+ for_each_string_list_item(item, &existing->cruft_packs)
+ fprintf(in, "-%s.pack\n", item->string);
+ }
for_each_string_list_item(item, &existing->kept_packs)
fprintf(in, "%s.pack\n", item->string);
fclose(in);
@@ -919,6 +1029,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
PACK_CRUFT),
OPT_STRING(0, "cruft-expiration", &cruft_expiration, N_("approxidate"),
N_("with --cruft, expire objects older than this")),
+ OPT_MAGNITUDE(0, "max-cruft-size", &cruft_po_args.max_pack_size,
+ N_("with --cruft, limit the size of new cruft packs")),
OPT_BOOL('d', NULL, &delete_redundant,
N_("remove redundant packs, and run git-prune-packed")),
OPT_BOOL('f', NULL, &po_args.no_reuse_delta,
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 69509d0c11..0d6b5c3b27 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -303,6 +303,33 @@ test_expect_success 'gc.bigPackThreshold ignores cruft packs' '
)
'
+cruft_max_size_opts="git repack -d -l --cruft --cruft-expiration=2.weeks.ago"
+
+test_expect_success 'setup for --max-cruft-size tests' '
+ git init cruft--max-size &&
+ (
+ cd cruft--max-size &&
+ prepare_cruft_history
+ )
+'
+
+test_expect_success '--max-cruft-size sets appropriate repack options' '
+ GIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size \
+ gc --cruft --max-cruft-size=1M &&
+ test_subcommand $cruft_max_size_opts --max-cruft-size=1048576 <trace2.txt
+'
+
+test_expect_success 'gc.maxCruftSize sets appropriate repack options' '
+ GIT_TRACE2_EVENT=$(pwd)/trace2.txt \
+ git -C cruft--max-size -c gc.maxCruftSize=2M gc --cruft &&
+ test_subcommand $cruft_max_size_opts --max-cruft-size=2097152 <trace2.txt &&
+
+ GIT_TRACE2_EVENT=$(pwd)/trace2.txt \
+ git -C cruft--max-size -c gc.maxCruftSize=2M gc --cruft \
+ --max-cruft-size=3M &&
+ test_subcommand $cruft_max_size_opts --max-cruft-size=3145728 <trace2.txt
+'
+
run_and_wait_for_auto_gc () {
# We read stdout from gc for the side effect of waiting until the
# background gc process exits, closing its fd 9. Furthermore, the
diff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh
index d91fcf1af1..dc86ca8269 100755
--- a/t/t7704-repack-cruft.sh
+++ b/t/t7704-repack-cruft.sh
@@ -5,6 +5,7 @@ test_description='git repack works correctly'
. ./test-lib.sh
objdir=.git/objects
+packdir=$objdir/pack
test_expect_success '--expire-to stores pruned objects (now)' '
git init expire-to-now &&
@@ -127,4 +128,248 @@ test_expect_success '--expire-to stores pruned objects (5.minutes.ago)' '
)
'
+generate_random_blob() {
+ test-tool genrandom "$@" >blob &&
+ git hash-object -w -t blob blob &&
+ rm blob
+}
+
+pack_random_blob () {
+ generate_random_blob "$@" &&
+ git repack -d -q >/dev/null
+}
+
+generate_cruft_pack () {
+ pack_random_blob "$@" >/dev/null &&
+
+ ls $packdir/pack-*.pack | xargs -n 1 basename >in &&
+ pack="$(git pack-objects --cruft $packdir/pack <in)" &&
+ git prune-packed &&
+
+ echo "$packdir/pack-$pack.mtimes"
+}
+
+test_expect_success '--max-cruft-size creates new packs when above threshold' '
+ git init max-cruft-size-large &&
+ (
+ cd max-cruft-size-large &&
+ test_commit base &&
+
+ foo="$(pack_random_blob foo $((1*1024*1024)))" &&
+ git repack --cruft -d &&
+ cruft_foo="$(ls $packdir/pack-*.mtimes)" &&
+
+ bar="$(pack_random_blob bar $((1*1024*1024)))" &&
+ git repack --cruft -d --max-cruft-size=1M &&
+ cruft_bar="$(ls $packdir/pack-*.mtimes | grep -v $cruft_foo)" &&
+
+ test-tool pack-mtimes $(basename "$cruft_foo") >foo.objects &&
+ test-tool pack-mtimes $(basename "$cruft_bar") >bar.objects &&
+
+ grep "^$foo" foo.objects &&
+ test_line_count = 1 foo.objects &&
+ grep "^$bar" bar.objects &&
+ test_line_count = 1 bar.objects
+ )
+'
+
+test_expect_success '--max-cruft-size combines existing packs when below threshold' '
+ git init max-cruft-size-small &&
+ (
+ cd max-cruft-size-small &&
+ test_commit base &&
+
+ foo="$(pack_random_blob foo $((1*1024*1024)))" &&
+ git repack --cruft -d &&
+
+ bar="$(pack_random_blob bar $((1*1024*1024)))" &&
+ git repack --cruft -d --max-cruft-size=10M &&
+
+ cruft=$(ls $packdir/pack-*.mtimes) &&
+ test-tool pack-mtimes $(basename "$cruft") >cruft.objects &&
+
+ grep "^$foo" cruft.objects &&
+ grep "^$bar" cruft.objects &&
+ test_line_count = 2 cruft.objects
+ )
+'
+
+test_expect_success '--max-cruft-size combines smaller packs first' '
+ git init max-cruft-size-consume-small &&
+ (
+ cd max-cruft-size-consume-small &&
+
+ test_commit base &&
+ git repack -ad &&
+
+ cruft_foo="$(generate_cruft_pack foo 524288)" && # 0.5 MiB
+ cruft_bar="$(generate_cruft_pack bar 524288)" && # 0.5 MiB
+ cruft_baz="$(generate_cruft_pack baz 1048576)" && # 1.0 MiB
+ cruft_quux="$(generate_cruft_pack quux 1572864)" && # 1.5 MiB
+
+ test-tool pack-mtimes "$(basename $cruft_foo)" >expect.raw &&
+ test-tool pack-mtimes "$(basename $cruft_bar)" >>expect.raw &&
+ sort expect.raw >expect.objects &&
+
+ # repacking with `--max-cruft-size=2M` should combine
+ # both 0.5 MiB packs together, instead of, say, one of
+ # the 0.5 MiB packs with the 1.0 MiB pack
+ ls $packdir/pack-*.mtimes | sort >cruft.before &&
+ git repack -d --cruft --max-cruft-size=2M &&
+ ls $packdir/pack-*.mtimes | sort >cruft.after &&
+
+ comm -13 cruft.before cruft.after >cruft.new &&
+ comm -23 cruft.before cruft.after >cruft.removed &&
+
+ test_line_count = 1 cruft.new &&
+ test_line_count = 2 cruft.removed &&
+
+ # the two smaller packs should be rolled up first
+ printf "%s\n" $cruft_foo $cruft_bar | sort >expect.removed &&
+ test_cmp expect.removed cruft.removed &&
+
+ # ...and contain the set of objects rolled up
+ test-tool pack-mtimes "$(basename $(cat cruft.new))" >actual.raw &&
+ sort actual.raw >actual.objects &&
+
+ test_cmp expect.objects actual.objects
+ )
+'
+
+test_expect_success 'setup --max-cruft-size with freshened objects' '
+ git init max-cruft-size-freshen &&
+ (
+ cd max-cruft-size-freshen &&
+
+ test_commit base &&
+ git repack -ad &&
+
+ foo="$(generate_random_blob foo 64)" &&
+ test-tool chmtime --get -10000 \
+ "$objdir/$(test_oid_to_path "$foo")" >foo.mtime &&
+
+ git repack --cruft -d &&
+
+ cruft="$(ls $packdir/pack-*.mtimes)" &&
+ test-tool pack-mtimes "$(basename $cruft)" >actual &&
+ echo "$foo $(cat foo.mtime)" >expect &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--max-cruft-size with freshened objects (loose)' '
+ (
+ cd max-cruft-size-freshen &&
+
+ # regenerate the object, setting its mtime to be more recent
+ foo="$(generate_random_blob foo 64)" &&
+ test-tool chmtime --get -100 \
+ "$objdir/$(test_oid_to_path "$foo")" >foo.mtime &&
+
+ git repack --cruft -d &&
+
+ cruft="$(ls $packdir/pack-*.mtimes)" &&
+ test-tool pack-mtimes "$(basename $cruft)" >actual &&
+ echo "$foo $(cat foo.mtime)" >expect &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--max-cruft-size with freshened objects (packed)' '
+ (
+ cd max-cruft-size-freshen &&
+
+ # regenerate the object and store it in a packfile,
+ # setting its mtime to be more recent
+ #
+ # store it alongside another cruft object so that we
+ # do not create an identical copy of the existing
+ # cruft pack (which contains $foo).
+ foo="$(generate_random_blob foo 64)" &&
+ bar="$(generate_random_blob bar 64)" &&
+ foo_pack="$(printf "%s\n" $foo $bar | git pack-objects $packdir/pack)" &&
+ git prune-packed &&
+
+ test-tool chmtime --get -10 \
+ "$packdir/pack-$foo_pack.pack" >foo.mtime &&
+
+ git repack --cruft -d &&
+
+ cruft="$(ls $packdir/pack-*.mtimes)" &&
+ test-tool pack-mtimes "$(basename $cruft)" >actual &&
+ echo "$foo $(cat foo.mtime)" >expect.raw &&
+ echo "$bar $(cat foo.mtime)" >>expect.raw &&
+ sort expect.raw >expect &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--max-cruft-size with pruning' '
+ git init max-cruft-size-prune &&
+ (
+ cd max-cruft-size-prune &&
+
+ test_commit base &&
+ foo="$(generate_random_blob foo $((1024*1024)))" &&
+ bar="$(generate_random_blob bar $((1024*1024)))" &&
+ baz="$(generate_random_blob baz $((1024*1024)))" &&
+
+ test-tool chmtime -10000 "$objdir/$(test_oid_to_path "$foo")" &&
+
+ git repack -d --cruft --max-cruft-size=1M &&
+
+ # backdate the mtimes of all cruft packs to validate
+ # that they were rewritten as a result of pruning
+ ls $packdir/pack-*.mtimes | sort >cruft.before &&
+ for cruft in $(cat cruft.before)
+ do
+ mtime="$(test-tool chmtime --get -10000 "$cruft")" &&
+ echo $cruft $mtime >>mtimes || return 1
+ done &&
+
+ # repack (and prune) with a --max-cruft-size to ensure
+ # that we appropriately split the resulting set of packs
+ git repack -d --cruft --max-cruft-size=1M \
+ --cruft-expiration=10.seconds.ago &&
+ ls $packdir/pack-*.mtimes | sort >cruft.after &&
+
+ for cruft in $(cat cruft.after)
+ do
+ old_mtime="$(grep $cruft mtimes | cut -d" " -f2)" &&
+ new_mtime="$(test-tool chmtime --get $cruft)" &&
+ test $old_mtime -lt $new_mtime || return 1
+ done &&
+
+ test_line_count = 3 cruft.before &&
+ test_line_count = 2 cruft.after &&
+ test_must_fail git cat-file -e $foo &&
+ git cat-file -e $bar &&
+ git cat-file -e $baz
+ )
+'
+
+test_expect_success '--max-cruft-size ignores non-local packs' '
+ repo="max-cruft-size-non-local" &&
+ git init $repo &&
+ (
+ cd $repo &&
+ test_commit base &&
+ generate_random_blob foo 64 &&
+ git repack --cruft -d
+ ) &&
+
+ git clone --reference=$repo $repo $repo-alt &&
+ (
+ cd $repo-alt &&
+
+ test_commit other &&
+ generate_random_blob bar 64 &&
+
+ # ensure that we do not attempt to pick up packs from
+ # the non-alternated repository, which would result in a
+ # crash
+ git repack --cruft --max-cruft-size=1M -d
+ )
+'
+
test_done
--
2.42.0.310.g9604b54f73.dirty
^ permalink raw reply related
* [PATCH v2 2/3] builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE
From: Taylor Blau @ 2023-10-03 0:44 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt, Eric Sunshine
In-Reply-To: <cover.1696293862.git.me@ttaylorr.com>
The repack builtin takes a `--max-pack-size` command-line argument which
it uses to feed into any of the pack-objects children that it may spawn
when generating a new pack.
This option is parsed with OPT_STRING, meaning that we'll accept
anything as input, punting on more fine-grained validation until we get
down into pack-objects.
This is fine, but it's wasteful to spend an entire sub-process just to
figure out that one of its option is bogus. Instead, parse the value of
`--max-pack-size` with OPT_MAGNITUDE in 'git repack', and then pass the
knonw-good result down to pack-objects.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/repack.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 529e13120d..8a5bbb9cba 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -51,7 +51,7 @@ struct pack_objects_args {
const char *window_memory;
const char *depth;
const char *threads;
- const char *max_pack_size;
+ unsigned long max_pack_size;
int no_reuse_delta;
int no_reuse_object;
int quiet;
@@ -242,7 +242,7 @@ static void prepare_pack_objects(struct child_process *cmd,
if (args->threads)
strvec_pushf(&cmd->args, "--threads=%s", args->threads);
if (args->max_pack_size)
- strvec_pushf(&cmd->args, "--max-pack-size=%s", args->max_pack_size);
+ strvec_pushf(&cmd->args, "--max-pack-size=%lu", args->max_pack_size);
if (args->no_reuse_delta)
strvec_pushf(&cmd->args, "--no-reuse-delta");
if (args->no_reuse_object)
@@ -946,7 +946,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
N_("limits the maximum delta depth")),
OPT_STRING(0, "threads", &po_args.threads, N_("n"),
N_("limits the maximum number of threads")),
- OPT_STRING(0, "max-pack-size", &po_args.max_pack_size, N_("bytes"),
+ OPT_MAGNITUDE(0, "max-pack-size", &po_args.max_pack_size,
N_("maximum size of each packfile")),
OPT_BOOL(0, "pack-kept-objects", &pack_kept_objects,
N_("repack objects in packs marked with .keep")),
--
2.42.0.310.g9604b54f73.dirty
^ permalink raw reply related
* [PATCH v2 1/3] t7700: split cruft-related tests to t7704
From: Taylor Blau @ 2023-10-03 0:44 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt, Eric Sunshine
In-Reply-To: <cover.1696293862.git.me@ttaylorr.com>
A small handful of the tests in t7700 (the main script for testing
functionality of 'git repack') are specifically related to cruft pack
operations.
Prepare for adding new cruft pack-related tests by moving the existing
set into a new test script.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
t/t7700-repack.sh | 121 -------------------------------------
t/t7704-repack-cruft.sh | 130 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 130 insertions(+), 121 deletions(-)
create mode 100755 t/t7704-repack-cruft.sh
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 27b66807cd..55ee3eb8ae 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -633,125 +633,4 @@ test_expect_success '-n overrides repack.updateServerInfo=true' '
test_server_info_missing
'
-test_expect_success '--expire-to stores pruned objects (now)' '
- git init expire-to-now &&
- (
- cd expire-to-now &&
-
- git branch -M main &&
-
- test_commit base &&
-
- git checkout -b cruft &&
- test_commit --no-tag cruft &&
-
- git rev-list --objects --no-object-names main..cruft >moved.raw &&
- sort moved.raw >moved.want &&
-
- git rev-list --all --objects --no-object-names >expect.raw &&
- sort expect.raw >expect &&
-
- git checkout main &&
- git branch -D cruft &&
- git reflog expire --all --expire=all &&
-
- git init --bare expired.git &&
- git repack -d \
- --cruft --cruft-expiration="now" \
- --expire-to="expired.git/objects/pack/pack" &&
-
- expired="$(ls expired.git/objects/pack/pack-*.idx)" &&
- test_path_is_file "${expired%.idx}.mtimes" &&
-
- # Since the `--cruft-expiration` is "now", the effective
- # behavior is to move _all_ unreachable objects out to
- # the location in `--expire-to`.
- git show-index <$expired >expired.raw &&
- cut -d" " -f2 expired.raw | sort >expired.objects &&
- git rev-list --all --objects --no-object-names \
- >remaining.objects &&
-
- # ...in other words, the combined contents of this
- # repository and expired.git should be the same as the
- # set of objects we started with.
- cat expired.objects remaining.objects | sort >actual &&
- test_cmp expect actual &&
-
- # The "moved" objects (i.e., those in expired.git)
- # should be the same as the cruft objects which were
- # expired in the previous step.
- test_cmp moved.want expired.objects
- )
-'
-
-test_expect_success '--expire-to stores pruned objects (5.minutes.ago)' '
- git init expire-to-5.minutes.ago &&
- (
- cd expire-to-5.minutes.ago &&
-
- git branch -M main &&
-
- test_commit base &&
-
- # Create two classes of unreachable objects, one which
- # is older than 5 minutes (stale), and another which is
- # newer (recent).
- for kind in stale recent
- do
- git checkout -b $kind main &&
- test_commit --no-tag $kind || return 1
- done &&
-
- git rev-list --objects --no-object-names main..stale >in &&
- stale="$(git pack-objects $objdir/pack/pack <in)" &&
- mtime="$(test-tool chmtime --get =-600 $objdir/pack/pack-$stale.pack)" &&
-
- # expect holds the set of objects we expect to find in
- # this repository after repacking
- git rev-list --objects --no-object-names recent >expect.raw &&
- sort expect.raw >expect &&
-
- # moved.want holds the set of objects we expect to find
- # in expired.git
- git rev-list --objects --no-object-names main..stale >out &&
- sort out >moved.want &&
-
- git checkout main &&
- git branch -D stale recent &&
- git reflog expire --all --expire=all &&
- git prune-packed &&
-
- git init --bare expired.git &&
- git repack -d \
- --cruft --cruft-expiration=5.minutes.ago \
- --expire-to="expired.git/objects/pack/pack" &&
-
- # Some of the remaining objects in this repository are
- # unreachable, so use `cat-file --batch-all-objects`
- # instead of `rev-list` to get their names
- git cat-file --batch-all-objects --batch-check="%(objectname)" \
- >remaining.objects &&
- sort remaining.objects >actual &&
- test_cmp expect actual &&
-
- (
- cd expired.git &&
-
- expired="$(ls objects/pack/pack-*.mtimes)" &&
- test-tool pack-mtimes $(basename $expired) >out &&
- cut -d" " -f1 out | sort >../moved.got &&
-
- # Ensure that there are as many objects with the
- # expected mtime as were moved to expired.git.
- #
- # In other words, ensure that the recorded
- # mtimes of any moved objects was written
- # correctly.
- grep " $mtime$" out >matching &&
- test_line_count = $(wc -l <../moved.want) matching
- ) &&
- test_cmp moved.want moved.got
- )
-'
-
test_done
diff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh
new file mode 100755
index 0000000000..d91fcf1af1
--- /dev/null
+++ b/t/t7704-repack-cruft.sh
@@ -0,0 +1,130 @@
+#!/bin/sh
+
+test_description='git repack works correctly'
+
+. ./test-lib.sh
+
+objdir=.git/objects
+
+test_expect_success '--expire-to stores pruned objects (now)' '
+ git init expire-to-now &&
+ (
+ cd expire-to-now &&
+
+ git branch -M main &&
+
+ test_commit base &&
+
+ git checkout -b cruft &&
+ test_commit --no-tag cruft &&
+
+ git rev-list --objects --no-object-names main..cruft >moved.raw &&
+ sort moved.raw >moved.want &&
+
+ git rev-list --all --objects --no-object-names >expect.raw &&
+ sort expect.raw >expect &&
+
+ git checkout main &&
+ git branch -D cruft &&
+ git reflog expire --all --expire=all &&
+
+ git init --bare expired.git &&
+ git repack -d \
+ --cruft --cruft-expiration="now" \
+ --expire-to="expired.git/objects/pack/pack" &&
+
+ expired="$(ls expired.git/objects/pack/pack-*.idx)" &&
+ test_path_is_file "${expired%.idx}.mtimes" &&
+
+ # Since the `--cruft-expiration` is "now", the effective
+ # behavior is to move _all_ unreachable objects out to
+ # the location in `--expire-to`.
+ git show-index <$expired >expired.raw &&
+ cut -d" " -f2 expired.raw | sort >expired.objects &&
+ git rev-list --all --objects --no-object-names \
+ >remaining.objects &&
+
+ # ...in other words, the combined contents of this
+ # repository and expired.git should be the same as the
+ # set of objects we started with.
+ cat expired.objects remaining.objects | sort >actual &&
+ test_cmp expect actual &&
+
+ # The "moved" objects (i.e., those in expired.git)
+ # should be the same as the cruft objects which were
+ # expired in the previous step.
+ test_cmp moved.want expired.objects
+ )
+'
+
+test_expect_success '--expire-to stores pruned objects (5.minutes.ago)' '
+ git init expire-to-5.minutes.ago &&
+ (
+ cd expire-to-5.minutes.ago &&
+
+ git branch -M main &&
+
+ test_commit base &&
+
+ # Create two classes of unreachable objects, one which
+ # is older than 5 minutes (stale), and another which is
+ # newer (recent).
+ for kind in stale recent
+ do
+ git checkout -b $kind main &&
+ test_commit --no-tag $kind || return 1
+ done &&
+
+ git rev-list --objects --no-object-names main..stale >in &&
+ stale="$(git pack-objects $objdir/pack/pack <in)" &&
+ mtime="$(test-tool chmtime --get =-600 $objdir/pack/pack-$stale.pack)" &&
+
+ # expect holds the set of objects we expect to find in
+ # this repository after repacking
+ git rev-list --objects --no-object-names recent >expect.raw &&
+ sort expect.raw >expect &&
+
+ # moved.want holds the set of objects we expect to find
+ # in expired.git
+ git rev-list --objects --no-object-names main..stale >out &&
+ sort out >moved.want &&
+
+ git checkout main &&
+ git branch -D stale recent &&
+ git reflog expire --all --expire=all &&
+ git prune-packed &&
+
+ git init --bare expired.git &&
+ git repack -d \
+ --cruft --cruft-expiration=5.minutes.ago \
+ --expire-to="expired.git/objects/pack/pack" &&
+
+ # Some of the remaining objects in this repository are
+ # unreachable, so use `cat-file --batch-all-objects`
+ # instead of `rev-list` to get their names
+ git cat-file --batch-all-objects --batch-check="%(objectname)" \
+ >remaining.objects &&
+ sort remaining.objects >actual &&
+ test_cmp expect actual &&
+
+ (
+ cd expired.git &&
+
+ expired="$(ls objects/pack/pack-*.mtimes)" &&
+ test-tool pack-mtimes $(basename $expired) >out &&
+ cut -d" " -f1 out | sort >../moved.got &&
+
+ # Ensure that there are as many objects with the
+ # expected mtime as were moved to expired.git.
+ #
+ # In other words, ensure that the recorded
+ # mtimes of any moved objects was written
+ # correctly.
+ grep " $mtime$" out >matching &&
+ test_line_count = $(wc -l <../moved.want) matching
+ ) &&
+ test_cmp moved.want moved.got
+ )
+'
+
+test_done
--
2.42.0.310.g9604b54f73.dirty
^ permalink raw reply related
* [PATCH v2 0/3] repack: implement `--cruft-max-size`
From: Taylor Blau @ 2023-10-03 0:44 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt, Eric Sunshine
In-Reply-To: <cover.1694123506.git.me@ttaylorr.com>
(The earlier round of these patches depended on a couple of in-flight
topics, but this series has been rebased onto the tip of 'master').
This is a reroll of my series that introduces more ergonomic options for
creating and dealing with multiple cruft packs.
Much is unchanged since last time, but some notable changes that have
taken place include:
- adding a missing commit message / s-o-b to the first patch to split
cruft-related tests out of t7700 into t7704.
- renamed the option to `--max-cruft-size` (instead of
`--cruft-max-size`), and made corresponding changes to the
documentation, configuration options, tests, etc.
- cleaned up our handling of how we mark packs to be deleted and
retained
- parsed `--max-cruft-size` as an unsigned long up front instead of
blindly passing a char * down to pack-objects to parse it for us.
The last of those also affects `--max-pack-size`, which caused this
series to grow a new second patch which cleans up the existing code
which suffers from the same issue.
As usual, a range-diff is available below. Thanks for all of the helpful
review on the earlier round, and thanks in advance for your review on
this one!
Taylor Blau (3):
t7700: split cruft-related tests to t7704
builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE
builtin/repack.c: implement support for `--max-cruft-size`
Documentation/config/gc.txt | 6 +
Documentation/git-gc.txt | 7 +
Documentation/git-repack.txt | 11 +
builtin/gc.c | 7 +
builtin/repack.c | 140 +++++++++++--
t/t6500-gc.sh | 27 +++
t/t7700-repack.sh | 121 -----------
t/t7704-repack-cruft.sh | 375 +++++++++++++++++++++++++++++++++++
8 files changed, 559 insertions(+), 135 deletions(-)
create mode 100755 t/t7704-repack-cruft.sh
Range-diff against v1:
1: 103e19c75a < -: ---------- builtin/repack.c: extract structure to store existing packs
2: 45d5f15308 < -: ---------- builtin/repack.c: extract marking packs for deletion
3: 30eccbfdcb < -: ---------- builtin/repack.c: extract redundant pack cleanup for --geometric
4: 57e322301d < -: ---------- builtin/repack.c: extract redundant pack cleanup for existing packs
5: 4df4a2e51e < -: ---------- builtin/repack.c: extract `has_existing_non_kept_packs()`
6: 360be582b4 < -: ---------- builtin/repack.c: store existing cruft packs separately
7: ef9c8d775b < -: ---------- builtin/repack.c: drop `DELETE_PACK` macro
8: 873bc16abb < -: ---------- builtin/repack.c: extract common cruft pack loop
9: de6c2a0d70 ! 1: 3ed4ab61f6 t7700: split cruft-related tests to t7704
@@ Metadata
## Commit message ##
t7700: split cruft-related tests to t7704
+ A small handful of the tests in t7700 (the main script for testing
+ functionality of 'git repack') are specifically related to cruft pack
+ operations.
+
+ Prepare for adding new cruft pack-related tests by moving the existing
+ set into a new test script.
+
+ Signed-off-by: Taylor Blau <me@ttaylorr.com>
+
## t/t7700-repack.sh ##
@@ t/t7700-repack.sh: test_expect_success '-n overrides repack.updateServerInfo=true' '
test_server_info_missing
-: ---------- > 2: 9ec999882d builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE
10: 7e4e42e1aa ! 3: e7beb2060d builtin/repack.c: implement support for `--cruft-max-size`
@@ Metadata
Author: Taylor Blau <me@ttaylorr.com>
## Commit message ##
- builtin/repack.c: implement support for `--cruft-max-size`
+ builtin/repack.c: implement support for `--max-cruft-size`
Cruft packs are an alternative mechanism for storing a collection of
unreachable objects whose mtimes are recent enough to avoid being
@@ Commit message
(for more, see some of the details in 3d89a8c118
(Documentation/technical: add cruft-packs.txt, 2022-05-20)).
- This all works, but can be costly from an I/O-perspective when a
- repository has either (a) many unreachable objects, (b) prunes objects
- relatively infrequently/never, or (c) both.
+ This all works, but can be costly from an I/O-perspective when
+ frequently repacking a repository that has many unreachable objects.
+ This problem is exacerbated when those unreachable objects are rarely
+ (if every) pruned.
Since there is at most one cruft pack in the above scheme, each time we
update the cruft pack it must be rewritten from scratch. Because much of
@@ Commit message
multiple cruft packs. This patch implements that support which we were
lacking.
- Introduce a new option `--cruft-max-size` which allows repositories to
+ Introduce a new option `--max-cruft-size` which allows repositories to
accumulate cruft packs up to a given size, after which point a new
generation of cruft packs can accumulate until it reaches the maximum
size, and so on. To generate a new cruft pack, the process works like
@@ Commit message
pack, along with any other unreachable objects which have since
entered the repository.
- This limits the I/O churn up to a quadratic function of the value
- specified by the `--cruft-max-size` option, instead of behaving
- quadratically in the number of total unreachable objects.
+ Once a cruft pack grows beyond the size specified via `--max-cruft-size`
+ the pack is effectively frozen. This limits the I/O churn up to a
+ quadratic function of the value specified by the `--max-cruft-size`
+ option, instead of behaving quadratically in the number of total
+ unreachable objects.
- When pruning unreachable objects, we bypass the new paths which combine
- small cruft packs together, and instead start from scratch, passing in
- the appropriate `--max-pack-size` down to `pack-objects`, putting it in
- charge of keeping the resulting set of cruft packs sized correctly.
+ When pruning unreachable objects, we bypass the new code paths which
+ combine small cruft packs together, and instead start from scratch,
+ passing in the appropriate `--max-pack-size` down to `pack-objects`,
+ putting it in charge of keeping the resulting set of cruft packs sized
+ correctly.
This may seem like further I/O churn, but in practice it isn't so bad.
We could prune old cruft packs for whom all or most objects are removed,
@@ Documentation/config/gc.txt: gc.cruftPacks::
linkgit:git-repack[1]) instead of as loose objects. The default
is `true`.
-+gc.cruftMaxSize::
++gc.maxCruftSize::
+ Limit the size of new cruft packs when repacking. When
-+ specified in addition to `--cruft-max-size`, the command line
-+ option takes priority. See the `--cruft-max-size` option of
++ specified in addition to `--max-cruft-size`, the command line
++ option takes priority. See the `--max-cruft-size` option of
+ linkgit:git-repack[1].
+
gc.pruneExpire::
@@ Documentation/git-gc.txt: be performed as well.
cruft pack instead of storing them as loose objects. `--cruft`
is on by default.
-+--cruft-max-size=<n>::
++--max-cruft-size=<n>::
+ When packing unreachable objects into a cruft pack, limit the
+ size of new cruft packs to be at most `<n>`. Overrides any
-+ value specified via the `gc.cruftMaxSize` configuration. See
-+ the `--cruft-max-size` option of linkgit:git-repack[1] for
++ value specified via the `gc.maxCruftSize` configuration. See
++ the `--max-cruft-size` option of linkgit:git-repack[1] for
+ more.
+
--prune=<date>::
@@ Documentation/git-repack.txt: to the new separate pack will be written.
immediately instead of waiting for the next `git gc` invocation.
Only useful with `--cruft -d`.
-+--cruft-max-size=<n>::
-+ Repack cruft objects into packs as large as `<n>` before
++--max-cruft-size=<n>::
++ Repack cruft objects into packs as large as `<n>` bytes before
+ creating new packs. As long as there are enough cruft packs
+ smaller than `<n>`, repacking will cause a new cruft pack to
+ be created containing objects from any combined cruft packs,
-+ along with any new unreachable objects. Cruft packs larger
-+ than `<n>` will not be modified. Only useful with `--cruft
-+ -d`.
++ along with any new unreachable objects. Cruft packs larger than
++ `<n>` will not be modified. When the new cruft pack is larger
++ than `<n>` bytes, it will be split into multiple packs, all of
++ which are guaranteed to be at most `<n>` bytes in size. Only
++ useful with `--cruft -d`.
+
--expire-to=<dir>::
Write a cruft pack containing pruned objects (if any) to the
@@ builtin/gc.c: static const char * const builtin_gc_usage[] = {
static int pack_refs = 1;
static int prune_reflogs = 1;
static int cruft_packs = 1;
-+static char *cruft_max_size;
++static unsigned long max_cruft_size;
static int aggressive_depth = 50;
static int aggressive_window = 250;
static int gc_auto_threshold = 6700;
@@ builtin/gc.c: static void gc_config(void)
git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
git_config_get_bool("gc.autodetach", &detach_auto);
git_config_get_bool("gc.cruftpacks", &cruft_packs);
-+ git_config_get_string("gc.cruftmaxsize", &cruft_max_size);
++ git_config_get_ulong("gc.maxcruftsize", &max_cruft_size);
git_config_get_expiry("gc.pruneexpire", &prune_expire);
git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
git_config_get_expiry("gc.logexpiry", &gc_log_expire);
@@ builtin/gc.c: static void add_repack_all_option(struct string_list *keep_pack)
strvec_push(&repack, "--cruft");
if (prune_expire)
strvec_pushf(&repack, "--cruft-expiration=%s", prune_expire);
-+ if (cruft_max_size)
-+ strvec_pushf(&repack, "--cruft-max-size=%s",
-+ cruft_max_size);
++ if (max_cruft_size)
++ strvec_pushf(&repack, "--max-cruft-size=%lu",
++ max_cruft_size);
} else {
strvec_push(&repack, "-A");
if (prune_expire)
@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
N_("prune unreferenced objects"),
PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
OPT_BOOL(0, "cruft", &cruft_packs, N_("pack unreferenced objects separately")),
-+ OPT_STRING(0, "cruft-max-size", &cruft_max_size,
-+ N_("bytes"),
-+ N_("with --cruft, limit the size of new cruft packs")),
++ OPT_MAGNITUDE(0, "max-cruft-size", &max_cruft_size,
++ N_("with --cruft, limit the size of new cruft packs")),
OPT_BOOL(0, "aggressive", &aggressive, N_("be more thorough (increased runtime)")),
OPT_BOOL_F(0, "auto", &auto_gc, N_("enable auto-gc mode"),
PARSE_OPT_NOCOMPLETE),
## builtin/repack.c ##
@@
- #define LOOSEN_UNREACHABLE 2
#define PACK_CRUFT 4
-+#define DELETE_PACK ((void*)(uintptr_t)1)
-+#define RETAIN_PACK ((uintptr_t)(1<<1))
-+
+ #define DELETE_PACK 1
++#define RETAIN_PACK 2
+
static int pack_everything;
static int delta_base_offset = 1;
- static int pack_kept_objects = -1;
-@@ builtin/repack.c: static int has_existing_non_kept_packs(const struct existing_packs *existing)
- return existing->non_kept_packs.nr || existing->cruft_packs.nr;
+@@ builtin/repack.c: static void pack_mark_for_deletion(struct string_list_item *item)
+ item->util = (void*)((uintptr_t)item->util | DELETE_PACK);
}
++static void pack_unmark_for_deletion(struct string_list_item *item)
++{
++ item->util = (void*)((uintptr_t)item->util & ~DELETE_PACK);
++}
++
+ static int pack_is_marked_for_deletion(struct string_list_item *item)
+ {
+ return (uintptr_t)item->util & DELETE_PACK;
+ }
+
++static void pack_mark_retained(struct string_list_item *item)
++{
++ item->util = (void*)((uintptr_t)item->util | RETAIN_PACK);
++}
++
+static int pack_is_retained(struct string_list_item *item)
+{
+ return (uintptr_t)item->util & RETAIN_PACK;
@@ builtin/repack.c: static void mark_packs_for_deletion_1(struct string_list *name
- * (if `-d` was given).
- */
- if (!string_list_has_string(names, sha1))
-- item->util = (void*)1;
+
+ if (pack_is_retained(item)) {
-+ item->util = NULL;
++ pack_unmark_for_deletion(item);
+ } else if (!string_list_has_string(names, sha1)) {
+ /*
+ * Mark this pack for deletion, which ensures
@@ builtin/repack.c: static void mark_packs_for_deletion_1(struct string_list *name
+ * will actually delete this pack (if `-d` was
+ * given).
+ */
-+ item->util = DELETE_PACK;
+ pack_mark_for_deletion(item);
+ }
}
}
@@ builtin/repack.c: static void mark_packs_for_deletion_1(struct string_list *name
+ if (!item)
+ BUG("could not find cruft pack '%s'", pack_basename(cruft));
+
-+ item->util = (void*)RETAIN_PACK;
++ pack_mark_retained(item);
+ strbuf_release(&buf);
+}
+
@@ builtin/repack.c: static void remove_redundant_bitmaps(struct string_list *inclu
+ return 0;
+}
+
-+static void collapse_small_cruft_packs(FILE *in, unsigned long max_size,
++static void collapse_small_cruft_packs(FILE *in, size_t max_size,
+ struct existing_packs *existing)
+{
+ struct packed_git **existing_cruft, *p;
+ struct strbuf buf = STRBUF_INIT;
-+ unsigned long total_size = 0;
++ size_t total_size = 0;
+ size_t existing_cruft_nr = 0;
+ size_t i;
+
@@ builtin/repack.c: static void remove_redundant_bitmaps(struct string_list *inclu
+ QSORT(existing_cruft, existing_cruft_nr, existing_cruft_pack_cmp);
+
+ for (i = 0; i < existing_cruft_nr; i++) {
-+ off_t proposed;
++ size_t proposed;
+
+ p = existing_cruft[i];
+ proposed = st_add(total_size, p->pack_size);
@@ builtin/repack.c: static int write_cruft_pack(const struct pack_objects_args *ar
- for_each_string_list_item(item, &existing->cruft_packs)
- fprintf(in, "-%s.pack\n", item->string);
+ if (args->max_pack_size && !cruft_expiration) {
-+ unsigned long max_pack_size;
-+ if (!git_parse_ulong(args->max_pack_size, &max_pack_size))
-+ return error(_("could not parse --cruft-max-size: '%s'"),
-+ args->max_pack_size);
-+ collapse_small_cruft_packs(in, max_pack_size, existing);
++ collapse_small_cruft_packs(in, args->max_pack_size, existing);
+ } else {
+ for_each_string_list_item(item, &existing->non_kept_packs)
+ fprintf(in, "-%s.pack\n", item->string);
@@ builtin/repack.c: int cmd_repack(int argc, const char **argv, const char *prefix
PACK_CRUFT),
OPT_STRING(0, "cruft-expiration", &cruft_expiration, N_("approxidate"),
N_("with --cruft, expire objects older than this")),
-+ OPT_STRING(0, "cruft-max-size", &cruft_po_args.max_pack_size,
-+ N_("bytes"),
++ OPT_MAGNITUDE(0, "max-cruft-size", &cruft_po_args.max_pack_size,
+ N_("with --cruft, limit the size of new cruft packs")),
OPT_BOOL('d', NULL, &delete_redundant,
N_("remove redundant packs, and run git-prune-packed")),
@@ t/t6500-gc.sh: test_expect_success 'gc.bigPackThreshold ignores cruft packs' '
+cruft_max_size_opts="git repack -d -l --cruft --cruft-expiration=2.weeks.ago"
+
-+test_expect_success 'setup for --cruft-max-size tests' '
++test_expect_success 'setup for --max-cruft-size tests' '
+ git init cruft--max-size &&
+ (
+ cd cruft--max-size &&
@@ t/t6500-gc.sh: test_expect_success 'gc.bigPackThreshold ignores cruft packs' '
+ )
+'
+
-+test_expect_success '--cruft-max-size sets appropriate repack options' '
++test_expect_success '--max-cruft-size sets appropriate repack options' '
+ GIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size \
-+ gc --cruft --cruft-max-size=1M &&
-+ test_subcommand $cruft_max_size_opts --cruft-max-size=1M <trace2.txt
++ gc --cruft --max-cruft-size=1M &&
++ test_subcommand $cruft_max_size_opts --max-cruft-size=1048576 <trace2.txt
+'
+
-+test_expect_success 'gc.cruftMaxSize sets appropriate repack options' '
++test_expect_success 'gc.maxCruftSize sets appropriate repack options' '
+ GIT_TRACE2_EVENT=$(pwd)/trace2.txt \
-+ git -C cruft--max-size -c gc.cruftMaxSize=2M gc --cruft &&
-+ test_subcommand $cruft_max_size_opts --cruft-max-size=2M <trace2.txt &&
++ git -C cruft--max-size -c gc.maxCruftSize=2M gc --cruft &&
++ test_subcommand $cruft_max_size_opts --max-cruft-size=2097152 <trace2.txt &&
+
+ GIT_TRACE2_EVENT=$(pwd)/trace2.txt \
-+ git -C cruft--max-size -c gc.cruftMaxSize=2M gc --cruft \
-+ --cruft-max-size=3M &&
-+ test_subcommand $cruft_max_size_opts --cruft-max-size=3M <trace2.txt
++ git -C cruft--max-size -c gc.maxCruftSize=2M gc --cruft \
++ --max-cruft-size=3M &&
++ test_subcommand $cruft_max_size_opts --max-cruft-size=3145728 <trace2.txt
+'
+
run_and_wait_for_auto_gc () {
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ echo "$packdir/pack-$pack.mtimes"
+}
+
-+test_expect_success '--cruft-max-size creates new packs when above threshold' '
-+ git init cruft-max-size-large &&
++test_expect_success '--max-cruft-size creates new packs when above threshold' '
++ git init max-cruft-size-large &&
+ (
-+ cd cruft-max-size-large &&
++ cd max-cruft-size-large &&
+ test_commit base &&
+
+ foo="$(pack_random_blob foo $((1*1024*1024)))" &&
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ cruft_foo="$(ls $packdir/pack-*.mtimes)" &&
+
+ bar="$(pack_random_blob bar $((1*1024*1024)))" &&
-+ git repack --cruft -d --cruft-max-size=1M &&
++ git repack --cruft -d --max-cruft-size=1M &&
+ cruft_bar="$(ls $packdir/pack-*.mtimes | grep -v $cruft_foo)" &&
+
+ test-tool pack-mtimes $(basename "$cruft_foo") >foo.objects &&
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ )
+'
+
-+test_expect_success '--cruft-max-size combines existing packs when below threshold' '
-+ git init cruft-max-size-small &&
++test_expect_success '--max-cruft-size combines existing packs when below threshold' '
++ git init max-cruft-size-small &&
+ (
-+ cd cruft-max-size-small &&
++ cd max-cruft-size-small &&
+ test_commit base &&
+
+ foo="$(pack_random_blob foo $((1*1024*1024)))" &&
+ git repack --cruft -d &&
+
+ bar="$(pack_random_blob bar $((1*1024*1024)))" &&
-+ git repack --cruft -d --cruft-max-size=10M &&
++ git repack --cruft -d --max-cruft-size=10M &&
+
+ cruft=$(ls $packdir/pack-*.mtimes) &&
+ test-tool pack-mtimes $(basename "$cruft") >cruft.objects &&
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ )
+'
+
-+test_expect_success '--cruft-max-size combines smaller packs first' '
-+ git init cruft-max-size-consume-small &&
++test_expect_success '--max-cruft-size combines smaller packs first' '
++ git init max-cruft-size-consume-small &&
+ (
-+ cd cruft-max-size-consume-small &&
++ cd max-cruft-size-consume-small &&
+
+ test_commit base &&
+ git repack -ad &&
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ test-tool pack-mtimes "$(basename $cruft_bar)" >>expect.raw &&
+ sort expect.raw >expect.objects &&
+
-+ # repacking with `--cruft-max-size=2M` should combine
++ # repacking with `--max-cruft-size=2M` should combine
+ # both 0.5 MiB packs together, instead of, say, one of
+ # the 0.5 MiB packs with the 1.0 MiB pack
+ ls $packdir/pack-*.mtimes | sort >cruft.before &&
-+ git repack -d --cruft --cruft-max-size=2M &&
++ git repack -d --cruft --max-cruft-size=2M &&
+ ls $packdir/pack-*.mtimes | sort >cruft.after &&
+
+ comm -13 cruft.before cruft.after >cruft.new &&
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ )
+'
+
-+test_expect_success 'setup --cruft-max-size with freshened objects' '
-+ git init cruft-max-size-freshen &&
++test_expect_success 'setup --max-cruft-size with freshened objects' '
++ git init max-cruft-size-freshen &&
+ (
-+ cd cruft-max-size-freshen &&
++ cd max-cruft-size-freshen &&
+
+ test_commit base &&
+ git repack -ad &&
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ )
+'
+
-+test_expect_success '--cruft-max-size with freshened objects (loose)' '
++test_expect_success '--max-cruft-size with freshened objects (loose)' '
+ (
-+ cd cruft-max-size-freshen &&
++ cd max-cruft-size-freshen &&
+
+ # regenerate the object, setting its mtime to be more recent
+ foo="$(generate_random_blob foo 64)" &&
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ )
+'
+
-+test_expect_success '--cruft-max-size with freshened objects (packed)' '
++test_expect_success '--max-cruft-size with freshened objects (packed)' '
+ (
-+ cd cruft-max-size-freshen &&
++ cd max-cruft-size-freshen &&
+
+ # regenerate the object and store it in a packfile,
+ # setting its mtime to be more recent
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ )
+'
+
-+test_expect_success '--cruft-max-size with pruning' '
-+ git init cruft-max-size-prune &&
++test_expect_success '--max-cruft-size with pruning' '
++ git init max-cruft-size-prune &&
+ (
-+ cd cruft-max-size-prune &&
++ cd max-cruft-size-prune &&
+
+ test_commit base &&
+ foo="$(generate_random_blob foo $((1024*1024)))" &&
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+
+ test-tool chmtime -10000 "$objdir/$(test_oid_to_path "$foo")" &&
+
-+ git repack -d --cruft --cruft-max-size=1M &&
++ git repack -d --cruft --max-cruft-size=1M &&
+
+ # backdate the mtimes of all cruft packs to validate
+ # that they were rewritten as a result of pruning
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ echo $cruft $mtime >>mtimes || return 1
+ done &&
+
-+ # repack (and prune) with a --cruft-max-size to ensure
++ # repack (and prune) with a --max-cruft-size to ensure
+ # that we appropriately split the resulting set of packs
-+ git repack -d --cruft --cruft-max-size=1M \
++ git repack -d --cruft --max-cruft-size=1M \
+ --cruft-expiration=10.seconds.ago &&
+ ls $packdir/pack-*.mtimes | sort >cruft.after &&
+
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ )
+'
+
-+test_expect_success '--cruft-max-size ignores non-local packs' '
-+ repo="cruft-max-size-non-local" &&
++test_expect_success '--max-cruft-size ignores non-local packs' '
++ repo="max-cruft-size-non-local" &&
+ git init $repo &&
+ (
+ cd $repo &&
@@ t/t7704-repack-cruft.sh: test_expect_success '--expire-to stores pruned objects
+ # ensure that we do not attempt to pick up packs from
+ # the non-alternated repository, which would result in a
+ # crash
-+ git repack --cruft --cruft-max-size=1M -d
++ git repack --cruft --max-cruft-size=1M -d
+ )
+'
+
--
2.42.0.310.g9604b54f73.dirty
^ permalink raw reply
* What's cooking in git.git (Oct 2023, #01; Mon, 2)
From: Junio C Hamano @ 2023-10-03 0:30 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* ds/stat-name-width-configuration (2023-09-18) 1 commit
(merged to 'next' on 2023-09-22 at dbf5bd96e8)
+ diff --stat: add config option to limit filename width
"git diff" learned diff.statNameWidth configuration variable, to
give the default width for the name part in the "--stat" output.
source: <87badb12f040d1c66cd9b89074d3de5015a45983.1694446743.git.dsimic@manjaro.org>
* hy/doc-show-is-like-log-not-diff-tree (2023-09-20) 1 commit
(merged to 'next' on 2023-09-22 at 5492c03eae)
+ show doc: redirect user to git log manual instead of git diff-tree
Doc update.
source: <20230920132731.1259-1-hanyang.tony@bytedance.com>
* jc/alias-completion (2023-09-20) 1 commit
(merged to 'next' on 2023-09-22 at 1d069e900b)
+ completion: loosen and document the requirement around completing alias
The command line completion script (in contrib/) can be told to
complete aliases by including ": git <cmd> ;" in the alias to tell
it that the alias should be completed similar to how "git <cmd>" is
completed. The parsing code for the alias as been loosened to
allow ';' without an extra space before it.
cf. <owlyjzssjro2.fsf@fine.c.googlers.com>
source: <xmqqy1h08zsp.fsf_-_@gitster.g>
* jc/unresolve-removal (2023-07-31) 7 commits
(merged to 'next' on 2023-09-25 at 0563c8d8a1)
+ checkout: allow "checkout -m path" to unmerge removed paths
+ checkout/restore: add basic tests for --merge
+ checkout/restore: refuse unmerging paths unless checking out of the index
+ update-index: remove stale fallback code for "--unresolve"
+ update-index: use unmerge_index_entry() to support removal
+ resolve-undo: allow resurrecting conflicted state that resolved to deletion
+ update-index: do not read HEAD and MERGE_HEAD unconditionally
(this branch is used by jc/rerere-cleanup.)
"checkout --merge -- path" and "update-index --unresolve path" did
not resurrect conflicted state that was resolved to remove path,
but now they do.
source: <20230731224409.4181277-1-gitster@pobox.com>
* jk/fsmonitor-unused-parameter (2023-09-18) 8 commits
(merged to 'next' on 2023-09-19 at bd06505f9e)
+ run-command: mark unused parameters in start_bg_wait callbacks
+ fsmonitor: mark unused hashmap callback parameters
+ fsmonitor/darwin: mark unused parameters in system callback
+ fsmonitor: mark unused parameters in stub functions
+ fsmonitor/win32: mark unused parameter in fsm_os__incompatible()
+ fsmonitor: mark some maybe-unused parameters
+ fsmonitor/win32: drop unused parameters
+ fsmonitor: prefer repo_git_path() to git_pathdup()
Unused parameters in fsmonitor related code paths have been marked
as such.
source: <20230918222908.GA2659096@coredump.intra.peff.net>
* jk/test-pass-ubsan-options-to-http-test (2023-09-21) 1 commit
(merged to 'next' on 2023-09-22 at bbe2f75937)
+ test-lib: set UBSAN_OPTIONS to match ASan
UBSAN options were not propagated through the test framework to git
run via the httpd, unlike ASAN options, which has been corrected.
source: <20230921041825.GA2814583@coredump.intra.peff.net>
* js/doc-status-with-submodules-mark-up-fix (2023-09-22) 1 commit
(merged to 'next' on 2023-09-25 at 7ed318fc91)
+ Documentation/git-status: add missing line breaks
Docfix.
source: <pull.1590.git.1695392082207.gitgitgadget@gmail.com>
* kh/range-diff-notes (2023-09-19) 1 commit
(merged to 'next' on 2023-09-22 at ac04978b4b)
+ range-diff: treat notes like `log`
"git range-diff --notes=foo" compared "log --notes=foo --notes" of
the two ranges, instead of using just the specified notes tree.
source: <6e114271a2e7d2323193bd58bb307f60101942ce.1695154855.git.code@khaugsbakk.name>
* ml/git-gui-exec-path-fix (2023-09-18) 3 commits
(merged to 'next' on 2023-09-19 at 0565b0b14b)
+ Merge git-gui into ml/git-gui-exec-path-fix
+ git-gui - use git-hook, honor core.hooksPath
+ git-gui - re-enable use of hook scripts
Fix recent regression in Git-GUI that fails to run hook scripts at
all.
* ob/am-msgfix (2023-09-21) 1 commit
(merged to 'next' on 2023-09-22 at 7f7589a06a)
+ am: fix error message in parse_opt_show_current_patch()
The parameters to generate an error message have been corrected.
source: <20230921110727.789156-1-oswald.buddenhagen@gmx.de>
--------------------------------------------------
[New Topics]
* cw/prelim-cleanup (2023-09-29) 4 commits
- parse: separate out parsing functions from config.h
- config: correct bad boolean env value error message
- wrapper: reduce scope of remove_or_warn()
- hex-ll: separate out non-hash-algo functions
Shuffle some bits across headers and sources to prepare for
libification effort.
Will merge to 'next'.
source: <cover.1696021277.git.jonathantanmy@google.com>
* ds/init-diffstat-width (2023-09-29) 1 commit
- diff --stat: set the width defaults in a helper function
Code clean-up.
Will merge to 'next'.
source: <d45d1dac1a20699e370905b88b6fd0ec296751e7.1695441501.git.dsimic@manjaro.org>
* ar/diff-index-merge-base-fix (2023-10-02) 1 commit
- diff: fix --merge-base with annotated tags
source: <20231001151845.3621551-1-hi@alyssa.is>
--------------------------------------------------
[Stalled]
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
- diff-lib: fix check_removed() when fsmonitor is active
- Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
- Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
It is unknown if the optimization is worth resurrecting, but in case...
source: <xmqqr0n0h0tw.fsf@gitster.g>
--------------------------------------------------
[Cooking]
* bb/unicode-width-table-15 (2023-09-25) 1 commit
(merged to 'next' on 2023-09-28 at bb76f46606)
+ unicode: update the width tables to Unicode 15.1
The display width table for unicode characters has been updated for
Unicode 15.1
Will merge to 'master'.
source: <20230925190704.157731-1-dev+git@drbeat.li>
* eb/limit-bulk-checkin-to-blobs (2023-09-26) 1 commit
(merged to 'next' on 2023-10-02 at 89c9c95966)
+ bulk-checkin: only support blobs in index_bulk_checkin
The "streaming" interface used for bulk-checkin codepath has been
narrowed to take only blob objects for now, with no real loss of
functionality.
Will merge to 'master'.
source: <87msx99b9o.fsf_-_@gmail.froward.int.ebiederm.org>
* jk/commit-graph-verify-fix (2023-09-28) 6 commits
(merged to 'next' on 2023-09-28 at e3ed560a2f)
+ commit-graph: report incomplete chains during verification
+ commit-graph: tighten chain size check
+ commit-graph: detect read errors when verifying graph chain
+ t5324: harmonize sha1/sha256 graph chain corruption
+ commit-graph: check mixed generation validation when loading chain file
+ commit-graph: factor out chain opening function
Various fixes to "git commit-graph verify".
Will merge to 'master'.
source: <20230928043746.GB57926@coredump.intra.peff.net>
* js/update-urls-in-doc-and-comment (2023-09-26) 4 commits
- doc: refer to internet archive
- doc: update links for andre-simon.de
- doc: update links to current pages
- doc: switch links to https
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
Needs eyeballing.
source: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>
* la/trailer-cleanups (2023-09-26) 4 commits
- trailer: only use trailer_block_* variables if trailers were found
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Needs review.
source: <pull.1563.v4.git.1695709372.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Breaks a few CI jobs when merged to 'seen'.
cf. <xmqqbkdmjbkp.fsf@gitster.g>
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* xz/commit-title-soft-limit-doc (2023-09-28) 1 commit
(merged to 'next' on 2023-09-28 at 20df852430)
+ doc: correct the 50 characters soft limit
Doc tweak.
Will merge to 'master'.
source: <pull.1580.git.git.1695895155985.gitgitgadget@gmail.com>
* jx/remote-archive-over-smart-http (2023-09-25) 3 commits
- archive: support remote archive from stateless transport
- transport-helper: run do_take_over in connect_helper
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Expecting a reroll.
cf. <CANYiYbFkG+CvrNFBkdNewZs7ADROVsjd051SDQsU0zVq8eBhew@mail.gmail.com>
source: <20230923152201.14741-1-worldhello.net@gmail.com>
* jx/sideband-chomp-newline-fix (2023-09-25) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Needs review.
source: <CANYiYbF+Xmk4rCNLMJe+i_CFafg8=QU5vbXWNUZbOVsDLTe5QQ@mail.gmail.com>
* ks/ref-filter-mailmap (2023-09-25) 3 commits
(merged to 'next' on 2023-09-28 at 0d3fd9959a)
+ ref-filter: add mailmap support
+ t/t6300: introduce test_bad_atom
+ t/t6300: cleanup test_atom
"git for-each-ref" and friends learn to apply mailmap to authorname
and other fields.
Will merge to 'master'.
source: <20230925175050.3498-1-five231003@gmail.com>
* ps/revision-cmdline-stdin-not (2023-09-25) 1 commit
(merged to 'next' on 2023-09-28 at a28201e0dd)
+ revision: make pseudo-opt flags read via stdin behave consistently
"git rev-list --stdin" learned to take non-revisions (like "--not")
recently from the standard input, but the way such a "--not" was
handled was quite confusing, which has been rethought. This is
potentially a change that breaks backward compatibility.
Will merge to 'master'.
source: <6221acd2796853144f8e84081655fbc79fdc6634.1695646898.git.ps@pks.im>
* ty/merge-tree-strategy-options (2023-09-25) 1 commit
(merged to 'next' on 2023-09-29 at aa65b54416)
+ merge-tree: add -X strategy option
"git merge-tree" learned to take strategy backend specific options
via the "-X" option, like "git merge" does.
Will merge to 'master'.
source: <pull.1565.v6.git.1695522222723.gitgitgadget@gmail.com>
* js/ci-coverity (2023-09-25) 7 commits
- SQUASH???
- coverity: detect and report when the token or project is incorrect
- coverity: allow running on macOS
- coverity: support building on Windows
- coverity: allow overriding the Coverity project
- coverity: cache the Coverity Build Tool
- ci: add a GitHub workflow to submit Coverity scans
GitHub CI workflow has learned to trigger Coverity check.
Looking good.
source: <pull.1588.v2.git.1695642662.gitgitgadget@gmail.com>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
source: <cover.1695330852.git.steadmon@google.com>
* jc/fake-lstat (2023-09-15) 1 commit
- cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Needs review.
source: <xmqqcyykig1l.fsf@gitster.g>
* kn/rev-list-ignore-missing-links (2023-09-20) 1 commit
- revision: add `--ignore-missing-links` user option
Surface the .ignore_missing_links bit that stops the revision
traversal from stopping and dying when encountering a missing
object to a new command line option of "git rev-list", so that the
objects that are required but are missing can be enumerated.
Waiting for review response.
source: <20230920104507.21664-1-karthik.188@gmail.com>
* rs/parse-options-value-int (2023-09-18) 2 commits
- parse-options: use and require int pointer for OPT_CMDMODE
- parse-options: add int value pointer to struct option
A bit of type safety for the "value" pointer used in the
parse-options API.
Comments?
source: <e6d8a291-03de-cfd3-3813-747fc2cad145@web.de>
* so/diff-merges-d (2023-09-11) 2 commits
- diff-merges: introduce '-d' option
- diff-merges: improve --diff-merges documentation
Teach a new "-d" option that shows the patch against the first
parent for merge commits (which is "--diff-merges=first-parent -p").
Letting a less useful combination of options squat on short-and-sweet "-d" feels dubious.
source: <20230909125446.142715-1-sorganov@gmail.com>
* cc/repack-sift-filtered-objects-to-separate-pack (2023-10-02) 9 commits
- gc: add `gc.repackFilterTo` config option
- repack: implement `--filter-to` for storing filtered out objects
- gc: add `gc.repackFilter` config option
- repack: add `--filter=<filter-spec>` option
- pack-bitmap-write: rebuild using new bitmap when remapping
- repack: refactor finding pack prefix
- repack: refactor finishing pack-objects command
- t/helper: add 'find-pack' test-tool
- pack-objects: allow `--filter` without `--stdout`
"git repack" machinery learns to pay attention to the "--filter="
option.
Will merge to 'next'.
cf. <ZRsknb4NxNHTR21E@nand.local>
source: <20231002165504.1325153-1-christian.couder@gmail.com>
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* cc/git-replay (2023-09-07) 15 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: disallow revision specific options and pathspecs
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: don't simplify history
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
Waiting for review response.
cf. <52277471-4ddd-b2e0-62ca-c2a5b59ae418@gmx.de>
cf. <58daa706-7efb-51dd-9061-202ef650b96a@gmx.de>
cf. <f0e75d47-c277-9fbb-7bcd-53e4e5686f3c@gmx.de>
May want to wait until tb/repack-existing-packs-cleanup stabilizes.
source: <20230907092521.733746-1-christian.couder@gmail.com>
* la/trailer-test-and-doc-updates (2023-09-07) 13 commits
- trailer doc: <token> is a <key> or <keyAlias>, not both
- trailer doc: separator within key suppresses default separator
- trailer doc: emphasize the effect of configuration variables
- trailer --unfold help: prefer "reformat" over "join"
- trailer --parse docs: add explanation for its usefulness
- trailer --only-input: prefer "configuration variables" over "rules"
- trailer --parse help: expose aliased options
- trailer --no-divider help: describe usual "---" meaning
- trailer: trailer location is a place, not an action
- trailer doc: narrow down scope of --where and related flags
- trailer: add tests to check defaulting behavior with --no-* flags
- trailer test description: this tests --where=after, not --where=before
- trailer tests: make test cases self-contained
Test coverage for trailers has been improved.
source: <pull.1564.v3.git.1694125209.gitgitgadget@gmail.com>
* js/doc-unit-tests (2023-08-17) 3 commits
- ci: run unit tests in CI
- unit tests: add TAP unit test framework
- unit tests: Add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Waiting for review response.
cf. <xmqq350hw6n7.fsf@gitster.g>
source: <cover.1692297001.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-09-25) 7 commits
- cmake: handle also unit tests
- cmake: use test names instead of full paths
- cmake: fix typo in variable name
- artifacts-tar: when including `.dll` files, don't forget the unit-tests
- unit-tests: do show relative file paths
- unit-tests: do not mistake `.pdb` files for being executable
- cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
Waiting for the base topic to settle.
source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
* tb/path-filter-fix (2023-08-30) 15 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Still being discussed.
cf. <20230830200218.GA5147@szeder.dev>
cf. <20230901205616.3572722-1-jonathantanmy@google.com>
cf. <20230924195900.GA1156862@szeder.dev>
source: <cover.1693413637.git.jonathantanmy@google.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-08-01) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
cf. <xmqqtttia3vn.fsf@gitster.g>
source: <48745298-f12b-8efb-4e48-90d2c22a8349@gmail.com>
--------------------------------------------------
[Discarded]
* tb/ci-coverity (2023-09-21) 1 commit
. .github/workflows: add coverity action
GitHub CI workflow has learned to trigger Coverity check.
Superseded by the js/ci-coverity topic.
source: <b23951c569660e1891a7fb3ad2c2ea1952897bd7.1695332105.git.me@ttaylorr.com>
* cw/git-std-lib (2023-09-11) 7 commits
. SQUASH???
. git-std-lib: add test file to call git-std-lib.a functions
. git-std-lib: introduce git standard library
. parse: create new library for parsing strings and env values
. config: correct bad boolean env value error message
. wrapper: remove dependency to Git-specific internal file
. hex-ll: split out functionality from hex
Another libification effort.
Superseded by the cw/prelim-cleanup topic.
cf. <xmqqy1hfrk6p.fsf@gitster.g>
cf. <20230915183927.1597414-1-jonathantanmy@google.com>
source: <20230908174134.1026823-1-calvinwan@google.com>
^ permalink raw reply
* Re: [RFC PATCH] Not computing changed path filter for root commits
From: Jonathan Tan @ 2023-10-02 22:55 UTC (permalink / raw)
To: Taylor Blau; +Cc: Jonathan Tan, SZEDER Gábor, git
In-Reply-To: <ZQnmTXUO94/Qy8mq@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
> diff --git a/revision.c b/revision.c
> index 2f4c53ea20..1d36df49e2 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -837,14 +837,24 @@ static int rev_compare_tree(struct rev_info *revs,
> static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
> {
> struct tree *t1 = repo_get_commit_tree(the_repository, commit);
> + int bloom_ret = 1;
>
> if (!t1)
> return 0;
>
> + if (revs->bloom_keys_nr) {
> + bloom_ret = check_maybe_different_in_bloom_filter(revs, commit);
> + if (!bloom_ret)
> + return 1;
> + }
> +
> tree_difference = REV_TREE_SAME;
> revs->pruning.flags.has_changes = 0;
> diff_tree_oid(NULL, &t1->object.oid, "", &revs->pruning);
>
> + if (bloom_ret == 1 && tree_difference == REV_TREE_SAME)
> + count_bloom_filter_false_positive++;
> +
> return tree_difference == REV_TREE_SAME;
> }
I'll concentrate on getting this patch in, and will look at (and
discuss) the other Bloom filter-related emails later.
This looks good, possibly except a code path in try_to_simplify_commit()
that calls this rev_same_tree_as_empty() function when
rev_compare_tree() between a commit and its parent returns REV_TREE_NEW.
So there are 2 issues: How can rev_compare_tree() ever return
REV_TREE_NEW? And it doesn't seem right to check Bloom filters in this
code path, since rev_same_tree_as_empty() was invoked here while we are
enumerating through a commit's parents, which necessarily implies that
the commit has parents, but here we're using the Bloom filter as if the
commit is known to have no parents.
As for the first issue, rev_compare_tree() returns REV_TREE_NEW when the
parent's tree is NULL. I'm not sure how this can happen - the tree can
be NULL if the parent commit is not parsed, but at this point I think
that it has been parsed. And I think every commit has a tree. This goes
back all the way to 3a5e860815 (revision: make tree comparison functions
take commits rather than trees, 2008-11-03) and even beyond that (I
didn't dig further).
As for the second issue, we can probably solve this by being defensive
in rev_same_tree_as_empty() by only using the Bloom filter when the
commit has no parents. Not sure if this is being overly defensive,
though.
There is also the issue that count_bloom_filter_false_positive is
incremented even when no Bloom filters are present, but I think this is
fine (it matches the behavior of rev_compare_tree()).
^ permalink raw reply
* Re: [PATCH 2/2] builtin/repack.c: implement support for `--cruft-max-size`
From: Taylor Blau @ 2023-10-02 20:30 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Jeff King, Jonathan Tan
In-Reply-To: <ZPsDyKOa76Mxb9u-@tanuki>
On Fri, Sep 08, 2023 at 01:21:44PM +0200, Patrick Steinhardt wrote:
> On Thu, Sep 07, 2023 at 05:52:04PM -0400, Taylor Blau wrote:
> [snip]
> > @@ -125,17 +133,39 @@ static void mark_packs_for_deletion_1(struct string_list *names,
> > if (len < hexsz)
> > continue;
> > sha1 = item->string + len - hexsz;
> > - /*
> > - * Mark this pack for deletion, which ensures that this
> > - * pack won't be included in a MIDX (if `--write-midx`
> > - * was given) and that we will actually delete this pack
> > - * (if `-d` was given).
> > - */
> > - if (!string_list_has_string(names, sha1))
> > - item->util = (void*)1;
> > +
> > + if (pack_is_retained(item)) {
> > + item->util = NULL;
> > + } else if (!string_list_has_string(names, sha1)) {
> > + /*
> > + * Mark this pack for deletion, which ensures
> > + * that this pack won't be included in a MIDX
> > + * (if `--write-midx` was given) and that we
> > + * will actually delete this pack (if `-d` was
> > + * given).
> > + */
> > + item->util = DELETE_PACK;
> > + }
>
> I find the behaviour of this function a tad surprising as it doesn't
> only mark a pack for deletion, but it also marks a pack as not being
> retained anymore. Shouldn't we rather:
>
> if (pack_is_retained(item)) {
> // Theoretically speaking we shouldn't even do this bit here as
> // we _un_mark the pack for deletion. But at least we shouldn't
> // be removing the `RETAIN_PACK` bit, I'd think.
> item->util &= ~DELETE_PACK;
> } else if (!string_list_has_string(names, sha1)) {
> // And here we shouldn't discard the `RETAIN_PACK` bit either.
> item->util |= DELETE_PACK;
> }
I think the new version should address these issues. But yeah, I
definitely understand your confusion here. I think what's written in
this patch is OK, because we check only whether the `->util` field is
non-NULL before deleting, which is why we have to remove the RETAINED
bit.
But the new version looks like this instead:
if (pack_is_retained(item))
pack_unmark_for_deletion(item);
else if (!string_list_has_string(names, sha1))
pack_mark_for_deletion(item);
the RETAINED bits still stick around (pack_unmark_for_deletion() just
does `item->util &= ~DELETE_PACK`), but we don't consult them after
mark_packs_for_deletion_1() has finished executing. Instead we just
check for the existence of the DELETE_PACK bit, rather than whether or
not the whole util field is NULL.
> > @@ -799,6 +831,72 @@ static void remove_redundant_bitmaps(struct string_list *include,
> > strbuf_release(&path);
> > }
> >
> > +static int existing_cruft_pack_cmp(const void *va, const void *vb)
> > +{
> > + struct packed_git *a = *(struct packed_git **)va;
> > + struct packed_git *b = *(struct packed_git **)vb;
> > +
> > + if (a->pack_size < b->pack_size)
> > + return -1;
> > + if (a->pack_size > b->pack_size)
> > + return 1;
> > + return 0;
> > +}
> > +
> > +static void collapse_small_cruft_packs(FILE *in, unsigned long max_size,
>
> We might want to use `size_t` to denote file sizes instead of `unsigned
> long`.
We can safely change these to use size_t, but let's leave OPT_MAGNITUDE
alone (and treat that portion as #leftoverbits).
> > + p = existing_cruft[i];
> > + proposed = st_add(total_size, p->pack_size);
> > +
> > + if (proposed <= max_size) {
> > + total_size = proposed;
> > + fprintf(in, "-%s\n", pack_basename(p));
> > + } else {
> > + retain_cruft_pack(existing, p);
> > + fprintf(in, "%s\n", pack_basename(p));
> > + }
>
> It's a bit funny that we re-check whether we have exceeded the maximum
> size in subsequente iterations once we hit the limit, but it arguably
> makes the logic a bit simpler.
Yeah. Those checks are all noops (IOW, once we end up in the else
branch, we'll stay there for the rest of the loop). But we don't want to
break early, because we have to call retain_cruft_pack() on everything.
In theory you could do something like:
for (i = 0; i < existing_cruft_nr; i++) {
size_t proposed;
p = existing_cruft[i];
proposed = st_add(total_size, p->pack_size);
if (proposed <= max_size) {
total_size = proposed;
fprintf(in, "-%s\n", pack_basename(p));
} else {
break;
}
}
for (; i < existing_cruft_nr; i++) {
retain_cruft_pack(existing, existing_cruft[i]);
fprintf(in, "%s\n", pack_basename(existing_cruft[i]));
}
But I think that the above is slightly more error-prone than what is
written in the original patch. I have only the vaguest of preferences
towards the former, but I'm happy to change it around if you feel
strongly.
> If I understand correctly, we only collapse small cruft packs in case
> we're not expiring any objects at the same time. Is there an inherent
> reason why? I would imagine that it can indeed be useful to expire
> objects contained in cruft packs and then have git-repack(1) recombine
> whatever is left into larger packs.
>
> If the reason is basically "it's complicated" then that is fine with me,
> we can still implement the functionality at a later point in time. But
> until then I think that we should let callers know that the two options
> are incompatible with each other by producing an error when both are
> passed.
Your understanding is correct. We could try to leave existing cruft
packs alone when none of their objects are removed as a result of
pruning, but that case should be relatively rare. Another thing you
could do is handle cruft packs which have only part of their objects
being pruned by combining the non-pruned parts into a new pack.
The latter should be mostly straightforward to implement, but since
we're often ending up with very few cruft objects post-pruning, it
likely wouldn't help much.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v8 0/9] Repack objects into separate packfiles based on a filter
From: Taylor Blau @ 2023-10-02 20:14 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, John Cai, Jonathan Tan, Jonathan Nieder,
Derrick Stolee, Patrick Steinhardt
In-Reply-To: <20231002165504.1325153-1-christian.couder@gmail.com>
On Mon, Oct 02, 2023 at 06:54:55PM +0200, Christian Couder wrote:
> # Range-diff since v7
>
> 1: eec0c09731 = 1: b23d216277 pack-objects: allow `--filter` without `--stdout`
> 2: 19c8b8a4b9 ! 2: 27e70ccf39 t/helper: add 'find-pack' test-tool
> @@ t/helper/test-tool.h: int cmd__dump_reftable(int argc, const char **argv);
> int cmd__genrandom(int argc, const char **argv);
> int cmd__genzeros(int argc, const char **argv);
>
> - ## t/t0080-find-pack.sh (new) ##
> + ## t/t0081-find-pack.sh (new) ##
> @@
> +#!/bin/sh
> +
> 3: aaaf40bd5d = 3: 7e692c4cfd repack: refactor finishing pack-objects command
> 4: 1eb6bc3f7e = 4: 227159ed4e repack: refactor finding pack prefix
> 5: b9159e1803 = 5: 79786eb5e1 pack-bitmap-write: rebuild using new bitmap when remapping
> 6: f2f5bb54d3 = 6: 205d33850e repack: add `--filter=<filter-spec>` option
> 7: 7ea0307628 = 7: 16b1621169 gc: add `gc.repackFilter` config option
> 8: 698647815b = 8: 92a5ff7cc7 repack: implement `--filter-to` for storing filtered out objects
> 9: 57b2ba444c = 9: 5bfd918c90 gc: add `gc.repackFilterTo` config option
These all look just as good as v7 ;-).
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH] git-status.txt: fix minor asciidoc format issue
From: Junio C Hamano @ 2023-10-02 18:58 UTC (permalink / raw)
To: Javier Mora; +Cc: cousteau via GitGitGadget, Josh Soref, git
In-Reply-To: <CAH1-q0g+xdb_mUi0sXrQjF4nkX1Nkpops_V1e86qACLxs1uPqg@mail.gmail.com>
Javier Mora <cousteaulecommandant@gmail.com> writes:
> Only problem is that that patch doesn't use the same formatting as the
> rest of the document (uses a code block / preformatted text instead of
> a list as other options in the document do) so my version of the patch
> is just a minor cosmetic improvement now.
Either one is a minor cosmetic improvement ;-) I do agree with you
that it is more appropriate to use the enumeration. A patch on top
of 'master' (which now has Josh's fix) would be very much welcome.
Thanks.
^ permalink raw reply
* Re: [PATCH] diff: fix --merge-base with annotated tags
From: Junio C Hamano @ 2023-10-02 18:54 UTC (permalink / raw)
To: Alyssa Ross; +Cc: git, Denton Liu
In-Reply-To: <20231001151845.3621551-1-hi@alyssa.is>
Alyssa Ross <hi@alyssa.is> writes:
> Checking early for OBJ_COMMIT excludes other objects that can be
> resolved to commits, like annotated tags. If we remove it, annotated
> tags will be resolved and handled just fine by
> lookup_commit_reference(), and if we are given something that can't be
> resolved to a commit, we'll still get a useful error message, e.g.:
>
>> error: object 21ab162211ac3ef13c37603ca88b27e9c7e0d40b is a tree, not a commit
>> fatal: no merge base found
Interesting. 0f5a1d44 (builtin/diff-index: learn --merge-base,
2020-09-20) claims that it took inspiration from "git diff A...B"
but forgot that it needs to accept any commit-ish.
With a devil's advocate hat on, I have to wonder if it is really a
useful error message to spew a long hexadecimal string when the user
would certainly have gave a more mnemonic HEAD^{tree} or something,
but the original message does not say which command line argument it
did not like anyway, so the patch is a net improvement.
Will queue. Thanks.
^ permalink raw reply
* [PATCH v8 6/9] repack: add `--filter=<filter-spec>` option
From: Christian Couder @ 2023-10-02 16:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, John Cai, Jonathan Tan, Jonathan Nieder,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Christian Couder,
Christian Couder
In-Reply-To: <20231002165504.1325153-1-christian.couder@gmail.com>
This new option puts the objects specified by `<filter-spec>` into a
separate packfile.
This could be useful if, for example, some blobs take up a lot of
precious space on fast storage while they are rarely accessed. It could
make sense to move them into a separate cheaper, though slower, storage.
It's possible to find which new packfile contains the filtered out
objects using one of the following:
- `git verify-pack -v ...`,
- `test-tool find-pack ...`, which a previous commit added,
- `--filter-to=<dir>`, which a following commit will add to specify
where the pack containing the filtered out objects will be.
This feature is implemented by running `git pack-objects` twice in a
row. The first command is run with `--filter=<filter-spec>`, using the
specified filter. It packs objects while omitting the objects specified
by the filter. Then another `git pack-objects` command is launched using
`--stdin-packs`. We pass it all the previously existing packs into its
stdin, so that it will pack all the objects in the previously existing
packs. But we also pass into its stdin, the pack created by the previous
`git pack-objects --filter=<filter-spec>` command as well as the kept
packs, all prefixed with '^', so that the objects in these packs will be
omitted from the resulting pack. The result is that only the objects
filtered out by the first `git pack-objects` command are in the pack
resulting from the second `git pack-objects` command.
As the interactions with kept packs are a bit tricky, a few related
tests are added.
Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-repack.txt | 12 ++++
builtin/repack.c | 70 ++++++++++++++++++
t/t7700-repack.sh | 135 +++++++++++++++++++++++++++++++++++
3 files changed, 217 insertions(+)
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 4017157949..6d5bec7716 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -143,6 +143,18 @@ depth is 4095.
a larger and slower repository; see the discussion in
`pack.packSizeLimit`.
+--filter=<filter-spec>::
+ Remove objects matching the filter specification from the
+ resulting packfile and put them into a separate packfile. Note
+ that objects used in the working directory are not filtered
+ out. So for the split to fully work, it's best to perform it
+ in a bare repo and to use the `-a` and `-d` options along with
+ this option. Also `--no-write-bitmap-index` (or the
+ `repack.writebitmaps` config option set to `false`) should be
+ used otherwise writing bitmap index will fail, as it supposes
+ a single packfile containing all the objects. See
+ linkgit:git-rev-list[1] for valid `<filter-spec>` forms.
+
-b::
--write-bitmap-index::
Write a reachability bitmap index as part of the repack. This
diff --git a/builtin/repack.c b/builtin/repack.c
index 9ef0044384..c7b564192f 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -21,6 +21,7 @@
#include "pack.h"
#include "pack-bitmap.h"
#include "refs.h"
+#include "list-objects-filter-options.h"
#define ALL_INTO_ONE 1
#define LOOSEN_UNREACHABLE 2
@@ -56,6 +57,7 @@ struct pack_objects_args {
int no_reuse_object;
int quiet;
int local;
+ struct list_objects_filter_options filter_options;
};
static int repack_config(const char *var, const char *value,
@@ -836,6 +838,56 @@ static int finish_pack_objects_cmd(struct child_process *cmd,
return finish_command(cmd);
}
+static int write_filtered_pack(const struct pack_objects_args *args,
+ const char *destination,
+ const char *pack_prefix,
+ struct existing_packs *existing,
+ struct string_list *names)
+{
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ struct string_list_item *item;
+ FILE *in;
+ int ret;
+ const char *caret;
+ const char *scratch;
+ int local = skip_prefix(destination, packdir, &scratch);
+
+ prepare_pack_objects(&cmd, args, destination);
+
+ strvec_push(&cmd.args, "--stdin-packs");
+
+ if (!pack_kept_objects)
+ strvec_push(&cmd.args, "--honor-pack-keep");
+ for_each_string_list_item(item, &existing->kept_packs)
+ strvec_pushf(&cmd.args, "--keep-pack=%s", item->string);
+
+ cmd.in = -1;
+
+ ret = start_command(&cmd);
+ if (ret)
+ return ret;
+
+ /*
+ * Here 'names' contains only the pack(s) that were just
+ * written, which is exactly the packs we want to keep. Also
+ * 'existing_kept_packs' already contains the packs in
+ * 'keep_pack_list'.
+ */
+ in = xfdopen(cmd.in, "w");
+ for_each_string_list_item(item, names)
+ fprintf(in, "^%s-%s.pack\n", pack_prefix, item->string);
+ for_each_string_list_item(item, &existing->non_kept_packs)
+ fprintf(in, "%s.pack\n", item->string);
+ for_each_string_list_item(item, &existing->cruft_packs)
+ fprintf(in, "%s.pack\n", item->string);
+ caret = pack_kept_objects ? "" : "^";
+ for_each_string_list_item(item, &existing->kept_packs)
+ fprintf(in, "%s%s.pack\n", caret, item->string);
+ fclose(in);
+
+ return finish_pack_objects_cmd(&cmd, names, local);
+}
+
static int write_cruft_pack(const struct pack_objects_args *args,
const char *destination,
const char *pack_prefix,
@@ -966,6 +1018,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
N_("limits the maximum number of threads")),
OPT_STRING(0, "max-pack-size", &po_args.max_pack_size, N_("bytes"),
N_("maximum size of each packfile")),
+ OPT_PARSE_LIST_OBJECTS_FILTER(&po_args.filter_options),
OPT_BOOL(0, "pack-kept-objects", &pack_kept_objects,
N_("repack objects in packs marked with .keep")),
OPT_STRING_LIST(0, "keep-pack", &keep_pack_list, N_("name"),
@@ -979,6 +1032,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
OPT_END()
};
+ list_objects_filter_init(&po_args.filter_options);
+
git_config(repack_config, &cruft_po_args);
argc = parse_options(argc, argv, prefix, builtin_repack_options,
@@ -1119,6 +1174,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
strvec_push(&cmd.args, "--incremental");
}
+ if (po_args.filter_options.choice)
+ strvec_pushf(&cmd.args, "--filter=%s",
+ expand_list_objects_filter_spec(&po_args.filter_options));
+
if (geometry.split_factor)
cmd.in = -1;
else
@@ -1205,6 +1264,16 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
}
}
+ if (po_args.filter_options.choice) {
+ ret = write_filtered_pack(&po_args,
+ packtmp,
+ find_pack_prefix(packdir, packtmp),
+ &existing,
+ &names);
+ if (ret)
+ goto cleanup;
+ }
+
string_list_sort(&names);
close_object_store(the_repository->objects);
@@ -1297,6 +1366,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
string_list_clear(&names, 1);
existing_packs_release(&existing);
free_pack_geometry(&geometry);
+ list_objects_filter_release(&po_args.filter_options);
return ret;
}
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 27b66807cd..39e89445fd 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -327,6 +327,141 @@ test_expect_success 'auto-bitmaps do not complain if unavailable' '
test_must_be_empty actual
'
+test_expect_success 'repacking with a filter works' '
+ git -C bare.git repack -a -d &&
+ test_stdout_line_count = 1 ls bare.git/objects/pack/*.pack &&
+ git -C bare.git -c repack.writebitmaps=false repack -a -d --filter=blob:none &&
+ test_stdout_line_count = 2 ls bare.git/objects/pack/*.pack &&
+ commit_pack=$(test-tool -C bare.git find-pack -c 1 HEAD) &&
+ blob_pack=$(test-tool -C bare.git find-pack -c 1 HEAD:file1) &&
+ test "$commit_pack" != "$blob_pack" &&
+ tree_pack=$(test-tool -C bare.git find-pack -c 1 HEAD^{tree}) &&
+ test "$tree_pack" = "$commit_pack" &&
+ blob_pack2=$(test-tool -C bare.git find-pack -c 1 HEAD:file2) &&
+ test "$blob_pack2" = "$blob_pack"
+'
+
+test_expect_success '--filter fails with --write-bitmap-index' '
+ test_must_fail \
+ env GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0 \
+ git -C bare.git repack -a -d --write-bitmap-index --filter=blob:none
+'
+
+test_expect_success 'repacking with two filters works' '
+ git init two-filters &&
+ (
+ cd two-filters &&
+ mkdir subdir &&
+ test_commit foo &&
+ test_commit subdir_bar subdir/bar &&
+ test_commit subdir_baz subdir/baz
+ ) &&
+ git clone --no-local --bare two-filters two-filters.git &&
+ (
+ cd two-filters.git &&
+ test_stdout_line_count = 1 ls objects/pack/*.pack &&
+ git -c repack.writebitmaps=false repack -a -d \
+ --filter=blob:none --filter=tree:1 &&
+ test_stdout_line_count = 2 ls objects/pack/*.pack &&
+ commit_pack=$(test-tool find-pack -c 1 HEAD) &&
+ blob_pack=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ root_tree_pack=$(test-tool find-pack -c 1 HEAD^{tree}) &&
+ subdir_tree_hash=$(git ls-tree --object-only HEAD -- subdir) &&
+ subdir_tree_pack=$(test-tool find-pack -c 1 "$subdir_tree_hash") &&
+
+ # Root tree and subdir tree are not in the same packfiles
+ test "$commit_pack" != "$blob_pack" &&
+ test "$commit_pack" = "$root_tree_pack" &&
+ test "$blob_pack" = "$subdir_tree_pack"
+ )
+'
+
+prepare_for_keep_packs () {
+ git init keep-packs &&
+ (
+ cd keep-packs &&
+ test_commit foo &&
+ test_commit bar
+ ) &&
+ git clone --no-local --bare keep-packs keep-packs.git &&
+ (
+ cd keep-packs.git &&
+
+ # Create two packs
+ # The first pack will contain all of the objects except one blob
+ git rev-list --objects --all >objs &&
+ grep -v "bar.t" objs | git pack-objects pack &&
+ # The second pack will contain the excluded object and be kept
+ packid=$(grep "bar.t" objs | git pack-objects pack) &&
+ >pack-$packid.keep &&
+
+ # Replace the existing pack with the 2 new ones
+ rm -f objects/pack/pack* &&
+ mv pack-* objects/pack/
+ )
+}
+
+test_expect_success '--filter works with .keep packs' '
+ prepare_for_keep_packs &&
+ (
+ cd keep-packs.git &&
+
+ foo_pack=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ bar_pack=$(test-tool find-pack -c 1 HEAD:bar.t) &&
+ head_pack=$(test-tool find-pack -c 1 HEAD) &&
+
+ test "$foo_pack" != "$bar_pack" &&
+ test "$foo_pack" = "$head_pack" &&
+
+ git -c repack.writebitmaps=false repack -a -d --filter=blob:none &&
+
+ foo_pack_1=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ bar_pack_1=$(test-tool find-pack -c 1 HEAD:bar.t) &&
+ head_pack_1=$(test-tool find-pack -c 1 HEAD) &&
+
+ # Object bar is still only in the old .keep pack
+ test "$foo_pack_1" != "$foo_pack" &&
+ test "$bar_pack_1" = "$bar_pack" &&
+ test "$head_pack_1" != "$head_pack" &&
+
+ test "$foo_pack_1" != "$bar_pack_1" &&
+ test "$foo_pack_1" != "$head_pack_1" &&
+ test "$bar_pack_1" != "$head_pack_1"
+ )
+'
+
+test_expect_success '--filter works with --pack-kept-objects and .keep packs' '
+ rm -rf keep-packs keep-packs.git &&
+ prepare_for_keep_packs &&
+ (
+ cd keep-packs.git &&
+
+ foo_pack=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ bar_pack=$(test-tool find-pack -c 1 HEAD:bar.t) &&
+ head_pack=$(test-tool find-pack -c 1 HEAD) &&
+
+ test "$foo_pack" != "$bar_pack" &&
+ test "$foo_pack" = "$head_pack" &&
+
+ git -c repack.writebitmaps=false repack -a -d --filter=blob:none \
+ --pack-kept-objects &&
+
+ foo_pack_1=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ test-tool find-pack -c 2 HEAD:bar.t >bar_pack_1 &&
+ head_pack_1=$(test-tool find-pack -c 1 HEAD) &&
+
+ test "$foo_pack_1" != "$foo_pack" &&
+ test "$foo_pack_1" != "$bar_pack" &&
+ test "$head_pack_1" != "$head_pack" &&
+
+ # Object bar is in both the old .keep pack and the new
+ # pack that contained the filtered out objects
+ grep "$bar_pack" bar_pack_1 &&
+ grep "$foo_pack_1" bar_pack_1 &&
+ test "$foo_pack_1" != "$head_pack_1"
+ )
+'
+
objdir=.git/objects
midx=$objdir/pack/multi-pack-index
--
2.42.0.305.g5bfd918c90
^ permalink raw reply related
* [PATCH v8 8/9] repack: implement `--filter-to` for storing filtered out objects
From: Christian Couder @ 2023-10-02 16:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, John Cai, Jonathan Tan, Jonathan Nieder,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Christian Couder,
Christian Couder
In-Reply-To: <20231002165504.1325153-1-christian.couder@gmail.com>
A previous commit has implemented `git repack --filter=<filter-spec>` to
allow users to filter out some objects from the main pack and move them
into a new different pack.
It would be nice if this new different pack could be created in a
different directory than the regular pack. This would make it possible
to move large blobs into a pack on a different kind of storage, for
example cheaper storage.
Even in a different directory, this pack can be accessible if, for
example, the Git alternates mechanism is used to point to it. In fact
not using the Git alternates mechanism can corrupt a repo as the
generated pack containing the filtered objects might not be accessible
from the repo any more. So setting up the Git alternates mechanism
should be done before using this feature if the user wants the repo to
be fully usable while this feature is used.
In some cases, like when a repo has just been cloned or when there is no
other activity in the repo, it's Ok to setup the Git alternates
mechanism afterwards though. It's also Ok to just inspect the generated
packfile containing the filtered objects and then just move it into the
'.git/objects/pack/' directory manually. That's why it's not necessary
for this command to check that the Git alternates mechanism has been
already setup.
While at it, as an example to show that `--filter` and `--filter-to`
work well with other options, let's also add a test to check that these
options work well with `--max-pack-size`.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-repack.txt | 11 +++++++
builtin/repack.c | 10 +++++-
t/t7700-repack.sh | 62 ++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 6d5bec7716..8545a32667 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -155,6 +155,17 @@ depth is 4095.
a single packfile containing all the objects. See
linkgit:git-rev-list[1] for valid `<filter-spec>` forms.
+--filter-to=<dir>::
+ Write the pack containing filtered out objects to the
+ directory `<dir>`. Only useful with `--filter`. This can be
+ used for putting the pack on a separate object directory that
+ is accessed through the Git alternates mechanism. **WARNING:**
+ If the packfile containing the filtered out objects is not
+ accessible, the repo can become corrupt as it might not be
+ possible to access the objects in that packfile. See the
+ `objects` and `objects/info/alternates` sections of
+ linkgit:gitrepository-layout[5].
+
-b::
--write-bitmap-index::
Write a reachability bitmap index as part of the repack. This
diff --git a/builtin/repack.c b/builtin/repack.c
index c7b564192f..db9277081d 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -977,6 +977,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
int write_midx = 0;
const char *cruft_expiration = NULL;
const char *expire_to = NULL;
+ const char *filter_to = NULL;
struct option builtin_repack_options[] = {
OPT_BIT('a', NULL, &pack_everything,
@@ -1029,6 +1030,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
N_("write a multi-pack index of the resulting packs")),
OPT_STRING(0, "expire-to", &expire_to, N_("dir"),
N_("pack prefix to store a pack containing pruned objects")),
+ OPT_STRING(0, "filter-to", &filter_to, N_("dir"),
+ N_("pack prefix to store a pack containing filtered out objects")),
OPT_END()
};
@@ -1177,6 +1180,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
if (po_args.filter_options.choice)
strvec_pushf(&cmd.args, "--filter=%s",
expand_list_objects_filter_spec(&po_args.filter_options));
+ else if (filter_to)
+ die(_("option '%s' can only be used along with '%s'"), "--filter-to", "--filter");
if (geometry.split_factor)
cmd.in = -1;
@@ -1265,8 +1270,11 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
}
if (po_args.filter_options.choice) {
+ if (!filter_to)
+ filter_to = packtmp;
+
ret = write_filtered_pack(&po_args,
- packtmp,
+ filter_to,
find_pack_prefix(packdir, packtmp),
&existing,
&names);
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 39e89445fd..48e92aa6f7 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -462,6 +462,68 @@ test_expect_success '--filter works with --pack-kept-objects and .keep packs' '
)
'
+test_expect_success '--filter-to stores filtered out objects' '
+ git -C bare.git repack -a -d &&
+ test_stdout_line_count = 1 ls bare.git/objects/pack/*.pack &&
+
+ git init --bare filtered.git &&
+ git -C bare.git -c repack.writebitmaps=false repack -a -d \
+ --filter=blob:none \
+ --filter-to=../filtered.git/objects/pack/pack &&
+ test_stdout_line_count = 1 ls bare.git/objects/pack/pack-*.pack &&
+ test_stdout_line_count = 1 ls filtered.git/objects/pack/pack-*.pack &&
+
+ commit_pack=$(test-tool -C bare.git find-pack -c 1 HEAD) &&
+ blob_pack=$(test-tool -C bare.git find-pack -c 0 HEAD:file1) &&
+ blob_hash=$(git -C bare.git rev-parse HEAD:file1) &&
+ test -n "$blob_hash" &&
+ blob_pack=$(test-tool -C filtered.git find-pack -c 1 $blob_hash) &&
+
+ echo $(pwd)/filtered.git/objects >bare.git/objects/info/alternates &&
+ blob_pack=$(test-tool -C bare.git find-pack -c 1 HEAD:file1) &&
+ blob_content=$(git -C bare.git show $blob_hash) &&
+ test "$blob_content" = "content1"
+'
+
+test_expect_success '--filter works with --max-pack-size' '
+ rm -rf filtered.git &&
+ git init --bare filtered.git &&
+ git init max-pack-size &&
+ (
+ cd max-pack-size &&
+ test_commit base &&
+ # two blobs which exceed the maximum pack size
+ test-tool genrandom foo 1048576 >foo &&
+ git hash-object -w foo &&
+ test-tool genrandom bar 1048576 >bar &&
+ git hash-object -w bar &&
+ git add foo bar &&
+ git commit -m "adding foo and bar"
+ ) &&
+ git clone --no-local --bare max-pack-size max-pack-size.git &&
+ (
+ cd max-pack-size.git &&
+ git -c repack.writebitmaps=false repack -a -d --filter=blob:none \
+ --max-pack-size=1M \
+ --filter-to=../filtered.git/objects/pack/pack &&
+ echo $(cd .. && pwd)/filtered.git/objects >objects/info/alternates &&
+
+ # Check that the 3 blobs are in different packfiles in filtered.git
+ test_stdout_line_count = 3 ls ../filtered.git/objects/pack/pack-*.pack &&
+ test_stdout_line_count = 1 ls objects/pack/pack-*.pack &&
+ foo_pack=$(test-tool find-pack -c 1 HEAD:foo) &&
+ bar_pack=$(test-tool find-pack -c 1 HEAD:bar) &&
+ base_pack=$(test-tool find-pack -c 1 HEAD:base.t) &&
+ test "$foo_pack" != "$bar_pack" &&
+ test "$foo_pack" != "$base_pack" &&
+ test "$bar_pack" != "$base_pack" &&
+ for pack in "$foo_pack" "$bar_pack" "$base_pack"
+ do
+ case "$foo_pack" in */filtered.git/objects/pack/*) true ;; *) return 1 ;; esac
+ done
+ )
+'
+
objdir=.git/objects
midx=$objdir/pack/multi-pack-index
--
2.42.0.305.g5bfd918c90
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox