Git development
 help / color / mirror / Atom feed
* [Outreachy PATCH] environment: move commit_encoding into repo_config_values
@ 2026-10-21 15:56 Okhuomon Ajayi
  2026-10-21 20:44 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Okhuomon Ajayi @ 2026-10-21 15:56 UTC (permalink / raw)
  To: git
  Cc: christian.couder, usmanakinyemi202, siddharthasthana31,
	kaartic.sivaraam, okhuomonajayi54

Move i18n.commitEncoding from the global git_commit_encoding variable
into struct repo_config_values.

Initialize and release the value with the other repository config
values.

Pass the repository to write_commit_tree().

Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
---
 commit.c      | 16 ++++++++++------
 environment.c | 15 +++++++++++----
 environment.h |  4 +++-
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/commit.c b/commit.c
index ad26f0b40a..91287289f4 100644
--- a/commit.c
+++ b/commit.c
@@ -1683,7 +1683,8 @@ N_("Warning: commit message did not conform to UTF-8.\n"
    "You may want to amend it after fixing the message, or set the config\n"
    "variable i18n.commitEncoding to the encoding your project uses.\n");
 
-static void write_commit_tree(struct strbuf *buffer, const char *msg, size_t msg_len,
+static void write_commit_tree(struct repository *r,
+			      struct strbuf *buffer, const char *msg, size_t msg_len,
 			      const struct object_id *tree,
 			      const struct object_id *parents, size_t parents_len,
 			      const char *author, const char *committer,
@@ -1691,9 +1692,10 @@ static void write_commit_tree(struct strbuf *buffer, const char *msg, size_t msg
 {
 	int encoding_is_utf8;
 	size_t i;
+	const char *encoding = repo_config_values(r)->commit_encoding;
 
 	/* Not having i18n.commitencoding is the same as having utf-8 */
-	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
+	encoding_is_utf8 = is_encoding_utf8(encoding);
 
 	strbuf_grow(buffer, 8192); /* should avoid reallocs for the headers */
 	strbuf_addf(buffer, "tree %s\n", oid_to_hex(tree));
@@ -1714,7 +1716,7 @@ static void write_commit_tree(struct strbuf *buffer, const char *msg, size_t msg
 		committer = git_committer_info(IDENT_STRICT);
 	strbuf_addf(buffer, "committer %s\n", committer);
 	if (!encoding_is_utf8)
-		strbuf_addf(buffer, "encoding %s\n", git_commit_encoding);
+		strbuf_addf(buffer, "encoding %s\n", encoding);
 
 	while (extra) {
 		add_extra_header(buffer, extra);
@@ -1734,6 +1736,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 			 const struct commit_extra_header *extra)
 {
 	struct repository *r = the_repository;
+	const char *encoding = repo_config_values(r)->commit_encoding;
 	int result = 0;
 	int encoding_is_utf8;
 	bool warned = false;
@@ -1744,7 +1747,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 	size_t i, nparents;
 
 	/* Not having i18n.commitencoding is the same as having utf-8 */
-	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
+	encoding_is_utf8 = is_encoding_utf8(encoding);
 
 	odb_assert_oid_type(the_repository->objects, tree, OBJ_TREE);
 
@@ -1757,7 +1760,8 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 	for (const struct commit_list *p = parents; p; p = p->next)
 		oidcpy(&parent_buf[i++], &p->item->object.oid);
 
-	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
+	write_commit_tree(r, &buffer, msg, msg_len, tree, parent_buf,
+			  nparents, author, committer, extra);
 
 	/* And check the encoding. */
 	if (encoding_is_utf8 && !ensure_utf8(&buffer)) {
@@ -1793,7 +1797,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 			free(mapped_parents);
 			goto out;
 		}
-		write_commit_tree(&compat_buffer, msg, msg_len, &mapped_tree,
+		write_commit_tree(r, &compat_buffer, msg, msg_len, &mapped_tree,
 				  mapped_parents, nparents, author, committer, compat_extra);
 		free_commit_extra_headers(compat_extra);
 		free(mapped_parents);
diff --git a/environment.c b/environment.c
index c83cf44839..632fbfad55 100644
--- a/environment.c
+++ b/environment.c
@@ -43,7 +43,6 @@ static int zlib_compression_seen;
 
 int minimum_abbrev = 4, default_abbrev = -1;
 int assume_unchanged;
-char *git_commit_encoding;
 char *git_log_output_encoding;
 int fsync_object_files = -1;
 int use_fsync = -1;
@@ -222,7 +221,9 @@ const char *get_log_output_encoding(void)
 
 const char *get_commit_output_encoding(void)
 {
-	return git_commit_encoding ? git_commit_encoding : "UTF-8";
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
+	return cfg->commit_encoding ? cfg->commit_encoding : "UTF-8";
 }
 
 int use_optional_locks(void)
@@ -597,9 +598,11 @@ static int git_default_sparse_config(const char *var, const char *value)
 
 static int git_default_i18n_config(const char *var, const char *value)
 {
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
 	if (!strcmp(var, "i18n.commitencoding")) {
-		FREE_AND_NULL(git_commit_encoding);
-		return git_config_string(&git_commit_encoding, var, value);
+		FREE_AND_NULL(cfg->commit_encoding);
+		return git_config_string(&cfg->commit_encoding, var, value);
 	}
 
 	if (!strcmp(var, "i18n.logoutputencoding")) {
@@ -783,6 +786,9 @@ void repo_config_values_init(struct repo_config_values *cfg)
 
 	/* section "branch" config values */
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
+
+	/* section "i18n" config values */
+	cfg->commit_encoding = NULL;
 }
 
 void repo_config_values_clear(struct repo_config_values *cfg)
@@ -794,4 +800,5 @@ void repo_config_values_clear(struct repo_config_values *cfg)
 	FREE_AND_NULL(cfg->askpass_program);
 	FREE_AND_NULL(cfg->apply_default_whitespace);
 	FREE_AND_NULL(cfg->apply_default_ignorewhitespace);
+	FREE_AND_NULL(cfg->commit_encoding);
 }
diff --git a/environment.h b/environment.h
index b336459e90..5b9c445146 100644
--- a/environment.h
+++ b/environment.h
@@ -147,6 +147,9 @@ struct repo_config_values {
 
 	/* section "branch" config values */
 	enum branch_track branch_track;
+
+	/* section "i18n" config values */
+	char *commit_encoding;
 };
 
 struct repo_config_values *repo_config_values(struct repository *repo);
@@ -242,7 +245,6 @@ extern int grafts_keep_true_parents;
 const char *get_log_output_encoding(void);
 const char *get_commit_output_encoding(void);
 
-extern char *git_commit_encoding;
 extern char *git_log_output_encoding;
 
 /*

base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Outreachy PATCH] environment: move commit_encoding into repo_config_values
  2026-10-21 15:56 [Outreachy PATCH] environment: move commit_encoding into repo_config_values Okhuomon Ajayi
@ 2026-10-21 20:44 ` Junio C Hamano
  2026-10-21 21:20   ` Okhuomon Ajayi
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2026-10-21 20:44 UTC (permalink / raw)
  To: Okhuomon Ajayi
  Cc: git, christian.couder, usmanakinyemi202, siddharthasthana31,
	kaartic.sivaraam

Okhuomon Ajayi <okhuomonajayi54@gmail.com> writes:

> Move i18n.commitEncoding from the global git_commit_encoding variable
> into struct repo_config_values.
>
> Initialize and release the value with the other repository config
> values.
>
> Pass the repository to write_commit_tree().
>
> Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
> ---
>  commit.c      | 16 ++++++++++------
>  environment.c | 15 +++++++++++----
>  environment.h |  4 +++-
>  3 files changed, 24 insertions(+), 11 deletions(-)

I am not sure if this change makes sense.  The 'git_commit_encoding'
variable is declared in 'environment.h' to be globally available and
defined in 'environment.c', but its only user is the 'commit.c' file.

I do not think it is worth contaminating the repository configuration
with it.  Instead, shouldn't we allow the functions that use this
global to take a "const char *git_commit_encoding" parameter (instead
of a 'repository_config' or a 'repository' structure) and pass it
through?  The callers may need to query the config subsystem to learn
the appropriate encoding for the repository they are working with
before calling these functions, but there is no need to pass the
entire repository structure down the call graph.

Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Outreachy PATCH] environment: move commit_encoding into repo_config_values
  2026-10-21 20:44 ` Junio C Hamano
@ 2026-10-21 21:20   ` Okhuomon Ajayi
  0 siblings, 0 replies; 3+ messages in thread
From: Okhuomon Ajayi @ 2026-10-21 21:20 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, christian.couder, usmanakinyemi202, siddharthasthana31,
	kaartic.sivaraam

On Wed, Oct 21, 2026 at 9:44 PM Junio C Hamano <gitster@pobox.com> wrote:
> I do not think it is worth contaminating the repository configuration
> with it.  Instead, shouldn't we allow the functions that use this
> global to take a "const char *git_commit_encoding" parameter (instead
> of a 'repository_config' or a 'repository' structure) and pass it
> through?  The callers may need to query the config subsystem to learn
> the appropriate encoding for the repository they are working with
> before calling these functions, but there is no need to pass the
> entire repository structure down the call graph.

Thanks, that makes sense.

So for v2, write_commit_tree() and commit_tree_extended() would both
take the encoding as a "const char *" parameter. The callers of
commit_tree_extended() would look up i18n.commitEncoding from the
config and pass it in, and commit_tree() would do the same for its
own callers. Nothing would be added to repo_config_values.

Does that match what you had in mind?

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-21 21:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-21 15:56 [Outreachy PATCH] environment: move commit_encoding into repo_config_values Okhuomon Ajayi
2026-10-21 20:44 ` Junio C Hamano
2026-10-21 21:20   ` Okhuomon Ajayi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox