All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Justin Tobler <jltobler@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/2] object-file: warn on failed unlink
Date: Wed, 04 Nov 2026 15:00:43 -0800	[thread overview]
Message-ID: <xmqqfqxgyzfo.fsf@gitster.g> (raw)
In-Reply-To: <20261103212410.1389988-2-jltobler@gmail.com> (Justin Tobler's message of "Tue, 3 Nov 2026 15:24:09 -0600")

Justin Tobler <jltobler@gmail.com> writes:

> When the "files" ODB transaction cleans up a temporary packfile or
> ".keep" lockfile on an error path, any failure to unlink the file is
> silently ignored. A ".keep" file left behind this way prevents its
> packfile from ever being removed by housekeeping.
>
> Use `unlink_or_warn()` instead so that such failures are reported.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  object-file.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 132aaab6f8..a9800143bd 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -823,7 +823,7 @@ static void flush_packfile_transaction(struct odb_transaction_files *transaction
>  	if (state->nr_written == 0) {
>  		close(state->f->fd);
>  		free_hashfile(state->f);
> -		unlink(state->pack_tmp_name);
> +		unlink_or_warn(state->pack_tmp_name);
>  		goto clear_exit;
>  	} else if (state->nr_written == 1) {
>  		finalize_hashfile(state->f, hash, FSYNC_COMPONENT_PACK,
> @@ -1276,13 +1276,13 @@ static int write_pack_lockfile(struct odb_source *source, const char *pack_name,
>  	if (write_in_full(fd, contents.buf, contents.len) < 0) {
>  		ret = error_errno(_("unable to write '%s'"), lock_path.buf);
>  		close(fd);
> -		unlink(lock_path.buf);
> +		unlink_or_warn(lock_path.buf);
>  		goto out;
>  	}
>  
>  	if (close(fd) < 0) {
>  		ret = error_errno(_("unable to close '%s'"), lock_path.buf);
> -		unlink(lock_path.buf);
> +		unlink_or_warn(lock_path.buf);
>  		goto out;
>  	}

Trivially correct.

  reply	other threads:[~2026-11-04 23:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03 21:24 [PATCH 0/2] object-file: small error handling follow-ups Justin Tobler
2026-11-03 21:24 ` [PATCH 1/2] object-file: warn on failed unlink Justin Tobler
2026-11-04 23:00   ` Junio C Hamano [this message]
2026-11-03 21:24 ` [PATCH 2/2] object-file: return error on index-pack abnormal exit Justin Tobler
2026-11-04 23:06   ` Junio C Hamano
2026-11-05 18:09     ` Justin Tobler
2026-11-10  9:04     ` Kaartic Sivaraam
2026-11-10 16:43       ` Junio C Hamano
2026-11-10 17:57         ` Kaartic Sivaraam
2026-11-10 19:25           ` Justin Tobler
2026-11-05 15:30   ` Patrick Steinhardt
2026-11-05 18:06     ` Justin Tobler
2026-11-04 13:37 ` [PATCH 0/2] object-file: small error handling follow-ups Karthik Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqfqxgyzfo.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jltobler@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.