On Mon, May 27, 2024 at 03:41:56PM +0000, Xing Xin via GitGitGadget wrote:
> From: Xing Xin <xingxin.xx@xxxxxxxxxxxxx>
>
> Currently we can use "transfer.fsckObjects" or "fetch.fsckObjects" to
> control whether to enable checks for broken objects during fetching. But
> these configs are only acknowledged by `fetch-pack.c:get_pack` and do
> not make sense when fetching from bundles or using bundle-uris.
Do they not make sense, or are they not effective? I assume you mean the
latter, right?
> This commit exposed the fetch-then-transfer configuration logic by
s/exposed/exposes/
> adding a new function `fetch_pack_fsck_objects` in fetch-pack.h. In next
> commit, this new function will be used by `unbundle` in fetching
> scenarios.
>
> Signed-off-by: Xing Xin <xingxin.xx@xxxxxxxxxxxxx>
> ---
> fetch-pack.c | 18 ++++++++++++------
> fetch-pack.h | 2 ++
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 7d2aef21add..81a64be6951 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -954,12 +954,7 @@ static int get_pack(struct fetch_pack_args *args,
> strvec_push(&cmd.args, alternate_shallow_file);
> }
>
> - if (fetch_fsck_objects >= 0
> - ? fetch_fsck_objects
> - : transfer_fsck_objects >= 0
> - ? transfer_fsck_objects
> - : 0)
> - fsck_objects = 1;
This statement is really weird to read, but that is certainly not the
fault of this patch, but...
> + fsck_objects = fetch_pack_fsck_objects();
>
> if (do_keep || args->from_promisor || index_pack_args || fsck_objects) {
> if (pack_lockfiles || fsck_objects)
> @@ -2046,6 +2041,17 @@ static const struct object_id *iterate_ref_map(void *cb_data)
> return &ref->old_oid;
> }
>
> +int fetch_pack_fsck_objects(void)
> +{
> + fetch_pack_setup();
> +
> + return fetch_fsck_objects >= 0
> + ? fetch_fsck_objects
> + : transfer_fsck_objects >= 0
> + ? transfer_fsck_objects
> + : 0;
> +}
... can we maybe rewrite it to something more customary here? The
following is way easier to read, at least for me.
int fetch_pack_fsck_objects(void)
{
fetch_pack_setup();
if (fetch_fsck_objects >= 0 ||
transfer_fsck_objects >= 0)
return 1;
return 0;
}
> struct ref *fetch_pack(struct fetch_pack_args *args,
> int fd[],
> const struct ref *ref,
> diff --git a/fetch-pack.h b/fetch-pack.h
> index 6775d265175..38956d9b748 100644
> --- a/fetch-pack.h
> +++ b/fetch-pack.h
> @@ -101,4 +101,6 @@ void negotiate_using_fetch(const struct oid_array *negotiation_tips,
> */
> int report_unmatched_refs(struct ref **sought, int nr_sought);
>
> +int fetch_pack_fsck_objects(void);
Let's add a comment here saying what this function does.
Patrick
Attachment:
signature.asc
Description: PGP signature
