config: warn on core.commentString=auto

index d25cc07a35..f821fdcfcc 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1783,6 +1783,9 @@ int cmd_commit(int argc, show_usage_with_options_if_asked(argc, argv, builtin_commit_usage, builtin_commit_options); +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ prepare_repo_settings(the_repository); the_repository->settings.command_requires_full_index = 0;

index dc4cb8fb14..794cb7bb26 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1378,6 +1378,9 @@ int cmd_merge(int argc, show_usage_with_options_if_asked(argc, argv, builtin_merge_usage, builtin_merge_options); +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ prepare_repo_settings(the_repository); the_repository->settings.command_requires_full_index = 0;

index 72a52bdfb9..962917ec48 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1242,6 +1242,9 @@ int cmd_rebase(int argc, builtin_rebase_usage, builtin_rebase_options); +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ prepare_repo_settings(the_repository); the_repository->settings.command_requires_full_index = 0;

index e07c2217fe..b197848bb0 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -4,6 +4,7 @@ #include "builtin.h" #include "parse-options.h" #include "diff.h" +#include "environment.h" #include "gettext.h" #include "revision.h" #include "rerere.h" @@ -285,6 +286,9 @@ int cmd_revert(int argc, struct replay_opts opts = REPLAY_OPTS_INIT; int res; +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ opts.action = REPLAY_REVERT; sequencer_init_config(&opts); res = run_sequencer(argc, argv, prefix, &opts); @@ -302,6 +306,9 @@ struct repository *repo UNUSED) struct replay_opts opts = REPLAY_OPTS_INIT; int res; +#ifndef WITH_BREAKING_CHANGES + warn_on_auto_comment_char = true; +#endif /* !WITH_BREAKING_CHANGES */ opts.action = REPLAY_PICK; sequencer_init_config(&opts); res = run_sequencer(argc, argv, prefix, &opts);

index 97ffef4270..18b4219709 100644 --- a/config.c +++ b/config.c @@ -11,6 +11,7 @@ #include "date.h" #include "branch.h" #include "config.h" +#include "dir.h" #include "parse.h" #include "convert.h" #include "environment.h" @@ -1951,10 +1952,110 @@ int git_configset_get_pathname(struct config_set *set, const char *key, char **d return 1; } +struct comment_char_config { + unsigned last_key_id; + bool auto_set; +}; + +#define COMMENT_CHAR_CFG_INIT { 0 } + +static const char *comment_key_name(unsigned id) +{ + static const char *name[] = { + "core.commentChar", + "core.commentString", + }; + + if (id >= ARRAY_SIZE(name)) + BUG("invalid comment key id"); + + return name[id]; +} + +static void comment_char_callback(const char *key, const char *value, + const struct config_context *ctx UNUSED, + void *data) +{ + struct comment_char_config *config = data; + unsigned key_id; + + if (!strcmp(key, "core.commentchar")) + key_id = 0; + else if (!strcmp(key, "core.commentstring")) + key_id = 1; + else + return; + + config->last_key_id = key_id; + config->auto_set = value && !strcmp(value, "auto"); +} + +struct repo_config { + struct repository *repo; + struct comment_char_config comment_char_config; +}; + +#define REPO_CONFIG_INIT(repo_) { \ + .comment_char_config = COMMENT_CHAR_CFG_INIT, \ + .repo = repo_, \ + }; + +#ifdef WITH_BREAKING_CHANGES +static void check_auto_comment_char_config(struct comment_char_config *config) +{ + if (!config->auto_set) + return; + + die_message(_("Support for '%s=auto' has been removed in Git 3.0"), + comment_key_name(config->last_key_id)); + die(NULL); +} +#else +static void check_auto_comment_char_config(struct comment_char_config *config) +{ + extern bool warn_on_auto_comment_char; + const char *DEPRECATED_CONFIG_ENV = + "GIT_AUTO_COMMENT_CHAR_CONFIG_WARNING_GIVEN"; + + if (!config->auto_set || !warn_on_auto_comment_char) + return; + + /* + * Use an environment variable to ensure that subprocesses do not repeat + * the warning. + */ + if (git_env_bool(DEPRECATED_CONFIG_ENV, false)) + return; + + setenv(DEPRECATED_CONFIG_ENV, "true", true); + + warning(_("Support for '%s=auto' is deprecated and will be removed in " + "Git 3.0"), comment_key_name(config->last_key_id)); +} +#endif /* WITH_BREAKING_CHANGES */ + +static void check_deprecated_config(struct repo_config *config) +{ + if (!config->repo->check_deprecated_config) + return; + + check_auto_comment_char_config(&config->comment_char_config); +} + +static int repo_config_callback(const char *key, const char *value, + const struct config_context *ctx, void *data) +{ + struct repo_config *config = data; + + comment_char_callback(key, value, ctx, &config->comment_char_config); + return config_set_callback(key, value, ctx, config->repo->config); +} + /* Functions use to read configuration from a repository */ static void repo_read_config(struct repository *repo) { struct config_options opts = { 0 }; + struct repo_config config = REPO_CONFIG_INIT(repo); opts.respect_includes = 1; opts.commondir = repo->commondir; @@ -1966,8 +2067,8 @@ static void repo_read_config(struct repository *repo) git_configset_clear(repo->config); git_configset_init(repo->config); - if (config_with_options(config_set_callback, repo->config, NULL, - repo, &opts) < 0) + if (config_with_options(repo_config_callback, &config, NULL, repo, + &opts) < 0) /* * config_with_options() normally returns only * zero, as most errors are fatal, and @@ -1980,6 +2081,7 @@ static void repo_read_config(struct repository *repo) * immediately. */ die(_("unknown error occurred while reading the configuration files")); + check_deprecated_config(&config); } static void git_config_check_init(struct repository *repo) @@ -2667,6 +2769,14 @@ int repo_config_set_multivar_in_file_gently(struct repository *r, char *contents = NULL; size_t contents_sz; struct config_store_data store = CONFIG_STORE_INIT; + bool saved_check_deprecated_config = r->check_deprecated_config; + + /* + * Do not warn or die if there are deprecated config settings as + * we want the user to be able to change those settings by running + * "git config". + */ + r->check_deprecated_config = false; validate_comment_string(comment); @@ -2898,6 +3008,7 @@ out_free: if (in_fd >= 0) close(in_fd); config_store_data_clear(&store); + r->check_deprecated_config = saved_check_deprecated_config; return ret; write_err_out:

index 4c87876d48..1ffa2ff30b 100644 --- a/environment.c +++ b/environment.c @@ -124,6 +124,7 @@ const char *comment_line_str = "#"; char *comment_line_str_to_free; #ifndef WITH_BREAKING_CHANGES int auto_comment_line_char; +bool warn_on_auto_comment_char; #endif /* !WITH_BREAKING_CHANGES */ /* This is set by setup_git_directory_gently() and/or git_default_config() */

index e75c4abb38..51898c99cd 100644 --- a/environment.h +++ b/environment.h @@ -210,6 +210,7 @@ extern const char *comment_line_str; extern char *comment_line_str_to_free; #ifndef WITH_BREAKING_CHANGES extern int auto_comment_line_char; +extern bool warn_on_auto_comment_char; #endif /* !WITH_BREAKING_CHANGES */ # endif /* USE_THE_REPOSITORY_VARIABLE */

index ecd691181f..8af73923d3 100644 --- a/repository.c +++ b/repository.c @@ -57,6 +57,7 @@ void initialize_repository(struct repository *repo) repo->parsed_objects = parsed_object_pool_new(repo); ALLOC_ARRAY(repo->index, 1); index_state_init(repo->index, repo); + repo->check_deprecated_config = true; /* * When a command runs inside a repository, it learns what

index 042dc93f0f..5808a5d610 100644 --- a/repository.h +++ b/repository.h @@ -161,6 +161,9 @@ struct repository { /* Indicate if a repository has a different 'commondir' from 'gitdir' */ unsigned different_commondir:1; + + /* Should repo_config() check for deprecated settings */ + bool check_deprecated_config; }; #ifdef USE_THE_REPOSITORY_VARIABLE

index ce0aebb9a7..3b2a46c25c 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1184,8 +1184,13 @@ test_expect_success !WITH_BREAKING_CHANGES 'rebase -i respects core.commentchar= test_when_finished "git rebase --abort || :" && ( test_set_editor "$(pwd)/copy-edit-script.sh" && - git rebase -i HEAD^ + git rebase -i HEAD^ 2>err ) && + sed -n "s/^warning: //p" err >actual && + cat >expect <<-EOF && + Support for ${SQ}core.commentChar=auto${SQ} is deprecated and will be removed in Git 3.0 + EOF + test_cmp expect actual && test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)" '

index 65b4519a71..a9dc1e416d 100755 --- a/t/t7502-commit-porcelain.sh +++ b/t/t7502-commit-porcelain.sh @@ -958,7 +958,12 @@ test_expect_success 'commit --status with custom comment character' ' test_expect_success !WITH_BREAKING_CHANGES 'switch core.commentchar' ' test_commit "#foo" foo && - GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend && + GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend 2>err && + sed -n "s/^warning: //p" err >actual && + cat >expect <<-EOF && + Support for ${SQ}core.commentChar=auto${SQ} is deprecated and will be removed in Git 3.0 + EOF + test_cmp expect actual && test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG ' @@ -982,4 +987,14 @@ EOF ) ' +test_expect_success WITH_BREAKING_CHANGES 'core.commentChar=auto is rejected' ' + test_config core.commentChar auto && + test_must_fail git rev-parse --git-dir 2>err && + sed -n "s/^fatal: //p" err >actual && + cat >expect <<-EOF && + Support for ${SQ}core.commentChar=auto${SQ} has been removed in Git 3.0 + EOF + test_cmp expect actual +' + test_done