diff mbox series

[08/20] config: introduce missing setters that take repo as parameter

Message ID feae2ad31ac91baae75c46c22c5c3ef3b58c1897.1723013714.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series Stop using `the_repository` in "config.c" | expand

Commit Message

Patrick Steinhardt Aug. 7, 2024, 6:57 a.m. UTC
While we already provide some of the config-setting interfaces with a
`struct repository` as parameter, others only have a variant that
implicitly depend on `the_repository`. Fill in those gaps such that we
can start to deprecate the repo-less variants.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 config.c | 93 ++++++++++++++++++++++++++++++++++++++++++++------------
 config.h | 15 ++++++++-
 2 files changed, 87 insertions(+), 21 deletions(-)

Comments

Justin Tobler Aug. 9, 2024, 8:07 p.m. UTC | #1
On 24/08/07 08:57AM, Patrick Steinhardt wrote:
> While we already provide some of the config-setting interfaces with a
> `struct repository` as parameter, others only have a variant that
> implicitly depend on `the_repository`. Fill in those gaps such that we

s/depend/depends/

> can start to deprecate the repo-less variants.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  config.c | 93 ++++++++++++++++++++++++++++++++++++++++++++------------
>  config.h | 15 ++++++++-
>  2 files changed, 87 insertions(+), 21 deletions(-)
> 
> diff --git a/config.c b/config.c
> index 6421894614..ac89b708e7 100644
> --- a/config.c
> +++ b/config.c
> @@ -3178,21 +3178,39 @@ static void maybe_remove_section(struct config_store_data *store,
>  		*end_offset = store->parsed[store->parsed_nr - 1].end;
>  }
>  
> +int repo_config_set_in_file_gently(struct repository *r, const char *config_filename,
> +				   const char *key, const char *comment, const char *value)

Instead of prefixing with `repo_`, should we instead use the `config_`
prefix? Maybe `config_repo_`? It might be nice for names to align with
the config subsystem here.

