pr-git-1497/chooglen/config/no-global-v2
tagged this
30 May 18:42
Thanks for the review on the previous round! This v2 is mostly a reorganization of v1, largely based off Jonathan's feedback in [1]. As such, nearly all of v1's cover letter still applies (except for the "Patch Overview" section [2]) so this cover letter omits those bits that have already been covered. The first non-RFC version will get rerolled with full cover letter. Since a lot of this series removes the config_reader stuff from gc/config-parsing-cleanup, it might be useful to diff this with "master" right before that: git diff 0a8c337394 HEAD -- config.c = Changes since v1 * Reorganize patches in a (hopefully) easier-to-review way. * Squash bugs in builtin/config.c that became apparent during the refactor. These could have been fixed in v1, but they dropped off my radar. = Patch overview * 1-5/14 add the "kvi" parameter to the config_fn_t signature. These are mostly unchanged from v1. * 6-11/14 converts the config.c machinery off "config_reader.config_kvi" and "config_reader.source" and onto the new "kvi" arg. Most of the changes from v1 are here. In v1, we converted all of the config.c machinery first, making the "kvi" arg available everywhere before the refactor. Thus we could convert all callers of the current_*() API to the "kvi" arg in a single step. However (as Jonathan rightfully pointed out), some of the changes to the machinery are non-trivial, and it's quite difficult to spot bugs in the intermediate patches. In v2, we convert the config.c machinery from "config_reader" to "kvi" one-by-one: configsets, then files, then CLI. To exercise the "kvi" arg as soon as possible, we convert from current_*() to "kvi" as soon as it is available. For example, in 6/14 "kvi" is available only in configsets, so we convert the current_*() call sites that are only reached via configsets and leave the others untouched. This means that we have a mix of current_*() and "kvi" in the middle, but auditing the changes is relatively easy (compared to v1's machinery changes), since you only need to verify that a callback isn't relying on the "kvi" arg before it is available, and that current_*() and "kvi" give the same value. * 8-9/14 squashes some bugs where builtin/config.c was calling the current_*() API outside of config callbacks. The "kvi" plumbing doesn't just make the bugs apparent, it also provides an obvious way to fix the bugs (by injecting "kvi" into the right places in builtin/config.c). These would have been nontrivial to fix if we were still using global state. * 12-14/14 remove config_reader by taking advantage of the "kvi" parameter and doing some other light plumbing. [1] https://lore.kernel.org/git/20230505210702.3359841-1-jonathantanmy@google.com [2] https://lore.kernel.org/pull.1497.git.git.1682104398.gitgitgadget@gmail.com Glen Choo (14): config: inline git_color_default_config urlmatch.h: use config_fn_t type (RFC-only) config: add kvi arg to config_fn_t (RFC-only) config: apply cocci to config_fn_t implementations (RFC-only) config: finish config_fn_t refactor config.c: pass kvi in configsets config: provide kvi with config files builtin/config.c: test misuse of format_config() config.c: provide kvi with CLI config trace2: plumb config kvi config: pass kvi to die_bad_number() config.c: remove config_reader from configsets config: add kvi.path, use it to evaluate includes config: pass source to config_parser_event_fn_t alias.c | 3 +- archive-tar.c | 5 +- archive-zip.c | 1 + builtin/add.c | 5 +- builtin/blame.c | 5 +- builtin/branch.c | 8 +- builtin/cat-file.c | 5 +- builtin/checkout.c | 7 +- builtin/clean.c | 9 +- builtin/clone.c | 10 +- builtin/column.c | 3 +- builtin/commit-graph.c | 3 +- builtin/commit.c | 20 +- builtin/config.c | 65 ++- builtin/difftool.c | 5 +- builtin/fetch.c | 12 +- builtin/fsmonitor--daemon.c | 11 +- builtin/grep.c | 12 +- builtin/help.c | 5 +- builtin/index-pack.c | 9 +- builtin/log.c | 12 +- builtin/merge.c | 7 +- builtin/multi-pack-index.c | 1 + builtin/pack-objects.c | 19 +- builtin/patch-id.c | 5 +- builtin/pull.c | 5 +- builtin/push.c | 5 +- builtin/read-tree.c | 5 +- builtin/rebase.c | 5 +- builtin/receive-pack.c | 15 +- builtin/reflog.c | 7 +- builtin/remote.c | 12 +- builtin/repack.c | 5 +- builtin/reset.c | 5 +- builtin/send-pack.c | 5 +- builtin/show-branch.c | 8 +- builtin/stash.c | 5 +- builtin/submodule--helper.c | 3 +- builtin/tag.c | 9 +- builtin/var.c | 5 +- builtin/worktree.c | 5 +- bundle-uri.c | 9 +- color.c | 8 - color.h | 6 +- compat/mingw.c | 3 +- compat/mingw.h | 4 +- config.c | 538 +++++++----------- config.h | 54 +- connect.c | 4 +- .../coccinelle/config_fn_kvi.pending.cocci | 146 +++++ contrib/coccinelle/git_config_number.cocci | 27 + convert.c | 4 +- credential.c | 1 + delta-islands.c | 3 +- diff.c | 19 +- diff.h | 7 +- fetch-pack.c | 5 +- fmt-merge-msg.c | 7 +- fmt-merge-msg.h | 3 +- fsck.c | 11 +- fsck.h | 4 +- git-compat-util.h | 2 + gpg-interface.c | 6 +- grep.c | 7 +- grep.h | 4 +- help.c | 10 +- http.c | 15 +- ident.c | 3 +- ident.h | 4 +- imap-send.c | 7 +- ll-merge.c | 1 + ls-refs.c | 2 +- mailinfo.c | 5 +- notes-utils.c | 3 +- notes.c | 3 +- pager.c | 5 +- pretty.c | 1 + promisor-remote.c | 4 +- remote.c | 7 +- revision.c | 3 +- scalar.c | 3 +- sequencer.c | 28 +- setup.c | 17 +- submodule-config.c | 31 +- submodule-config.h | 3 +- t/helper/test-config.c | 21 +- t/helper/test-userdiff.c | 4 +- t/t1300-config.sh | 27 + trace2.c | 4 +- trace2.h | 3 +- trace2/tr2_cfg.c | 12 +- trace2/tr2_sysenv.c | 3 +- trace2/tr2_tgt.h | 4 +- trace2/tr2_tgt_event.c | 4 +- trace2/tr2_tgt_normal.c | 4 +- trace2/tr2_tgt_perf.c | 4 +- trailer.c | 2 + upload-pack.c | 18 +- urlmatch.c | 7 +- urlmatch.h | 8 +- worktree.c | 2 +- xdiff-interface.c | 5 +- xdiff-interface.h | 5 +- 103 files changed, 883 insertions(+), 642 deletions(-) create mode 100644 contrib/coccinelle/config_fn_kvi.pending.cocci create mode 100644 contrib/coccinelle/git_config_number.cocci base-commit: 9857273be005833c71e2d16ba48e193113e12276 Submitted-As: https://lore.kernel.org/git/pull.1497.v2.git.git.1685472132.gitgitgadget@gmail.com In-Reply-To: https://lore.kernel.org/git/pull.1497.git.git.1682104398.gitgitgadget@gmail.com
Assets 2
-
2023-05-30T18:42:14Z -
2023-05-30T18:42:14Z - Loading