On Fri, Mar 28, 2025 at 11:37:25AM -0400, Todd Zullinger wrote:
> I wrote:
> > I don't have a lot of time to poke at this today, but I'll
> > make another test run on an s390x build host without that
> > patch, but where I can save the output and post it
> > somewhere.
> >
> > For the Fedora packaging, it will be something like this:
> >
> > make -C t all || {
> > (cd t && ./t5620-backfill.sh -ix);
> > ./print-failed-test-output;
> > }
>
> The matches file is empty.
>
> $ ls -lhn batch-trace matches
> -rw-r--r--. 1 1000 1000 31K Mar 28 11:09 batch-trace
> -rw-r--r--. 1 1000 1000 0 Mar 28 11:09 matches
>
> The only match in batch-trace for promisor fetch_count is
> from the previous test:
>
> $ grep -e '"category":"promisor","key":"fetch_count","value":' batch-trace
> {"event":"data","sid":"20250328T150939.623820Z-H9aa15b67-P0008f613","thread":"main","time":"2025-03-28T15:09:39.625484Z","file":"promisor-remote.c","line":55,"repo":1,"t_abs":0.001777,"t_rel":0.001777,"nesting":1,"category":"promisor","key":"fetch_count","value":"48"}
>
> The trash directory for the test run is here, in case anyone
> wants to poke at it:
>
> https://tmz.fedorapeople.org/t5620-backfill-trash-dir.tar.gz
>
> The full build log is available as well:
>
> https://tmz.fedorapeople.org/git-2.49.0-s390x-build.log
>
> If you search for 'BEGIN BASE64 MESSAGE' in that, it
> provides a command which can be used to extract the full
> test-results directory. That's used to get the output from
> the build hosts where shell access isn't available. I don't
> know that it's got anything which isn't in the trash
> directory tarball which I already extracted, but it's there
> just in case.
Thanks for the additional information!
One thing I stumbled over: the `--min-batch-size` parameter is parsed
using `OPT_INTEGER()`, which expects the value pointer to point to an
integer. But we pass `struct backfill_context::min_batch_size`, which is
of type `size_t`. Maybe that's causing us to end up with an invalid
value?
Could you please check whether the below diff fixes the issue for you?
If so I can turn it into a proper patch.
Patrick
-- >8 --
diff --git a/builtin/backfill.c b/builtin/backfill.c
index 33e1ea2f84f..1dd0d746538 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -119,11 +119,11 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
struct backfill_context ctx = {
.repo = repo,
.current_batch = OID_ARRAY_INIT,
- .min_batch_size = 50000,
.sparse = 0,
};
+ unsigned long min_batch_size = 50000;
struct option options[] = {
- OPT_INTEGER(0, "min-batch-size", &ctx.min_batch_size,
+ OPT_MAGNITUDE(0, "min-batch-size", &min_batch_size,
N_("Minimum number of objects to request at a time")),
OPT_BOOL(0, "sparse", &ctx.sparse,
N_("Restrict the missing objects to the current sparse-checkout")),
@@ -140,6 +140,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
if (ctx.sparse < 0)
ctx.sparse = core_apply_sparse_checkout;
+ ctx.min_batch_size = min_batch_size;
result = do_backfill(&ctx);
backfill_context_clear(&ctx);