[snip]
> diff --git a/config.h b/config.h
> index 54b47dec9e..b13e1bfb8d 100644
> --- a/config.h
> +++ b/config.h
> @@ -298,14 +298,18 @@ int git_config_pathname(char **, const char *, const char *);
>  int git_config_expiry_date(timestamp_t *, const char *, const char *);
>  int git_config_color(char *, const char *, const char *);
>  int git_config_set_in_file_gently(const char *, const char *, const char *, const char *);
> +int repo_config_set_in_file_gently(struct repository *r, const char *config_filename,
> +				   const char *key, const char *comment, const char *value);
>  
>  /**
>   * write config values to a specific config file, takes a key/value pair as
>   * parameter.
>   */
>  void git_config_set_in_file(const char *, const char *, const char *);
> +void repo_config_set_in_file(struct repository *, const char *, const char *, const char *);
>  
>  int git_config_set_gently(const char *, const char *);
> +int repo_config_set_gently(struct repository *r, const char *, const char *);
>  
>  /**
>   * Write a config value that should apply to the current worktree. If
> @@ -318,6 +322,7 @@ int repo_config_set_worktree_gently(struct repository *, const char *, const cha
>   * write config values to `.git/config`, takes a key/value pair as parameter.
>   */
>  void git_config_set(const char *, const char *);
> +void repo_config_set(struct repository *, const char *, const char *);
>  
>  int git_config_parse_key(const char *, char **, size_t *);
>  
> @@ -341,9 +346,11 @@ int git_config_parse_key(const char *, char **, size_t *);
>  #define CONFIG_FLAGS_FIXED_VALUE (1 << 1)
>  
>  int git_config_set_multivar_gently(const char *, const char *, const char *, unsigned);
> -void git_config_set_multivar(const char *, const char *, const char *, unsigned);
>  int repo_config_set_multivar_gently(struct repository *, const char *, const char *, const char *, unsigned);
> +void git_config_set_multivar(const char *, const char *, const char *, unsigned);
> +void repo_config_set_multivar(struct repository *r, const char *, const char *, const char *, unsigned);
>  int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, const char *, unsigned);
> +int repo_config_set_multivar_in_file_gently(struct repository *, const char *, const char *, const char *, const char *, const char *, unsigned);
>  
>  char *git_config_prepare_comment_string(const char *);
>  
> @@ -372,6 +379,12 @@ void git_config_set_multivar_in_file(const char *config_filename,
>  				     const char *value,
>  				     const char *value_pattern,
>  				     unsigned flags);
> +void repo_config_set_multivar_in_file(struct repository *r,
> +				      const char *config_filename,
> +				      const char *key,
> +				      const char *value,
> +				      const char *value_pattern,
> +				      unsigned flags);
>  
>  /**
>   * rename or remove sections in the config file

The rest of this patch is simply implementing variations of existing
functions that explicitly inject a repository and looks good to me.
Patrick Steinhardt Aug. 13, 2024, 9:25 a.m. UTC | #2
On Fri, Aug 09, 2024 at 03:07:37PM -0500, Justin Tobler wrote:
> On 24/08/07 08:57AM, Patrick Steinhardt wrote:
> > diff --git a/config.c b/config.c
> > index 6421894614..ac89b708e7 100644
> > --- a/config.c
> > +++ b/config.c
> > @@ -3178,21 +3178,39 @@ static void maybe_remove_section(struct config_store_data *store,
> >  		*end_offset = store->parsed[store->parsed_nr - 1].end;
> >  }
> >  
> > +int repo_config_set_in_file_gently(struct repository *r, const char *config_filename,
> > +				   const char *key, const char *comment, const char *value)
> 
> Instead of prefixing with `repo_`, should we instead use the `config_`
> prefix? Maybe `config_repo_`? It might be nice for names to align with
> the config subsystem here.

I think I agree, but we extensively use the `repo_config_` prefix for
all functions that take a repository as input in this subsystem. So in
this case I think we should stick with that, but we may eventually want
to revisit this.

Patrick
diff mbox series

Patch

diff --git a/config.c b/config.c
index 6421894614..ac89b708e7 100644
--- a/config.c
+++ b/config.c
@@ -3178,21 +3178,39 @@  static void maybe_remove_section(struct config_store_data *store,
 		*end_offset = store->parsed[store->parsed_nr - 1].end;
 }
 
+int repo_config_set_in_file_gently(struct repository *r, const char *config_filename,
+				   const char *key, const char *comment, const char *value)
+{
+	return repo_config_set_multivar_in_file_gently(r, config_filename, key, value, NULL, comment, 0);
+}
+
 int git_config_set_in_file_gently(const char *config_filename,
 				  const char *key, const char *comment, const char *value)
 {
-	return git_config_set_multivar_in_file_gently(config_filename, key, value, NULL, comment, 0);
+	return repo_config_set_in_file_gently(the_repository, config_filename,
+					      key, comment, value);
+}
+
+void repo_config_set_in_file(struct repository *r, const char *config_filename,
+			     const char *key, const char *value)
+{
+	repo_config_set_multivar_in_file(r, config_filename, key, value, NULL, 0);
 }
 
 void git_config_set_in_file(const char *config_filename,
 			    const char *key, const char *value)
 {
-	git_config_set_multivar_in_file(config_filename, key, value, NULL, 0);
+	repo_config_set_in_file(the_repository, config_filename, key, value);
+}
+
+int repo_config_set_gently(struct repository *r, const char *key, const char *value)
+{
+	return repo_config_set_multivar_gently(r, key, value, NULL, 0);
 }
 
 int git_config_set_gently(const char *key, const char *value)
 {
-	return git_config_set_multivar_gently(key, value, NULL, 0);
+	return repo_config_set_gently(the_repository, key, value);
 }
 
 int repo_config_set_worktree_gently(struct repository *r,
@@ -3209,13 +3227,18 @@  int repo_config_set_worktree_gently(struct repository *r,
 	return repo_config_set_multivar_gently(r, key, value, NULL, 0);
 }
 
-void git_config_set(const char *key, const char *value)
+void repo_config_set(struct repository *r, const char *key, const char *value)
 {
-	git_config_set_multivar(key, value, NULL, 0);
+	repo_config_set_multivar(r, key, value, NULL, 0);
 
 	trace2_cmd_set_config(key, value);
 }
 
+void git_config_set(const char *key, const char *value)
+{
+	repo_config_set(the_repository, key, value);
+}
+
 char *git_config_prepare_comment_string(const char *comment)
 {
 	size_t leading_blanks;
@@ -3293,11 +3316,12 @@  static void validate_comment_string(const char *comment)
  * - the config file is removed and the lock file rename()d to it.
  *
  */
