{"thread":{"id":"51286","subject":"[PATCH v2 0/3] Test oidmap","startedAt":"2019-06-11T08:23:50Z","lastAt":"2019-06-12T17:09:24Z","messageCount":7,"participants":["Christian Couder","SZEDER Gábor"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"376997","messageId":"20190611082325.28878-1-chriscool@tuxfamily.org","threadId":"51286","inReplyTo":null,"subject":"[PATCH v2 0/3] Test oidmap","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-06-11T08:23:22Z","receivedAt":"2019-06-11T08:23:50Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Unlike hashmap that has t/helper/test-hashmap.c and t/t0011-hashmap.sh\noidmap has no specific test. The goal of this small patch series is to\nchange that and also improve oidmap a bit while at it.\n\nChanges compared to V1 are the following:\n\n  - use printf(\"%x\\n\", ntohl(sha1hash(oid.hash))) to print hashes as\n    suggested by Gábor and approved by Junio,\n\n  - use `git rev-parse \"$1\" | cut -c1-8` to check hashes in t0016 as\n    suggested by Gábor,\n\n  - removed PERL prereq on \"hash\" test in t0016 as allowed by the\n    above change and suggested by Gábor,\n\n  - removed suprious space between \">\" and \"expect\" in t0016 as\n    suggested by Gábor.\n\nI decided against hardcoding values as I think it might help\ntransitionning from SHA1 to SHA256.\n\nChristian Couder (3):\n  t/helper: add test-oidmap.c\n  t: add t0016-oidmap.sh\n  oidmap: use sha1hash() instead of static hash() function\n\n Makefile               |   1 +\n oidmap.c               |  13 +---\n t/helper/test-oidmap.c | 134 +++++++++++++++++++++++++++++++++++++++++\n t/helper/test-tool.c   |   1 +\n t/helper/test-tool.h   |   1 +\n t/t0016-oidmap.sh      | 100 ++++++++++++++++++++++++++++++\n 6 files changed, 240 insertions(+), 10 deletions(-)\n create mode 100644 t/helper/test-oidmap.c\n create mode 100755 t/t0016-oidmap.sh\n\n-- \n2.22.0.6.gde8b105b43\n\n"},{"id":"376998","messageId":"20190611082325.28878-2-chriscool@tuxfamily.org","threadId":"51286","inReplyTo":"20190611082325.28878-1-chriscool@tuxfamily.org","subject":"[PATCH v2 1/3] t/helper: add test-oidmap.c","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-06-11T08:23:23Z","receivedAt":"2019-06-11T08:23:56Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This new helper is very similar to \"test-hashmap.c\" and will help\ntest how `struct oidmap` from oidmap.{c,h} can be used.\n\nHelped-by: SZEDER Gábor <szeder.dev@gmail.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Makefile               |   1 +\n t/helper/test-oidmap.c | 134 +++++++++++++++++++++++++++++++++++++++++\n t/helper/test-tool.c   |   1 +\n t/helper/test-tool.h   |   1 +\n 4 files changed, 137 insertions(+)\n create mode 100644 t/helper/test-oidmap.c\n\ndiff --git a/Makefile b/Makefile\nindex 8a7e235352..5efc7700ed 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -727,6 +727,7 @@ TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o\n TEST_BUILTINS_OBJS += test-match-trees.o\n TEST_BUILTINS_OBJS += test-mergesort.o\n TEST_BUILTINS_OBJS += test-mktemp.o\n+TEST_BUILTINS_OBJS += test-oidmap.o\n TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n TEST_BUILTINS_OBJS += test-path-utils.o\ndiff --git a/t/helper/test-oidmap.c b/t/helper/test-oidmap.c\nnew file mode 100644\nindex 0000000000..60e92096a1\n--- /dev/null\n+++ b/t/helper/test-oidmap.c\n@@ -0,0 +1,134 @@\n+#include \"test-tool.h\"\n+#include \"cache.h\"\n+#include \"oidmap.h\"\n+#include \"strbuf.h\"\n+\n+/* key is an oid and value is a name (could be a refname for example) */\n+struct test_entry {\n+\tstruct oidmap_entry entry;\n+\tchar name[FLEX_ARRAY];\n+};\n+\n+#define DELIM \" \\t\\r\\n\"\n+\n+/*\n+ * Read stdin line by line and print result of commands to stdout:\n+ *\n+ * hash oidkey -> sha1hash(oidkey)\n+ * put oidkey namevalue -> NULL / old namevalue\n+ * get oidkey -> NULL / namevalue\n+ * remove oidkey -> NULL / old namevalue\n+ * iterate -> oidkey1 namevalue1\\noidkey2 namevalue2\\n...\n+ *\n+ */\n+int cmd__oidmap(int argc, const char **argv)\n+{\n+\tstruct strbuf line = STRBUF_INIT;\n+\tstruct oidmap map = OIDMAP_INIT;\n+\n+\tsetup_git_directory();\n+\n+\t/* init oidmap */\n+\toidmap_init(&map, 0);\n+\n+\t/* process commands from stdin */\n+\twhile (strbuf_getline(&line, stdin) != EOF) {\n+\t\tchar *cmd, *p1 = NULL, *p2 = NULL;\n+\t\tstruct test_entry *entry;\n+\t\tstruct object_id oid;\n+\n+\t\t/* break line into command and up to two parameters */\n+\t\tcmd = strtok(line.buf, DELIM);\n+\t\t/* ignore empty lines */\n+\t\tif (!cmd || *cmd == '#')\n+\t\t\tcontinue;\n+\n+\t\tp1 = strtok(NULL, DELIM);\n+\t\tif (p1)\n+\t\t\tp2 = strtok(NULL, DELIM);\n+\n+\t\tif (!strcmp(\"hash\", cmd) && p1) {\n+\n+\t\t\t/* print hash of oid */\n+\t\t\tif (!get_oid(p1, &oid))\n+\t\t\t\tprintf(\"%x\\n\", ntohl(sha1hash(oid.hash)));\n+\t\t\telse\n+\t\t\t\tprintf(\"Unknown oid: %s\\n\", p1);\n+\n+\t\t} else if (!strcmp(\"add\", cmd) && p1 && p2) {\n+\n+\t\t\tif (get_oid(p1, &oid)) {\n+\t\t\t\tprintf(\"Unknown oid: %s\\n\", p1);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\t/* create entry with oidkey from p1, value = p2 */\n+\t\t\tFLEX_ALLOC_STR(entry, name, p2);\n+\t\t\toidcpy(&entry->entry.oid, &oid);\n+\n+\t\t\t/* add to oidmap */\n+\t\t\toidmap_put(&map, entry);\n+\n+\t\t} else if (!strcmp(\"put\", cmd) && p1 && p2) {\n+\n+\t\t\tif (get_oid(p1, &oid)) {\n+\t\t\t\tprintf(\"Unknown oid: %s\\n\", p1);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\t/* create entry with oid_key = p1, name_value = p2 */\n+\t\t\tFLEX_ALLOC_STR(entry, name, p2);\n+\t\t\toidcpy(&entry->entry.oid, &oid);\n+\n+\t\t\t/* add / replace entry */\n+\t\t\tentry = oidmap_put(&map, entry);\n+\n+\t\t\t/* print and free replaced entry, if any */\n+\t\t\tputs(entry ? entry->name : \"NULL\");\n+\t\t\tfree(entry);\n+\n+\t\t} else if (!strcmp(\"get\", cmd) && p1) {\n+\n+\t\t\tif (get_oid(p1, &oid)) {\n+\t\t\t\tprintf(\"Unknown oid: %s\\n\", p1);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\t/* lookup entry in oidmap */\n+\t\t\tentry = oidmap_get(&map, &oid);\n+\n+\t\t\t/* print result */\n+\t\t\tputs(entry ? entry->name : \"NULL\");\n+\n+\t\t} else if (!strcmp(\"remove\", cmd) && p1) {\n+\n+\t\t\tif (get_oid(p1, &oid)) {\n+\t\t\t\tprintf(\"Unknown oid: %s\\n\", p1);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\t/* remove entry from oidmap */\n+\t\t\tentry = oidmap_remove(&map, &oid);\n+\n+\t\t\t/* print result and free entry*/\n+\t\t\tputs(entry ? entry->name : \"NULL\");\n+\t\t\tfree(entry);\n+\n+\t\t} else if (!strcmp(\"iterate\", cmd)) {\n+\n+\t\t\tstruct oidmap_iter iter;\n+\t\t\toidmap_iter_init(&map, &iter);\n+\t\t\twhile ((entry = oidmap_iter_next(&iter)))\n+\t\t\t\tprintf(\"%s %s\\n\", oid_to_hex(&entry->entry.oid), entry->name);\n+\n+\t\t} else {\n+\n+\t\t\tprintf(\"Unknown command %s\\n\", cmd);\n+\n+\t\t}\n+\t}\n+\n+\tstrbuf_release(&line);\n+\toidmap_free(&map, 1);\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 087a8c0cc9..1eac25233f 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -35,6 +35,7 @@ static struct test_cmd cmds[] = {\n \t{ \"match-trees\", cmd__match_trees },\n \t{ \"mergesort\", cmd__mergesort },\n \t{ \"mktemp\", cmd__mktemp },\n+\t{ \"oidmap\", cmd__oidmap },\n \t{ \"online-cpus\", cmd__online_cpus },\n \t{ \"parse-options\", cmd__parse_options },\n \t{ \"path-utils\", cmd__path_utils },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 7e703f3038..c7a46dc320 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -25,6 +25,7 @@ int cmd__lazy_init_name_hash(int argc, const char **argv);\n int cmd__match_trees(int argc, const char **argv);\n int cmd__mergesort(int argc, const char **argv);\n int cmd__mktemp(int argc, const char **argv);\n+int cmd__oidmap(int argc, const char **argv);\n int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n int cmd__path_utils(int argc, const char **argv);\n-- \n2.22.0.6.gde8b105b43\n\n"},{"id":"376999","messageId":"20190611082325.28878-3-chriscool@tuxfamily.org","threadId":"51286","inReplyTo":"20190611082325.28878-1-chriscool@tuxfamily.org","subject":"[PATCH v2 2/3] t: add t0016-oidmap.sh","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-06-11T08:23:24Z","receivedAt":"2019-06-11T08:24:00Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder <christian.couder@gmail.com>\n\nAdd actual tests for operations using `struct oidmap` from oidmap.{c,h}.\n\nHelped-by: SZEDER Gábor <szeder.dev@gmail.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/t0016-oidmap.sh | 100 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 100 insertions(+)\n create mode 100755 t/t0016-oidmap.sh\n\ndiff --git a/t/t0016-oidmap.sh b/t/t0016-oidmap.sh\nnew file mode 100755\nindex 0000000000..cbd2cb71d6\n--- /dev/null\n+++ b/t/t0016-oidmap.sh\n@@ -0,0 +1,100 @@\n+#!/bin/sh\n+\n+test_description='test oidmap'\n+. ./test-lib.sh\n+\n+# This purposefully is very similar to t0011-hashmap.sh\n+\n+test_oidmap() {\n+\techo \"$1\" | test-tool oidmap $3 > actual &&\n+\techo \"$2\" >expect &&\n+\ttest_cmp expect actual\n+}\n+\n+\n+test_expect_success 'setup' '\n+\n+\ttest_commit one &&\n+\ttest_commit two &&\n+\ttest_commit three &&\n+\ttest_commit four\n+\n+'\n+\n+test_oidhash() {\n+\tgit rev-parse \"$1\" | cut -c1-8\n+}\n+\n+test_expect_success 'hash' '\n+\n+test_oidmap \"hash one\n+hash two\n+hash invalidOid\n+hash three\" \"$(test_oidhash one)\n+$(test_oidhash two)\n+Unknown oid: invalidOid\n+$(test_oidhash three)\"\n+\n+'\n+\n+test_expect_success 'put' '\n+\n+test_oidmap \"put one 1\n+put two 2\n+put invalidOid 4\n+put three 3\" \"NULL\n+NULL\n+Unknown oid: invalidOid\n+NULL\"\n+\n+'\n+\n+test_expect_success 'replace' '\n+\n+test_oidmap \"put one 1\n+put two 2\n+put three 3\n+put invalidOid 4\n+put two deux\n+put one un\" \"NULL\n+NULL\n+NULL\n+Unknown oid: invalidOid\n+2\n+1\"\n+\n+'\n+\n+test_expect_success 'get' '\n+\n+test_oidmap \"put one 1\n+put two 2\n+put three 3\n+get two\n+get four\n+get invalidOid\n+get one\" \"NULL\n+NULL\n+NULL\n+2\n+NULL\n+Unknown oid: invalidOid\n+1\"\n+\n+'\n+\n+test_expect_success 'iterate' '\n+\n+test_oidmap \"put one 1\n+put two 2\n+put three 3\n+iterate\" \"NULL\n+NULL\n+NULL\n+$(git rev-parse two) 2\n+$(git rev-parse one) 1\n+$(git rev-parse three) 3\"\n+\n+'\n+\n+test_done\n-- \n2.22.0.6.gde8b105b43\n\n"},{"id":"377000","messageId":"20190611082325.28878-4-chriscool@tuxfamily.org","threadId":"51286","inReplyTo":"20190611082325.28878-1-chriscool@tuxfamily.org","subject":"[PATCH v2 3/3] oidmap: use sha1hash() instead of static hash() function","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-06-11T08:23:25Z","receivedAt":"2019-06-11T08:24:03Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder <christian.couder@gmail.com>\n\nGet rid of the static hash() function in oidmap.c which is redundant\nwith sha1hash(). Use sha1hash() directly instead.\n\nLet's be more consistent and not use several hash functions doing\nnearly exactly the same thing.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n oidmap.c | 13 +++----------\n 1 file changed, 3 insertions(+), 10 deletions(-)\n\ndiff --git a/oidmap.c b/oidmap.c\nindex b0841a0f58..01c206aaef 100644\n--- a/oidmap.c\n+++ b/oidmap.c\n@@ -12,13 +12,6 @@ static int oidmap_neq(const void *hashmap_cmp_fn_data,\n \t\t      &((const struct oidmap_entry *) entry_or_key)->oid);\n }\n \n-static int hash(const struct object_id *oid)\n-{\n-\tint hash;\n-\tmemcpy(&hash, oid->hash, sizeof(hash));\n-\treturn hash;\n-}\n-\n void oidmap_init(struct oidmap *map, size_t initial_size)\n {\n \thashmap_init(&map->map, oidmap_neq, NULL, initial_size);\n@@ -36,7 +29,7 @@ void *oidmap_get(const struct oidmap *map, const struct object_id *key)\n \tif (!map->map.cmpfn)\n \t\treturn NULL;\n \n-\treturn hashmap_get_from_hash(&map->map, hash(key), key);\n+\treturn hashmap_get_from_hash(&map->map, sha1hash(key->hash), key);\n }\n \n void *oidmap_remove(struct oidmap *map, const struct object_id *key)\n@@ -46,7 +39,7 @@ void *oidmap_remove(struct oidmap *map, const struct object_id *key)\n \tif (!map->map.cmpfn)\n \t\toidmap_init(map, 0);\n \n-\thashmap_entry_init(&entry, hash(key));\n+\thashmap_entry_init(&entry, sha1hash(key->hash));\n \treturn hashmap_remove(&map->map, &entry, key);\n }\n \n@@ -57,6 +50,6 @@ void *oidmap_put(struct oidmap *map, void *entry)\n \tif (!map->map.cmpfn)\n \t\toidmap_init(map, 0);\n \n-\thashmap_entry_init(&to_put->internal_entry, hash(&to_put->oid));\n+\thashmap_entry_init(&to_put->internal_entry, sha1hash(to_put->oid.hash));\n \treturn hashmap_put(&map->map, to_put);\n }\n-- \n2.22.0.6.gde8b105b43\n\n"},{"id":"377001","messageId":"20190611101234.GD4012@szeder.dev","threadId":"51286","inReplyTo":"20190611082325.28878-1-chriscool@tuxfamily.org","subject":"Re: [PATCH v2 0/3] Test oidmap","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-06-11T10:12:34Z","receivedAt":"2019-06-11T10:12:41Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Jun 11, 2019 at 10:23:22AM +0200, Christian Couder wrote:\n> I decided against hardcoding values as I think it might help\n> transitionning from SHA1 to SHA256.\n\nOk, that makes sense.\n\n"},{"id":"377002","messageId":"20190611101250.GE4012@szeder.dev","threadId":"51286","inReplyTo":"20190611082325.28878-3-chriscool@tuxfamily.org","subject":"Re: [PATCH v2 2/3] t: add t0016-oidmap.sh","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-06-11T10:12:50Z","receivedAt":"2019-06-11T10:12:55Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Jun 11, 2019 at 10:23:24AM +0200, Christian Couder wrote:\n> diff --git a/t/t0016-oidmap.sh b/t/t0016-oidmap.sh\n> new file mode 100755\n> index 0000000000..cbd2cb71d6\n> --- /dev/null\n> +++ b/t/t0016-oidmap.sh\n> @@ -0,0 +1,100 @@\n> +#!/bin/sh\n> +\n> +test_description='test oidmap'\n> +. ./test-lib.sh\n> +\n> +# This purposefully is very similar to t0011-hashmap.sh\n> +\n> +test_oidmap() {\n> +\techo \"$1\" | test-tool oidmap $3 > actual &&\n\nStyle nit: space between redirection op and filename ;)\n\n"},{"id":"377090","messageId":"CAP8UFD1Cn15P4KeuPbK1ZOcZuWHNRq690PtdcvQSWnSc_T+Beg@mail.gmail.com","threadId":"51286","inReplyTo":"20190611101250.GE4012@szeder.dev","subject":"Re: [PATCH v2 2/3] t: add t0016-oidmap.sh","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-06-12T17:09:06Z","receivedAt":"2019-06-12T17:09:24Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Jun 11, 2019 at 12:12 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:\n>\n> On Tue, Jun 11, 2019 at 10:23:24AM +0200, Christian Couder wrote:\n> > diff --git a/t/t0016-oidmap.sh b/t/t0016-oidmap.sh\n> > new file mode 100755\n> > index 0000000000..cbd2cb71d6\n> > --- /dev/null\n> > +++ b/t/t0016-oidmap.sh\n> > @@ -0,0 +1,100 @@\n> > +#!/bin/sh\n> > +\n> > +test_description='test oidmap'\n> > +. ./test-lib.sh\n> > +\n> > +# This purposefully is very similar to t0011-hashmap.sh\n> > +\n> > +test_oidmap() {\n> > +     echo \"$1\" | test-tool oidmap $3 > actual &&\n>\n> Style nit: space between redirection op and filename ;)\n\nAargh! I will resend soon with a fix.\n\nIt will actually have everything that Junio put in 7f2a91c1a6\n(SQUASH??? sh style, 2019-06-11) which is in pu.\n\nThanks,\nChristian.\n"}]}