Re: [Outreachy PATCH v2 1/2] notes-utils: use the passed repository for notes refs

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> Okhuomon Ajayi <okhuomonajayi54@gmail.com> writes:
>
>> Keep USE_THE_REPOSITORY_VARIABLE because
>> init_copy_notes_for_rewrite() still uses the_repository.

Unfortunately, this is a highly misleading comment to leave here.

The next person would be misled into thinking that it would be just
a matter of giving a "struct repository *r" argument to that
function to lift the USE_THE_REPOSITORY_VARIABLE declaration from
there.

Looking at what <notes-utils.h> exports, the file has quite some way
to go to be free from the_repository.

 * create_notes_commit() takes "struct repository *" to pretend as
   if it can work with any repository, but the first function it
   calls is write_notes_tree() that eventually calls into
   odb_write_object() with hardcoded the_repository->objects.

 * commit_notes() calls the above create_notes_commit() and shares
   the same problem.

 * init_copy_notes_for_rewrite(), in addition to having its own
   reference to the_repository, calls load_notes_trees() that makes
   calls to init_notes() that works only on the_repository.

 * copy_note_for_rewrite() calls copy_note() which calls add_note(),
   which eventually calls into load_subtree() that can only work
   with the_repository.

In short, I think adding "struct repository *" to anything in
<notes-utils.h> is not yet a productive exercise at this point,
before cleaning up what is in the underlying things exported from
<notes.h>.

This is a tangent but there are precedents to mark the functions
that still work only on the_repository or the_hash_algo:

    $ git grep 'ifdef *USE_THE' \*.h
    environment.h:# ifdef USE_THE_REPOSITORY_VARIABLE
    hash.h:#ifdef USE_THE_REPOSITORY_VARIABLE
    hex.h:#ifdef USE_THE_REPOSITORY_VARIABLE
    path.h:# ifdef USE_THE_REPOSITORY_VARIABLE
    repository.h:#ifdef USE_THE_REPOSITORY_VARIABLE

Perhaps we need similar protection to forbid use of these functions
that still assume the_repository is available in this file?

 notes-utils.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git c/notes-utils.h w/notes-utils.h
index c54b1fe141..1503991cf6 100644
--- c/notes-utils.h
+++ w/notes-utils.h
@@ -7,6 +7,9 @@ struct commit_list;
 struct object_id;
 struct repository;
 
+/* most of the things in notes.h still only work with the_repository */
+# ifdef USE_THE_REPOSITORY_VARIABLE
+
 /*
  * Create new notes commit from the given notes tree
  *
@@ -52,4 +55,5 @@ void finish_copy_notes_for_rewrite(struct repository *r,
 				   struct notes_rewrite_cfg *c,
 				   const char *msg);
 
+# endif
 #endif