-int git_config_set_multivar_in_file_gently(const char *config_filename,
-					   const char *key, const char *value,
-					   const char *value_pattern,
-					   const char *comment,
-					   unsigned flags)
+int repo_config_set_multivar_in_file_gently(struct repository *r,
+					    const char *config_filename,
+					    const char *key, const char *value,
+					    const char *value_pattern,
+					    const char *comment,
+					    unsigned flags)
 {
 	int fd = -1, in_fd = -1;
 	int ret;
@@ -3317,7 +3341,7 @@  int git_config_set_multivar_in_file_gently(const char *config_filename,
 	store.multi_replace = (flags & CONFIG_FLAGS_MULTI_REPLACE) != 0;
 
 	if (!config_filename)
-		config_filename = filename_buf = git_pathdup("config");
+		config_filename = filename_buf = repo_git_path(r, "config");
 
 	/*
 	 * The lock serves a purpose in addition to locking: the new
@@ -3526,7 +3550,7 @@  int git_config_set_multivar_in_file_gently(const char *config_filename,
 	ret = 0;
 
 	/* Invalidate the config cache */
-	git_config_clear();
+	repo_config_clear(r);
 
 out_free:
 	rollback_lock_file(&lock);
@@ -3543,12 +3567,24 @@  int git_config_set_multivar_in_file_gently(const char *config_filename,
 	goto out_free;
 }
 
-void git_config_set_multivar_in_file(const char *config_filename,
-				     const char *key, const char *value,
-				     const char *value_pattern, unsigned flags)
+int git_config_set_multivar_in_file_gently(const char *config_filename,
+					   const char *key, const char *value,
+					   const char *value_pattern,
+					   const char *comment,
+					   unsigned flags)
 {
-	if (!git_config_set_multivar_in_file_gently(config_filename, key, value,
-						    value_pattern, NULL, flags))
+	return repo_config_set_multivar_in_file_gently(the_repository, config_filename,
+						       key, value, value_pattern,
+						       comment, flags);
+}
+
+void repo_config_set_multivar_in_file(struct repository *r,
+				      const char *config_filename,
+				      const char *key, const char *value,
+				      const char *value_pattern, unsigned flags)
+{
+	if (!repo_config_set_multivar_in_file_gently(r, config_filename, key, value,
+						     value_pattern, NULL, flags))
 		return;
 	if (value)
 		die(_("could not set '%s' to '%s'"), key, value);
@@ -3556,6 +3592,14 @@  void git_config_set_multivar_in_file(const char *config_filename,
 		die(_("could not unset '%s'"), key);
 }
 
+void git_config_set_multivar_in_file(const char *config_filename,
+				     const char *key, const char *value,
+				     const char *value_pattern, unsigned flags)
+{
+	repo_config_set_multivar_in_file(the_repository, config_filename,
+					 key, value, value_pattern, flags);
+}
+
 int git_config_set_multivar_gently(const char *key, const char *value,
 				   const char *value_pattern, unsigned flags)
 {
@@ -3576,12 +3620,21 @@  int repo_config_set_multivar_gently(struct repository *r, const char *key,
 	return res;
 }
 
+void repo_config_set_multivar(struct repository *r,
+			      const char *key, const char *value,
+			      const char *value_pattern, unsigned flags)
+{
+	char *file = repo_git_path(r, "config");
+	git_config_set_multivar_in_file(file, key, value,
+					value_pattern, flags);
+	free(file);
+}
+
 void git_config_set_multivar(const char *key, const char *value,
 			     const char *value_pattern, unsigned flags)
 {
-	git_config_set_multivar_in_file(git_path("config"),
-					key, value, value_pattern,
-					flags);
+	repo_config_set_multivar(the_repository, key, value,
+				 value_pattern, flags);
 }
 
 static size_t section_name_match (const char *buf, const char *name)
diff --git a/config.h b/config.h
index 54b47dec9e..b13e1bfb8d 100644
--- a/config.h
+++ b/config.h
@@ -298,14 +298,18 @@  int git_config_pathname(char **, const char *, const char *);
 int git_config_expiry_date(timestamp_t *, const char *, const char *);
 int git_config_color(char *, const char *, const char *);
 int git_config_set_in_file_gently(const char *, const char *, const char *, const char *);
+int repo_config_set_in_file_gently(struct repository *r, const char *config_filename,
+				   const char *key, const char *comment, const char *value);
 
 /**
  * write config values to a specific config file, takes a key/value pair as
  * parameter.
  */
 void git_config_set_in_file(const char *, const char *, const char *);
+void repo_config_set_in_file(struct repository *, const char *, const char *, const char *);
 
 int git_config_set_gently(const char *, const char *);
+int repo_config_set_gently(struct repository *r, const char *, const char *);
 
 /**
  * Write a config value that should apply to the current worktree. If
@@ -318,6 +322,7 @@  int repo_config_set_worktree_gently(struct repository *, const char *, const cha
  * write config values to `.git/config`, takes a key/value pair as parameter.
  */
 void git_config_set(const char *, const char *);
+void repo_config_set(struct repository *, const char *, const char *);
 
 int git_config_parse_key(const char *, char **, size_t *);
 
@@ -341,9 +346,11 @@  int git_config_parse_key(const char *, char **, size_t *);
 #define CONFIG_FLAGS_FIXED_VALUE (1 << 1)
 
 int git_config_set_multivar_gently(const char *, const char *, const char *, unsigned);
-void git_config_set_multivar(const char *, const char *, const char *, unsigned);
 int repo_config_set_multivar_gently(struct repository *, const char *, const char *, const char *, unsigned);
+void git_config_set_multivar(const char *, const char *, const char *, unsigned);
+void repo_config_set_multivar(struct repository *r, const char *, const char *, const char *, unsigned);
 int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, const char *, unsigned);
+int repo_config_set_multivar_in_file_gently(struct repository *, const char *, const char *, const char *, const char *, const char *, unsigned);
 
 char *git_config_prepare_comment_string(const char *);
 
@@ -372,6 +379,12 @@  void git_config_set_multivar_in_file(const char *config_filename,
 				     const char *value,
 				     const char *value_pattern,
 				     unsigned flags);
+void repo_config_set_multivar_in_file(struct repository *r,
+				      const char *config_filename,
+				      const char *key,
+				      const char *value,
+				      const char *value_pattern,
+				      unsigned flags);
 
 /**
  * rename or remove sections in the config file