Sorry for late comment.
On 02/08/2022 08:54, Vegard Nossum wrote:
> Notes are currently always fetched from the current repo. However, in
> certain situations you may want to keep notes in a separate repository
> altogether.
>
> In my specific case, I am using cgit to display notes for repositories
> that are owned by others but hosted on a shared machine, so I cannot
> really add the notes directly to their repositories.
>
> This patch makes it so that you can do:
>
> const char *notes_repo_path = "path/to/notes.git";
> const char *notes_ref = "refs/notes/commits";
>
> struct repository notes_repo;
> struct display_notes_opt notes_opt;
>
> repo_init(¬es_repo, notes_repo_path, NULL);
> add_to_alternates_memory(notes_repo.objects->odb->path);
>
> init_display_notes(¬es_opt);
> notes_opt.repo = ¬es_repo;
> notes_opt.use_default_notes = 0;
>
> string_list_append(¬es_opt.extra_notes_refs, notes_ref);
> load_display_notes(¬es_opt);
>
> ...and then notes will be taken from the given ref in the external
> repository.
>
> Given that the functionality is not exposed through the command line,
> there is currently no way to add regression tests for this.
>
> Cc: Johan Herland <johan@xxxxxxxxxxx>
> Cc: Jason A. Donenfeld <Jason@xxxxxxxxx>
> Cc: Christian Hesse <mail@xxxxxxxx>
> Signed-off-by: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
> ---
> common-main.c | 2 ++
> notes.c | 15 ++++++++++++---
> notes.h | 3 +++
> refs.c | 12 +++++++++---
> refs.h | 2 ++
> 5 files changed, 28 insertions(+), 6 deletions(-)
Where's the documentation? Without a clarity of purpose and usage then,
for me, it doesn't fly.
I feel that underlying this there may something that's interesting, but
without the 'SPIN' narrative (situation, problem, implication, and
need-payoff) I'm not sure what it's trying to do from a broad user
perspective. (Spin is just one approach to 'selling' the patches;-)
I'd agree that Notes are 'odd' in that they are out of sequence relative
to commits, and may not be common between users viewing the same repo.
I'd like to understand the issues in a wider context.
--
Philip
>
> diff --git a/common-main.c b/common-main.c
> index c531372f3f..74b69a4632 100644
> --- a/common-main.c
> +++ b/common-main.c
> @@ -1,6 +1,7 @@
> #include "cache.h"
> #include "exec-cmd.h"
> #include "attr.h"
> +#include "notes.h"
>
> /*
> * Many parts of Git have subprograms communicate via pipe, expect the
> @@ -43,6 +44,7 @@ int main(int argc, const char **argv)
> git_setup_gettext();
>
> initialize_the_repository();
> + init_default_notes_repository();
>
> attr_start();
>
> diff --git a/notes.c b/notes.c
> index 7452e71cc8..90ec625192 100644
> --- a/notes.c
> +++ b/notes.c
> @@ -73,11 +73,17 @@ struct non_note {
> #define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
> (memcmp(key_sha1, subtree_sha1, subtree_sha1[KEY_INDEX]))
>
> +struct repository *default_notes_repo;
> struct notes_tree default_notes_tree;
>
> static struct string_list display_notes_refs = STRING_LIST_INIT_NODUP;
> static struct notes_tree **display_notes_trees;
>
> +void init_default_notes_repository()
> +{
> + default_notes_repo = the_repository;
> +}
> +
> static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
> struct int_node *node, unsigned int n);
>
> @@ -940,10 +946,10 @@ void string_list_add_refs_by_glob(struct string_list *list, const char *glob)
> {
> assert(list->strdup_strings);
> if (has_glob_specials(glob)) {
> - for_each_glob_ref(string_list_add_one_ref, glob, list);
> + repo_for_each_glob_ref_in(default_notes_repo, string_list_add_one_ref, glob, NULL, list);
> } else {
> struct object_id oid;
> - if (get_oid(glob, &oid))
> + if (repo_get_oid(default_notes_repo, glob, &oid))
> warning("notes ref %s is invalid", glob);
> if (!unsorted_string_list_has_string(list, glob))
> string_list_append(list, glob);
> @@ -1019,7 +1025,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
> t->dirty = 0;
>
> if (flags & NOTES_INIT_EMPTY || !notes_ref ||
> - get_oid_treeish(notes_ref, &object_oid))
> + repo_get_oid_treeish(default_notes_repo, notes_ref, &object_oid))
> return;
> if (flags & NOTES_INIT_WRITABLE && read_ref(notes_ref, &object_oid))
> die("Cannot use notes ref %s", notes_ref);
> @@ -1088,6 +1094,9 @@ void load_display_notes(struct display_notes_opt *opt)
>
> assert(!display_notes_trees);
>
> + if (opt->repo)
> + default_notes_repo = opt->repo;
> +
> if (!opt || opt->use_default_notes > 0 ||
> (opt->use_default_notes == -1 && !opt->extra_notes_refs.nr)) {
> string_list_append(&display_notes_refs, default_notes_ref());
> diff --git a/notes.h b/notes.h
> index c1682c39a9..c7aae85ea6 100644
> --- a/notes.h
> +++ b/notes.h
> @@ -6,6 +6,8 @@
> struct object_id;
> struct strbuf;
>
> +void init_default_notes_repository();
> +
> /*
> * Function type for combining two notes annotating the same object.
> *
> @@ -256,6 +258,7 @@ void free_notes(struct notes_tree *t);
> struct string_list;
>
> struct display_notes_opt {
> + struct repository *repo;
> int use_default_notes;
> struct string_list extra_notes_refs;
> };
> diff --git a/refs.c b/refs.c
> index 90bcb27168..cf0dc85872 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -468,8 +468,8 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,
> strbuf_release(&normalized_pattern);
> }
>
> -int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
> - const char *prefix, void *cb_data)
> +int repo_for_each_glob_ref_in(struct repository *r, each_ref_fn fn,
> + const char *pattern, const char *prefix, void *cb_data)
> {
> struct strbuf real_pattern = STRBUF_INIT;
> struct ref_filter filter;
> @@ -492,12 +492,18 @@ int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
> filter.prefix = prefix;
> filter.fn = fn;
> filter.cb_data = cb_data;
> - ret = for_each_ref(filter_refs, &filter);
> + ret = refs_for_each_ref(get_main_ref_store(r), filter_refs, &filter);
>
> strbuf_release(&real_pattern);
> return ret;
> }
>
> +int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
> + const char *prefix, void *cb_data)
> +{
> + return repo_for_each_glob_ref_in(the_repository, fn, pattern, prefix, cb_data);
> +}
> +
> int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)
> {
> return for_each_glob_ref_in(fn, pattern, NULL, cb_data);
> diff --git a/refs.h b/refs.h
> index 47cb9edbaa..1375c8531f 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -366,6 +366,8 @@ int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_dat
> /* iterates all refs that match the specified glob pattern. */
> int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
>
> +int repo_for_each_glob_ref_in(struct repository *r, each_ref_fn fn, const char *pattern,
> + const char *prefix, void *cb_data);
> int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
> const char *prefix, void *cb_data);
>
