{"thread":{"id":"53361","subject":"[PATCH v2 0/4] shallow: extract a header file","startedAt":"2020-04-30T19:48:49Z","lastAt":"2020-04-30T21:11:34Z","messageCount":8,"participants":["Taylor Blau","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"396682","messageId":"cover.1588275891.git.me@ttaylorr.com","threadId":"53361","inReplyTo":null,"subject":"[PATCH v2 0/4] shallow: extract a header file","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-30T19:48:43Z","receivedAt":"2020-04-30T19:48:49Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nHere's a reroll of my series to introduce 'shallow.h' after some very\nhelpful review from yesterday evening and early this morning.\n\nNot a ton has changed since last time, but here are the main points. A\nrange-diff since v1 is included below for a more detailed look at the\nchanges.\n\n  * The #include in 'builtin.h' was dropped and instead the burden of\n    including 'shallow.h' falls on the callers. Six builtins needed to\n    be updated to include 'shallow.h', so this seems a net positive for\n    the others that don't need to include it.\n\n  * Comments were updated in the final two patches for increased\n    readability and helpfulness.\n\n  * The third patch was folded into the second patch to make the\n    refactoring occur in a single step.\n\n  * A comment was added to 'commit_graft_pos' to indicate what position\n    is being returned.\n\nLike last time, this is based on my earlier series 'tb/reset-shallow',\nwhich should be on master shortly (perhaps in the next push-out? Not\nsure.).\n\nThanks in advance for a re-review :).\n\nTaylor Blau (4):\n  commit: make 'commit_graft_pos' non-static\n  shallow: extract a header file for shallow-related functions\n  shallow.h: document '{commit,rollback}_shallow_file'\n  shallow: use struct 'shallow_lock' for additional safety\n\n builtin/fetch.c        |  1 +\n builtin/pack-objects.c |  1 +\n builtin/prune.c        |  1 +\n builtin/receive-pack.c |  3 +-\n builtin/repack.c       |  1 +\n builtin/rev-parse.c    |  1 +\n commit-graph.c         |  1 +\n commit.c               | 16 ++-------\n commit.h               | 49 +------------------------\n environment.c          |  1 +\n fetch-pack.c           |  3 +-\n git.c                  |  1 +\n send-pack.c            |  1 +\n shallow.c              | 36 +++++++++++++------\n shallow.h              | 81 ++++++++++++++++++++++++++++++++++++++++++\n upload-pack.c          |  1 +\n 16 files changed, 123 insertions(+), 75 deletions(-)\n create mode 100644 shallow.h\n\nRange-diff against v1:\n1:  8ac4c63735 = 1:  cb8dde2ae2 commit: make 'commit_graft_pos' non-static\n2:  8ee7ed0666 < -:  ---------- shallow: take 'unregister_shallow' from 'commit.c'\n3:  3fb71045b6 ! 2:  0631e2a87d shallow: extract a header file for shallow-related functions\n    @@ Commit message\n         But, now there are a good number of shallow-related functions, and\n         placing them all in 'commit.h' doesn't make sense.\n\n    -    This patch extracts a 'shallow.h', which takes all of the headers from\n    -    'commit.h' for functions which already exist in 'shallow.c'. We will\n    -    bring the remaining shallow-related functions defined in 'commit.c' in a\n    -    subsequent patch.\n    +    This patch extracts a 'shallow.h', which takes all of the declarations\n    +    from 'commit.h' for functions which already exist in 'shallow.c'. We\n    +    will bring the remaining shallow-related functions defined in 'commit.c'\n    +    in a subsequent patch.\n\n         For now, move only the ones that already are implemented in 'shallow.c',\n         and update the necessary includes.\n\n         Signed-off-by: Taylor Blau <me@ttaylorr.com>\n\n    - ## builtin.h ##\n    + ## builtin/fetch.c ##\n     @@\n    - #include \"strbuf.h\"\n    - #include \"cache.h\"\n    - #include \"commit.h\"\n    + #include \"branch.h\"\n    + #include \"promisor-remote.h\"\n    + #include \"commit-graph.h\"\n     +#include \"shallow.h\"\n\n    - /*\n    -  * builtin API\n    + #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)\n    +\n    +\n    + ## builtin/pack-objects.c ##\n    +@@\n    + #include \"dir.h\"\n    + #include \"midx.h\"\n    + #include \"trace2.h\"\n    ++#include \"shallow.h\"\n    +\n    + #define IN_PACK(obj) oe_in_pack(&to_pack, obj)\n    + #define SIZE(obj) oe_size(&to_pack, obj)\n    +\n    + ## builtin/prune.c ##\n    +@@\n    + #include \"parse-options.h\"\n    + #include \"progress.h\"\n    + #include \"object-store.h\"\n    ++#include \"shallow.h\"\n    +\n    + static const char * const prune_usage[] = {\n    + \tN_(\"git prune [-n] [-v] [--progress] [--expire <time>] [--] [<head>...]\"),\n    +\n    + ## builtin/receive-pack.c ##\n    +@@\n    + #include \"protocol.h\"\n    + #include \"commit-reach.h\"\n    + #include \"worktree.h\"\n    ++#include \"shallow.h\"\n    +\n    + static const char * const receive_pack_usage[] = {\n    + \tN_(\"git receive-pack <git-dir>\"),\n    +\n    + ## builtin/repack.c ##\n    +@@\n    + #include \"packfile.h\"\n    + #include \"object-store.h\"\n    + #include \"promisor-remote.h\"\n    ++#include \"shallow.h\"\n    +\n    + static int delta_base_offset = 1;\n    + static int pack_kept_objects = -1;\n    +\n    + ## builtin/rev-parse.c ##\n    +@@\n    + #include \"split-index.h\"\n    + #include \"submodule.h\"\n    + #include \"commit-reach.h\"\n    ++#include \"shallow.h\"\n    +\n    + #define DO_REVS\t\t1\n    + #define DO_NOREV\t2\n\n      ## commit-graph.c ##\n     @@\n    @@ commit.c\n\n      static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);\n\n    +@@ commit.c: int for_each_commit_graft(each_commit_graft_fn fn, void *cb_data)\n    + \treturn ret;\n    + }\n    +\n    +-int unregister_shallow(const struct object_id *oid)\n    +-{\n    +-\tint pos = commit_graft_pos(the_repository, oid->hash);\n    +-\tif (pos < 0)\n    +-\t\treturn -1;\n    +-\tif (pos + 1 < the_repository->parsed_objects->grafts_nr)\n    +-\t\tMOVE_ARRAY(the_repository->parsed_objects->grafts + pos,\n    +-\t\t\t   the_repository->parsed_objects->grafts + pos + 1,\n    +-\t\t\t   the_repository->parsed_objects->grafts_nr - pos - 1);\n    +-\tthe_repository->parsed_objects->grafts_nr--;\n    +-\treturn 0;\n    +-}\n    +-\n    + struct commit_buffer {\n    + \tvoid *buffer;\n    + \tunsigned long size;\n\n      ## commit.h ##\n     @@ commit.h: struct commit *get_fork_point(const char *refname, struct commit *commit);\n    @@ fetch-pack.c\n      static int transfer_unpack_limit = -1;\n      static int fetch_unpack_limit = -1;\n\n    + ## git.c ##\n    +@@\n    + #include \"help.h\"\n    + #include \"run-command.h\"\n    + #include \"alias.h\"\n    ++#include \"shallow.h\"\n    +\n    + #define RUN_SETUP\t\t(1<<0)\n    + #define RUN_SETUP_GENTLY\t(1<<1)\n    +\n      ## send-pack.c ##\n     @@\n      #include \"sha1-array.h\"\n    @@ shallow.c\n\n      void set_alternate_shallow_file(struct repository *r, const char *path, int override)\n      {\n    +@@ shallow.c: int register_shallow(struct repository *r, const struct object_id *oid)\n    + \treturn register_commit_graft(r, graft, 0);\n    + }\n    +\n    ++int unregister_shallow(const struct object_id *oid)\n    ++{\n    ++\tint pos = commit_graft_pos(the_repository, oid->hash);\n    ++\tif (pos < 0)\n    ++\t\treturn -1;\n    ++\tif (pos + 1 < the_repository->parsed_objects->grafts_nr)\n    ++\t\tMOVE_ARRAY(the_repository->parsed_objects->grafts + pos,\n    ++\t\t\t   the_repository->parsed_objects->grafts + pos + 1,\n    ++\t\t\t   the_repository->parsed_objects->grafts_nr - pos - 1);\n    ++\tthe_repository->parsed_objects->grafts_nr--;\n    ++\treturn 0;\n    ++}\n    ++\n    + int is_repository_shallow(struct repository *r)\n    + {\n    + \tFILE *fp;\n\n      ## shallow.h (new) ##\n     @@\n4:  ff3620d50c < -:  ---------- shallow.h: document '{commit,rollback}_shallow_file'\n-:  ---------- > 3:  c251cf3ef9 shallow.h: document '{commit,rollback}_shallow_file'\n5:  839d5d0d8e ! 4:  08d8a915a0 shallow: use struct 'shallow_lock' for additional safety\n    @@ Commit message\n         callers to use it.\n\n         Suggested-by: Junio C Hamano <gitster@pobox.com>\n    +    Helped-by: Jonathan Nieder <jrnieder@gmail.com>\n         Signed-off-by: Taylor Blau <me@ttaylorr.com>\n\n      ## builtin/receive-pack.c ##\n    @@ shallow.c: static void reset_repository_shallow(struct repository *r)\n     +int commit_shallow_file(struct repository *r, struct shallow_lock *lk)\n      {\n     -\tint res = commit_lock_file(lk);\n    -+\tint res = commit_lock_file(&lk->lk);\n    ++\tint res = commit_lock_file(&lk->lock);\n      \treset_repository_shallow(r);\n      \treturn res;\n      }\n    @@ shallow.c: static void reset_repository_shallow(struct repository *r)\n     +void rollback_shallow_file(struct repository *r, struct shallow_lock *lk)\n      {\n     -\trollback_lock_file(lk);\n    -+\trollback_lock_file(&lk->lk);\n    ++\trollback_lock_file(&lk->lock);\n      \treset_repository_shallow(r);\n      }\n\n    @@ shallow.c: const char *setup_temporary_shallow(const struct oid_array *extra)\n      \tint fd;\n\n     -\tfd = hold_lock_file_for_update(shallow_lock,\n    -+\tfd = hold_lock_file_for_update(&shallow_lock->lk,\n    ++\tfd = hold_lock_file_for_update(&shallow_lock->lock,\n      \t\t\t\t       git_path_shallow(the_repository),\n      \t\t\t\t       LOCK_DIE_ON_ERROR);\n      \tcheck_shallow_file_for_update(the_repository);\n    @@ shallow.c: const char *setup_temporary_shallow(const struct oid_array *extra)\n      \t\t\tdie_errno(\"failed to write to %s\",\n     -\t\t\t\t  get_lock_file_path(shallow_lock));\n     -\t\t*alternate_shallow_file = get_lock_file_path(shallow_lock);\n    -+\t\t\t\t  get_lock_file_path(&shallow_lock->lk));\n    -+\t\t*alternate_shallow_file = get_lock_file_path(&shallow_lock->lk);\n    ++\t\t\t\t  get_lock_file_path(&shallow_lock->lock));\n    ++\t\t*alternate_shallow_file = get_lock_file_path(&shallow_lock->lock);\n      \t} else\n      \t\t/*\n      \t\t * is_repository_shallow() sees empty string as \"no\n    @@ shallow.c: void prune_shallow(unsigned options)\n      \t\treturn;\n      \t}\n     -\tfd = hold_lock_file_for_update(&shallow_lock,\n    -+\tfd = hold_lock_file_for_update(&shallow_lock.lk,\n    ++\tfd = hold_lock_file_for_update(&shallow_lock.lock,\n      \t\t\t\t       git_path_shallow(the_repository),\n      \t\t\t\t       LOCK_DIE_ON_ERROR);\n      \tcheck_shallow_file_for_update(the_repository);\n    @@ shallow.c: void prune_shallow(unsigned options)\n      \t\tif (write_in_full(fd, sb.buf, sb.len) < 0)\n      \t\t\tdie_errno(\"failed to write to %s\",\n     -\t\t\t\t  get_lock_file_path(&shallow_lock));\n    -+\t\t\t\t  get_lock_file_path(&shallow_lock.lk));\n    ++\t\t\t\t  get_lock_file_path(&shallow_lock.lock));\n      \t\tcommit_shallow_file(the_repository, &shallow_lock);\n      \t} else {\n      \t\tunlink(git_path_shallow(the_repository));\n    @@ shallow.h: void set_alternate_shallow_file(struct repository *r, const char *pat\n      int is_repository_shallow(struct repository *r);\n     +\n     +/*\n    -+ * shallow_lock is a thin wrapper around 'struct lock_file' in order to restrict\n    -+ * which locks can be used with '{commit,rollback}_shallow_file()'.\n    ++ * Lock for updating the $GIT_DIR/shallow file.\n    ++ *\n    ++ * Use `commit_shallow_file()` to commit an update, or\n    ++ * `rollback_shallow_file()` to roll it back. In either case, any\n    ++ * in-memory cached information about which commits are shallow will be\n    ++ * appropriately invalidated so that future operations reflect the new\n    ++ * state.\n     + */\n     +struct shallow_lock {\n    -+\tstruct lock_file lk;\n    ++\tstruct lock_file lock;\n     +};\n     +#define SHALLOW_LOCK_INIT { LOCK_INIT }\n     +\n    - /*\n    -  * {commit,rollback}_shallow_file commits or performs a rollback to the\n    -  * '.git/shallow' file, respectively, and resets stat-validity checks.\n    -  */\n    + /* commit $GIT_DIR/shallow and reset stat-validity checks */\n     -int commit_shallow_file(struct repository *r, struct lock_file *lk);\n    --void rollback_shallow_file(struct repository *r, struct lock_file *lk);\n     +int commit_shallow_file(struct repository *r, struct shallow_lock *lk);\n    + /* rollback $GIT_DIR/shallow and reset stat-validity checks */\n    +-void rollback_shallow_file(struct repository *r, struct lock_file *lk);\n     +void rollback_shallow_file(struct repository *r, struct shallow_lock *lk);\n\n      struct commit_list *get_shallow_commits(struct object_array *heads,\n--\n2.26.0.113.ge9739cdccc\n"},{"id":"396683","messageId":"cb8dde2ae2e78d0bcfb61382fe7769c12804336b.1588275891.git.me@ttaylorr.com","threadId":"53361","inReplyTo":"cover.1588275891.git.me@ttaylorr.com","subject":"[PATCH v2 1/4] commit: make 'commit_graft_pos' non-static","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-30T19:48:47Z","receivedAt":"2020-04-30T19:48:51Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In the next patch, some functions will be moved from 'commit.c' to have\nprototypes in a new 'shallow.h' and their implementations in\n'shallow.c'.\n\nThree functions in 'commit.c' use 'commit_graft_pos()' (they are\n'register_commit_graft()', 'lookup_commit_graft()', and\n'unregister_shallow()'). The first two of these will stay in 'commit.c',\nbut the latter will move to 'shallow.c', and thus needs\n'commit_graft_pos' to be non-static.\n\nPrepare for that by making 'commit_graft_pos' non-static so that it can\nbe called from both 'commit.c' and 'shallow.c'.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n commit.c | 2 +-\n commit.h | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/commit.c b/commit.c\nindex c7099daeac..b76f7d72be 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -110,7 +110,7 @@ static const unsigned char *commit_graft_sha1_access(size_t index, void *table)\n \treturn commit_graft_table[index]->oid.hash;\n }\n \n-static int commit_graft_pos(struct repository *r, const unsigned char *sha1)\n+int commit_graft_pos(struct repository *r, const unsigned char *sha1)\n {\n \treturn sha1_pos(sha1, r->parsed_objects->grafts,\n \t\t\tr->parsed_objects->grafts_nr,\ndiff --git a/commit.h b/commit.h\nindex ab91d21131..eb42e8b6d2 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -236,6 +236,7 @@ struct commit_graft {\n typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);\n \n struct commit_graft *read_graft_line(struct strbuf *line);\n+int commit_graft_pos(struct repository *r, const unsigned char *sha1);\n int register_commit_graft(struct repository *r, struct commit_graft *, int);\n void prepare_commit_graft(struct repository *r);\n struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid);\n-- \n2.26.0.113.ge9739cdccc\n\n"},{"id":"396684","messageId":"0631e2a87dc83220af27f7be7d7d8cd2bbb117fc.1588275891.git.me@ttaylorr.com","threadId":"53361","inReplyTo":"cover.1588275891.git.me@ttaylorr.com","subject":"[PATCH v2 2/4] shallow: extract a header file for shallow-related functions","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-30T19:48:50Z","receivedAt":"2020-04-30T19:48:55Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"There are many functions in commit.h that are more related to shallow\nrepositories than they are to any sort of generic commit machinery.\nLikely this began when there were only a few shallow-related functions,\nand commit.h seemed a reasonable enough place to put them.\n\nBut, now there are a good number of shallow-related functions, and\nplacing them all in 'commit.h' doesn't make sense.\n\nThis patch extracts a 'shallow.h', which takes all of the declarations\nfrom 'commit.h' for functions which already exist in 'shallow.c'. We\nwill bring the remaining shallow-related functions defined in 'commit.c'\nin a subsequent patch.\n\nFor now, move only the ones that already are implemented in 'shallow.c',\nand update the necessary includes.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n builtin/fetch.c        |  1 +\n builtin/pack-objects.c |  1 +\n builtin/prune.c        |  1 +\n builtin/receive-pack.c |  1 +\n builtin/repack.c       |  1 +\n builtin/rev-parse.c    |  1 +\n commit-graph.c         |  1 +\n commit.c               | 14 +--------\n commit.h               | 48 -------------------------------\n environment.c          |  1 +\n fetch-pack.c           |  1 +\n git.c                  |  1 +\n send-pack.c            |  1 +\n shallow.c              | 14 +++++++++\n shallow.h              | 64 ++++++++++++++++++++++++++++++++++++++++++\n upload-pack.c          |  1 +\n 16 files changed, 91 insertions(+), 61 deletions(-)\n create mode 100644 shallow.h\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex bf6bab80fa..a292acd1c0 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -27,6 +27,7 @@\n #include \"branch.h\"\n #include \"promisor-remote.h\"\n #include \"commit-graph.h\"\n+#include \"shallow.h\"\n \n #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)\n \ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex dc7c58ce3f..303ae312a9 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -34,6 +34,7 @@\n #include \"dir.h\"\n #include \"midx.h\"\n #include \"trace2.h\"\n+#include \"shallow.h\"\n \n #define IN_PACK(obj) oe_in_pack(&to_pack, obj)\n #define SIZE(obj) oe_size(&to_pack, obj)\ndiff --git a/builtin/prune.c b/builtin/prune.c\nindex 2b76872ad2..028ff786d6 100644\n--- a/builtin/prune.c\n+++ b/builtin/prune.c\n@@ -7,6 +7,7 @@\n #include \"parse-options.h\"\n #include \"progress.h\"\n #include \"object-store.h\"\n+#include \"shallow.h\"\n \n static const char * const prune_usage[] = {\n \tN_(\"git prune [-n] [-v] [--progress] [--expire <time>] [--] [<head>...]\"),\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 652661fa99..8e3f9bc35f 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -28,6 +28,7 @@\n #include \"protocol.h\"\n #include \"commit-reach.h\"\n #include \"worktree.h\"\n+#include \"shallow.h\"\n \n static const char * const receive_pack_usage[] = {\n \tN_(\"git receive-pack <git-dir>\"),\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 0781763b06..2550603608 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -12,6 +12,7 @@\n #include \"packfile.h\"\n #include \"object-store.h\"\n #include \"promisor-remote.h\"\n+#include \"shallow.h\"\n \n static int delta_base_offset = 1;\n static int pack_kept_objects = -1;\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 06056434ed..669dd2fd6f 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -16,6 +16,7 @@\n #include \"split-index.h\"\n #include \"submodule.h\"\n #include \"commit-reach.h\"\n+#include \"shallow.h\"\n \n #define DO_REVS\t\t1\n #define DO_NOREV\t2\ndiff --git a/commit-graph.c b/commit-graph.c\nindex f013a84e29..d316de5021 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -16,6 +16,7 @@\n #include \"hashmap.h\"\n #include \"replace-object.h\"\n #include \"progress.h\"\n+#include \"shallow.h\"\n \n #define GRAPH_SIGNATURE 0x43475048 /* \"CGPH\" */\n #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* \"OIDF\" */\ndiff --git a/commit.c b/commit.c\nindex b76f7d72be..87686a7055 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -20,6 +20,7 @@\n #include \"refs.h\"\n #include \"commit-reach.h\"\n #include \"run-command.h\"\n+#include \"shallow.h\"\n \n static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);\n \n@@ -245,19 +246,6 @@ int for_each_commit_graft(each_commit_graft_fn fn, void *cb_data)\n \treturn ret;\n }\n \n-int unregister_shallow(const struct object_id *oid)\n-{\n-\tint pos = commit_graft_pos(the_repository, oid->hash);\n-\tif (pos < 0)\n-\t\treturn -1;\n-\tif (pos + 1 < the_repository->parsed_objects->grafts_nr)\n-\t\tMOVE_ARRAY(the_repository->parsed_objects->grafts + pos,\n-\t\t\t   the_repository->parsed_objects->grafts + pos + 1,\n-\t\t\t   the_repository->parsed_objects->grafts_nr - pos - 1);\n-\tthe_repository->parsed_objects->grafts_nr--;\n-\treturn 0;\n-}\n-\n struct commit_buffer {\n \tvoid *buffer;\n \tunsigned long size;\ndiff --git a/commit.h b/commit.h\nindex eb42e8b6d2..5cd984939b 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -248,55 +248,7 @@ struct commit *get_fork_point(const char *refname, struct commit *commit);\n \n struct oid_array;\n struct ref;\n-int register_shallow(struct repository *r, const struct object_id *oid);\n-int unregister_shallow(const struct object_id *oid);\n-int commit_shallow_file(struct repository *r, struct lock_file *lk);\n-void rollback_shallow_file(struct repository *r, struct lock_file *lk);\n int for_each_commit_graft(each_commit_graft_fn, void *);\n-int is_repository_shallow(struct repository *r);\n-struct commit_list *get_shallow_commits(struct object_array *heads,\n-\t\t\t\t\tint depth, int shallow_flag, int not_shallow_flag);\n-struct commit_list *get_shallow_commits_by_rev_list(\n-\t\tint ac, const char **av, int shallow_flag, int not_shallow_flag);\n-void set_alternate_shallow_file(struct repository *r, const char *path, int override);\n-int write_shallow_commits(struct strbuf *out, int use_pack_protocol,\n-\t\t\t  const struct oid_array *extra);\n-void setup_alternate_shallow(struct lock_file *shallow_lock,\n-\t\t\t     const char **alternate_shallow_file,\n-\t\t\t     const struct oid_array *extra);\n-const char *setup_temporary_shallow(const struct oid_array *extra);\n-void advertise_shallow_grafts(int);\n-\n-/*\n- * Initialize with prepare_shallow_info() or zero-initialize (equivalent to\n- * prepare_shallow_info with a NULL oid_array).\n- */\n-struct shallow_info {\n-\tstruct oid_array *shallow;\n-\tint *ours, nr_ours;\n-\tint *theirs, nr_theirs;\n-\tstruct oid_array *ref;\n-\n-\t/* for receive-pack */\n-\tuint32_t **used_shallow;\n-\tint *need_reachability_test;\n-\tint *reachable;\n-\tint *shallow_ref;\n-\tstruct commit **commits;\n-\tint nr_commits;\n-};\n-\n-void prepare_shallow_info(struct shallow_info *, struct oid_array *);\n-void clear_shallow_info(struct shallow_info *);\n-void remove_nonexistent_theirs_shallow(struct shallow_info *);\n-void assign_shallow_commits_to_refs(struct shallow_info *info,\n-\t\t\t\t    uint32_t **used,\n-\t\t\t\t    int *ref_status);\n-int delayed_reachability_test(struct shallow_info *si, int c);\n-#define PRUNE_SHOW_ONLY 1\n-#define PRUNE_QUICK 2\n-void prune_shallow(unsigned options);\n-extern struct trace_key trace_shallow;\n \n int interactive_add(int argc, const char **argv, const char *prefix, int patch);\n int run_add_interactive(const char *revision, const char *patch_mode,\ndiff --git a/environment.c b/environment.c\nindex 10c9061c43..aaca0e91ac 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -17,6 +17,7 @@\n #include \"argv-array.h\"\n #include \"object-store.h\"\n #include \"chdir-notify.h\"\n+#include \"shallow.h\"\n \n int trust_executable_bit = 1;\n int trust_ctime = 1;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex a618f3b029..401d028e41 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -22,6 +22,7 @@\n #include \"connected.h\"\n #include \"fetch-negotiator.h\"\n #include \"fsck.h\"\n+#include \"shallow.h\"\n \n static int transfer_unpack_limit = -1;\n static int fetch_unpack_limit = -1;\ndiff --git a/git.c b/git.c\nindex b07198fe03..9b689143c6 100644\n--- a/git.c\n+++ b/git.c\n@@ -4,6 +4,7 @@\n #include \"help.h\"\n #include \"run-command.h\"\n #include \"alias.h\"\n+#include \"shallow.h\"\n \n #define RUN_SETUP\t\t(1<<0)\n #define RUN_SETUP_GENTLY\t(1<<1)\ndiff --git a/send-pack.c b/send-pack.c\nindex 0407841ae8..e0ccfef75a 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -15,6 +15,7 @@\n #include \"sha1-array.h\"\n #include \"gpg-interface.h\"\n #include \"cache.h\"\n+#include \"shallow.h\"\n \n int option_parse_push_signed(const struct option *opt,\n \t\t\t     const char *arg, int unset)\ndiff --git a/shallow.c b/shallow.c\nindex 5010a6c732..76e00893fe 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -14,6 +14,7 @@\n #include \"commit-slab.h\"\n #include \"list-objects.h\"\n #include \"commit-reach.h\"\n+#include \"shallow.h\"\n \n void set_alternate_shallow_file(struct repository *r, const char *path, int override)\n {\n@@ -38,6 +39,19 @@ int register_shallow(struct repository *r, const struct object_id *oid)\n \treturn register_commit_graft(r, graft, 0);\n }\n \n+int unregister_shallow(const struct object_id *oid)\n+{\n+\tint pos = commit_graft_pos(the_repository, oid->hash);\n+\tif (pos < 0)\n+\t\treturn -1;\n+\tif (pos + 1 < the_repository->parsed_objects->grafts_nr)\n+\t\tMOVE_ARRAY(the_repository->parsed_objects->grafts + pos,\n+\t\t\t   the_repository->parsed_objects->grafts + pos + 1,\n+\t\t\t   the_repository->parsed_objects->grafts_nr - pos - 1);\n+\tthe_repository->parsed_objects->grafts_nr--;\n+\treturn 0;\n+}\n+\n int is_repository_shallow(struct repository *r)\n {\n \tFILE *fp;\ndiff --git a/shallow.h b/shallow.h\nnew file mode 100644\nindex 0000000000..b50a85ed7e\n--- /dev/null\n+++ b/shallow.h\n@@ -0,0 +1,64 @@\n+#ifndef SHALLOW_H\n+#define SHALLOW_H\n+\n+#include \"lockfile.h\"\n+#include \"object.h\"\n+#include \"repository.h\"\n+#include \"strbuf.h\"\n+\n+void set_alternate_shallow_file(struct repository *r, const char *path, int override);\n+int register_shallow(struct repository *r, const struct object_id *oid);\n+int unregister_shallow(const struct object_id *oid);\n+int is_repository_shallow(struct repository *r);\n+int commit_shallow_file(struct repository *r, struct lock_file *lk);\n+void rollback_shallow_file(struct repository *r, struct lock_file *lk);\n+\n+struct commit_list *get_shallow_commits(struct object_array *heads,\n+\t\t\t\t\tint depth, int shallow_flag, int not_shallow_flag);\n+struct commit_list *get_shallow_commits_by_rev_list(\n+\t\tint ac, const char **av, int shallow_flag, int not_shallow_flag);\n+int write_shallow_commits(struct strbuf *out, int use_pack_protocol,\n+\t\t\t  const struct oid_array *extra);\n+\n+void setup_alternate_shallow(struct lock_file *shallow_lock,\n+\t\t\t     const char **alternate_shallow_file,\n+\t\t\t     const struct oid_array *extra);\n+\n+const char *setup_temporary_shallow(const struct oid_array *extra);\n+\n+void advertise_shallow_grafts(int);\n+\n+#define PRUNE_SHOW_ONLY 1\n+#define PRUNE_QUICK 2\n+void prune_shallow(unsigned options);\n+\n+/*\n+ * Initialize with prepare_shallow_info() or zero-initialize (equivalent to\n+ * prepare_shallow_info with a NULL oid_array).\n+ */\n+struct shallow_info {\n+\tstruct oid_array *shallow;\n+\tint *ours, nr_ours;\n+\tint *theirs, nr_theirs;\n+\tstruct oid_array *ref;\n+\n+\t/* for receive-pack */\n+\tuint32_t **used_shallow;\n+\tint *need_reachability_test;\n+\tint *reachable;\n+\tint *shallow_ref;\n+\tstruct commit **commits;\n+\tint nr_commits;\n+};\n+\n+void prepare_shallow_info(struct shallow_info *, struct oid_array *);\n+void clear_shallow_info(struct shallow_info *);\n+void remove_nonexistent_theirs_shallow(struct shallow_info *);\n+void assign_shallow_commits_to_refs(struct shallow_info *info,\n+\t\t\t\t    uint32_t **used,\n+\t\t\t\t    int *ref_status);\n+int delayed_reachability_test(struct shallow_info *si, int c);\n+\n+extern struct trace_key trace_shallow;\n+\n+#endif /* SHALLOW_H */\ndiff --git a/upload-pack.c b/upload-pack.c\nindex c53249cac1..e71b068291 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -26,6 +26,7 @@\n #include \"serve.h\"\n #include \"commit-graph.h\"\n #include \"commit-reach.h\"\n+#include \"shallow.h\"\n \n /* Remember to update object flag allocation in object.h */\n #define THEY_HAVE\t(1u << 11)\n-- \n2.26.0.113.ge9739cdccc\n\n"},{"id":"396685","messageId":"c251cf3ef9fe34e78db30c3170b56a496bc45345.1588275891.git.me@ttaylorr.com","threadId":"53361","inReplyTo":"cover.1588275891.git.me@ttaylorr.com","subject":"[PATCH v2 3/4] shallow.h: document '{commit,rollback}_shallow_file'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-30T19:48:54Z","receivedAt":"2020-04-30T19:48:59Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"When 'commit_shallow_file()' and 'rollback_shallow_file()' were\nintroduced, they did not have a documenting comment, when they could\nhave benefited from one.\n\nAdd a brief note about what these functions do, and make a special note\nthat they reset stat-validity checks.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n shallow.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/shallow.h b/shallow.h\nindex b50a85ed7e..b3bf200bf9 100644\n--- a/shallow.h\n+++ b/shallow.h\n@@ -10,7 +10,9 @@ void set_alternate_shallow_file(struct repository *r, const char *path, int over\n int register_shallow(struct repository *r, const struct object_id *oid);\n int unregister_shallow(const struct object_id *oid);\n int is_repository_shallow(struct repository *r);\n+/* commit $GIT_DIR/shallow and reset stat-validity checks */\n int commit_shallow_file(struct repository *r, struct lock_file *lk);\n+/* rollback $GIT_DIR/shallow and reset stat-validity checks */\n void rollback_shallow_file(struct repository *r, struct lock_file *lk);\n \n struct commit_list *get_shallow_commits(struct object_array *heads,\n-- \n2.26.0.113.ge9739cdccc\n\n"},{"id":"396686","messageId":"08d8a915a0d1a19f8ad119f43141ba2b8f52df2c.1588275891.git.me@ttaylorr.com","threadId":"53361","inReplyTo":"cover.1588275891.git.me@ttaylorr.com","subject":"[PATCH v2 4/4] shallow: use struct 'shallow_lock' for additional safety","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-30T19:48:57Z","receivedAt":"2020-04-30T19:49:02Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In previous patches, the functions 'commit_shallow_file' and\n'rollback_shallow_file' were introduced to reset the shallowness\nvalidity checks on a repository after potentially modifying\n'.git/shallow'.\n\nThese functions can be made safer by wrapping the 'struct lockfile *' in\na new type, 'shallow_lock', so that they cannot be called with a raw\nlock (and potentially misused by other code that happens to possess a\nlockfile, but has nothing to do with shallowness).\n\nThis patch introduces that type as a thin wrapper around 'struct\nlockfile', and updates the two aforementioned functions and their\ncallers to use it.\n\nSuggested-by: Junio C Hamano <gitster@pobox.com>\nHelped-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n builtin/receive-pack.c |  2 +-\n fetch-pack.c           |  2 +-\n shallow.c              | 22 +++++++++++-----------\n shallow.h              | 21 ++++++++++++++++++---\n 4 files changed, 31 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 8e3f9bc35f..73cebc2f55 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -856,7 +856,7 @@ static void refuse_unconfigured_deny_delete_current(void)\n static int command_singleton_iterator(void *cb_data, struct object_id *oid);\n static int update_shallow_ref(struct command *cmd, struct shallow_info *si)\n {\n-\tstruct lock_file shallow_lock = LOCK_INIT;\n+\tstruct shallow_lock shallow_lock = SHALLOW_LOCK_INIT;\n \tstruct oid_array extra = OID_ARRAY_INIT;\n \tstruct check_connected_options opt = CHECK_CONNECTED_INIT;\n \tuint32_t mask = 1 << (cmd->index % 32);\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 401d028e41..27d2d08cc0 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -35,7 +35,7 @@ static int fetch_fsck_objects = -1;\n static int transfer_fsck_objects = -1;\n static int agent_supported;\n static int server_supports_filtering;\n-static struct lock_file shallow_lock;\n+static struct shallow_lock shallow_lock;\n static const char *alternate_shallow_file;\n static struct strbuf fsck_msg_types = STRBUF_INIT;\n \ndiff --git a/shallow.c b/shallow.c\nindex 76e00893fe..7e0a41ea68 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -92,16 +92,16 @@ static void reset_repository_shallow(struct repository *r)\n \tstat_validity_clear(r->parsed_objects->shallow_stat);\n }\n \n-int commit_shallow_file(struct repository *r, struct lock_file *lk)\n+int commit_shallow_file(struct repository *r, struct shallow_lock *lk)\n {\n-\tint res = commit_lock_file(lk);\n+\tint res = commit_lock_file(&lk->lock);\n \treset_repository_shallow(r);\n \treturn res;\n }\n \n-void rollback_shallow_file(struct repository *r, struct lock_file *lk)\n+void rollback_shallow_file(struct repository *r, struct shallow_lock *lk)\n {\n-\trollback_lock_file(lk);\n+\trollback_lock_file(&lk->lock);\n \treset_repository_shallow(r);\n }\n \n@@ -366,22 +366,22 @@ const char *setup_temporary_shallow(const struct oid_array *extra)\n \treturn \"\";\n }\n \n-void setup_alternate_shallow(struct lock_file *shallow_lock,\n+void setup_alternate_shallow(struct shallow_lock *shallow_lock,\n \t\t\t     const char **alternate_shallow_file,\n \t\t\t     const struct oid_array *extra)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tint fd;\n \n-\tfd = hold_lock_file_for_update(shallow_lock,\n+\tfd = hold_lock_file_for_update(&shallow_lock->lock,\n \t\t\t\t       git_path_shallow(the_repository),\n \t\t\t\t       LOCK_DIE_ON_ERROR);\n \tcheck_shallow_file_for_update(the_repository);\n \tif (write_shallow_commits(&sb, 0, extra)) {\n \t\tif (write_in_full(fd, sb.buf, sb.len) < 0)\n \t\t\tdie_errno(\"failed to write to %s\",\n-\t\t\t\t  get_lock_file_path(shallow_lock));\n-\t\t*alternate_shallow_file = get_lock_file_path(shallow_lock);\n+\t\t\t\t  get_lock_file_path(&shallow_lock->lock));\n+\t\t*alternate_shallow_file = get_lock_file_path(&shallow_lock->lock);\n \t} else\n \t\t/*\n \t\t * is_repository_shallow() sees empty string as \"no\n@@ -414,7 +414,7 @@ void advertise_shallow_grafts(int fd)\n  */\n void prune_shallow(unsigned options)\n {\n-\tstruct lock_file shallow_lock = LOCK_INIT;\n+\tstruct shallow_lock shallow_lock = SHALLOW_LOCK_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tunsigned flags = SEEN_ONLY;\n \tint fd;\n@@ -428,14 +428,14 @@ void prune_shallow(unsigned options)\n \t\tstrbuf_release(&sb);\n \t\treturn;\n \t}\n-\tfd = hold_lock_file_for_update(&shallow_lock,\n+\tfd = hold_lock_file_for_update(&shallow_lock.lock,\n \t\t\t\t       git_path_shallow(the_repository),\n \t\t\t\t       LOCK_DIE_ON_ERROR);\n \tcheck_shallow_file_for_update(the_repository);\n \tif (write_shallow_commits_1(&sb, 0, NULL, flags)) {\n \t\tif (write_in_full(fd, sb.buf, sb.len) < 0)\n \t\t\tdie_errno(\"failed to write to %s\",\n-\t\t\t\t  get_lock_file_path(&shallow_lock));\n+\t\t\t\t  get_lock_file_path(&shallow_lock.lock));\n \t\tcommit_shallow_file(the_repository, &shallow_lock);\n \t} else {\n \t\tunlink(git_path_shallow(the_repository));\ndiff --git a/shallow.h b/shallow.h\nindex b3bf200bf9..5b4a96dcd6 100644\n--- a/shallow.h\n+++ b/shallow.h\n@@ -10,10 +10,25 @@ void set_alternate_shallow_file(struct repository *r, const char *path, int over\n int register_shallow(struct repository *r, const struct object_id *oid);\n int unregister_shallow(const struct object_id *oid);\n int is_repository_shallow(struct repository *r);\n+\n+/*\n+ * Lock for updating the $GIT_DIR/shallow file.\n+ *\n+ * Use `commit_shallow_file()` to commit an update, or\n+ * `rollback_shallow_file()` to roll it back. In either case, any\n+ * in-memory cached information about which commits are shallow will be\n+ * appropriately invalidated so that future operations reflect the new\n+ * state.\n+ */\n+struct shallow_lock {\n+\tstruct lock_file lock;\n+};\n+#define SHALLOW_LOCK_INIT { LOCK_INIT }\n+\n /* commit $GIT_DIR/shallow and reset stat-validity checks */\n-int commit_shallow_file(struct repository *r, struct lock_file *lk);\n+int commit_shallow_file(struct repository *r, struct shallow_lock *lk);\n /* rollback $GIT_DIR/shallow and reset stat-validity checks */\n-void rollback_shallow_file(struct repository *r, struct lock_file *lk);\n+void rollback_shallow_file(struct repository *r, struct shallow_lock *lk);\n \n struct commit_list *get_shallow_commits(struct object_array *heads,\n \t\t\t\t\tint depth, int shallow_flag, int not_shallow_flag);\n@@ -22,7 +37,7 @@ struct commit_list *get_shallow_commits_by_rev_list(\n int write_shallow_commits(struct strbuf *out, int use_pack_protocol,\n \t\t\t  const struct oid_array *extra);\n \n-void setup_alternate_shallow(struct lock_file *shallow_lock,\n+void setup_alternate_shallow(struct shallow_lock *shallow_lock,\n \t\t\t     const char **alternate_shallow_file,\n \t\t\t     const struct oid_array *extra);\n \n-- \n2.26.0.113.ge9739cdccc\n"},{"id":"396688","messageId":"20200430194952.GA2014@syl.local","threadId":"53361","inReplyTo":"cover.1588275891.git.me@ttaylorr.com","subject":"Re: [PATCH v2 0/4] shallow: extract a header file","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-30T19:49:52Z","receivedAt":"2020-04-30T19:49:57Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Apr 30, 2020 at 01:48:43PM -0600, Taylor Blau wrote:\n> Hi,\n>\n> Here's a reroll of my series to introduce 'shallow.h' after some very\n> helpful review from yesterday evening and early this morning.\n\nOops. Forgot to set --in-reply-to to the v1 of this series, which can be\nfound in [1]. Since I already sent the cover letter and patches, let's\njust pick it up from here.\n\nThanks,\nTaylor\n\n[1]: https://lore.kernel.org/git/cover.1588199705.git.me@ttaylorr.com/\n"},{"id":"396697","messageId":"xmqqbln8r8wg.fsf@gitster.c.googlers.com","threadId":"53361","inReplyTo":"cb8dde2ae2e78d0bcfb61382fe7769c12804336b.1588275891.git.me@ttaylorr.com","subject":"Re: [PATCH v2 1/4] commit: make 'commit_graft_pos' non-static","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-30T20:55:11Z","receivedAt":"2020-04-30T20:55:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> -static int commit_graft_pos(struct repository *r, const unsigned char *sha1)\n> +int commit_graft_pos(struct repository *r, const unsigned char *sha1)\n>  {\n>  \treturn sha1_pos(sha1, r->parsed_objects->grafts,\n>  \t\t\tr->parsed_objects->grafts_nr,\n> diff --git a/commit.h b/commit.h\n> index ab91d21131..eb42e8b6d2 100644\n> --- a/commit.h\n> +++ b/commit.h\n> @@ -236,6 +236,7 @@ struct commit_graft {\n>  typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);\n>  \n>  struct commit_graft *read_graft_line(struct strbuf *line);\n> +int commit_graft_pos(struct repository *r, const unsigned char *sha1);\n\nIn an earlier exchange, I saw this:\n\n>> - could include a comment saying that it's an index into\n>>   r->parsed_objects->grafts\n>\n> This and the below are both good ideas to me. I prefer this one, since\n> we'd have to duplicate yet another static function\n> ('commit_graft_sha1_access()' directly above) that is called by this\n> one.\n>\n>> - I'm usually loathe to suggest unnecessary duplication of code, but\n>>   it might make sense to duplicate the function into shallow.c.  Or\n>>   even to inline it there (in the single call site, that ends up\n>>   being pretty readable).\n>\n> I am not at all offended by duplication of code where it makes sense to\n> do so, but having to duplicate two functions seems like we'd be better\n> off simply documenting the function in commit.h.\n\nand I think I agree with that direction.  Forgot to add those\ncomments?\n\n\n"},{"id":"396698","messageId":"20200430211128.GA24821@syl.local","threadId":"53361","inReplyTo":"xmqqbln8r8wg.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 1/4] commit: make 'commit_graft_pos' non-static","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-30T21:11:28Z","receivedAt":"2020-04-30T21:11:34Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Apr 30, 2020 at 01:55:11PM -0700, Junio C Hamano wrote:\n> Taylor Blau <me@ttaylorr.com> writes:\n>\n> > -static int commit_graft_pos(struct repository *r, const unsigned char *sha1)\n> > +int commit_graft_pos(struct repository *r, const unsigned char *sha1)\n> >  {\n> >  \treturn sha1_pos(sha1, r->parsed_objects->grafts,\n> >  \t\t\tr->parsed_objects->grafts_nr,\n> > diff --git a/commit.h b/commit.h\n> > index ab91d21131..eb42e8b6d2 100644\n> > --- a/commit.h\n> > +++ b/commit.h\n> > @@ -236,6 +236,7 @@ struct commit_graft {\n> >  typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);\n> >\n> >  struct commit_graft *read_graft_line(struct strbuf *line);\n> > +int commit_graft_pos(struct repository *r, const unsigned char *sha1);\n>\n> In an earlier exchange, I saw this:\n>\n> >> - could include a comment saying that it's an index into\n> >>   r->parsed_objects->grafts\n> >\n> > This and the below are both good ideas to me. I prefer this one, since\n> > we'd have to duplicate yet another static function\n> > ('commit_graft_sha1_access()' directly above) that is called by this\n> > one.\n> >\n> >> - I'm usually loathe to suggest unnecessary duplication of code, but\n> >>   it might make sense to duplicate the function into shallow.c.  Or\n> >>   even to inline it there (in the single call site, that ends up\n> >>   being pretty readable).\n> >\n> > I am not at all offended by duplication of code where it makes sense to\n> > do so, but having to duplicate two functions seems like we'd be better\n> > off simply documenting the function in commit.h.\n>\n> and I think I agree with that direction.  Forgot to add those\n> comments?\n\nThanks for remembering. I did apply this locally, but must've dropped it\non the floor during a rebase. In any case, please swap in this patch\ninstead:\n\n-- >8 --\n\nSubject: [PATCH] commit: make 'commit_graft_pos' non-static\n\nIn the next patch, some functions will be moved from 'commit.c' to have\nprototypes in a new 'shallow.h' and their implementations in\n'shallow.c'.\n\nThree functions in 'commit.c' use 'commit_graft_pos()' (they are\n'register_commit_graft()', 'lookup_commit_graft()', and\n'unregister_shallow()'). The first two of these will stay in 'commit.c',\nbut the latter will move to 'shallow.c', and thus needs\n'commit_graft_pos' to be non-static.\n\nPrepare for that by making 'commit_graft_pos' non-static so that it can\nbe called from both 'commit.c' and 'shallow.c'.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n commit.c | 2 +-\n commit.h | 2 ++\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/commit.c b/commit.c\nindex c7099daeac..b76f7d72be 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -110,7 +110,7 @@ static const unsigned char *commit_graft_sha1_access(size_t index, void *table)\n \treturn commit_graft_table[index]->oid.hash;\n }\n\n-static int commit_graft_pos(struct repository *r, const unsigned char *sha1)\n+int commit_graft_pos(struct repository *r, const unsigned char *sha1)\n {\n \treturn sha1_pos(sha1, r->parsed_objects->grafts,\n \t\t\tr->parsed_objects->grafts_nr,\ndiff --git a/commit.h b/commit.h\nindex ab91d21131..0fe1e1b570 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -236,6 +236,8 @@ struct commit_graft {\n typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);\n\n struct commit_graft *read_graft_line(struct strbuf *line);\n+/* commit_graft_pos returns an index into r->parsed_objects->grafts. */\n+int commit_graft_pos(struct repository *r, const unsigned char *sha1);\n int register_commit_graft(struct repository *r, struct commit_graft *, int);\n void prepare_commit_graft(struct repository *r);\n struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid);\n--\n2.26.0.113.ge9739cdccc\n\n"}]}