# [PATCH] loose: don't rely on repository global state 4 messages from 2024-10-09 to 2024-10-10. Participants: Karthik Nayak, Junio C Hamano, Patrick Steinhardt. Thread: https://gitlist.dev/t/62291 ## Karthik Nayak, 2024-10-09 09:58 Subject: [PATCH] loose: don't rely on repository global state Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZRMaw-PAsZ9s0zJ2zp_suMppi%3DZrT67B__LU1tWZSvuUQ%40mail.gmail.com ``` 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 --- 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) -- 2.46.2 ``` ## Junio C Hamano, 2024-10-09 18:52 Subject: Re: [PATCH] loose: don't rely on repository global state Message-ID: URL: https://gitlist.dev/e/xmqqcyk9rtwv.fsf%40gitster.g In-Reply-To: ``` Karthik Nayak 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, 2024-10-09 19:41 Subject: Re: [PATCH] loose: don't rely on repository global state Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZThTs%3DEzFEAHM%2BaLJ5Eq_E-RPfGrKiceDADsBGJQE859w%40mail.gmail.com In-Reply-To: ``` On Wed, Oct 9, 2024 at 8:52 PM Junio C Hamano wrote: > > Karthik Nayak 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, 2024-10-10 06:45 Subject: Re: [PATCH] loose: don't rely on repository global state Message-ID: URL: https://gitlist.dev/e/Zwd393AHNAGQ_zAx%40pks.im In-Reply-To: ``` On Wed, Oct 09, 2024 at 02:58:59AM -0700, Karthik Nayak wrote: > 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 > --- > 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 ```