threads / patch / 62291

patchloose: don't rely on repository global state

Subject: [PATCH] loose: don't rely on repository global state

## tl;dr

4 messages between Oct 9, 2024 and Oct 10, 2024. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Karthik Nayak· Oct 9, 2024, 09:58 UTC · lore

In `loose.c`, we rely on the global variable `the_hash_algo`. As such we have guarded the file with the 'USE_THE_REPOSITORY_VARIABLE' definition. Let's derive the hash algorithm from the available repository variable and remove this guard. This brings us one step closer to removing the global 'the_repository' variable.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 loose.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
Show changes to loose.c +3 −4
diff --git a/loose.c b/loose.c
index 6d6a41b7e5..897ba389da 100644
--- a/loose.c
+++ b/loose.c
@@ -1,10 +1,9 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "git-compat-util.h"
 #include "hash.h"
 #include "path.h"
 #include "object-store.h"
 #include "hex.h"
+#include "repository.h"
 #include "wrapper.h"
 #include "gettext.h"
 #include "loose.h"
@@ -142,8 +141,8 @@ int repo_write_loose_object_map(struct repository *repo)

 	for (; iter != kh_end(map); iter++) {
 		if (kh_exist(map, iter)) {
-			if (oideq(&kh_key(map, iter), the_hash_algo->empty_tree) ||
-			    oideq(&kh_key(map, iter), the_hash_algo->empty_blob))
+			if (oideq(&kh_key(map, iter), repo->hash_algo->empty_tree) ||
+			    oideq(&kh_key(map, iter), repo->hash_algo->empty_blob))
 				continue;
 			strbuf_addf(&buf, "%s %s\n", oid_to_hex(&kh_key(map, iter)),
oid_to_hex(kh_value(map, iter)));
 			if (write_in_full(fd, buf.buf, buf.len) < 0)
-- 
2.46.2
Junio C Hamano· Oct 9, 2024, 18:52 UTC · re: Karthik Nayak · lore

Re: [PATCH] loose: don't rely on repository global state

Karthik Nayak <karthik.188@gmail.com> writes:
>  			strbuf_addf(&buf, "%s %s\n", oid_to_hex(&kh_key(map, iter)),
> oid_to_hex(kh_value(map, iter)));

The line seems corrupt. Joining them with one SP in between makes the patch cleanly apply.

Karthik Nayak· Oct 9, 2024, 19:41 UTC · re: Junio C Hamano · lore

Re: [PATCH] loose: don't rely on repository global state

On Wed, Oct 9, 2024 at 8:52 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
>
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> >                       strbuf_addf(&buf, "%s %s\n", oid_to_hex(&kh_key(map, iter)),
> > oid_to_hex(kh_value(map, iter)));
>
> The line seems corrupt.  Joining them with one SP in between makes
> the patch cleanly apply.

Indeed. I used `format-patch` and `send-email` right after, I don't remember messing with it, but perhaps I did. Thanks for fixing it up and checking.

Patrick Steinhardt· Oct 10, 2024, 06:45 UTC · re: Karthik Nayak · lore

Re: [PATCH] loose: don't rely on repository global state

On Wed, Oct 09, 2024 at 02:58:59AM -0700, Karthik Nayak wrote:
Show 39 quoted lines
> In `loose.c`, we rely on the global variable `the_hash_algo`. As such we
> have guarded the file with the 'USE_THE_REPOSITORY_VARIABLE' definition.
> Let's derive the hash algorithm from the available repository variable
> and remove this guard. This brings us one step closer to removing the
> global 'the_repository' variable.
> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  loose.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/loose.c b/loose.c
> index 6d6a41b7e5..897ba389da 100644
> --- a/loose.c
> +++ b/loose.c
> @@ -1,10 +1,9 @@
> -#define USE_THE_REPOSITORY_VARIABLE
> -
>  #include "git-compat-util.h"
>  #include "hash.h"
>  #include "path.h"
>  #include "object-store.h"
>  #include "hex.h"
> +#include "repository.h"
>  #include "wrapper.h"
>  #include "gettext.h"
>  #include "loose.h"
> @@ -142,8 +141,8 @@ int repo_write_loose_object_map(struct repository *repo)
> 
>  	for (; iter != kh_end(map); iter++) {
>  		if (kh_exist(map, iter)) {
> -			if (oideq(&kh_key(map, iter), the_hash_algo->empty_tree) ||
> -			    oideq(&kh_key(map, iter), the_hash_algo->empty_blob))
> +			if (oideq(&kh_key(map, iter), repo->hash_algo->empty_tree) ||
> +			    oideq(&kh_key(map, iter), repo->hash_algo->empty_blob))
>  				continue;
>  			strbuf_addf(&buf, "%s %s\n", oid_to_hex(&kh_key(map, iter)),
> oid_to_hex(kh_value(map, iter)));
>  			if (write_in_full(fd, buf.buf, buf.len) < 0)

I've already had a look internally and couldn't spot any other things that'd need to be converted in this file. So this looks good to me, except for the whitespace-broken parts.

Thanks!
Patrick

← back to recent threads