From: Junio C Hamano Date: Thu, 28 Aug 2025 20:45:36 GMT Subject: Re: [PATCH 2/5] midx-write: put failing response value back Message-ID: In-Reply-To: <709555c5316014b2b4d6648db407d15160769146.1756402795.git.gitgitgadget@gmail.com> "Derrick Stolee via GitGitGadget" writes: > From: Derrick Stolee > > 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 > --- > 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; > }