[PATCH v3 0/5] config-parse: create config parsing library

[Josh Steadmon <steadmon@google.com>]

Config parsing no longer uses global state as of gc/config-context, so the
natural next step for libification is to turn that into its own library.
This series starts that process by moving config parsing into
config-parse.[c|h] so that other programs can include this functionality
without pulling in all of config.[c|h].

Open questions:
- How do folks feel about the do_event() refactor in patches 2 & 3?

Changes since v2:
- Added patch 2/5 to refactor do_event() into start_event() and
  flush_event().
- In patch 3/5, we can now add do_event_and_flush() to immediately run
  an event callback, rather than having to do_event() twice in a row.

Changes since v1.5:
- Dropped patch 1/5: config: return positive from git_config_parse_key()


Glen Choo (4):
  config: split out config_parse_options
  config: report config parse errors using cb
  config.c: accept config_parse_options in git_config_from_stdin
  config-parse: split library out of config.[c|h]

Josh Steadmon (1):
  config: split do_event() into start and flush operations

 Makefile           |   1 +
 builtin/config.c   |   4 +-
 bundle-uri.c       |   4 +-
 config-parse.c     | 601 +++++++++++++++++++++++++++++++++++++++++
 config-parse.h     | 155 +++++++++++
 config.c           | 658 ++++-----------------------------------------
 config.h           | 134 +--------
 fsck.c             |   4 +-
 submodule-config.c |   9 +-
 9 files changed, 836 insertions(+), 734 deletions(-)
 create mode 100644 config-parse.c
 create mode 100644 config-parse.h

Range-diff against v2:
1:  5c676fbac3 ! 1:  fa55b7836f config: split out config_parse_options
    @@ Metadata
      ## Commit message ##
         config: split out config_parse_options
     
    -    "struct config_options" is a disjoint set of options options used by the
    -    config parser (e.g. event listners) and options used by
    -    config_with_options() (e.g. to handle includes, choose which config
    -    files to parse). Split parser-only options into config_parse_options.
    +    "struct config_options" is a disjoint set of options used by the config
    +    parser (e.g. event listeners) and options used by config_with_options()
    +    (e.g. to handle includes, choose which config files to parse). Split
    +    parser-only options into config_parse_options.
     
         Signed-off-by: Glen Choo <chooglen@google.com>
     
      ## bundle-uri.c ##
