Subject: Re: [PATCH 2/5] midx-write: put failing response value back

From: Junio C Hamano <[email-masked]>
Date: Thu, 28 Aug 2025 13:45:36 -0700

"Derrick Stolee via GitGitGadget" <[email-masked]> writes:

> From: Derrick Stolee <[email-masked]>
>
> This instance of setting the result to 1 before going to cleanup was
> accidentally removed in fcb2205b77 (midx: implement support for writing
> incremental MIDX chains, 2024-08-06).
>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  midx-write.c | 1 +
>  1 file changed, 1 insertion(+)

The cover letter made it sound as if [1/5] was the only fix and the
rest was clean-up, but unless all callers of write_midx_internal()
ignores the return value from it, this surely would change the
behaviour of the program, no?

And the results from write-midx_file_only(), write_midx_file(), and
expire_midx_packs(), the three callers of this _internal() function,
all seem to be used in builtin/multi-pack-index.c so wouldn't this
also be a fix?

Not that I endorse "0 is success and any non-zero value is an error",
but this does not look like a mere clean-up to me.

> diff --git a/midx-write.c b/midx-write.c
> index d8f9679868..85b2d471ef 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -1106,6 +1106,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
>  			m = m->base_midx;
>  		}
>  	} else if (ctx.m && fill_packs_from_midx(&ctx)) {
> +		result = 1;
>  		goto cleanup;
>  	}