Jeff King <peff@xxxxxxxx> writes:
> This patch also fixes another small problem. We only hook signals, and
> don't set up an atexit handler. So if we see an error that causes us to
> die(), we'll leave the .tmp-* files in place. But since the tempfile API
> handles this for us, this is now fixed for free. The new test covers
> this by stimulating a failure of pack-objects when generating a cruft
> pack. Before this patch, the .tmp-* file for the main pack would have
> been left, but now we correctly clean it up.
>
> Reported-by: Jan Pokorný <poki@xxxxxxxx>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> builtin/repack.c | 17 ++++-------------
> t/t7700-repack.sh | 8 ++++++++
> 2 files changed, 12 insertions(+), 13 deletions(-)
Nice.
> struct generated_pack_data {
> - char exts[ARRAY_SIZE(exts)];
> + struct tempfile *tempfiles[ARRAY_SIZE(exts)];
> ...
> - data->exts[i] = 1;
> + data->tempfiles[i] = register_tempfile(path.buf);
OK.
> }
>
> strbuf_release(&path);
> @@ -867,8 +860,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> split_pack_geometry(geometry, geometric_factor);
> }
>
> - sigchain_push_common(remove_pack_on_signal);
> -
> prepare_pack_objects(&cmd, &po_args);
>
> show_progress = !po_args.quiet && isatty(2);
> @@ -1020,14 +1011,14 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> fname_old = mkpathdup("%s-%s%s",
> packtmp, item->string, exts[ext].name);
>
> - if (data->exts[ext]) {
> + if (data->tempfiles[ext]) {
> struct stat statbuffer;
> if (!stat(fname_old, &statbuffer)) {
> statbuffer.st_mode &= ~(S_IWUSR | S_IWGRP | S_IWOTH);
> chmod(fname_old, statbuffer.st_mode);
> }
>
> - if (rename(fname_old, fname))
> + if (rename_tempfile(&data->tempfiles[ext], fname))
> die_errno(_("renaming '%s' failed"), fname_old);
It now got a bit confusing that we have 'fname', 'fname_old', and
the tempfile. The path.buf used as the argument to register_tempfile()
matches what is used to compute fname_old above. I wonder if tempfile
API does not give us that name so that we can stop using fname_old here?
