Re: [PATCH v3 3/4] fetch-pack: expose fsckObjects configuration logic
From: Patrick Steinhardt <hidden>
Date: 2024-05-28 12:03:24
On Mon, May 27, 2024 at 03:41:56PM +0000, Xing Xin via GitGitGadget wrote:
From: Xing Xin <redacted> 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/
quoted hunk ↗ jump to hunk
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 <redacted> --- 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...
quoted hunk ↗ jump to hunk
+ 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;
}
quoted hunk ↗ jump to hunk
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
Attachments
- signature.asc [application/pgp-signature] 833 bytes