-:  ---------- > 2:  8a1463c223 config: split do_event() into start and flush operations
2:  cb92a1f2e3 ! 3:  a888045c04 config: report config parse errors using cb
    @@ Commit message
         CONFIG_ERROR_UNSET and the config_source 'default', since all callers
         are now expected to specify the error handling they want.
     
    +    Add a new "do_event_and_flush" function for running event callbacks
    +    immediately, where the event does not need to calculate an end offset.
    +
         Signed-off-by: Glen Choo <chooglen@google.com>
     
      ## builtin/config.c ##
    @@ config.c: static int add_remote_url(const char *var, const char *value,
      
      	opts = *inc->opts;
      	opts.unconditional_remote_url = 1;
    +@@ config.c: static int do_event(struct config_source *cs, enum config_event_t type,
    + 	return 0;
    + }
    + 
    ++static int do_event_and_flush(struct config_source *cs,
    ++			      enum config_event_t type,
    ++			      struct parse_event_data *data)
    ++{
    ++	int maybe_ret;
    ++
    ++	if ((maybe_ret = flush_event(cs, type, data)) < 1)
    ++		return maybe_ret;
    ++
    ++	start_event(cs, type, data);
    ++
    ++	if ((maybe_ret = flush_event(cs, type, data)) < 1)
    ++		return maybe_ret;
    ++
    ++	/*
    ++	 * Not actually EOF, but this indicates we don't have a valid event
    ++	 * to flush next time around.
    ++	 */
    ++	data->previous_type = CONFIG_EVENT_EOF;
    ++
    ++	return 0;
    ++}
    ++
    + static void kvi_from_source(struct config_source *cs,
    + 			    enum config_scope scope,
    + 			    struct key_value_info *out)
     @@ config.c: static void kvi_from_source(struct config_source *cs,
      	out->path = cs->path;
      }
    @@ config.c: static int git_parse_source(struct config_source *cs, config_fn_t fn,
     -
     -	free(error_msg);
     -	return error_return;
    -+	/*
    -+	 * FIXME for whatever reason, do_event passes the _previous_ event, so
    -+	 * in order for our callback to receive the error event, we have to call
    -+	 * do_event twice
    -+	 */
    -+	do_event(cs, CONFIG_EVENT_ERROR, &event_data);
    -+	do_event(cs, CONFIG_EVENT_ERROR, &event_data);
    ++	do_event_and_flush(cs, CONFIG_EVENT_ERROR, &event_data);
     +	return -1;
      }
      
    @@ config.c: void read_early_config(config_fn_t cb, void *data)
      void read_very_early_config(config_fn_t cb, void *data)
      {
     -	struct config_options opts = { 0 };
    +-
    +-	opts.respect_includes = 1;
    +-	opts.ignore_repo = 1;
    +-	opts.ignore_worktree = 1;
    +-	opts.ignore_cmdline = 1;
    +-	opts.system_gently = 1;
     +	struct config_options opts = {
    ++		.respect_includes = 1,
    ++		.ignore_repo = 1,
    ++		.ignore_worktree = 1,
    ++		.ignore_cmdline = 1,
    ++		.system_gently = 1,
     +		.parse_options = CP_OPTS_INIT(CONFIG_ERROR_DIE),
     +	};
      
    - 	opts.respect_includes = 1;
    - 	opts.ignore_repo = 1;
    + 	config_with_options(cb, data, NULL, NULL, &opts);
    + }
     @@ config.c: int git_configset_get_pathname(struct config_set *set, const char *key, const ch
      /* Functions use to read configuration from a repository */
      static void repo_read_config(struct repository *repo)
    @@ config.c: int git_configset_get_pathname(struct config_set *set, const char *key
      
      	opts.respect_includes = 1;
      	opts.commondir = repo->commondir;
    -@@ config.c: int repo_config_get_pathname(struct repository *repo,
    - static void read_protected_config(void)
    - {
    - 	struct config_options opts = {
    --		.respect_includes = 1,
    --		.ignore_repo = 1,
    --		.ignore_worktree = 1,
    --		.system_gently = 1,
    +@@ config.c: static void read_protected_config(void)
    + 		.ignore_repo = 1,
    + 		.ignore_worktree = 1,
    + 		.system_gently = 1,
     +		.parse_options = CP_OPTS_INIT(CONFIG_ERROR_DIE),
      	};
      
    -+	opts.respect_includes = 1;
    -+	opts.ignore_repo = 1;
    -+	opts.ignore_worktree = 1;
    -+	opts.system_gently = 1;
     +
      	git_configset_init(&protected_config);
      	config_with_options(config_set_callback, &protected_config, NULL,
3:  e034d0780c = 4:  49d4b64991 config.c: accept config_parse_options in git_config_from_stdin
4:  74c5dcd5a2 ! 5:  e59ca992d0 config-parse: split library out of config.[c|h]
    @@ config-parse.c (new)
     +	const struct config_parse_options *opts;
     +};
     +
    -+static int do_event(struct config_source *cs, enum config_event_t type,
    -+		    struct parse_event_data *data)
    ++static size_t get_corrected_offset(struct config_source *cs,
    ++				   enum config_event_t type)
     +{
    -+	size_t offset;
    -+
    -+	if (!data->opts || !data->opts->event_fn)
    -+		return 0;
    ++	size_t offset = cs->do_ftell(cs);
     +
    -+	if (type == CONFIG_EVENT_WHITESPACE &&
    -+	    data->previous_type == type)
    -+		return 0;
    -+
    -+	offset = cs->do_ftell(cs);
     +	/*
     +	 * At EOF, the parser always "inserts" an extra '\n', therefore
     +	 * the end offset of the event is the current file position, otherwise
    @@ config-parse.c (new)
     +	 */
     +	if (type != CONFIG_EVENT_EOF)
     +		offset--;
    ++	return offset;
    ++}
    ++
    ++static void start_event(struct config_source *cs, enum config_event_t type,
    ++		       struct parse_event_data *data)
    ++{
    ++	data->previous_type = type;
    ++	data->previous_offset = get_corrected_offset(cs, type);
    ++}
    ++
    ++static int flush_event(struct config_source *cs, enum config_event_t type,
    ++		       struct parse_event_data *data)
    ++{
    ++	if (!data->opts || !data->opts->event_fn)
    ++		return 0;
    ++
    ++	if (type == CONFIG_EVENT_WHITESPACE &&
    ++	    data->previous_type == type)
    ++		return 0;
     +
     +	if (data->previous_type != CONFIG_EVENT_EOF &&
     +	    data->opts->event_fn(data->previous_type, data->previous_offset,
    -+				 offset, cs, data->opts->event_fn_data) < 0)
    ++				 get_corrected_offset(cs, type), cs,
    ++				 data->opts->event_fn_data) < 0)
     +		return -1;
     +
    -+	data->previous_type = type;
    -+	data->previous_offset = offset;
    ++	return 1;
    ++}
    ++
    ++static int do_event(struct config_source *cs, enum config_event_t type,
    ++		    struct parse_event_data *data)
    ++{
    ++	int maybe_ret;
    ++
    ++	if ((maybe_ret = flush_event(cs, type, data)) < 1)
    ++		return maybe_ret;
    ++
    ++	start_event(cs, type, data);
    ++
    ++	return 0;
    ++}
    ++
    ++static int do_event_and_flush(struct config_source *cs,
    ++			      enum config_event_t type,
    ++			      struct parse_event_data *data)
    ++{
    ++	int maybe_ret;
    ++
    ++	if ((maybe_ret = flush_event(cs, type, data)) < 1)
    ++		return maybe_ret;
    ++
    ++	start_event(cs, type, data);
    ++
    ++	if ((maybe_ret = flush_event(cs, type, data)) < 1)
    ++		return maybe_ret;
    ++
    ++	/*
    ++	 * Not actually EOF, but this indicates we don't have a valid event
    ++	 * to flush next time around.
    ++	 */
    ++	data->previous_type = CONFIG_EVENT_EOF;
     +
     +	return 0;
     +}
    @@ config-parse.c (new)
     +		if (get_value(cs, kvi, fn, data, var) < 0)
     +			break;
     +	}
    -+	/*
    -+	 * FIXME for whatever reason, do_event passes the _previous_ event, so
    -+	 * in order for our callback to receive the error event, we have to call
    -+	 * do_event twice
    -+	 */
    -+	do_event(cs, CONFIG_EVENT_ERROR, &event_data);
    -+	do_event(cs, CONFIG_EVENT_ERROR, &event_data);
    ++
    ++	do_event_and_flush(cs, CONFIG_EVENT_ERROR, &event_data);
     +	return -1;
     +}
     +
    @@ config.c: int git_config_from_parameters(config_fn_t fn, void *data)
     -	const struct config_parse_options *opts;
     -};
     -
    --static int do_event(struct config_source *cs, enum config_event_t type,
    --		    struct parse_event_data *data)
    +-static size_t get_corrected_offset(struct config_source *cs,
    +-				   enum config_event_t type)
     -{
    --	size_t offset;
    --
    --	if (!data->opts || !data->opts->event_fn)
    --		return 0;
    +-	size_t offset = cs->do_ftell(cs);
     -
    --	if (type == CONFIG_EVENT_WHITESPACE &&
    --	    data->previous_type == type)
    --		return 0;
    --
    --	offset = cs->do_ftell(cs);
     -	/*
     -	 * At EOF, the parser always "inserts" an extra '\n', therefore
     -	 * the end offset of the event is the current file position, otherwise
    @@ config.c: int git_config_from_parameters(config_fn_t fn, void *data)
     -	 */
     -	if (type != CONFIG_EVENT_EOF)
     -		offset--;
    +-	return offset;
    +-}
    +-
    +-static void start_event(struct config_source *cs, enum config_event_t type,
    +-		       struct parse_event_data *data)
    +-{
    +-	data->previous_type = type;
    +-	data->previous_offset = get_corrected_offset(cs, type);
    +-}
    +-
    +-static int flush_event(struct config_source *cs, enum config_event_t type,
    +-		       struct parse_event_data *data)
    +-{
    +-	if (!data->opts || !data->opts->event_fn)
    +-		return 0;
    +-
    +-	if (type == CONFIG_EVENT_WHITESPACE &&
    +-	    data->previous_type == type)
    +-		return 0;
     -
     -	if (data->previous_type != CONFIG_EVENT_EOF &&
     -	    data->opts->event_fn(data->previous_type, data->previous_offset,
    --				 offset, cs, data->opts->event_fn_data) < 0)
    +-				 get_corrected_offset(cs, type), cs,
    +-				 data->opts->event_fn_data) < 0)
     -		return -1;
     -
    --	data->previous_type = type;
    --	data->previous_offset = offset;
    +-	return 1;
    +-}
    +-
    +-static int do_event(struct config_source *cs, enum config_event_t type,
    +-		    struct parse_event_data *data)
    +-{
    +-	int maybe_ret;
    +-
    +-	if ((maybe_ret = flush_event(cs, type, data)) < 1)
    +-		return maybe_ret;
    +-
    +-	start_event(cs, type, data);
    +-
    +-	return 0;
    +-}
    +-
    +-static int do_event_and_flush(struct config_source *cs,
    +-			      enum config_event_t type,
    +-			      struct parse_event_data *data)
    +-{
    +-	int maybe_ret;
    +-
    +-	if ((maybe_ret = flush_event(cs, type, data)) < 1)
    +-		return maybe_ret;
    +-
    +-	start_event(cs, type, data);
    +-
    +-	if ((maybe_ret = flush_event(cs, type, data)) < 1)
    +-		return maybe_ret;
    +-
    +-	/*
    +-	 * Not actually EOF, but this indicates we don't have a valid event
    +-	 * to flush next time around.
    +-	 */
    +-	data->previous_type = CONFIG_EVENT_EOF;
     -
     -	return 0;
     -}
    @@ config.c: int git_config_err_fn(enum config_event_t type, size_t begin_offset UN
     -			break;
     -	}
     -
    --	/*
    --	 * FIXME for whatever reason, do_event passes the _previous_ event, so
    --	 * in order for our callback to receive the error event, we have to call
    --	 * do_event twice
    --	 */
    --	do_event(cs, CONFIG_EVENT_ERROR, &event_data);
    --	do_event(cs, CONFIG_EVENT_ERROR, &event_data);
    +-	do_event_and_flush(cs, CONFIG_EVENT_ERROR, &event_data);
     -	return -1;
     -}
     -

base-commit: aa9166bcc0ba654fc21f198a30647ec087f733ed
-- 
2.42.0.515.g380fc7ccd1-goog