Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 5ad80b85b4..11f0fafd33 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -120,7 +120,7 @@ static int nr_threads;
> static int from_stdin;
> static int strict;
> static int do_fsck_object;
> -static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
> +static struct fsck_options fsck_options = FSCK_OPTIONS_MISSING_GITMODULES;
Hmph, I do not think this is a good way to go. Specifically,
fsck-cb.c with the definition of what this thing is, and in fsck.h
file the normal "options" initializers being defined quite far away
from where this is defined, it is hard to see what is different
between the normal strict one and MISSING_GITMODULES one.
Rather, it may be far simpler to keep only DEFAULT and STRICT, and
override .error_func at runtime in the codepath(s) that needs to,
which would make it more clear what is going on. That way, we do
not need the split initializers with _ERROR_FUNC, which is another
reason why the approach taken by this series is not a good idea (it
does not scale---error-func may seem so special to deserve having
two sets of macros that use the default one and leave the member
unspecified, but it won't stay to be special forever).
IOW,
> @@ -1761,7 +1743,6 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
>
> read_replace_refs = 0;
> fsck_options.walk = mark_link;
> - fsck_options.error_func = print_dangling_gitmodules;
I doubt this hunk is an improvement.
> diff --git a/fsck-cb.c b/fsck-cb.c
> new file mode 100644
> index 0000000000..465a49235a
> --- /dev/null
> +++ b/fsck-cb.c
> @@ -0,0 +1,16 @@
> +#include "git-compat-util.h"
> +#include "fsck.h"
> +
> +int fsck_error_cb_print_missing_gitmodules(struct fsck_options *o,
> + const struct object_id *oid,
> + enum object_type object_type,
> + enum fsck_msg_type msg_type,
> + enum fsck_msg_id msg_id,
> + const char *message)
> +{
> + if (msg_id == FSCK_MSG_GITMODULES_MISSING) {
> + puts(oid_to_hex(oid));
> + return 0;
> + }
> + return fsck_error_function(o, oid, object_type, msg_type, msg_id, message);
> +}
As Derrick noticed, I do not know if we want to have a separate file
for this single function. Shouldn't it be part of builtin/index-pack.c,
or do we want other places to do the same kind of checks?
Thanks.
