{"thread":{"id":"58256","subject":"[RFC PATCH 2/2] notes: create interface to iterate over notes for a given oid","startedAt":"2022-08-02T07:54:24Z","lastAt":"2022-10-19T12:46:47Z","messageCount":8,"participants":["Vegard Nossum","Junio C Hamano","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"460406","messageId":"20220802075401.2393-2-vegard.nossum@oracle.com","threadId":"58256","inReplyTo":"20220802075401.2393-1-vegard.nossum@oracle.com","subject":"[RFC PATCH 2/2] notes: create interface to iterate over notes for a given oid","fromName":"Vegard Nossum","fromEmail":"vegard.nossum@oracle.com","sentAt":"2022-08-02T07:54:01Z","receivedAt":"2022-08-02T07:54:24Z","isPatch":true,"sender":{"key":"vegard.nossum@oracle.com","avatar":"https://avatars.githubusercontent.com/u/24173?v=4"},"body":"format_display_notes() outputs notes in a specific format which is\nsuitable for displaying in a terminal with \"git log\"/\"git show\". Other\nusers may want a different format.\n\nThis patch adds a new function -- for_each_oid_note() -- which, given the\noid for a commit, iterates over notes refs and calls the given callback\nfunction for each note ref that contains a corresponding note.\n\nThe old functionality can easily be implemented using the new interface,\nso I'm doing that at the same time.\n\nCc: Johan Herland <johan@herland.net>\nCc: Jason A. Donenfeld <Jason@zx2c4.com>\nCc: Christian Hesse <mail@eworm.de>\nSigned-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n---\n notes.c | 108 ++++++++++++++++++++++++++++++++++----------------------\n notes.h |   5 +++\n 2 files changed, 70 insertions(+), 43 deletions(-)\n\ndiff --git a/notes.c b/notes.c\nindex 90ec625192..4c7e883758 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1242,56 +1242,68 @@ void free_notes(struct notes_tree *t)\n \tmemset(t, 0, sizeof(struct notes_tree));\n }\n \n-/*\n- * Fill the given strbuf with the notes associated with the given object.\n- *\n- * If the given notes_tree structure is not initialized, it will be auto-\n- * initialized to the default value (see documentation for init_notes() above).\n- * If the given notes_tree is NULL, the internal/default notes_tree will be\n- * used instead.\n- *\n- * (raw != 0) gives the %N userformat; otherwise, the note message is given\n- * for human consumption.\n- */\n-static void format_note(struct notes_tree *t, const struct object_id *object_oid,\n-\t\t\tstruct strbuf *sb, const char *output_encoding, int raw)\n+void for_each_oid_note(const struct object_id *object_oid,\n+\t\t       const char *output_encoding, int raw,\n+\t\t       each_oid_note_fn fn, void *cb_data)\n {\n \tstatic const char utf8[] = \"utf-8\";\n-\tconst struct object_id *oid;\n-\tchar *msg, *msg_p;\n-\tunsigned long linelen, msglen;\n-\tenum object_type type;\n \n-\tif (!t)\n-\t\tt = &default_notes_tree;\n-\tif (!t->initialized)\n-\t\tinit_notes(t, NULL, NULL, 0);\n+\tint i;\n+\tassert(display_notes_trees);\n+\tfor (i = 0; display_notes_trees[i]; i++) {\n+\t\tstruct notes_tree *t = display_notes_trees[i];\n+\t\tconst struct object_id *oid;\n+\t\tchar *msg;\n+\t\tunsigned long msglen;\n+\t\tenum object_type type;\n+\n+\t\tif (!t)\n+\t\t\tt = &default_notes_tree;\n+\t\tif (!t->initialized)\n+\t\t\tinit_notes(t, NULL, NULL, 0);\n+\n+\t\toid = get_note(t, object_oid);\n+\t\tif (!oid)\n+\t\t\tcontinue;\n \n-\toid = get_note(t, object_oid);\n-\tif (!oid)\n-\t\treturn;\n+\t\tif (!(msg = read_object_file(oid, &type, &msglen)) || type != OBJ_BLOB) {\n+\t\t\tfree(msg);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (output_encoding && *output_encoding &&\n+\t\t    !is_encoding_utf8(output_encoding)) {\n+\t\t\tchar *reencoded = reencode_string(msg, output_encoding, utf8);\n+\t\t\tif (reencoded) {\n+\t\t\t\tfree(msg);\n+\t\t\t\tmsg = reencoded;\n+\t\t\t\tmsglen = strlen(msg);\n+\t\t\t}\n+\t\t}\n \n-\tif (!(msg = read_object_file(oid, &type, &msglen)) || type != OBJ_BLOB) {\n+\t\tfn(t->ref, msg, msglen, cb_data);\n \t\tfree(msg);\n-\t\treturn;\n \t}\n+}\n \n-\tif (output_encoding && *output_encoding &&\n-\t    !is_encoding_utf8(output_encoding)) {\n-\t\tchar *reencoded = reencode_string(msg, output_encoding, utf8);\n-\t\tif (reencoded) {\n-\t\t\tfree(msg);\n-\t\t\tmsg = reencoded;\n-\t\t\tmsglen = strlen(msg);\n-\t\t}\n-\t}\n+struct format_display_notes_cb {\n+\tint raw;\n+\tstruct strbuf *output;\n+};\n+\n+static void format_note(const char *ref, const char *msg, unsigned long msglen, void *cb_data)\n+{\n+\tstruct format_display_notes_cb *cb = cb_data;\n+\tint raw = cb->raw;\n+\tstruct strbuf *sb = cb->output;\n+\tconst char *msg_p;\n+\tunsigned long linelen;\n \n \t/* we will end the annotation by a newline anyway */\n \tif (msglen && msg[msglen - 1] == '\\n')\n \t\tmsglen--;\n \n \tif (!raw) {\n-\t\tconst char *ref = t->ref;\n \t\tif (!ref || !strcmp(ref, GIT_NOTES_DEFAULT_REF)) {\n \t\t\tstrbuf_addstr(sb, \"\\nNotes:\\n\");\n \t\t} else {\n@@ -1309,18 +1321,28 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid\n \t\tstrbuf_add(sb, msg_p, linelen);\n \t\tstrbuf_addch(sb, '\\n');\n \t}\n-\n-\tfree(msg);\n }\n \n+/*\n+ * Fill the given strbuf with the notes associated with the given object.\n+ *\n+ * If the given notes_tree structure is not initialized, it will be auto-\n+ * initialized to the default value (see documentation for init_notes() above).\n+ * If the given notes_tree is NULL, the internal/default notes_tree will be\n+ * used instead.\n+ *\n+ * (raw != 0) gives the %N userformat; otherwise, the note message is given\n+ * for human consumption.\n+ */\n void format_display_notes(const struct object_id *object_oid,\n \t\t\t  struct strbuf *sb, const char *output_encoding, int raw)\n {\n-\tint i;\n-\tassert(display_notes_trees);\n-\tfor (i = 0; display_notes_trees[i]; i++)\n-\t\tformat_note(display_notes_trees[i], object_oid, sb,\n-\t\t\t    output_encoding, raw);\n+\tstruct format_display_notes_cb cb = {\n+\t\t.raw = raw,\n+\t\t.output = sb,\n+\t};\n+\n+\tfor_each_oid_note(object_oid, output_encoding, raw, format_note, &cb);\n }\n \n int copy_note(struct notes_tree *t,\ndiff --git a/notes.h b/notes.h\nindex c7aae85ea6..833af94fae 100644\n--- a/notes.h\n+++ b/notes.h\n@@ -309,6 +309,11 @@ void load_display_notes(struct display_notes_opt *opt);\n void format_display_notes(const struct object_id *object_oid,\n \t\t\t  struct strbuf *sb, const char *output_encoding, int raw);\n \n+typedef void (*each_oid_note_fn)(const char *ref, const char *msg, unsigned long msglen, void *cb_data);\n+\n+void for_each_oid_note(const struct object_id *object_oid,\n+\t\t       const char *output_encoding, int raw, each_oid_note_fn fn, void *cb_data);\n+\n /*\n  * Load the notes tree from each ref listed in 'refs'.  The output is\n  * an array of notes_tree*, terminated by a NULL.\n-- \n2.35.1.46.g38062e73e0\n\n"},{"id":"460407","messageId":"20220802075401.2393-1-vegard.nossum@oracle.com","threadId":"58256","inReplyTo":null,"subject":"[RFC PATCH 1/2] notes: support fetching notes from an external repo","fromName":"Vegard Nossum","fromEmail":"vegard.nossum@oracle.com","sentAt":"2022-08-02T07:54:00Z","receivedAt":"2022-08-02T07:54:33Z","isPatch":true,"sender":{"key":"vegard.nossum@oracle.com","avatar":"https://avatars.githubusercontent.com/u/24173?v=4"},"body":"Notes are currently always fetched from the current repo. However, in\ncertain situations you may want to keep notes in a separate repository\naltogether.\n\nIn my specific case, I am using cgit to display notes for repositories\nthat are owned by others but hosted on a shared machine, so I cannot\nreally add the notes directly to their repositories.\n\nThis patch makes it so that you can do:\n\n    const char *notes_repo_path = \"path/to/notes.git\";\n    const char *notes_ref = \"refs/notes/commits\";\n\n    struct repository notes_repo;\n    struct display_notes_opt notes_opt;\n\n    repo_init(&notes_repo, notes_repo_path, NULL);\n    add_to_alternates_memory(notes_repo.objects->odb->path);\n\n    init_display_notes(&notes_opt);\n    notes_opt.repo = &notes_repo;\n    notes_opt.use_default_notes = 0;\n\n    string_list_append(&notes_opt.extra_notes_refs, notes_ref);\n    load_display_notes(&notes_opt);\n\n...and then notes will be taken from the given ref in the external\nrepository.\n\nGiven that the functionality is not exposed through the command line,\nthere is currently no way to add regression tests for this.\n\nCc: Johan Herland <johan@herland.net>\nCc: Jason A. Donenfeld <Jason@zx2c4.com>\nCc: Christian Hesse <mail@eworm.de>\nSigned-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n---\n common-main.c |  2 ++\n notes.c       | 15 ++++++++++++---\n notes.h       |  3 +++\n refs.c        | 12 +++++++++---\n refs.h        |  2 ++\n 5 files changed, 28 insertions(+), 6 deletions(-)\n\ndiff --git a/common-main.c b/common-main.c\nindex c531372f3f..74b69a4632 100644\n--- a/common-main.c\n+++ b/common-main.c\n@@ -1,6 +1,7 @@\n #include \"cache.h\"\n #include \"exec-cmd.h\"\n #include \"attr.h\"\n+#include \"notes.h\"\n \n /*\n  * Many parts of Git have subprograms communicate via pipe, expect the\n@@ -43,6 +44,7 @@ int main(int argc, const char **argv)\n \tgit_setup_gettext();\n \n \tinitialize_the_repository();\n+\tinit_default_notes_repository();\n \n \tattr_start();\n \ndiff --git a/notes.c b/notes.c\nindex 7452e71cc8..90ec625192 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -73,11 +73,17 @@ struct non_note {\n #define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \\\n \t(memcmp(key_sha1, subtree_sha1, subtree_sha1[KEY_INDEX]))\n \n+struct repository *default_notes_repo;\n struct notes_tree default_notes_tree;\n \n static struct string_list display_notes_refs = STRING_LIST_INIT_NODUP;\n static struct notes_tree **display_notes_trees;\n \n+void init_default_notes_repository()\n+{\n+\tdefault_notes_repo = the_repository;\n+}\n+\n static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,\n \t\tstruct int_node *node, unsigned int n);\n \n@@ -940,10 +946,10 @@ void string_list_add_refs_by_glob(struct string_list *list, const char *glob)\n {\n \tassert(list->strdup_strings);\n \tif (has_glob_specials(glob)) {\n-\t\tfor_each_glob_ref(string_list_add_one_ref, glob, list);\n+\t\trepo_for_each_glob_ref_in(default_notes_repo, string_list_add_one_ref, glob, NULL, list);\n \t} else {\n \t\tstruct object_id oid;\n-\t\tif (get_oid(glob, &oid))\n+\t\tif (repo_get_oid(default_notes_repo, glob, &oid))\n \t\t\twarning(\"notes ref %s is invalid\", glob);\n \t\tif (!unsorted_string_list_has_string(list, glob))\n \t\t\tstring_list_append(list, glob);\n@@ -1019,7 +1025,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,\n \tt->dirty = 0;\n \n \tif (flags & NOTES_INIT_EMPTY || !notes_ref ||\n-\t    get_oid_treeish(notes_ref, &object_oid))\n+\t    repo_get_oid_treeish(default_notes_repo, notes_ref, &object_oid))\n \t\treturn;\n \tif (flags & NOTES_INIT_WRITABLE && read_ref(notes_ref, &object_oid))\n \t\tdie(\"Cannot use notes ref %s\", notes_ref);\n@@ -1088,6 +1094,9 @@ void load_display_notes(struct display_notes_opt *opt)\n \n \tassert(!display_notes_trees);\n \n+\tif (opt->repo)\n+\t\tdefault_notes_repo = opt->repo;\n+\n \tif (!opt || opt->use_default_notes > 0 ||\n \t    (opt->use_default_notes == -1 && !opt->extra_notes_refs.nr)) {\n \t\tstring_list_append(&display_notes_refs, default_notes_ref());\ndiff --git a/notes.h b/notes.h\nindex c1682c39a9..c7aae85ea6 100644\n--- a/notes.h\n+++ b/notes.h\n@@ -6,6 +6,8 @@\n struct object_id;\n struct strbuf;\n \n+void init_default_notes_repository();\n+\n /*\n  * Function type for combining two notes annotating the same object.\n  *\n@@ -256,6 +258,7 @@ void free_notes(struct notes_tree *t);\n struct string_list;\n \n struct display_notes_opt {\n+\tstruct repository *repo;\n \tint use_default_notes;\n \tstruct string_list extra_notes_refs;\n };\ndiff --git a/refs.c b/refs.c\nindex 90bcb27168..cf0dc85872 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -468,8 +468,8 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,\n \tstrbuf_release(&normalized_pattern);\n }\n \n-int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n-\tconst char *prefix, void *cb_data)\n+int repo_for_each_glob_ref_in(struct repository *r, each_ref_fn fn,\n+\tconst char *pattern, const char *prefix, void *cb_data)\n {\n \tstruct strbuf real_pattern = STRBUF_INIT;\n \tstruct ref_filter filter;\n@@ -492,12 +492,18 @@ int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n \tfilter.prefix = prefix;\n \tfilter.fn = fn;\n \tfilter.cb_data = cb_data;\n-\tret = for_each_ref(filter_refs, &filter);\n+\tret = refs_for_each_ref(get_main_ref_store(r), filter_refs, &filter);\n \n \tstrbuf_release(&real_pattern);\n \treturn ret;\n }\n \n+int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n+\tconst char *prefix, void *cb_data)\n+{\n+\treturn repo_for_each_glob_ref_in(the_repository, fn, pattern, prefix, cb_data);\n+}\n+\n int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)\n {\n \treturn for_each_glob_ref_in(fn, pattern, NULL, cb_data);\ndiff --git a/refs.h b/refs.h\nindex 47cb9edbaa..1375c8531f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -366,6 +366,8 @@ int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_dat\n /* iterates all refs that match the specified glob pattern. */\n int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);\n \n+int repo_for_each_glob_ref_in(struct repository *r, each_ref_fn fn, const char *pattern,\n+\t\t\t const char *prefix, void *cb_data);\n int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n \t\t\t const char *prefix, void *cb_data);\n \n-- \n2.35.1.46.g38062e73e0\n\n"},{"id":"460444","messageId":"xmqqczdiirh8.fsf@gitster.g","threadId":"58256","inReplyTo":"20220802075401.2393-1-vegard.nossum@oracle.com","subject":"Re: [RFC PATCH 1/2] notes: support fetching notes from an external repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-02T15:40:19Z","receivedAt":"2022-08-02T15:40:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vegard Nossum <vegard.nossum@oracle.com> writes:\n\n> Notes are currently always fetched from the current repo. However, in\n> certain situations you may want to keep notes in a separate repository\n> altogether.\n>\n> In my specific case, I am using cgit to display notes for repositories\n> that are owned by others but hosted on a shared machine, so I cannot\n> really add the notes directly to their repositories.\n\nMy gut reaction is that I am not interested at all in the above\napproach, even though the problem you are trying to solve is\ninteresting.  Mostly because notes are not the only decorations your\nusers may want.  What if you want to \"log --decorate\" their\nrepository contents with your own tags that annotate their commits?\nA notes-only approach to mix repositories is way too narrow.\n\nA usable alternative _might_ be to introduce a way to \"borrow\" refs\nand objects from a different repository as if you cloned from and\ncontinuously fetching from them.  We already have a mechanism to\nborrow objects from another repository in the form of \"alternate\nobject database\" that lets us pretend objects in their repository\nare locally available.  We can invent a similar mechanism that lets\nany of their ref as if it were our local ref, e.g. their \"main\"\nbranch at their refs/heads/main might appear to exist at our\nrefs/borrowed/X/heads/main.  \n\nOnce the mechanism for doing so is in place, setting up such a\nparasite repository might be\n\n    $ git clone --local-parasite=X /path/to/theirs mine\n\nwhich would create an empty repository 'mine' that uses\n/path/to/theirs/.git/objects as one of its alternate object store,\nand their refs are borrowed under our refs/borrowed/X/.\n\nThen you can tell your cgit to show refs/borrowed/X/{heads,tags}\nhierarchies as if they are the branches and tags, and use your own\nrefs/notes/ hiearchy to store whatever notes they do not let you\nstore in theirs.\n"},{"id":"460539","messageId":"69fd9dcf-7769-6c5c-ca0e-ea61e6d616d9@oracle.com","threadId":"58256","inReplyTo":"xmqqczdiirh8.fsf@gitster.g","subject":"Re: [RFC PATCH 1/2] notes: support fetching notes from an external repo","fromName":"Vegard Nossum","fromEmail":"vegard.nossum@oracle.com","sentAt":"2022-08-03T08:09:03Z","receivedAt":"2022-08-03T08:09:29Z","isPatch":true,"sender":{"key":"vegard.nossum@oracle.com","avatar":"https://avatars.githubusercontent.com/u/24173?v=4"},"body":"\nOn 8/2/22 17:40, Junio C Hamano wrote:\n> Vegard Nossum <vegard.nossum@oracle.com> writes:\n> \n>> Notes are currently always fetched from the current repo. However, in\n>> certain situations you may want to keep notes in a separate repository\n>> altogether.\n>>\n>> In my specific case, I am using cgit to display notes for repositories\n>> that are owned by others but hosted on a shared machine, so I cannot\n>> really add the notes directly to their repositories.\n> \n> My gut reaction is that I am not interested at all in the above\n> approach, even though the problem you are trying to solve is\n> interesting.  Mostly because notes are not the only decorations your\n> users may want.  What if you want to \"log --decorate\" their\n> repository contents with your own tags that annotate their commits?\n> A notes-only approach to mix repositories is way too narrow.\n> \n> A usable alternative _might_ be to introduce a way to \"borrow\" refs\n> and objects from a different repository as if you cloned from and\n> continuously fetching from them.  We already have a mechanism to\n> borrow objects from another repository in the form of \"alternate\n> object database\" that lets us pretend objects in their repository\n> are locally available.  We can invent a similar mechanism that lets\n> any of their ref as if it were our local ref, e.g. their \"main\"\n> branch at their refs/heads/main might appear to exist at our\n> refs/borrowed/X/heads/main.  \n\nHi Junio,\n\nThanks for the reply.\n\nTo be clear, are you saying there is no way you would ever take my\npatches in their current form, even though they only rearrange internal\nworkings (and have no other user-observable effects) to solve a problem\nI am currently facing?\n\nThe thing is, I personally have no use for displaying refs borrowed from\nanother repository at this time, and I'm not sure I have either the time\nor the ability to provide what you are asking for.\n\nI don't think my patches preclude adding \"borrowed refs\" as a feature at\na later time, so can we not do that when somebody actually has a use for it?\n\nJust to provide a bit more background: These two patches are just the\nfirst two in a bigger project to make extensive use of git notes to\nprovide added value to the whole Linux kernel community -- in other\nwords, this is not just for myself, I am trying here to upstream our\ninternal patches for the benefit of everybody. I have cgit patches as\nwell (but I'm waiting to submit them until git can support them) and\nhundreds of thousands of notes annotating Linux kernel commits with\nuseful information.\n\nRespectfully,\n\n\nVegard\n"},{"id":"460579","messageId":"xmqq35ed7ybl.fsf@gitster.g","threadId":"58256","inReplyTo":"69fd9dcf-7769-6c5c-ca0e-ea61e6d616d9@oracle.com","subject":"Re: [RFC PATCH 1/2] notes: support fetching notes from an external repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-03T22:32:30Z","receivedAt":"2022-08-03T22:32:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vegard Nossum <vegard.nossum@oracle.com> writes:\n\n>> My gut reaction is that I am not interested at all in the above\n>> approach, even though the problem you are trying to solve is\n>> interesting.  Mostly because notes are not the only decorations your\n>> users may want.  What if you want to \"log --decorate\" their\n>> repository contents with your own tags that annotate their commits?\n>> A notes-only approach to mix repositories is way too narrow.\n> ...\n> To be clear, are you saying there is no way you would ever take my\n> patches in their current form,\n\nCorrect.\n\n> Just to provide a bit more background: These two patches are just the\n> first two in a bigger project to make extensive use of git notes to\n> provide added value to the whole Linux kernel community ...\n\nI already said that the problem being solved is interesting.  The\nfact that a solution aims to address a problem worth solving does\nnot diminish the need for the solution to be sensibly designed,\nand I again already said that an approach to special case notes ref\nspecially is unwanted.\n\nThanks.\n\n\n"},{"id":"462198","messageId":"96b04fc0-eadc-af01-502a-e5236a393ac4@iee.email","threadId":"58256","inReplyTo":"20220802075401.2393-1-vegard.nossum@oracle.com","subject":"Re: [RFC PATCH 1/2] notes: support fetching notes from an external repo","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-08-30T14:17:03Z","receivedAt":"2022-08-30T14:17:14Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Sorry for late comment.\n\nOn 02/08/2022 08:54, Vegard Nossum wrote:\n> Notes are currently always fetched from the current repo. However, in\n> certain situations you may want to keep notes in a separate repository\n> altogether.\n>\n> In my specific case, I am using cgit to display notes for repositories\n> that are owned by others but hosted on a shared machine, so I cannot\n> really add the notes directly to their repositories.\n>\n> This patch makes it so that you can do:\n>\n>     const char *notes_repo_path = \"path/to/notes.git\";\n>     const char *notes_ref = \"refs/notes/commits\";\n>\n>     struct repository notes_repo;\n>     struct display_notes_opt notes_opt;\n>\n>     repo_init(&notes_repo, notes_repo_path, NULL);\n>     add_to_alternates_memory(notes_repo.objects->odb->path);\n>\n>     init_display_notes(&notes_opt);\n>     notes_opt.repo = &notes_repo;\n>     notes_opt.use_default_notes = 0;\n>\n>     string_list_append(&notes_opt.extra_notes_refs, notes_ref);\n>     load_display_notes(&notes_opt);\n>\n> ...and then notes will be taken from the given ref in the external\n> repository.\n>\n> Given that the functionality is not exposed through the command line,\n> there is currently no way to add regression tests for this.\n>\n> Cc: Johan Herland <johan@herland.net>\n> Cc: Jason A. Donenfeld <Jason@zx2c4.com>\n> Cc: Christian Hesse <mail@eworm.de>\n> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n> ---\n>  common-main.c |  2 ++\n>  notes.c       | 15 ++++++++++++---\n>  notes.h       |  3 +++\n>  refs.c        | 12 +++++++++---\n>  refs.h        |  2 ++\n>  5 files changed, 28 insertions(+), 6 deletions(-)\n\nWhere's the documentation? Without a clarity of purpose and usage then,\nfor me, it doesn't fly.\n\nI feel that underlying this there may something that's interesting, but\nwithout the 'SPIN' narrative (situation, problem, implication, and\nneed-payoff) I'm not sure what it's trying to do from a broad user\nperspective. (Spin is just one approach to 'selling' the patches;-)\n\nI'd agree that Notes are 'odd' in that they are out of sequence relative\nto commits, and may not be common between users viewing the same repo.\nI'd like to understand the issues in a wider context.\n--\nPhilip\n\n>\n> diff --git a/common-main.c b/common-main.c\n> index c531372f3f..74b69a4632 100644\n> --- a/common-main.c\n> +++ b/common-main.c\n> @@ -1,6 +1,7 @@\n>  #include \"cache.h\"\n>  #include \"exec-cmd.h\"\n>  #include \"attr.h\"\n> +#include \"notes.h\"\n>  \n>  /*\n>   * Many parts of Git have subprograms communicate via pipe, expect the\n> @@ -43,6 +44,7 @@ int main(int argc, const char **argv)\n>  \tgit_setup_gettext();\n>  \n>  \tinitialize_the_repository();\n> +\tinit_default_notes_repository();\n>  \n>  \tattr_start();\n>  \n> diff --git a/notes.c b/notes.c\n> index 7452e71cc8..90ec625192 100644\n> --- a/notes.c\n> +++ b/notes.c\n> @@ -73,11 +73,17 @@ struct non_note {\n>  #define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \\\n>  \t(memcmp(key_sha1, subtree_sha1, subtree_sha1[KEY_INDEX]))\n>  \n> +struct repository *default_notes_repo;\n>  struct notes_tree default_notes_tree;\n>  \n>  static struct string_list display_notes_refs = STRING_LIST_INIT_NODUP;\n>  static struct notes_tree **display_notes_trees;\n>  \n> +void init_default_notes_repository()\n> +{\n> +\tdefault_notes_repo = the_repository;\n> +}\n> +\n>  static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,\n>  \t\tstruct int_node *node, unsigned int n);\n>  \n> @@ -940,10 +946,10 @@ void string_list_add_refs_by_glob(struct string_list *list, const char *glob)\n>  {\n>  \tassert(list->strdup_strings);\n>  \tif (has_glob_specials(glob)) {\n> -\t\tfor_each_glob_ref(string_list_add_one_ref, glob, list);\n> +\t\trepo_for_each_glob_ref_in(default_notes_repo, string_list_add_one_ref, glob, NULL, list);\n>  \t} else {\n>  \t\tstruct object_id oid;\n> -\t\tif (get_oid(glob, &oid))\n> +\t\tif (repo_get_oid(default_notes_repo, glob, &oid))\n>  \t\t\twarning(\"notes ref %s is invalid\", glob);\n>  \t\tif (!unsorted_string_list_has_string(list, glob))\n>  \t\t\tstring_list_append(list, glob);\n> @@ -1019,7 +1025,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,\n>  \tt->dirty = 0;\n>  \n>  \tif (flags & NOTES_INIT_EMPTY || !notes_ref ||\n> -\t    get_oid_treeish(notes_ref, &object_oid))\n> +\t    repo_get_oid_treeish(default_notes_repo, notes_ref, &object_oid))\n>  \t\treturn;\n>  \tif (flags & NOTES_INIT_WRITABLE && read_ref(notes_ref, &object_oid))\n>  \t\tdie(\"Cannot use notes ref %s\", notes_ref);\n> @@ -1088,6 +1094,9 @@ void load_display_notes(struct display_notes_opt *opt)\n>  \n>  \tassert(!display_notes_trees);\n>  \n> +\tif (opt->repo)\n> +\t\tdefault_notes_repo = opt->repo;\n> +\n>  \tif (!opt || opt->use_default_notes > 0 ||\n>  \t    (opt->use_default_notes == -1 && !opt->extra_notes_refs.nr)) {\n>  \t\tstring_list_append(&display_notes_refs, default_notes_ref());\n> diff --git a/notes.h b/notes.h\n> index c1682c39a9..c7aae85ea6 100644\n> --- a/notes.h\n> +++ b/notes.h\n> @@ -6,6 +6,8 @@\n>  struct object_id;\n>  struct strbuf;\n>  \n> +void init_default_notes_repository();\n> +\n>  /*\n>   * Function type for combining two notes annotating the same object.\n>   *\n> @@ -256,6 +258,7 @@ void free_notes(struct notes_tree *t);\n>  struct string_list;\n>  \n>  struct display_notes_opt {\n> +\tstruct repository *repo;\n>  \tint use_default_notes;\n>  \tstruct string_list extra_notes_refs;\n>  };\n> diff --git a/refs.c b/refs.c\n> index 90bcb27168..cf0dc85872 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -468,8 +468,8 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,\n>  \tstrbuf_release(&normalized_pattern);\n>  }\n>  \n> -int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n> -\tconst char *prefix, void *cb_data)\n> +int repo_for_each_glob_ref_in(struct repository *r, each_ref_fn fn,\n> +\tconst char *pattern, const char *prefix, void *cb_data)\n>  {\n>  \tstruct strbuf real_pattern = STRBUF_INIT;\n>  \tstruct ref_filter filter;\n> @@ -492,12 +492,18 @@ int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n>  \tfilter.prefix = prefix;\n>  \tfilter.fn = fn;\n>  \tfilter.cb_data = cb_data;\n> -\tret = for_each_ref(filter_refs, &filter);\n> +\tret = refs_for_each_ref(get_main_ref_store(r), filter_refs, &filter);\n>  \n>  \tstrbuf_release(&real_pattern);\n>  \treturn ret;\n>  }\n>  \n> +int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n> +\tconst char *prefix, void *cb_data)\n> +{\n> +\treturn repo_for_each_glob_ref_in(the_repository, fn, pattern, prefix, cb_data);\n> +}\n> +\n>  int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)\n>  {\n>  \treturn for_each_glob_ref_in(fn, pattern, NULL, cb_data);\n> diff --git a/refs.h b/refs.h\n> index 47cb9edbaa..1375c8531f 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -366,6 +366,8 @@ int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_dat\n>  /* iterates all refs that match the specified glob pattern. */\n>  int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);\n>  \n> +int repo_for_each_glob_ref_in(struct repository *r, each_ref_fn fn, const char *pattern,\n> +\t\t\t const char *prefix, void *cb_data);\n>  int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,\n>  \t\t\t const char *prefix, void *cb_data);\n>  \n\n"},{"id":"465086","messageId":"66d96a5c-ce6f-9241-a766-f396674798c9@oracle.com","threadId":"58256","inReplyTo":"96b04fc0-eadc-af01-502a-e5236a393ac4@iee.email","subject":"Re: [RFC PATCH 1/2] notes: support fetching notes from an external repo","fromName":"Vegard Nossum","fromEmail":"vegard.nossum@oracle.com","sentAt":"2022-10-17T13:14:10Z","receivedAt":"2022-10-17T13:15:48Z","isPatch":true,"sender":{"key":"vegard.nossum@oracle.com","avatar":"https://avatars.githubusercontent.com/u/24173?v=4"},"body":"\nOn 8/30/22 16:17, Philip Oakley wrote:\n> Sorry for late comment.\n\nAnd sorry for late response! I didn't receive your email for some\nreason, but I noticed it in the list archives.\n\n> On 02/08/2022 08:54, Vegard Nossum wrote:\n>> Notes are currently always fetched from the current repo. However, in\n>> certain situations you may want to keep notes in a separate repository\n>> altogether.\n>>\n>> In my specific case, I am using cgit to display notes for repositories\n>> that are owned by others but hosted on a shared machine, so I cannot\n>> really add the notes directly to their repositories.\n>>\n>> This patch makes it so that you can do:\n>>\n>>      const char *notes_repo_path = \"path/to/notes.git\";\n>>      const char *notes_ref = \"refs/notes/commits\";\n>>\n>>      struct repository notes_repo;\n>>      struct display_notes_opt notes_opt;\n>>\n>>      repo_init(&notes_repo, notes_repo_path, NULL);\n>>      add_to_alternates_memory(notes_repo.objects->odb->path);\n>>\n>>      init_display_notes(&notes_opt);\n>>      notes_opt.repo = &notes_repo;\n>>      notes_opt.use_default_notes = 0;\n>>\n>>      string_list_append(&notes_opt.extra_notes_refs, notes_ref);\n>>      load_display_notes(&notes_opt);\n>>\n>> ...and then notes will be taken from the given ref in the external\n>> repository.\n>>\n>> Given that the functionality is not exposed through the command line,\n>> there is currently no way to add regression tests for this.\n>>\n>> Cc: Johan Herland <johan@herland.net>\n>> Cc: Jason A. Donenfeld <Jason@zx2c4.com>\n>> Cc: Christian Hesse <mail@eworm.de>\n>> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n>> ---\n>>   common-main.c |  2 ++\n>>   notes.c       | 15 ++++++++++++---\n>>   notes.h       |  3 +++\n>>   refs.c        | 12 +++++++++---\n>>   refs.h        |  2 ++\n>>   5 files changed, 28 insertions(+), 6 deletions(-)\n> \n> Where's the documentation? Without a clarity of purpose and usage then,\n> for me, it doesn't fly.\n> \n> I feel that underlying this there may something that's interesting, but\n> without the 'SPIN' narrative (situation, problem, implication, and\n> need-payoff) I'm not sure what it's trying to do from a broad user\n> perspective. (Spin is just one approach to 'selling' the patches;-)\n> \n> I'd agree that Notes are 'odd' in that they are out of sequence relative\n> to commits, and may not be common between users viewing the same repo.\n> I'd like to understand the issues in a wider context.\n> --\n> Philip\n\nPerhaps the best way to showcase this is with a screenshot of what we're\ntrying to upstream:\n\nhttps://vegard.github.io/cgit/6399f1fae4ec.png\n\nSince git commits cannot be changed without rewriting history, git notes\nis the mechanism by which we can attach new information to existing\ncommits. We're internally using these notes for cross-referencing\ninformation like references to subsequent fixes, backports in other\ntrees, mailing list discussions, etc.\n\nThere is also a bit more information in my cgit patch submission from\ntoday: https://lists.zx2c4.com/pipermail/cgit/2022-October/004764.html\n\nMy \"problem\" is that there are many moving parts to this, and the two\ngit.git patches sit at the top of the dependency chain:\n\n1. these git patches\n2. the cgit patches\n3. the Linux kernel-specific notes generation scripts/logic\n4. the Linux kernel notes themselves\n5. displaying notes on kernel.org\n\nAlmost all of these steps involve different people with different\nstandards, different motivations, different priorities.\n\nAs I wrote earlier, I am trying to be a good citizen and upstream as\nmuch of this as I can. But it's hard to justify what Junio asked for:\nscrapping my current patches (which we are currently using...) in favour\nof a complete rewrite with more functionality that does not buy us\nanything from my point of view.\n\nDoes this clarify things?\n\nI think my patches are a good cleanup regardless of motivation and\neverything was fairly well documented in the changelogs, so I'm\nsurprised to see skepticism in the git community.\n\n\nVegard\n"},{"id":"465242","messageId":"1d6d6047-6993-d4fd-c506-6c9be9a789dd@iee.email","threadId":"58256","inReplyTo":"66d96a5c-ce6f-9241-a766-f396674798c9@oracle.com","subject":"Re: [RFC PATCH 1/2] notes: support fetching notes from an external repo","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-10-19T09:15:19Z","receivedAt":"2022-10-19T12:46:47Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Vegard,\n\nOn 17/10/2022 14:14, Vegard Nossum wrote:\n>\n> On 8/30/22 16:17, Philip Oakley wrote:\n>> Sorry for late comment.\n>\n> And sorry for late response! I didn't receive your email for some\n> reason, but I noticed it in the list archives.\n>\n>> On 02/08/2022 08:54, Vegard Nossum wrote:\n>>> Notes are currently always fetched from the current repo. However, in\n>>> certain situations you may want to keep notes in a separate repository\n>>> altogether.\n>>>\n>>> In my specific case, I am using cgit to display notes for repositories\n>>> that are owned by others but hosted on a shared machine, so I cannot\n>>> really add the notes directly to their repositories.\n>>>\n>>> This patch makes it so that you can do:\n>>>\n>>>      const char *notes_repo_path = \"path/to/notes.git\";\n>>>      const char *notes_ref = \"refs/notes/commits\";\n>>>\n>>>      struct repository notes_repo;\n>>>      struct display_notes_opt notes_opt;\n>>>\n>>>      repo_init(&notes_repo, notes_repo_path, NULL);\n>>>      add_to_alternates_memory(notes_repo.objects->odb->path);\n>>>\n>>>      init_display_notes(&notes_opt);\n>>>      notes_opt.repo = &notes_repo;\n>>>      notes_opt.use_default_notes = 0;\n>>>\n>>>      string_list_append(&notes_opt.extra_notes_refs, notes_ref);\n>>>      load_display_notes(&notes_opt);\n>>>\n>>> ...and then notes will be taken from the given ref in the external\n>>> repository.\n>>>\n>>> Given that the functionality is not exposed through the command line,\n>>> there is currently no way to add regression tests for this.\n>>>\n>>> Cc: Johan Herland <johan@herland.net>\n>>> Cc: Jason A. Donenfeld <Jason@zx2c4.com>\n>>> Cc: Christian Hesse <mail@eworm.de>\n>>> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n>>> ---\n>>>   common-main.c |  2 ++\n>>>   notes.c       | 15 ++++++++++++---\n>>>   notes.h       |  3 +++\n>>>   refs.c        | 12 +++++++++---\n>>>   refs.h        |  2 ++\n>>>   5 files changed, 28 insertions(+), 6 deletions(-)\n>>\n>> Where's the documentation? Without a clarity of purpose and usage then,\n>> for me, it doesn't fly.\n>>\n>> I feel that underlying this there may something that's interesting, but\n>> without the 'SPIN' narrative (situation, problem, implication, and\n>> need-payoff) I'm not sure what it's trying to do from a broad user\n>> perspective. (Spin is just one approach to 'selling' the patches;-)\n>>\n>> I'd agree that Notes are 'odd' in that they are out of sequence relative\n>> to commits, and may not be common between users viewing the same repo.\n>> I'd like to understand the issues in a wider context.\n>> -- \n>> Philip\n>\n> Perhaps the best way to showcase this is with a screenshot of what we're\n> trying to upstream:\n>\n> https://vegard.github.io/cgit/6399f1fae4ec.png\n>\n> Since git commits cannot be changed without rewriting history, git notes\n> is the mechanism by which we can attach new information to existing\n> commits. We're internally using these notes for cross-referencing\n> information like references to subsequent fixes, backports in other\n> trees, mailing list discussions, etc.\n>\n> There is also a bit more information in my cgit patch submission from\n> today: https://lists.zx2c4.com/pipermail/cgit/2022-October/004764.html\n>\n> My \"problem\" is that there are many moving parts to this, and the two\n> git.git patches sit at the top of the dependency chain:\n>\n> 1. these git patches\n> 2. the cgit patches\n> 3. the Linux kernel-specific notes generation scripts/logic\n> 4. the Linux kernel notes themselves\n> 5. displaying notes on kernel.org\n>\n> Almost all of these steps involve different people with different\n> standards, different motivations, different priorities.\n>\n> As I wrote earlier, I am trying to be a good citizen and upstream as\n> much of this as I can. But it's hard to justify what Junio asked for:\n> scrapping my current patches (which we are currently using...) in favour\n> of a complete rewrite with more functionality that does not buy us\n> anything from my point of view.\n>\n> Does this clarify things?\n\nYes, and No;\nEven without Junio's desire for a broader functionality of the\n`alternate object database` (is it that, or an ext repo?), I still felt\nthat given the new and improved functionality, it would need some extra\ntext to go into the documentation and man pages, along with a short\nabstract to go into the release notes. Somehow the prospective users\n(e.g. me) would need to be told - I.e. be able to read from the man\npages what to expect.\n\nThe commit messages also didn't really bring out where the benefit would\nbe seen i.e. the cgit display (as per your screen shot). Also some\nannotation of the screen shot with an arrow pointing to what was 'new'\ncould help.\n\nAlso you didn't really explain the point you make above about the\n\"shared machine\", which cuts across the normal \"personal machine\" view\nof the 'distributed' in DVCS.\n\n>\n> I think my patches are a good cleanup regardless of motivation and\n> everything was fairly well documented in the changelogs, so I'm\n> surprised to see skepticism in the git community.\n\nIn a sense, I hear your frustration. It does feel common that that every\nknife has to be converted to scissors (two knives working together) or a\nmulti-tool Swiss Army knife, and in some cases loosing the original\n'obviousness' of a simple thing done well.\n\nA first step may be to write out \"what would the man pages say\" that\nexplains how the the `alternate object database` is used and set up, and\nthen maybe look at whether Junio's example, to see if you have explained\nthis new capability well enough.\n\nI hope that helps clarify my original comments.\n\nPhilip\n"}]}