Git development
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
Cc: git@vger.kernel.org, christian.couder@gmail.com,
	usmanakinyemi202@gmail.com, siddharthasthana31@gmail.com,
	kaartic.sivaraam@gmail.com
Subject: Re: [Outreachy PATCH] bundle-uri: use the passed repository for refs and tracing
Date: Thu, 29 Oct 2026 13:07:40 +0100	[thread overview]
Message-ID: <auM3DGcWTL5OdRnD@pks.im> (raw)
In-Reply-To: <20261028093455.76247-1-okhuomonajayi54@gmail.com>

On Wed, Oct 28, 2026 at 10:34:55AM +0100, Okhuomon Ajayi wrote:
> unbundle_from_file() receives r and unbundles the objects into
> it, but reads and writes the refs in the_repository.
> fetch_bundle_uri() also has r, but uses the_repository for its
> trace2 region.
> 
> Use r for both.

Okay.

> Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
> ---
> unbundle_from_file() still depends on the_repository through
> fetch_pack_fsck_objects(), which reads fetch.fsckObjects and
> transfer.fsckObjects from the_repository's config. I left that out.

That's fine, we know that subsystems may still implicitly depend on
it even though they don't declare `USE_THE_REPOSITORY_VARIABLE` anymore.

What I find curious though is why you didn't go the last mile so that we
could drop that define from this subsystem altogether. There's only two
sites left that use `the_repository`, and both of them are trivial to
convert (see below patch).

So shouldn't we maybe also fix those cases while already at it and then
drop the define?

Patrick

diff --git a/bundle-uri.c b/bundle-uri.c
index 1f1683b52e9..abdb99f4375 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -285,7 +285,7 @@ int bundle_uri_parse_config_format(const char *uri,
 	return result;
 }
 
-static char *find_temp_filename(void)
+static char *find_temp_filename(struct repository *r)
 {
 	int fd;
 	struct strbuf name = STRBUF_INIT;
@@ -293,7 +293,7 @@ static char *find_temp_filename(void)
 	 * Find a temporary filename that is available. This is briefly
 	 * racy, but unlikely to collide.
 	 */
-	fd = odb_mkstemp(the_repository->objects, &name,
+	fd = odb_mkstemp(r->objects, &name,
 			 "bundles/tmp_uri_XXXXXX");
 	if (fd < 0) {
 		warning(_("failed to create temporary file"));
@@ -383,7 +383,7 @@ static int download_https_uri_to_file(const char *file, const char *uri)
 	return result;
 }
 
-static int copy_uri_to_file(const char *filename, const char *uri)
+static int copy_uri_to_file(struct repository *r, const char *filename, const char *uri)
 {
 	const char *out;
 
@@ -395,7 +395,7 @@ static int copy_uri_to_file(const char *filename, const char *uri)
 		uri = out;
 
 	/* Copy as a file */
-	return copy_file(the_repository, filename, uri, 0);
+	return copy_file(r, filename, uri, 0);
 }
 
 static int unbundle_from_file(struct repository *r, const char *file)
@@ -773,12 +773,12 @@ static int fetch_bundle_uri_internal(struct repository *r,
 	}
 
 	if (!bundle->file &&
-	    !(bundle->file = find_temp_filename())) {
+	    !(bundle->file = find_temp_filename(r))) {
 		result = -1;
 		goto cleanup;
 	}
 
-	if ((result = copy_uri_to_file(bundle->file, bundle->uri))) {
+	if ((result = copy_uri_to_file(r, bundle->file, bundle->uri))) {
 		warning(_("failed to download bundle from URI '%s'"), bundle->uri);
 		goto cleanup;
 	}

  parent reply	other threads:[~2026-10-29 12:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-28  9:34 [Outreachy PATCH] bundle-uri: use the passed repository for refs and tracing Okhuomon Ajayi
2026-10-28 11:05 ` Karthik Nayak
2026-10-28 11:18   ` Okhuomon Ajayi
2026-10-28 16:36   ` Junio C Hamano
2026-10-29 12:07 ` Patrick Steinhardt [this message]
2026-10-29 12:47   ` Okhuomon Ajayi
2026-10-29 13:13   ` [Outreachy PATCH v2] bundle-uri: stop using the_repository Okhuomon Ajayi
2026-10-29 13:37     ` Patrick Steinhardt

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=auM3DGcWTL5OdRnD@pks.im \
    --to=ps@pks.im \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=okhuomonajayi54@gmail.com \
    --cc=siddharthasthana31@gmail.com \
    --cc=usmanakinyemi202@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox