[PATCH v4 0/5] revision.[ch]: add --stdin parsing API, use in pack-objects

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

A re-roll of v3[1], this v4 fixes specific issues noted there, but
more importantly I've tried to address Junio's comments to the effect
of "what's the point of this churny thing?"" in [2].

As the new 3/5 explains what I really wanted this for is a new feature
for "git bundle create" that I have working locally, which allows it
to do:

    $ printf "e83c5163316f89bfbde7\trefs/heads/first-git-dot-git-commit\n" |
    git bundle create initial.bundle --stdin

As well as things like:

    $ git for-each-ref 'refs/remotes/origin' |
    sed 's!\trefs/remotes/origin/!\trefs/heads/!' |
    git bundle create origin.bundle --stdin

As elaborated on in 3/5 for that it's much better to have an API like
this, but since those changes also needed what I've got in
ab/bundle-updates and ab/bundle-doc (well, not that one really, but
the merge conflict would be painful otherwise) I split this series off
so I wouldn't end up with another 10-20 patch monster.

Junio: I hope that'll convence you to accept this, I can also submit
that "monster", but it'll be harder to read & review sinc so much of
it (e.g. this series, and doc refactoring) will be needed just to get
to the point of implementing that feature.

1. https://lore.kernel.org/git/cover-0.4-00000000000-20210621T150651Z-avarab@gmail.com/
2. https://lore.kernel.org/git/xmqqfswoa1ku.fsf@gitster.g/

Ævar Arnfjörð Bjarmason (5):
  upload-pack: run is_repository_shallow() before setup_revisions()
  revision.h: refactor "disable_stdin" and "read_from_stdin"
  revision.[ch]: add a "handle_stdin_line" API
  pack-objects.c: do stdin parsing via revision.c's API
  pack-objects.c: make use of REV_INFO_STDIN_LINE_PROCESS

 builtin/am.c           |  4 +--
 builtin/blame.c        |  2 +-
 builtin/diff-tree.c    |  2 +-
 builtin/pack-objects.c | 61 +++++++++++++++++++-----------------------
 builtin/rev-list.c     |  2 +-
 revision.c             | 39 ++++++++++++++++++++++-----
 revision.h             | 59 ++++++++++++++++++++++++++++++++++++----
 sequencer.c            |  4 +--
 8 files changed, 120 insertions(+), 53 deletions(-)

