git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/4] reftable/stack: register new tables as tempfiles

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 6, 2024, 11:59 UTC
Message-ID
<Zehav4V_8GGZG94Q@tanuki>
In-Reply-To
<6cw6d3ubo2kbogzdbniyoznij2zfoh5t3htwb4oaghaltcgeqg@kkrw4g6atr2k>
On Tue, Mar 05, 2024 at 04:30:18PM -0600, Justin Tobler wrote:
Show 51 quoted lines
> On 24/03/04 12:10PM, Patrick Steinhardt wrote:
> > We do not register new tables which we're about to add to the stack with
> > the tempfile API. Those tables will thus not be deleted in case Git gets
> > killed.
> > 
> > Refactor the code to register tables as tempfiles.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  reftable/stack.c | 29 ++++++++++++-----------------
> >  1 file changed, 12 insertions(+), 17 deletions(-)
> > 
> > diff --git a/reftable/stack.c b/reftable/stack.c
> > index b64e55648a..81544fbfa0 100644
> > --- a/reftable/stack.c
> > +++ b/reftable/stack.c
> > @@ -737,8 +737,9 @@ int reftable_addition_add(struct reftable_addition *add,
> >  	struct strbuf tab_file_name = STRBUF_INIT;
> >  	struct strbuf next_name = STRBUF_INIT;
> >  	struct reftable_writer *wr = NULL;
> > +	struct tempfile *tab_file = NULL;
> >  	int err = 0;
> > -	int tab_fd = 0;
> > +	int tab_fd;
> >  
> >  	strbuf_reset(&next_name);
> >  	format_name(&next_name, add->next_update_index, add->next_update_index);
> > @@ -746,17 +747,20 @@ int reftable_addition_add(struct reftable_addition *add,
> >  	stack_filename(&temp_tab_file_name, add->stack, next_name.buf);
> >  	strbuf_addstr(&temp_tab_file_name, ".temp.XXXXXX");
> >  
> > -	tab_fd = mkstemp(temp_tab_file_name.buf);
> > -	if (tab_fd < 0) {
> > +	tab_file = mks_tempfile(temp_tab_file_name.buf);
> > +	if (!tab_file) {
> >  		err = REFTABLE_IO_ERROR;
> >  		goto done;
> >  	}
> >  	if (add->stack->config.default_permissions) {
> > -		if (chmod(temp_tab_file_name.buf, add->stack->config.default_permissions)) {
> > +		if (chmod(get_tempfile_path(tab_file),
> > +			  add->stack->config.default_permissions)) {
> >  			err = REFTABLE_IO_ERROR;
> >  			goto done;
> >  		}
> >  	}
> 
> Since the tempfile is now being created through the tempfile API, I
> think the file mode can be set directly through `mks_tempfile_m()`
> instead of creating the tempfile and then using chmod. Just something I
> thought to mention.

Unfortunately not. The problem is that `mks_tempfile_m()` will munge passed-in permissions via "core.sharedRepository", but we already pre calculated the target mode in `config.default_permissions`. Thus, the result would have wrong permissions if we used `mks_tempfile_m()`.

Patrick
Previous: Justin ToblerNext: Justin Tobler
Message 7 of 26 in “reftable/stack: register temporary files”
  1. 0/4 reftable/stack: register temporary filesPatrick Steinhardt, Mar 4, 2024
  2. 1/4 lockfile: report when rollback failsPatrick Steinhardt, Mar 4, 2024
  3. Justin ToblerMar 5, 2024
  4. Patrick SteinhardtMar 6, 2024
  5. 2/4 reftable/stack: register new tables as tempfilesPatrick Steinhardt, Mar 4, 2024
  6. Justin ToblerMar 5, 2024
  7. Patrick SteinhardtMar 6, 2024
  8. Justin ToblerMar 6, 2024
  9. Junio C HamanoMar 6, 2024
  10. Patrick SteinhardtMar 7, 2024
  11. Junio C HamanoMar 7, 2024
  12. Patrick SteinhardtMar 7, 2024
  13. Junio C HamanoMar 7, 2024
  14. 3/4 reftable/stack: register lockfiles during compactionPatrick Steinhardt, Mar 4, 2024
  15. Justin ToblerMar 5, 2024
  16. Patrick SteinhardtMar 6, 2024
  17. Junio C HamanoMar 6, 2024
  18. Justin ToblerMar 6, 2024
  19. 4/4 reftable/stack: register compacted tables as tempfilesPatrick Steinhardt, Mar 4, 2024
  20. Toon claesMar 7, 2024
  21. Patrick SteinhardtMar 7, 2024
  22. 0/4 reftable/stack: register temporary filesPatrick Steinhardt, Mar 7, 2024
  23. 1/4 lockfile: report when rollback failsPatrick Steinhardt, Mar 7, 2024
  24. 2/4 reftable/stack: register new tables as tempfilesPatrick Steinhardt, Mar 7, 2024
  25. 3/4 reftable/stack: register lockfiles during compactionPatrick Steinhardt, Mar 7, 2024
  26. 4/4 reftable/stack: register compacted tables as tempfilesPatrick Steinhardt, Mar 7, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.