[PATCH] repo-settings: move warn_on_object_refname_ambiguity to repo_settings

[PATCH] repo-settings: move warn_on_object_refname_ambiguity to repo_settings

[drona]

From: Dorna Raj Gyawali <dronarajgyawali@gmail.com>

This flag (default: true) controls whether warnings are printed when
disambiguating object names that match multiple refname prefixes.
Move it from a global variable in environment.c to struct repo_settings
to reduce global state.

- Added field and default value to repo-settings.h
- Added getter and setter helpers in repo-settings.[ch]
- Updated all call sites in object-name.c, submodule.c, revision.c,
  builtin/cat-file.c and builtin/pack-objects.c
- Removed the old global from environment.[ch]

No functional change intended.

The USE_THE_REPOSITORY_VARIABLE macros are kept in this version.
Will remove them in v2 if requested.

Signed-off-by: drona <dronarajgyawali@gmail.com>
Signed-off-by: Dorna Raj Gyawali <dronarajgyawali@gmail.com>
---
 builtin/cat-file.c     |  6 +++---
 builtin/pack-objects.c |  6 +++---
 environment.c          |  1 -
 environment.h          |  1 -
 object-name.c          |  2 +-
 repo-settings.c        | 10 ++++++++++
 repo-settings.h        |  7 ++++++-
 revision.c             |  6 +++---
 submodule.c            |  6 +++---
 9 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index df8e87a81f..bc48ae9f1e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -947,8 +947,8 @@ static int batch_objects(struct batch_options *opt)
 	 * warn) ends up dwarfing the actual cost of the object lookups
 	 * themselves. We can work around it by just turning off the warning.
 	 */
-	save_warning = warn_on_object_refname_ambiguity;
-	warn_on_object_refname_ambiguity = 0;
+	save_warning = repo_settings_get_warn_on_object_refname_ambiguity(the_repository);
+	repo_settings_set_warn_on_object_refname_ambiguity(the_repository, 0);
 
 	if (opt->batch_mode == BATCH_MODE_QUEUE_AND_DISPATCH) {
 		batch_objects_command(opt, &output, &data);
@@ -976,7 +976,7 @@ static int batch_objects(struct batch_options *opt)
  cleanup:
 	strbuf_release(&input);
 	strbuf_release(&output);
-	warn_on_object_refname_ambiguity = save_warning;
+	repo_settings_set_warn_on_object_refname_ambiguity(the_repository, save_warning);
 	return retval;
 }
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index cfb03d4c09..da008c7e98 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4679,8 +4679,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
 	/* make sure shallows are read */
 	is_repository_shallow(the_repository);
 
-	save_warning = warn_on_object_refname_ambiguity;
-	warn_on_object_refname_ambiguity = 0;
+	save_warning = repo_settings_get_warn_on_object_refname_ambiguity(the_repository);
+	repo_settings_set_warn_on_object_refname_ambiguity(the_repository, 0);
 
 	while (fgets(line, sizeof(line), stdin) != NULL) {
 		int len = strlen(line);
@@ -4708,7 +4708,7 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
 			die(_("bad revision '%s'"), line);
 	}
 
-	warn_on_object_refname_ambiguity = save_warning;
+	repo_settings_set_warn_on_object_refname_ambiguity(the_repository, save_warning);
 
 	if (use_bitmap_index && !get_object_list_from_bitmap(revs))
 		return;
diff --git a/environment.c b/environment.c
index 0026eb2274..c76adc02c1 100644
--- a/environment.c
+++ b/environment.c
@@ -49,7 +49,6 @@ int minimum_abbrev = 4, default_abbrev = -1;
 int ignore_case;
 int assume_unchanged;
 int is_bare_repository_cfg = -1; /* unspecified */
-int warn_on_object_refname_ambiguity = 1;
 char *git_commit_encoding;
 char *git_log_output_encoding;
 char *apply_default_whitespace;
diff --git a/environment.h b/environment.h
index 27f657af04..f2438634ce 100644
--- a/environment.h
+++ b/environment.h
@@ -151,7 +151,6 @@ extern int has_symlinks;
 extern int minimum_abbrev, default_abbrev;
 extern int ignore_case;
 extern int assume_unchanged;
-extern int warn_on_object_refname_ambiguity;
 extern char *apply_default_whitespace;
 extern char *apply_default_ignorewhitespace;
 extern char *git_attributes_file;
diff --git a/object-name.c b/object-name.c
index 7b14c3bf9b..e420152064 100644
--- a/object-name.c
+++ b/object-name.c
@@ -973,7 +973,7 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 	if (len == r->hash_algo->hexsz && !get_oid_hex(str, oid)) {
 		if (!(flags & GET_OID_SKIP_AMBIGUITY_CHECK) &&
 		    repo_settings_get_warn_ambiguous_refs(r) &&
-		    warn_on_object_refname_ambiguity) {
+		    repo_settings_get_warn_on_object_refname_ambiguity(r)) {
 			refs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref, 0);
 			if (refs_found > 0) {
 				warning(warn_msg, len, str);
diff --git a/repo-settings.c b/repo-settings.c
index 208e09ff17..c415779b9b 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -233,3 +233,13 @@ void repo_settings_reset_shared_repository(struct repository *repo)
 {
 	repo->settings.shared_repository_initialized = 0;
 }
+
+int repo_settings_get_warn_on_object_refname_ambiguity(struct repository *repo)
+{
+    return repo->settings.warn_on_object_refname_ambiguity;
+}
+
+void repo_settings_set_warn_on_object_refname_ambiguity(struct repository *repo, int value)
+{
+    repo->settings.warn_on_object_refname_ambiguity = value;
+}
\ No newline at end of file
diff --git a/repo-settings.h b/repo-settings.h
index cad9c3f0cc..f74d6b1a6b 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -61,7 +61,7 @@ struct repo_settings {
 
 	int core_multi_pack_index;
 	int warn_ambiguous_refs; /* lazily loaded via accessor */
-
+	int warn_on_object_refname_ambiguity;
 	size_t delta_base_cache_limit;
 	size_t packed_git_window_size;
 	size_t packed_git_limit;
@@ -77,6 +77,7 @@ struct repo_settings {
 	.core_untracked_cache = UNTRACKED_CACHE_KEEP, \
 	.fetch_negotiation_algorithm = FETCH_NEGOTIATION_CONSECUTIVE, \
 	.warn_ambiguous_refs = -1, \
+	.warn_on_object_refname_ambiguity = 1, \
 	.delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT, \
 	.packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE, \
 	.packed_git_limit = DEFAULT_PACKED_GIT_LIMIT, \
@@ -102,4 +103,8 @@ int repo_settings_get_shared_repository(struct repository *repo);
 void repo_settings_set_shared_repository(struct repository *repo, int value);
 void repo_settings_reset_shared_repository(struct repository *repo);
 
+/* Read the value for "warn_on_object_refname_ambiguity" (internal flag) */
+int repo_settings_get_warn_on_object_refname_ambiguity(struct repository *repo);
+void repo_settings_set_warn_on_object_refname_ambiguity(struct repository *repo, int value);
+
 #endif /* REPO_SETTINGS_H */
diff --git a/revision.c b/revision.c
index 047ff7e458..7072f9c331 100644
--- a/revision.c
+++ b/revision.c
@@ -2905,8 +2905,8 @@ static void read_revisions_from_stdin(struct rev_info *revs,
 	int save_warning;
 	int flags = 0;
 
-	save_warning = warn_on_object_refname_ambiguity;
-	warn_on_object_refname_ambiguity = 0;
+	save_warning = repo_settings_get_warn_on_object_refname_ambiguity(revs->repo);
+	repo_settings_set_warn_on_object_refname_ambiguity(revs->repo, 0);
 
 	strbuf_init(&sb, 1000);
 	while (strbuf_getline(&sb, stdin) != EOF) {
@@ -2940,7 +2940,7 @@ static void read_revisions_from_stdin(struct rev_info *revs,
 		read_pathspec_from_stdin(&sb, prune);
 
 	strbuf_release(&sb);
-	warn_on_object_refname_ambiguity = save_warning;
+	repo_settings_set_warn_on_object_refname_ambiguity(revs->repo, save_warning);
 }
 
 static void NORETURN diagnose_missing_default(const char *def)
diff --git a/submodule.c b/submodule.c
index 508938e4da..a1d618e802 100644
--- a/submodule.c
+++ b/submodule.c
@@ -899,11 +899,11 @@ static void collect_changed_submodules(struct repository *r,
 		.assume_dashdash = 1,
 	};
 
-	save_warning = warn_on_object_refname_ambiguity;
-	warn_on_object_refname_ambiguity = 0;
+	save_warning = repo_settings_get_warn_on_object_refname_ambiguity(r);
+	repo_settings_set_warn_on_object_refname_ambiguity(r, 0);
 	repo_init_revisions(r, &rev, NULL);
 	setup_revisions_from_strvec(argv, &rev, &s_r_opt);
-	warn_on_object_refname_ambiguity = save_warning;
+	repo_settings_set_warn_on_object_refname_ambiguity(r, save_warning);
 	if (prepare_revision_walk(&rev))
 		die(_("revision walk setup failed"));
 
-- 
2.43.0

Re: [PATCH] repo-settings: move warn_on_object_refname_ambiguity to repo_settings

[Junio C Hamano]

drona <dronarajgyawali@gmail.com> writes:

> From: Dorna Raj Gyawali <dronarajgyawali@gmail.com>
>
> This flag (default: true) controls whether warnings are printed when
> disambiguating object names that match multiple refname prefixes.
> Move it from a global variable in environment.c to struct repo_settings
> to reduce global state.

I am not sure why this needs to be per-repository, though.  It is
very understandable if this were a per-user preference coming from
the ~/.gitconfig file, and then it makes a lot more sense to keep it
a process-wide global.

>  builtin/cat-file.c     |  6 +++---
>  builtin/pack-objects.c |  6 +++---
>  environment.c          |  1 -
>  environment.h          |  1 -
>  object-name.c          |  2 +-
>  repo-settings.c        | 10 ++++++++++
>  repo-settings.h        |  7 ++++++-
>  revision.c             |  6 +++---
>  submodule.c            |  6 +++---
>  9 files changed, 29 insertions(+), 16 deletions(-)

Even in a distant future where these files are used to implement a
long running multi-user multi-repository "git server", we would
probably want to create a per-user structure that holds personal
preference items and throw this variable in there, not in the
per-repository repo_settings struct.

So,... I dunno.

[PATCH v2] v2: revert per-repo move + add clarifying comment

[drona]

From: Dorna Raj Gyawali <dronarajgyawali@gmail.com>

Following Junio's feedback: this flag is a user/process preference,
not repository-specific. Reverted the repo_settings changes.

Added a small comment for future contributors.

Signed-off-by: Dorna Raj Gyawali <dronarajgyawali@gmail.com>
---
 environment.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/environment.c b/environment.c
index 0026eb2274..09c34d1776 100644
--- a/environment.c
+++ b/environment.c
@@ -49,7 +49,7 @@ int minimum_abbrev = 4, default_abbrev = -1;
 int ignore_case;
 int assume_unchanged;
 int is_bare_repository_cfg = -1; /* unspecified */
-int warn_on_object_refname_ambiguity = 1;
+int warn_on_object_refname_ambiguity = 1; /* process-wide user preference */
 char *git_commit_encoding;
 char *git_log_output_encoding;
 char *apply_default_whitespace;
-- 
2.43.0