Re: [PATCH 4/4] fetch-pack: print and use dangling .gitmodules

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jan 24, 2021, 12:18 UTC
Message-ID: <87ft2q7ck4.fsf@evledraar.gmail.com>
In-Reply-To: <2d6d8c2671fe424c752994dcb5277d4d923e17a0.1611455251.git.jonathantanmy@google.com>

On Sun, Jan 24 2021, Jonathan Tan wrote:

Show 5 quoted lines

> +void register_found_gitmodules(const struct object_id *oid)
> +{
> +	oidset_insert(&gitmodules_found, oid);
> +}
> +

In fsck.c we only use this variable to insert into it, or in fsck_blob() to do the actual check, but then we either abort early if we've found it, or right after that:

        if (object_on_skiplist(options, oid))
                return 0;

So (along with comments I have below...) you could just use the existing "skiplist" option instead, no?

Show 47 quoted lines

>  int fsck_finish(struct fsck_options *options)
>  {
>  	int ret = 0;
> @@ -1262,10 +1267,13 @@ int fsck_finish(struct fsck_options *options)
>  		if (!buf) {
>  			if (is_promisor_object(oid))
>  				continue;
> -			ret |= report(options,
> -				      oid, OBJ_BLOB,
> -				      FSCK_MSG_GITMODULES_MISSING,
> -				      "unable to read .gitmodules blob");
> +			if (options->print_dangling_gitmodules)
> +				printf("%s\n", oid_to_hex(oid));
> +			else
> +				ret |= report(options,
> +					      oid, OBJ_BLOB,
> +					      FSCK_MSG_GITMODULES_MISSING,
> +					      "unable to read .gitmodules blob");
>  			continue;
>  		}
>  
> diff --git a/fsck.h b/fsck.h
> index 69cf715e79..4b8cf03445 100644
> --- a/fsck.h
> +++ b/fsck.h
> @@ -41,6 +41,12 @@ struct fsck_options {
>  	int *msg_type;
>  	struct oidset skiplist;
>  	kh_oid_map_t *object_names;
> +
> +	/*
> +	 * If 1, print the hashes of missing .gitmodules blobs instead of
> +	 * considering them to be errors.
> +	 */
> +	unsigned print_dangling_gitmodules:1;
>  };
>  
>  #define FSCK_OPTIONS_DEFAULT { NULL, fsck_error_function, 0, NULL, OIDSET_INIT }
> @@ -62,6 +68,8 @@ int fsck_walk(struct object *obj, void *data, struct fsck_options *options);
>  int fsck_object(struct object *obj, void *data, unsigned long size,
>  	struct fsck_options *options);
>  
> +void register_found_gitmodules(const struct object_id *oid);
> +
>  /*
>   * Some fsck checks are context-dependent, and may end up queued; run this
>   * after completing all fsck_object() calls in order to resolve any remaining

This whole thing seems just like the bad path I took in earlier rounds of my in-flight mktag series. You don't need this new custom API. You just setup an error handler for your fsck which ignores / prints / logs / whatever the OIDs you want if you get a FSCK_MSG_GITMODULES_MISSING error, which you then "return 0" on.

If you don't have FSCK_MSG_GITMODULES_MISSING punt and call fsck_error_function().