Re: Testsuite failure on s390x and sparc64 after 6840fe9ee2
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Mar 31, 2025, 12:27 UTC
- Message-ID
- <Z-qKGqpbdaW9WCrP@pks.im>
- In-Reply-To
- <Z-bCNdOOLrM2Chb8@teonanacatl.net>
On Fri, Mar 28, 2025 at 11:37:25AM -0400, Todd Zullinger wrote:
Show 41 quoted lines
> 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);