[PATCH v2 0/4] reftable/stack: register temporary files

From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Justin Tobler <jltobler@gmail.com>, Toon claes <toon@iotcl.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/4] reftable/stack: register temporary files
Date: Thu, 7 Mar 2024 14:10:27 +0100	[thread overview]
Message-ID: <cover.1709816483.git.ps@pks.im> (raw)
In-Reply-To: <cover.1709549619.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 5192 bytes --]

Hi,

this is the second version of my patch series that registers temporary
files written by the reftable library with the tempfile framework. This
ensures that those files get cleaned up even when Git dies or otherwise
gets signalled.

Changes compared to v1:

  - Patch 1: Clarify that rolling back a deactivated lockfile will not
    result in an error.

  - Patch 4: Rename some local variables for improved consistency with
    other code.

Patrick

Patrick Steinhardt (4):
  lockfile: report when rollback fails
  reftable/stack: register new tables as tempfiles
  reftable/stack: register lockfiles during compaction
  reftable/stack: register compacted tables as tempfiles

 lockfile.h        |   6 +-
 reftable/stack.c  | 330 ++++++++++++++++++++++------------------------
 reftable/system.h |   2 +
 tempfile.c        |  21 +--
 tempfile.h        |   2 +-
 5 files changed, 178 insertions(+), 183 deletions(-)

Range-diff against v1:
1:  1acaa9ca1a ! 1:  782e96a678 lockfile: report when rollback fails
    @@ Commit message

      ## lockfile.h ##
     @@ lockfile.h: static inline int commit_lock_file_to(struct lock_file *lk, const char *path)
    +  * Roll back `lk`: close the file descriptor and/or file pointer and
    +  * remove the lockfile. It is a NOOP to call `rollback_lock_file()`
       * for a `lock_file` object that has already been committed or rolled
    -  * back.
    +- * back.
    ++ * back. No error will be returned in this case.
       */
     -static inline void rollback_lock_file(struct lock_file *lk)
     +static inline int rollback_lock_file(struct lock_file *lk)
2:  02bf41d419 = 2:  5dbc93d5be reftable/stack: register new tables as tempfiles
3:  45b5c3167f = 3:  c88c85443e reftable/stack: register lockfiles during compaction
4:  b952d54a05 ! 4:  4023d78f08 reftable/stack: register compacted tables as tempfiles
    @@ reftable/stack.c: uint64_t reftable_stack_next_update_index(struct reftable_stac
     -				struct strbuf *temp_tab,
     -				struct reftable_log_expiry_config *config)
     +				struct reftable_log_expiry_config *config,
    -+				struct tempfile **temp_table_out)
    ++				struct tempfile **tab_file_out)
      {
      	struct strbuf next_name = STRBUF_INIT;
     -	int tab_fd = -1;
    -+	struct strbuf table_path = STRBUF_INIT;
    ++	struct strbuf tab_file_path = STRBUF_INIT;
      	struct reftable_writer *wr = NULL;
    -+	struct tempfile *temp_table;
    -+	int temp_table_fd;
    - 	int err = 0;
    +-	int err = 0;
    ++	struct tempfile *tab_file;
    ++	int tab_fd, err = 0;

      	format_name(&next_name,
      		    reftable_reader_min_update_index(st->readers[first]),
      		    reftable_reader_max_update_index(st->readers[last]));
    -+	stack_filename(&table_path, st, next_name.buf);
    -+	strbuf_addstr(&table_path, ".temp.XXXXXX");
    ++	stack_filename(&tab_file_path, st, next_name.buf);
    ++	strbuf_addstr(&tab_file_path, ".temp.XXXXXX");

     -	stack_filename(temp_tab, st, next_name.buf);
     -	strbuf_addstr(temp_tab, ".temp.XXXXXX");
    -+	temp_table = mks_tempfile(table_path.buf);
    -+	if (!temp_table) {
    ++	tab_file = mks_tempfile(tab_file_path.buf);
    ++	if (!tab_file) {
     +		err = REFTABLE_IO_ERROR;
     +		goto done;
     +	}
    -+	temp_table_fd = get_tempfile_fd(temp_table);
    ++	tab_fd = get_tempfile_fd(tab_file);

     -	tab_fd = mkstemp(temp_tab->buf);
      	if (st->config.default_permissions &&
     -	    chmod(temp_tab->buf, st->config.default_permissions) < 0) {
    -+	    chmod(get_tempfile_path(temp_table), st->config.default_permissions) < 0) {
    ++	    chmod(get_tempfile_path(tab_file), st->config.default_permissions) < 0) {
      		err = REFTABLE_IO_ERROR;
      		goto done;
      	}
    @@ reftable/stack.c: uint64_t reftable_stack_next_update_index(struct reftable_stac
     -	wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd, &st->config);
     -
     +	wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush,
    -+				 &temp_table_fd, &st->config);
    ++				 &tab_fd, &st->config);
      	err = stack_write_compact(st, wr, first, last, config);
      	if (err < 0)
      		goto done;
    @@ reftable/stack.c: uint64_t reftable_stack_next_update_index(struct reftable_stac

     -	err = close(tab_fd);
     -	tab_fd = 0;
    -+	err = close_tempfile_gently(temp_table);
    ++	err = close_tempfile_gently(tab_file);
     +	if (err < 0)
     +		goto done;
     +
    -+	*temp_table_out = temp_table;
    -+	temp_table = NULL;
    ++	*tab_file_out = tab_file;
    ++	tab_file = NULL;

      done:
    -+	delete_tempfile(&temp_table);
    ++	delete_tempfile(&tab_file);
      	reftable_writer_free(wr);
     -	if (tab_fd > 0) {
     -		close(tab_fd);
    @@ reftable/stack.c: uint64_t reftable_stack_next_update_index(struct reftable_stac
     -		strbuf_release(temp_tab);
     -	}
      	strbuf_release(&next_name);
    -+	strbuf_release(&table_path);
    ++	strbuf_release(&tab_file_path);
      	return err;
      }

-- 
2.44.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]