Re: Testsuite failure on s390x and sparc64 after 6840fe9ee2

[Patrick Steinhardt <ps@pks.im>]

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);