On Thu, Apr 30, 2020 at 01:55:11PM -0700, Junio C Hamano wrote:

Taylor Blau [off-list ref] writes:

quoted

-static int commit_graft_pos(struct repository *r, const unsigned char *sha1)
+int commit_graft_pos(struct repository *r, const unsigned char *sha1)
 {
 	return sha1_pos(sha1, r->parsed_objects->grafts,
 			r->parsed_objects->grafts_nr,

diff --git a/commit.h b/commit.h
index ab91d21131..eb42e8b6d2 100644
--- a/commit.h
+++ b/commit.h

@@ -236,6 +236,7 @@ struct commit_graft {
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);

 struct commit_graft *read_graft_line(struct strbuf *line);
+int commit_graft_pos(struct repository *r, const unsigned char *sha1);

In an earlier exchange, I saw this:

quoted

quoted

- could include a comment saying that it's an index into
  r->parsed_objects->grafts

This and the below are both good ideas to me. I prefer this one, since
we'd have to duplicate yet another static function
('commit_graft_sha1_access()' directly above) that is called by this
one.

quoted

- I'm usually loathe to suggest unnecessary duplication of code, but
  it might make sense to duplicate the function into shallow.c.  Or
  even to inline it there (in the single call site, that ends up
  being pretty readable).

I am not at all offended by duplication of code where it makes sense to
do so, but having to duplicate two functions seems like we'd be better
off simply documenting the function in commit.h.

and I think I agree with that direction.  Forgot to add those
comments?

Thanks for remembering. I did apply this locally, but must've dropped it
on the floor during a rebase. In any case, please swap in this patch
instead:

-- >8 --

Subject: [PATCH] commit: make 'commit_graft_pos' non-static

In the next patch, some functions will be moved from 'commit.c' to have
prototypes in a new 'shallow.h' and their implementations in
'shallow.c'.

Three functions in 'commit.c' use 'commit_graft_pos()' (they are
'register_commit_graft()', 'lookup_commit_graft()', and
'unregister_shallow()'). The first two of these will stay in 'commit.c',
but the latter will move to 'shallow.c', and thus needs
'commit_graft_pos' to be non-static.

Prepare for that by making 'commit_graft_pos' non-static so that it can
be called from both 'commit.c' and 'shallow.c'.

Signed-off-by: Taylor Blau <redacted>
---
 commit.c | 2 +-
 commit.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/commit.c b/commit.c
index c7099daeac..b76f7d72be 100644
--- a/commit.c
+++ b/commit.c

@@ -110,7 +110,7 @@ static const unsigned char *commit_graft_sha1_access(size_t index, void *table)
 	return commit_graft_table[index]->oid.hash;
 }

-static int commit_graft_pos(struct repository *r, const unsigned char *sha1)
+int commit_graft_pos(struct repository *r, const unsigned char *sha1)
 {
 	return sha1_pos(sha1, r->parsed_objects->grafts,
 			r->parsed_objects->grafts_nr,

diff --git a/commit.h b/commit.h
index ab91d21131..0fe1e1b570 100644
--- a/commit.h
+++ b/commit.h

@@ -236,6 +236,8 @@ struct commit_graft {
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);

 struct commit_graft *read_graft_line(struct strbuf *line);
+/* commit_graft_pos returns an index into r->parsed_objects->grafts. */
+int commit_graft_pos(struct repository *r, const unsigned char *sha1);
 int register_commit_graft(struct repository *r, struct commit_graft *, int);
 void prepare_commit_graft(struct repository *r);
 struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid);
--

2.26.0.113.ge9739cdccc