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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 28, 2025, 20:45 UTC
Message-ID: <xmqq1povuqdr.fsf@gitster.g>
In-Reply-To: <709555c5316014b2b4d6648db407d15160769146.1756402795.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 10 quoted lines

> From: Derrick Stolee <stolee@gmail.com>
>
> 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 <stolee@gmail.com>
> ---
>  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.

Show 11 quoted lines

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