Range-diff against v3:
1:  dd2efd1cfb9 < -:  ----------- pack-objects tests: cover blindspots in stdin handling
2:  a9702132385 < -:  ----------- pack-objects: fix segfault in --stdin-packs option
3:  3840ac28e8d = 1:  8b2872227c5 upload-pack: run is_repository_shallow() before setup_revisions()
4:  6f69644b808 ! 2:  630feb06650 revision.h: refactor "disable_stdin" and "read_from_stdin"
    @@ Commit message
         of "disable_stdin", rather we read stdin if we see the "--stdin"
         option.
     
    -    The "read" is intended to understood as "I read it", not "you should
    -    read it". Let's avoid this confusion by using "consume" and "consumed"
    -    instead, i.e. a word whose present and past tense isn't the same.
    +    The "read" is intended to understood as "I've read it already", not
    +    "you should read it". Let's avoid this confusion by using "consume"
    +    and "consumed" instead, i.e. a word whose present and past tense isn't
    +    the same.
     
         See 8b3dce56508 (Teach --stdin option to "log" family, 2009-11-03)
         where we added the "disable_stdin" flag, and a12cbe23ef7 (rev-list:
    @@ revision.c: int setup_revisions(int argc, const char **argv, struct rev_info *re
      					argv[left++] = arg;
      					continue;
     +				case REV_INFO_STDIN_CONSUME_ON_OPTION:
    -+					if (revs->consumed_stdin++)
    ++					if (revs->consumed_stdin)
     +						die("--stdin given twice?");
     +					read_revisions_from_stdin(revs, &prune_data);
    ++					revs->consumed_stdin = 1;
     +					continue;
      				}
     -				if (revs->read_from_stdin++)
    @@ revision.h: struct rev_cmdline_info {
      struct topo_walk_info;
      
     +enum rev_info_stdin {
    -+	REV_INFO_STDIN_CONSUME_ON_OPTION = 0,
    ++	REV_INFO_STDIN_CONSUME_ON_OPTION,
     +	REV_INFO_STDIN_IGNORE,
     +};
     +
    @@ revision.h: struct rev_info {
     +	 * option?
      	 */
     -	int read_from_stdin;
    -+	int consumed_stdin;
    ++	int consumed_stdin:1;
      
      	/* topo-sort */
      	enum rev_sort_order sort_order;
-:  ----------- > 3:  60ef49df3b8 revision.[ch]: add a "handle_stdin_line" API
5:  943b1b4c12a ! 4:  ebffeb5891f pack-objects.c: do stdin parsing via revision.c's API
    @@ Metadata
      ## Commit message ##
         pack-objects.c: do stdin parsing via revision.c's API
     
    -    Extend the rev_info stdin parsing API to support hooking into its
    -    read_revisions_from_stdin() function, and change the custom stdin
    -    parsing in pack-objects.c to use it.
    +    Use the new "handle_stdin_line" API in revision.c to parse stdin in
    +    pack-objects.c, instead of using custom pack-objects.c-specific code
    +    to do so.
    +
    +    This means that we can remove the "if (len && line[len - 1] == '\n')"
    +    check, it's now redundant to using strbuf_getline(), and we get to
    +    skip the whole "warn_on_object_refname_ambiguity" dance. The
    +    read_revisions_from_stdin() function in revision.c we're now using
    +    does it for us.
     
         The pack-objects.c code being refactored away here was first added in
         Linus's c323ac7d9c5 (git-pack-objects: create a packed object
    @@ Commit message
         and it moved to a more simpler strbuf API in 6e8d46f9d4b (revision:
         read --stdin with strbuf_getline(), 2015-10-28).
     
    -    Let's do the same here, as in 6e8d46f9d4b we can remove the "if (len
    -    && line[len - 1] == '\n')" check, it's now redundant to using
    -    strbuf_getline(), and we get to skip the whole
    -    "warn_on_object_refname_ambiguity" dance. The
    -    read_revisions_from_stdin() function in revision.c we're now using
    -    does it for us.
    -
    -    For now there isn't all that much point in this whole exercises. We
    -    just end up calling setup_revisions() to loop over stdin for us, but
    -    the callback we define makes no use of REV_INFO_STDIN_LINE_PROCESS, we
    +    For now we've just made setup_revisions() loop over stdin for us, but
    +    the callback we define makes no use of REV_INFO_STDIN_LINE_PROCESS. We
         still need to call handle_revision_arg() ourselves because we'd like
         to call it with different flags.
     
    @@ builtin/pack-objects.c: static void get_object_list(int ac, const char **av)
      	if (use_bitmap_index && !get_object_list_from_bitmap(&revs))
      		return;
      
    -
    - ## revision.c ##
    -@@ revision.c: static void read_revisions_from_stdin(struct rev_info *revs,
    - 		int len = sb.len;
    - 		if (!len)
    - 			break;
    -+
    -+		if (revs->handle_stdin_line) {
    -+			enum rev_info_stdin_line ret = revs->handle_stdin_line(
    -+				revs, &sb, revs->stdin_line_priv);
    -+
    -+			switch (ret) {
    -+			case REV_INFO_STDIN_LINE_PROCESS:
    -+				break;
    -+			case REV_INFO_STDIN_LINE_CONTINUE:
    -+				continue;
    -+			}
    -+		}
    -+
    - 		if (sb.buf[0] == '-') {
    - 			if (len == 2 && sb.buf[1] == '-') {
    - 				seen_dashdash = 1;
    -@@ revision.c: int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
    - 
    - 			if (!strcmp(arg, "--stdin")) {
    - 				switch (revs->stdin_handling) {
    -+				case REV_INFO_STDIN_ALWAYS_READ:
    - 				case REV_INFO_STDIN_IGNORE:
    - 					argv[left++] = arg;
    - 					continue;
    -@@ revision.c: int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
    - 		}
    - 	}
    - 
    -+	/*
    -+	 * We're asked to ALWAYS_READ from stdin, but no --stdin
    -+	 * option (or "consumed_stdin" would be set).
    -+	 */
    -+	if (!revs->consumed_stdin &&
    -+	    revs->stdin_handling == REV_INFO_STDIN_ALWAYS_READ)
    -+		read_revisions_from_stdin(revs, &prune_data);
    -+
    - 	if (prune_data.nr) {
    - 		/*
    - 		 * If we need to introduce the magic "a lone ':' means no
    -
    - ## revision.h ##
    -@@ revision.h: struct topo_walk_info;
    - enum rev_info_stdin {
    - 	REV_INFO_STDIN_CONSUME_ON_OPTION = 0,
    - 	REV_INFO_STDIN_IGNORE,
    -+	REV_INFO_STDIN_ALWAYS_READ,
    - };
    - 
    -+enum rev_info_stdin_line {
    -+	REV_INFO_STDIN_LINE_PROCESS,
    -+	REV_INFO_STDIN_LINE_CONTINUE,
    -+};
    -+
    -+typedef enum rev_info_stdin_line (*rev_info_stdin_line_func)(
    -+	struct rev_info *revs, struct strbuf *line, void *stdin_line_priv);
    -+
    - struct rev_info {
    - 	/* Starting list */
    - 	struct commit_list *commits;
    -@@ revision.h: struct rev_info {
    - 	 *
    - 	 * Can be set to REV_INFO_STDIN_IGNORE to ignore the --stdin
    - 	 * option.
    -+	 *
    -+	 * Set it to REV_INFO_STDIN_ALWAYS_READ if there's always data
    -+	 * on stdin to be read, even if no --stdin option is provided.
    - 	 */
    - 	enum rev_info_stdin stdin_handling;
    - 
    -@@ revision.h: struct rev_info {
    - 	 */
    - 	int consumed_stdin;
    - 
    -+	/*
    -+	 * When reading from stdin (see "stdin_handling" above) define
    -+	 * a handle_stdin_line function to consume the lines.
    -+	 *
    -+	 * - Return REV_INFO_STDIN_LINE_PROCESS to continue
    -+	 *   revision.c's normal processing of the line (after
    -+	 *   possibly munging the provided strbuf).
    -+	 *
    -+	 * - Return REV_INFO_STDIN_LINE_CONTINUE to indicate that the
    -+	 *   line is fully processed, moving onto the next line (if
    -+	 *   any)
    -+	 *
    -+	 * Use the "stdin_line_priv" to optionally pass your own data
    -+	 * around.
    -+	 */
    -+	rev_info_stdin_line_func handle_stdin_line;
    -+	void *stdin_line_priv;
    -+
    - 	/* topo-sort */
    - 	enum rev_sort_order sort_order;
    - 
6:  34750ab81cf ! 5:  0c049aee67c pack-objects.c: make use of REV_INFO_STDIN_LINE_PROCESS
    @@ Commit message
             anticipation of a future in-tree user.
     
          2. I have patches for such an in-tree user already in a series
    -        that'll be submitted after this one.
    +        that'll be submitted after this one. See the reference to "git
    +        bundle create --stdin" in the commit that added the "handle_stdin_line"
    +        API.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
-- 
2.32.0.636.g43e71d69cff