| Message ID | 339d668da837ab5b4b11399ece4efaf5bc27d313.1718106285.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce `USE_THE_REPOSITORY_VARIABLE` macro | expand |
On Tue, 11 Jun 2024, Patrick Steinhardt <ps@pks.im> wrote: > The "oidtree" test helper sets up a Git repository, but this is really > only used such that `get_oid_hex()` can parse both SHA1 and SHA256 > object hashes. The `struct oidtree` interface itself does not care at > all about the object hash of `the_repository`, and always asserts that > inserted object IDs have their hash algorithm set. > > Stop initializing the repository and instead use `get_oid_hex_any()` to > parse object IDs for the "contains" action, like we already do when > parsing the "insert" action. I think the motive of this patch is already achieved in 'gt/unit-test-oidtree'[1], if this is to be merged after that. Thanks. [1]: https://github.com/git/git/commit/79d9e08db3a08c5a06e2633a39cd38b980e654f4 mailing list: https://lore.kernel.org/git/20240608165731.29467-1-shyamthakkar001@gmail.com/ > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > t/helper/test-oidtree.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/t/helper/test-oidtree.c b/t/helper/test-oidtree.c > index c7a1d4c642..04ec24cc84 100644 > --- a/t/helper/test-oidtree.c > +++ b/t/helper/test-oidtree.c > @@ -1,7 +1,6 @@ > #include "test-tool.h" > #include "hex.h" > #include "oidtree.h" > -#include "setup.h" > #include "strbuf.h" > > static enum cb_next print_oid(const struct object_id *oid, void *data UNUSED) > @@ -14,11 +13,9 @@ int cmd__oidtree(int argc UNUSED, const char **argv UNUSED) > { > struct oidtree ot; > struct strbuf line = STRBUF_INIT; > - int nongit_ok; > int algo = GIT_HASH_UNKNOWN; > > oidtree_init(&ot); > - setup_git_directory_gently(&nongit_ok); > > while (strbuf_getline(&line, stdin) != EOF) { > const char *arg; > @@ -30,7 +27,7 @@ int cmd__oidtree(int argc UNUSED, const char **argv UNUSED) > algo = oid.algo; > oidtree_insert(&ot, &oid); > } else if (skip_prefix(line.buf, "contains ", &arg)) { > - if (get_oid_hex(arg, &oid)) > + if (get_oid_hex_any(arg, &oid) == GIT_HASH_UNKNOWN) > die("contains not a hexadecimal oid: %s", arg); > printf("%d\n", oidtree_contains(&ot, &oid)); > } else if (skip_prefix(line.buf, "each ", &arg)) { > -- > 2.45.2.436.gcd77e87115.dirty >
On 2024-06-11 at 11:59:01, Patrick Steinhardt wrote: > diff --git a/t/helper/test-oidtree.c b/t/helper/test-oidtree.c > index c7a1d4c642..04ec24cc84 100644 > --- a/t/helper/test-oidtree.c > +++ b/t/helper/test-oidtree.c > @@ -1,7 +1,6 @@ > #include "test-tool.h" > #include "hex.h" > #include "oidtree.h" > -#include "setup.h" > #include "strbuf.h" > > static enum cb_next print_oid(const struct object_id *oid, void *data UNUSED) > @@ -14,11 +13,9 @@ int cmd__oidtree(int argc UNUSED, const char **argv UNUSED) > { > struct oidtree ot; > struct strbuf line = STRBUF_INIT; > - int nongit_ok; > int algo = GIT_HASH_UNKNOWN; > > oidtree_init(&ot); > - setup_git_directory_gently(&nongit_ok); > > while (strbuf_getline(&line, stdin) != EOF) { > const char *arg; > @@ -30,7 +27,7 @@ int cmd__oidtree(int argc UNUSED, const char **argv UNUSED) > algo = oid.algo; > oidtree_insert(&ot, &oid); > } else if (skip_prefix(line.buf, "contains ", &arg)) { > - if (get_oid_hex(arg, &oid)) > + if (get_oid_hex_any(arg, &oid) == GIT_HASH_UNKNOWN) > die("contains not a hexadecimal oid: %s", arg); This is not a problem in your code, but this might read more naturally as "does not contain a hexadecimal oid" or "contains no hexadecimal oid".
On Tue, Jun 11, 2024 at 06:27:51PM +0530, Ghanshyam Thakkar wrote: > On Tue, 11 Jun 2024, Patrick Steinhardt <ps@pks.im> wrote: > > The "oidtree" test helper sets up a Git repository, but this is really > > only used such that `get_oid_hex()` can parse both SHA1 and SHA256 > > object hashes. The `struct oidtree` interface itself does not care at > > all about the object hash of `the_repository`, and always asserts that > > inserted object IDs have their hash algorithm set. > > > > Stop initializing the repository and instead use `get_oid_hex_any()` to > > parse object IDs for the "contains" action, like we already do when > > parsing the "insert" action. > > I think the motive of this patch is already achieved in > 'gt/unit-test-oidtree'[1], if this is to be merged after that. > > Thanks. > > [1]: https://github.com/git/git/commit/79d9e08db3a08c5a06e2633a39cd38b980e654f4 > mailing list: https://lore.kernel.org/git/20240608165731.29467-1-shyamthakkar001@gmail.com/ Okay. I'll make sure to evict this patch once your patch series gets merged down to "next". Thanks for the hint. Patrick
On Tue, Jun 11, 2024 at 11:17:57PM +0000, brian m. carlson wrote: > On 2024-06-11 at 11:59:01, Patrick Steinhardt wrote: > > diff --git a/t/helper/test-oidtree.c b/t/helper/test-oidtree.c > > index c7a1d4c642..04ec24cc84 100644 > > --- a/t/helper/test-oidtree.c > > +++ b/t/helper/test-oidtree.c > > @@ -30,7 +27,7 @@ int cmd__oidtree(int argc UNUSED, const char **argv UNUSED) > > algo = oid.algo; > > oidtree_insert(&ot, &oid); > > } else if (skip_prefix(line.buf, "contains ", &arg)) { > > - if (get_oid_hex(arg, &oid)) > > + if (get_oid_hex_any(arg, &oid) == GIT_HASH_UNKNOWN) > > die("contains not a hexadecimal oid: %s", arg); > > This is not a problem in your code, but this might read more naturally > as "does not contain a hexadecimal oid" or "contains no hexadecimal > oid". True. I'll leave this as-is though given that Ghanshyam is already busy converting this test to be a unit test anyway, so I don't want to make his life harder here. Patrick
diff --git a/t/helper/test-oidtree.c b/t/helper/test-oidtree.c index c7a1d4c642..04ec24cc84 100644 --- a/t/helper/test-oidtree.c +++ b/t/helper/test-oidtree.c @@ -1,7 +1,6 @@ #include "test-tool.h" #include "hex.h" #include "oidtree.h" -#include "setup.h" #include "strbuf.h" static enum cb_next print_oid(const struct object_id *oid, void *data UNUSED) @@ -14,11 +13,9 @@ int cmd__oidtree(int argc UNUSED, const char **argv UNUSED) { struct oidtree ot; struct strbuf line = STRBUF_INIT; - int nongit_ok; int algo = GIT_HASH_UNKNOWN; oidtree_init(&ot); - setup_git_directory_gently(&nongit_ok); while (strbuf_getline(&line, stdin) != EOF) { const char *arg; @@ -30,7 +27,7 @@ int cmd__oidtree(int argc UNUSED, const char **argv UNUSED) algo = oid.algo; oidtree_insert(&ot, &oid); } else if (skip_prefix(line.buf, "contains ", &arg)) { - if (get_oid_hex(arg, &oid)) + if (get_oid_hex_any(arg, &oid) == GIT_HASH_UNKNOWN) die("contains not a hexadecimal oid: %s", arg); printf("%d\n", oidtree_contains(&ot, &oid)); } else if (skip_prefix(line.buf, "each ", &arg)) {
The "oidtree" test helper sets up a Git repository, but this is really only used such that `get_oid_hex()` can parse both SHA1 and SHA256 object hashes. The `struct oidtree` interface itself does not care at all about the object hash of `the_repository`, and always asserts that inserted object IDs have their hash algorithm set. Stop initializing the repository and instead use `get_oid_hex_any()` to parse object IDs for the "contains" action, like we already do when parsing the "insert" action. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- t/helper/test-oidtree.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)