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

Re: [PATCH 3/4] builtin/repack.c: write cruft packs to arbitrary locations

From
Taylor Blau <me@ttaylorr.com>
Date
Oct 28, 2022, 19:42 UTC
Message-ID
<Y1wwt0H9JXFsE0Yn@nand.local>
In-Reply-To
<xmqq1qqwsyd2.fsf@gitster.g>
On Mon, Oct 24, 2022 at 02:30:49PM -0700, Junio C Hamano wrote:
Show 31 quoted lines
> > -	prepare_pack_objects(&cmd, args, packtmp);
> > +	prepare_pack_objects(&cmd, args, destination);
> >
> >  	strvec_push(&cmd.args, "--cruft");
> >  	if (cruft_expiration)
> > @@ -714,7 +717,12 @@ static int write_cruft_pack(const struct pack_objects_args *args,
> >  		if (line.len != the_hash_algo->hexsz)
> >  			die(_("repack: Expecting full hex object ID lines only "
> >  			      "from pack-objects."));
> > -		string_list_append(names, line.buf);
> > +                /*
> > +		 * avoid putting packs written outside of the repository in the
> > +		 * list of names
> > +		 */
> > +		if (local)
> > +			string_list_append(names, line.buf);
> >  	}
>
> Even if we do not want to contaminate the "names" list with packs
> that are not in the repository, wouldn't our caller still want to be
> able to tell what packs they are?
>
> What I am wondering is if it makes more sense to have the caller
> pass &names (which can be NULL to just discard the output from the
> pack-objects command) so that this function can concentrate on what
> it does (i.e. formulate the command to write cruft packs and then
> report the packs that are created), without having to worry about
> the management of the &names thing, which can be done by the caller
> of this function?  We are already passing &names, so it may be the
> matter of caller deciding to pass &names or NULL based on the value
> of destination it passes to the function?

It would be nice if it were possible to avoid passing `names` entirely here, but we still need it to determine the set of packs we already wrote a few lines above when we write the input to `pack-objects --cruft`.

Thanks, Taylor

Previous: Junio C HamanoNext: Derrick Stolee
Message 12 of 14 in “repack: implement `--expire-to` option”
  1. 0/4 repack: implement `--expire-to` optionTaylor Blau, Oct 24, 2022
  2. 1/4 builtin/repack.c: pass "out" to `prepare_pack_objects`Taylor Blau, Oct 24, 2022
  3. Junio C HamanoOct 24, 2022
  4. Derrick StoleeNov 7, 2022
  5. 2/4 builtin/repack.c: pass "cruft_expiration" to `write_cruft_pack`Taylor Blau, Oct 24, 2022
  6. Junio C HamanoOct 24, 2022
  7. 4/4 builtin/repack.c: implement `--expire-to` for storing pruned objectsTaylor Blau, Oct 24, 2022
  8. Derrick StoleeNov 7, 2022
  9. Taylor BlauNov 7, 2022
  10. 3/4 builtin/repack.c: write cruft packs to arbitrary locationsTaylor Blau, Oct 24, 2022
  11. Junio C HamanoOct 24, 2022
  12. Taylor BlauOct 28, 2022
  13. Derrick StoleeNov 7, 2022
  14. Taylor BlauNov 7, 2022

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.