{"thread":{"id":"57577","subject":"[PATCH 0/5] refs: designated init & missing debug in ps/fetch-mirror-optim","startedAt":"2022-03-17T17:27:39Z","lastAt":"2022-03-21T19:10:20Z","messageCount":11,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Han-Wen Nienhuys","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"451570","messageId":"cover-0.5-00000000000-20220317T171618Z-avarab@gmail.com","threadId":"57577","inReplyTo":null,"subject":"[PATCH 0/5] refs: designated init & missing debug in ps/fetch-mirror-optim","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-17T17:27:14Z","receivedAt":"2022-03-17T17:27:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When I recently submitted a topic[1] to convert various things to\ndesignated initializers Junio (presumably) ejected the refs changes\ndue to conflicts with ps/fetch-mirror-optim. The rest of the series\nwas down already in 20d34c07ea4 (Merge branch\n'ab/c99-designated-initializers', 2022-03-06)\n\nNow that that series has landed 1-3/5 are those previous patches\nre-submitted (with a minor change for the new \"read_symbolic_ref\").\n\nThe 4/5 is then a deletion of packed-backend.c stub functions that I\nthink are not worth carrying anymore, and which has the fringe benefit\nof squashing a noisy warning under SunCC.\n\n5/5 is then a minor fix-up for ps/fetch-mirror-optim: It didn't define\na debug wrapper for refs_read_symbolic_ref(). Now that we do we don't\nneed an implementation that'll ever fall back on\nrefs_read_raw_ref(). It's now a stand-alone API function without a\nfallback (which in effect it was before, sans debug backend).\n\nAside: It seems that the GIT_TRACE_REFS facility has been broken since\nit was added, i.e. running the test site with e.g.:\n\n    GIT_TRACE_REFS=/tmp/log.txt make test\n\nWill fail, and not due to a test being confused about the logs. I\nthink something in how the iterators are wrapped is confusing it, but\nthat's a long-standing issue in any case...\n\n1. https://lore.kernel.org/git/cover-00.12-00000000000-20220224T092805Z-avarab@gmail.com\n\nÆvar Arnfjörð Bjarmason (5):\n  refs: use designated initializers for \"struct ref_storage_be\"\n  refs: use designated initializers for \"struct ref_iterator_vtable\"\n  misc *.c: use designated initializers for struct assignments\n  packed-backend: remove stub BUG(...) functions\n  refs debug: add a wrapper for \"read_symbolic_ref\"\n\n attr.c                |   2 +-\n notes-merge.c         |   1 +\n object-file.c         |   9 +--\n refs.c                |  13 +----\n refs/debug.c          |  82 ++++++++++++++++++---------\n refs/files-backend.c  |  64 ++++++++++-----------\n refs/iterator.c       |  18 +++---\n refs/packed-backend.c | 128 ++++++++++--------------------------------\n refs/ref-cache.c      |   6 +-\n 9 files changed, 135 insertions(+), 188 deletions(-)\n\n-- \n2.35.1.1384.g7d2906948a1\n\n"},{"id":"451571","messageId":"patch-1.5-a5dcca0b41d-20220317T171618Z-avarab@gmail.com","threadId":"57577","inReplyTo":"cover-0.5-00000000000-20220317T171618Z-avarab@gmail.com","subject":"[PATCH 1/5] refs: use designated initializers for \"struct ref_storage_be\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-17T17:27:15Z","receivedAt":"2022-03-17T17:27:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the definition of the three refs backends we currently carry to\nuse designated initializers.\n\nThe \"= NULL\" assignments being retained here are redundant, and could\nbe removed, but let's keep them for clarity. All of these backends\ndefine almost all fields, so we're not saving much in terms of line\ncount by omitting these, but e.g. for \"refs_be_debug\" it's immediately\napparent that we're omitting \"init\" when comparing its assignment to\nthe others.\n\nThis is a follow-up to similar work merged in bd4232fac33 (Merge\nbranch 'ab/struct-init', 2021-07-16), a4b9fb6a5cf (Merge branch\n'ab/designated-initializers-more', 2021-10-18) and a30321b9eae (Merge\nbranch 'ab/designated-initializers' into\nab/designated-initializers-more, 2021-09-27).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n refs/debug.c          | 52 +++++++++++++++++++++----------------------\n refs/files-backend.c  | 52 +++++++++++++++++++++----------------------\n refs/packed-backend.c | 52 +++++++++++++++++++++----------------------\n 3 files changed, 78 insertions(+), 78 deletions(-)\n\ndiff --git a/refs/debug.c b/refs/debug.c\nindex c590d377200..b03a83258be 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -418,30 +418,30 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname,\n }\n \n struct ref_storage_be refs_be_debug = {\n-\tNULL,\n-\t\"debug\",\n-\tNULL,\n-\tdebug_init_db,\n-\tdebug_transaction_prepare,\n-\tdebug_transaction_finish,\n-\tdebug_transaction_abort,\n-\tdebug_initial_transaction_commit,\n-\n-\tdebug_pack_refs,\n-\tdebug_create_symref,\n-\tdebug_delete_refs,\n-\tdebug_rename_ref,\n-\tdebug_copy_ref,\n-\n-\tdebug_ref_iterator_begin,\n-\tdebug_read_raw_ref,\n-\tNULL,\n-\n-\tdebug_reflog_iterator_begin,\n-\tdebug_for_each_reflog_ent,\n-\tdebug_for_each_reflog_ent_reverse,\n-\tdebug_reflog_exists,\n-\tdebug_create_reflog,\n-\tdebug_delete_reflog,\n-\tdebug_reflog_expire,\n+\t.next = NULL,\n+\t.name = \"debug\",\n+\t.init = NULL,\n+\t.init_db = debug_init_db,\n+\t.transaction_prepare = debug_transaction_prepare,\n+\t.transaction_finish = debug_transaction_finish,\n+\t.transaction_abort = debug_transaction_abort,\n+\t.initial_transaction_commit = debug_initial_transaction_commit,\n+\n+\t.pack_refs = debug_pack_refs,\n+\t.create_symref = debug_create_symref,\n+\t.delete_refs = debug_delete_refs,\n+\t.rename_ref = debug_rename_ref,\n+\t.copy_ref = debug_copy_ref,\n+\n+\t.iterator_begin = debug_ref_iterator_begin,\n+\t.read_raw_ref = debug_read_raw_ref,\n+\t.read_symbolic_ref = NULL,\n+\n+\t.reflog_iterator_begin = debug_reflog_iterator_begin,\n+\t.for_each_reflog_ent = debug_for_each_reflog_ent,\n+\t.for_each_reflog_ent_reverse = debug_for_each_reflog_ent_reverse,\n+\t.reflog_exists = debug_reflog_exists,\n+\t.create_reflog = debug_create_reflog,\n+\t.delete_reflog = debug_delete_reflog,\n+\t.reflog_expire = debug_reflog_expire,\n };\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 0457ecdb42d..f95552f9263 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3291,30 +3291,30 @@ static int files_init_db(struct ref_store *ref_store, struct strbuf *err)\n }\n \n struct ref_storage_be refs_be_files = {\n-\tNULL,\n-\t\"files\",\n-\tfiles_ref_store_create,\n-\tfiles_init_db,\n-\tfiles_transaction_prepare,\n-\tfiles_transaction_finish,\n-\tfiles_transaction_abort,\n-\tfiles_initial_transaction_commit,\n-\n-\tfiles_pack_refs,\n-\tfiles_create_symref,\n-\tfiles_delete_refs,\n-\tfiles_rename_ref,\n-\tfiles_copy_ref,\n-\n-\tfiles_ref_iterator_begin,\n-\tfiles_read_raw_ref,\n-\tfiles_read_symbolic_ref,\n-\n-\tfiles_reflog_iterator_begin,\n-\tfiles_for_each_reflog_ent,\n-\tfiles_for_each_reflog_ent_reverse,\n-\tfiles_reflog_exists,\n-\tfiles_create_reflog,\n-\tfiles_delete_reflog,\n-\tfiles_reflog_expire\n+\t.next = NULL,\n+\t.name = \"files\",\n+\t.init = files_ref_store_create,\n+\t.init_db = files_init_db,\n+\t.transaction_prepare = files_transaction_prepare,\n+\t.transaction_finish = files_transaction_finish,\n+\t.transaction_abort = files_transaction_abort,\n+\t.initial_transaction_commit = files_initial_transaction_commit,\n+\n+\t.pack_refs = files_pack_refs,\n+\t.create_symref = files_create_symref,\n+\t.delete_refs = files_delete_refs,\n+\t.rename_ref = files_rename_ref,\n+\t.copy_ref = files_copy_ref,\n+\n+\t.iterator_begin = files_ref_iterator_begin,\n+\t.read_raw_ref = files_read_raw_ref,\n+\t.read_symbolic_ref = files_read_symbolic_ref,\n+\n+\t.reflog_iterator_begin = files_reflog_iterator_begin,\n+\t.for_each_reflog_ent = files_for_each_reflog_ent,\n+\t.for_each_reflog_ent_reverse = files_for_each_reflog_ent_reverse,\n+\t.reflog_exists = files_reflog_exists,\n+\t.create_reflog = files_create_reflog,\n+\t.delete_reflog = files_delete_reflog,\n+\t.reflog_expire = files_reflog_expire\n };\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex f56e2cc635b..47f01fa5c98 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1667,30 +1667,30 @@ static int packed_reflog_expire(struct ref_store *ref_store,\n }\n \n struct ref_storage_be refs_be_packed = {\n-\tNULL,\n-\t\"packed\",\n-\tpacked_ref_store_create,\n-\tpacked_init_db,\n-\tpacked_transaction_prepare,\n-\tpacked_transaction_finish,\n-\tpacked_transaction_abort,\n-\tpacked_initial_transaction_commit,\n-\n-\tpacked_pack_refs,\n-\tpacked_create_symref,\n-\tpacked_delete_refs,\n-\tpacked_rename_ref,\n-\tpacked_copy_ref,\n-\n-\tpacked_ref_iterator_begin,\n-\tpacked_read_raw_ref,\n-\tNULL,\n-\n-\tpacked_reflog_iterator_begin,\n-\tpacked_for_each_reflog_ent,\n-\tpacked_for_each_reflog_ent_reverse,\n-\tpacked_reflog_exists,\n-\tpacked_create_reflog,\n-\tpacked_delete_reflog,\n-\tpacked_reflog_expire\n+\t.next = NULL,\n+\t.name = \"packed\",\n+\t.init = packed_ref_store_create,\n+\t.init_db = packed_init_db,\n+\t.transaction_prepare = packed_transaction_prepare,\n+\t.transaction_finish = packed_transaction_finish,\n+\t.transaction_abort = packed_transaction_abort,\n+\t.initial_transaction_commit = packed_initial_transaction_commit,\n+\n+\t.pack_refs = packed_pack_refs,\n+\t.create_symref = packed_create_symref,\n+\t.delete_refs = packed_delete_refs,\n+\t.rename_ref = packed_rename_ref,\n+\t.copy_ref = packed_copy_ref,\n+\n+\t.iterator_begin = packed_ref_iterator_begin,\n+\t.read_raw_ref = packed_read_raw_ref,\n+\t.read_symbolic_ref = NULL,\n+\n+\t.reflog_iterator_begin = packed_reflog_iterator_begin,\n+\t.for_each_reflog_ent = packed_for_each_reflog_ent,\n+\t.for_each_reflog_ent_reverse = packed_for_each_reflog_ent_reverse,\n+\t.reflog_exists = packed_reflog_exists,\n+\t.create_reflog = packed_create_reflog,\n+\t.delete_reflog = packed_delete_reflog,\n+\t.reflog_expire = packed_reflog_expire\n };\n-- \n2.35.1.1384.g7d2906948a1\n\n"},{"id":"451572","messageId":"patch-2.5-3f82f24a17d-20220317T171618Z-avarab@gmail.com","threadId":"57577","inReplyTo":"cover-0.5-00000000000-20220317T171618Z-avarab@gmail.com","subject":"[PATCH 2/5] refs: use designated initializers for \"struct ref_iterator_vtable\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-17T17:27:16Z","receivedAt":"2022-03-17T17:27:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n refs/debug.c          |  5 +++--\n refs/files-backend.c  | 12 ++++++------\n refs/iterator.c       | 18 +++++++++---------\n refs/packed-backend.c |  6 +++---\n refs/ref-cache.c      |  6 +++---\n 5 files changed, 24 insertions(+), 23 deletions(-)\n\ndiff --git a/refs/debug.c b/refs/debug.c\nindex b03a83258be..b83b5817118 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -220,8 +220,9 @@ static int debug_ref_iterator_abort(struct ref_iterator *ref_iterator)\n }\n \n static struct ref_iterator_vtable debug_ref_iterator_vtable = {\n-\tdebug_ref_iterator_advance, debug_ref_iterator_peel,\n-\tdebug_ref_iterator_abort\n+\t.advance = debug_ref_iterator_advance,\n+\t.peel = debug_ref_iterator_peel,\n+\t.abort = debug_ref_iterator_abort,\n };\n \n static struct ref_iterator *\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex f95552f9263..aa4e7182b6e 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -822,9 +822,9 @@ static int files_ref_iterator_abort(struct ref_iterator *ref_iterator)\n }\n \n static struct ref_iterator_vtable files_ref_iterator_vtable = {\n-\tfiles_ref_iterator_advance,\n-\tfiles_ref_iterator_peel,\n-\tfiles_ref_iterator_abort\n+\t.advance = files_ref_iterator_advance,\n+\t.peel = files_ref_iterator_peel,\n+\t.abort = files_ref_iterator_abort,\n };\n \n static struct ref_iterator *files_ref_iterator_begin(\n@@ -2231,9 +2231,9 @@ static int files_reflog_iterator_abort(struct ref_iterator *ref_iterator)\n }\n \n static struct ref_iterator_vtable files_reflog_iterator_vtable = {\n-\tfiles_reflog_iterator_advance,\n-\tfiles_reflog_iterator_peel,\n-\tfiles_reflog_iterator_abort\n+\t.advance = files_reflog_iterator_advance,\n+\t.peel = files_reflog_iterator_peel,\n+\t.abort = files_reflog_iterator_abort,\n };\n \n static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,\ndiff --git a/refs/iterator.c b/refs/iterator.c\nindex a89d132d4fe..b2e56bae1c6 100644\n--- a/refs/iterator.c\n+++ b/refs/iterator.c\n@@ -64,9 +64,9 @@ static int empty_ref_iterator_abort(struct ref_iterator *ref_iterator)\n }\n \n static struct ref_iterator_vtable empty_ref_iterator_vtable = {\n-\tempty_ref_iterator_advance,\n-\tempty_ref_iterator_peel,\n-\tempty_ref_iterator_abort\n+\t.advance = empty_ref_iterator_advance,\n+\t.peel = empty_ref_iterator_peel,\n+\t.abort = empty_ref_iterator_abort,\n };\n \n struct ref_iterator *empty_ref_iterator_begin(void)\n@@ -201,9 +201,9 @@ static int merge_ref_iterator_abort(struct ref_iterator *ref_iterator)\n }\n \n static struct ref_iterator_vtable merge_ref_iterator_vtable = {\n-\tmerge_ref_iterator_advance,\n-\tmerge_ref_iterator_peel,\n-\tmerge_ref_iterator_abort\n+\t.advance = merge_ref_iterator_advance,\n+\t.peel = merge_ref_iterator_peel,\n+\t.abort = merge_ref_iterator_abort,\n };\n \n struct ref_iterator *merge_ref_iterator_begin(\n@@ -378,9 +378,9 @@ static int prefix_ref_iterator_abort(struct ref_iterator *ref_iterator)\n }\n \n static struct ref_iterator_vtable prefix_ref_iterator_vtable = {\n-\tprefix_ref_iterator_advance,\n-\tprefix_ref_iterator_peel,\n-\tprefix_ref_iterator_abort\n+\t.advance = prefix_ref_iterator_advance,\n+\t.peel = prefix_ref_iterator_peel,\n+\t.abort = prefix_ref_iterator_abort,\n };\n \n struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 47f01fa5c98..03002451f15 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -911,9 +911,9 @@ static int packed_ref_iterator_abort(struct ref_iterator *ref_iterator)\n }\n \n static struct ref_iterator_vtable packed_ref_iterator_vtable = {\n-\tpacked_ref_iterator_advance,\n-\tpacked_ref_iterator_peel,\n-\tpacked_ref_iterator_abort\n+\t.advance = packed_ref_iterator_advance,\n+\t.peel = packed_ref_iterator_peel,\n+\t.abort = packed_ref_iterator_abort\n };\n \n static struct ref_iterator *packed_ref_iterator_begin(\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nindex be4aa5e0981..3080ef944d9 100644\n--- a/refs/ref-cache.c\n+++ b/refs/ref-cache.c\n@@ -456,9 +456,9 @@ static int cache_ref_iterator_abort(struct ref_iterator *ref_iterator)\n }\n \n static struct ref_iterator_vtable cache_ref_iterator_vtable = {\n-\tcache_ref_iterator_advance,\n-\tcache_ref_iterator_peel,\n-\tcache_ref_iterator_abort\n+\t.advance = cache_ref_iterator_advance,\n+\t.peel = cache_ref_iterator_peel,\n+\t.abort = cache_ref_iterator_abort\n };\n \n struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache,\n-- \n2.35.1.1384.g7d2906948a1\n\n"},{"id":"451573","messageId":"patch-3.5-11bfe9fabb4-20220317T171618Z-avarab@gmail.com","threadId":"57577","inReplyTo":"cover-0.5-00000000000-20220317T171618Z-avarab@gmail.com","subject":"[PATCH 3/5] misc *.c: use designated initializers for struct assignments","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-17T17:27:17Z","receivedAt":"2022-03-17T17:27:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a few miscellaneous non-designated initializer assignments to\nuse designated initializers.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n attr.c        | 2 +-\n notes-merge.c | 1 +\n object-file.c | 9 +++++----\n 3 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 79adaa50ea1..9ad12578cce 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -80,7 +80,7 @@ static int attr_hash_entry_cmp(const void *unused_cmp_data,\n  * Access to this dictionary must be surrounded with a mutex.\n  */\n static struct attr_hashmap g_attr_hashmap = {\n-\tHASHMAP_INIT(attr_hash_entry_cmp, NULL)\n+\t.map = HASHMAP_INIT(attr_hash_entry_cmp, NULL),\n };\n \n /*\ndiff --git a/notes-merge.c b/notes-merge.c\nindex 878b6c571b9..b4cc594a790 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -113,6 +113,7 @@ static struct notes_merge_pair *find_notes_merge_pair_pos(\n }\n \n static struct object_id uninitialized = {\n+\t.hash =\n \t\"\\xff\\xff\\xff\\xff\\xff\\xff\\xff\\xff\\xff\\xff\" \\\n \t\"\\xff\\xff\\xff\\xff\\xff\\xff\\xff\\xff\\xff\\xff\"\n };\ndiff --git a/object-file.c b/object-file.c\nindex bdc5cbdd386..f0a75b3ff1e 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -274,10 +274,11 @@ static struct cached_object {\n static int cached_object_nr, cached_object_alloc;\n \n static struct cached_object empty_tree = {\n-\t{ EMPTY_TREE_SHA1_BIN_LITERAL },\n-\tOBJ_TREE,\n-\t\"\",\n-\t0\n+\t.oid = {\n+\t\t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n+\t},\n+\t.type = OBJ_TREE,\n+\t.buf = \"\",\n };\n \n static struct cached_object *find_cached_object(const struct object_id *oid)\n-- \n2.35.1.1384.g7d2906948a1\n\n"},{"id":"451574","messageId":"patch-4.5-c9a7004bc38-20220317T171618Z-avarab@gmail.com","threadId":"57577","inReplyTo":"cover-0.5-00000000000-20220317T171618Z-avarab@gmail.com","subject":"[PATCH 4/5] packed-backend: remove stub BUG(...) functions","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-17T17:27:18Z","receivedAt":"2022-03-17T17:27:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the stub BUG(...) functions previously used by the \"struct\nref_storage_be refs_be_packed\" backend.\n\nWe never call any functions in the packed backend by using it as a\n\"normal\" primary ref store, instead we'll always initialize a \"files\"\nbackend ref-store.\n\nIt will then via the \"packed_ref_store\" member of \"struct\nfiles_ref_store\" call selected functions in the \"packed\" backend, and\nwe'll in addition call others via wrappers in refs.c.\n\nSo while these would arguably give us *slightly* more meaningful error\nmessages we'll NULL the missing members in the initializer anyway, so\nwe'll reliably get a segfault if we're ever changing the backend and\nhaving it call something it doesn't have.\n\nSo there's no need for this verbose boilerplate, and as shown in a\nsubsequent commit it might even lead to some confusion about the\npacked backend being a \"real\" backend. Let's make it clear that it's\nnot.\n\nAs an aside, this also fixes a warning emitted by SunCC in at least\nversions 12.5 and 12.6 of Oracle Developer Studio:\n\n    \"refs/packed-backend.c\", line 1599: warning: Function has no return statement : packed_create_symref\n    \"refs/packed-backend.c\", line 1606: warning: Function has no return statement : packed_rename_ref)\n    \"refs/packed-backend.c\", line 1613: warning: Function has no return statement : packed_copy_ref\n    \"refs/packed-backend.c\", line 1648: warning: Function has no return statement : packed_create_reflog\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n refs/packed-backend.c | 88 +++++--------------------------------------\n 1 file changed, 9 insertions(+), 79 deletions(-)\n\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 03002451f15..310c2a72026 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1591,81 +1591,11 @@ static int packed_pack_refs(struct ref_store *ref_store, unsigned int flags)\n \treturn 0;\n }\n \n-static int packed_create_symref(struct ref_store *ref_store,\n-\t\t\t       const char *refname, const char *target,\n-\t\t\t       const char *logmsg)\n-{\n-\tBUG(\"packed reference store does not support symrefs\");\n-}\n-\n-static int packed_rename_ref(struct ref_store *ref_store,\n-\t\t\t    const char *oldrefname, const char *newrefname,\n-\t\t\t    const char *logmsg)\n-{\n-\tBUG(\"packed reference store does not support renaming references\");\n-}\n-\n-static int packed_copy_ref(struct ref_store *ref_store,\n-\t\t\t   const char *oldrefname, const char *newrefname,\n-\t\t\t   const char *logmsg)\n-{\n-\tBUG(\"packed reference store does not support copying references\");\n-}\n-\n static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store)\n {\n \treturn empty_ref_iterator_begin();\n }\n \n-static int packed_for_each_reflog_ent(struct ref_store *ref_store,\n-\t\t\t\t      const char *refname,\n-\t\t\t\t      each_reflog_ent_fn fn, void *cb_data)\n-{\n-\tBUG(\"packed reference store does not support reflogs\");\n-\treturn 0;\n-}\n-\n-static int packed_for_each_reflog_ent_reverse(struct ref_store *ref_store,\n-\t\t\t\t\t      const char *refname,\n-\t\t\t\t\t      each_reflog_ent_fn fn,\n-\t\t\t\t\t      void *cb_data)\n-{\n-\tBUG(\"packed reference store does not support reflogs\");\n-\treturn 0;\n-}\n-\n-static int packed_reflog_exists(struct ref_store *ref_store,\n-\t\t\t       const char *refname)\n-{\n-\tBUG(\"packed reference store does not support reflogs\");\n-\treturn 0;\n-}\n-\n-static int packed_create_reflog(struct ref_store *ref_store,\n-\t\t\t\tconst char *refname, struct strbuf *err)\n-{\n-\tBUG(\"packed reference store does not support reflogs\");\n-}\n-\n-static int packed_delete_reflog(struct ref_store *ref_store,\n-\t\t\t       const char *refname)\n-{\n-\tBUG(\"packed reference store does not support reflogs\");\n-\treturn 0;\n-}\n-\n-static int packed_reflog_expire(struct ref_store *ref_store,\n-\t\t\t\tconst char *refname,\n-\t\t\t\tunsigned int flags,\n-\t\t\t\treflog_expiry_prepare_fn prepare_fn,\n-\t\t\t\treflog_expiry_should_prune_fn should_prune_fn,\n-\t\t\t\treflog_expiry_cleanup_fn cleanup_fn,\n-\t\t\t\tvoid *policy_cb_data)\n-{\n-\tBUG(\"packed reference store does not support reflogs\");\n-\treturn 0;\n-}\n-\n struct ref_storage_be refs_be_packed = {\n \t.next = NULL,\n \t.name = \"packed\",\n@@ -1677,20 +1607,20 @@ struct ref_storage_be refs_be_packed = {\n \t.initial_transaction_commit = packed_initial_transaction_commit,\n \n \t.pack_refs = packed_pack_refs,\n-\t.create_symref = packed_create_symref,\n+\t.create_symref = NULL,\n \t.delete_refs = packed_delete_refs,\n-\t.rename_ref = packed_rename_ref,\n-\t.copy_ref = packed_copy_ref,\n+\t.rename_ref = NULL,\n+\t.copy_ref = NULL,\n \n \t.iterator_begin = packed_ref_iterator_begin,\n \t.read_raw_ref = packed_read_raw_ref,\n \t.read_symbolic_ref = NULL,\n \n \t.reflog_iterator_begin = packed_reflog_iterator_begin,\n-\t.for_each_reflog_ent = packed_for_each_reflog_ent,\n-\t.for_each_reflog_ent_reverse = packed_for_each_reflog_ent_reverse,\n-\t.reflog_exists = packed_reflog_exists,\n-\t.create_reflog = packed_create_reflog,\n-\t.delete_reflog = packed_delete_reflog,\n-\t.reflog_expire = packed_reflog_expire\n+\t.for_each_reflog_ent = NULL,\n+\t.for_each_reflog_ent_reverse = NULL,\n+\t.reflog_exists = NULL,\n+\t.create_reflog = NULL,\n+\t.delete_reflog = NULL,\n+\t.reflog_expire = NULL,\n };\n-- \n2.35.1.1384.g7d2906948a1\n\n"},{"id":"451575","messageId":"patch-5.5-ad45319b19a-20220317T171618Z-avarab@gmail.com","threadId":"57577","inReplyTo":"cover-0.5-00000000000-20220317T171618Z-avarab@gmail.com","subject":"[PATCH 5/5] refs debug: add a wrapper for \"read_symbolic_ref\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-17T17:27:19Z","receivedAt":"2022-03-17T17:27:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In cd475b3b038 (refs: add ability for backends to special-case reading\nof symbolic refs, 2022-03-01) when the \"read_symbolic_ref\" callback\nwas added we'd fall back on \"refs_read_raw_ref\" if there wasn't any\nbackend implementation of \"read_symbolic_ref\".\n\nAs discussed in the preceding commit this would only happen if we were\nrunning the \"debug\" backend, e.g. in the \"setup for ref completion\"\ntest in t9902-completion.sh with:\n\n    GIT_TRACE_REFS=1 git fetch --no-tags other\n\nLet's improve the trace output, but and also eliminate the\nnow-redundant refs_read_raw_ref() fallback case. As noted in the\npreceding commit the \"packed\" backend will never call\nrefs_read_symbolic_ref() (nor is it ever going to). For any future\nbackend such as reftable it's OK to ask that they either implement\nthis (or a wrapper) themselves.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n refs.c       | 13 +------------\n refs/debug.c | 27 ++++++++++++++++++++++++++-\n 2 files changed, 27 insertions(+), 13 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0b79bdd7c37..1a964505f92 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1676,18 +1676,7 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,\n int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname,\n \t\t\t   struct strbuf *referent)\n {\n-\tstruct object_id oid;\n-\tint ret, failure_errno = 0;\n-\tunsigned int type = 0;\n-\n-\tif (ref_store->be->read_symbolic_ref)\n-\t\treturn ref_store->be->read_symbolic_ref(ref_store, refname, referent);\n-\n-\tret = refs_read_raw_ref(ref_store, refname, &oid, referent, &type, &failure_errno);\n-\tif (ret || !(type & REF_ISSYMREF))\n-\t\treturn -1;\n-\n-\treturn 0;\n+\treturn ref_store->be->read_symbolic_ref(ref_store, refname, referent);\n }\n \n const char *refs_resolve_ref_unsafe(struct ref_store *refs,\ndiff --git a/refs/debug.c b/refs/debug.c\nindex b83b5817118..eed8bc94b04 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -262,6 +262,24 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \treturn res;\n }\n \n+static int debug_read_symbolic_ref(struct ref_store *ref_store, const char *refname,\n+\t\t\t\t   struct strbuf *referent)\n+{\n+\tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n+\tstruct ref_store *refs = drefs->refs;\n+\tint res;\n+\n+\tres = refs->be->read_symbolic_ref(refs, refname, referent);\n+\tif (!res)\n+\t\ttrace_printf_key(&trace_refs, \"read_symbolic_ref: %s: (%s)\\n\",\n+\t\t\t\t refname, referent->buf);\n+\telse\n+\t\ttrace_printf_key(&trace_refs,\n+\t\t\t\t \"read_symbolic_ref: %s: %d\\n\", refname, res);\n+\treturn res;\n+\n+}\n+\n static struct ref_iterator *\n debug_reflog_iterator_begin(struct ref_store *ref_store)\n {\n@@ -423,6 +441,13 @@ struct ref_storage_be refs_be_debug = {\n \t.name = \"debug\",\n \t.init = NULL,\n \t.init_db = debug_init_db,\n+\n+\t/*\n+\t * None of these should be NULL. If the \"files\" backend (in\n+\t * \"struct ref_storage_be refs_be_files\" in files-backend.c)\n+\t * has a function we should also have a wrapper for it here.\n+\t * Test the output with \"GIT_TRACE_REFS=1\".\n+\t */\n \t.transaction_prepare = debug_transaction_prepare,\n \t.transaction_finish = debug_transaction_finish,\n \t.transaction_abort = debug_transaction_abort,\n@@ -436,7 +461,7 @@ struct ref_storage_be refs_be_debug = {\n \n \t.iterator_begin = debug_ref_iterator_begin,\n \t.read_raw_ref = debug_read_raw_ref,\n-\t.read_symbolic_ref = NULL,\n+\t.read_symbolic_ref = debug_read_symbolic_ref,\n \n \t.reflog_iterator_begin = debug_reflog_iterator_begin,\n \t.for_each_reflog_ent = debug_for_each_reflog_ent,\n-- \n2.35.1.1384.g7d2906948a1\n\n"},{"id":"451576","messageId":"xmqqlex8h40g.fsf@gitster.g","threadId":"57577","inReplyTo":"patch-4.5-c9a7004bc38-20220317T171618Z-avarab@gmail.com","subject":"Re: [PATCH 4/5] packed-backend: remove stub BUG(...) functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-17T17:39:59Z","receivedAt":"2022-03-17T17:40:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> -\t.for_each_reflog_ent = packed_for_each_reflog_ent,\n> -\t.for_each_reflog_ent_reverse = packed_for_each_reflog_ent_reverse,\n> -\t.reflog_exists = packed_reflog_exists,\n> -\t.create_reflog = packed_create_reflog,\n> -\t.delete_reflog = packed_delete_reflog,\n> -\t.reflog_expire = packed_reflog_expire\n> +\t.for_each_reflog_ent = NULL,\n> +\t.for_each_reflog_ent_reverse = NULL,\n> +\t.reflog_exists = NULL,\n> +\t.create_reflog = NULL,\n> +\t.delete_reflog = NULL,\n> +\t.reflog_expire = NULL,\n\nYup, the entry in vtable being set to NULL is a more direct sign the\nreaders would appreciate than a name natural for the method, whose\ndefinition needs to be consulted before the readers can realize that\nit is a \"missing\" method.\n\nVery sensible.\n\nThanks.\n\n"},{"id":"451577","messageId":"CAFQ2z_M7-D4ZCJbj8JituTbR3d7ocRHd=ObdZUnmE=WD7qW0cg@mail.gmail.com","threadId":"57577","inReplyTo":"cover-0.5-00000000000-20220317T171618Z-avarab@gmail.com","subject":"Re: [PATCH 0/5] refs: designated init & missing debug in ps/fetch-mirror-optim","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-03-17T17:45:13Z","receivedAt":"2022-03-17T17:45:30Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Thu, Mar 17, 2022 at 6:27 PM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> Aside: It seems that the GIT_TRACE_REFS facility has been broken since\n> it was added, i.e. running the test site with e.g.:\n>\n>     GIT_TRACE_REFS=/tmp/log.txt make test\n\nI wasn't aware that this is even possible. I've only used it with\nGIT_TRACE_REFS=1\n\nI looked over your patches and they LGTM.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n"},{"id":"451620","messageId":"220318.86fsnfctzg.gmgdl@evledraar.gmail.com","threadId":"57577","inReplyTo":"CAFQ2z_M7-D4ZCJbj8JituTbR3d7ocRHd=ObdZUnmE=WD7qW0cg@mail.gmail.com","subject":"Re: [PATCH 0/5] refs: designated init & missing debug in ps/fetch-mirror-optim","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-18T12:40:56Z","receivedAt":"2022-03-18T12:42:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Mar 17 2022, Han-Wen Nienhuys wrote:\n\n> On Thu, Mar 17, 2022 at 6:27 PM Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>> Aside: It seems that the GIT_TRACE_REFS facility has been broken since\n>> it was added, i.e. running the test site with e.g.:\n>>\n>>     GIT_TRACE_REFS=/tmp/log.txt make test\n>\n> I wasn't aware that this is even possible. I've only used it with\n> GIT_TRACE_REFS=1\n\nYes, it's possible with all the GIT_TRACE* variables. To clarify the\nfailure has nothing to do with the \"log to file\" feature of it, the same\nhappens e.g. with this on master:\n\n    GIT_TRACE_REFS=1 ./t1400-update-ref.sh -vx --run=1-46,55-169\n\nI.e. here I skipped some tests that would fail because we emitted the\noutput on stderr, but this one fails due to the debug tracing and\ntransactions (I think, but not 100% sure) tripping over one another\nsomehow.\n\n> I looked over your patches and they LGTM.\n\nThanks!\n"},{"id":"451723","messageId":"YjhdOUEG1RbWDApX@ncase","threadId":"57577","inReplyTo":"CAFQ2z_M7-D4ZCJbj8JituTbR3d7ocRHd=ObdZUnmE=WD7qW0cg@mail.gmail.com","subject":"Re: [PATCH 0/5] refs: designated init & missing debug in ps/fetch-mirror-optim","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2022-03-21T11:10:49Z","receivedAt":"2022-03-21T11:11:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Mar 17, 2022 at 06:45:13PM +0100, Han-Wen Nienhuys wrote:\n> On Thu, Mar 17, 2022 at 6:27 PM Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n> > Aside: It seems that the GIT_TRACE_REFS facility has been broken since\n> > it was added, i.e. running the test site with e.g.:\n> >\n> >     GIT_TRACE_REFS=/tmp/log.txt make test\n> \n> I wasn't aware that this is even possible. I've only used it with\n> GIT_TRACE_REFS=1\n> \n> I looked over your patches and they LGTM.\n\nSeconded, the patches look good to me.\n\nPatrick\n"},{"id":"451750","messageId":"xmqqczifxgtn.fsf@gitster.g","threadId":"57577","inReplyTo":"YjhdOUEG1RbWDApX@ncase","subject":"Re: [PATCH 0/5] refs: designated init & missing debug in ps/fetch-mirror-optim","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-21T19:10:12Z","receivedAt":"2022-03-21T19:10:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Thu, Mar 17, 2022 at 06:45:13PM +0100, Han-Wen Nienhuys wrote:\n>> On Thu, Mar 17, 2022 at 6:27 PM Ævar Arnfjörð Bjarmason\n>> <avarab@gmail.com> wrote:\n>> > Aside: It seems that the GIT_TRACE_REFS facility has been broken since\n>> > it was added, i.e. running the test site with e.g.:\n>> >\n>> >     GIT_TRACE_REFS=/tmp/log.txt make test\n>> \n>> I wasn't aware that this is even possible. I've only used it with\n>> GIT_TRACE_REFS=1\n>> \n>> I looked over your patches and they LGTM.\n>\n> Seconded, the patches look good to me.\n\nThanks, all.  The topic will be in 'master' hopefully by the end of\nthe week (I'll be offline part of today, mostly tomorrow and also\nFriday, so things may slow, though).\n\n"}]}