| Message ID | pull.1493.v2.git.1679379968.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Header cleanups | expand |
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > * Patch 1: leftover cleanup from en/header-cleanups (now in master) that > should have been included in that series > * Patches 2-4: cleanups around gettext.h. Without these patches, many > gettext inclusions would have to be squashed into later patches and make > them harder to review. Also, the implicit dependency on gettext.h was > making it much harder to find other dependencies. > * Patch 5: move some global var declarations from cache.h into the header > associated with where the vars are declared > * Patches 6-16: move header declarations from cache.h into a header named > the same as the source file containing the actual functions (e.g. move > declarations for abspath.c functions from cache.h into abspath.h), and > reduce the dependencies on cache.h accordingly. Thanks for the effort. All the changes look good to me. (As for whether gettext.h should go into cache.h/git-compat-util.h, I think that individual files that use it should include it themselves, just as you have done in this patch set. This means, among other things, that we can reuse Git code in other projects that do not use gettext.)
Maintainer note: I dropped multiple changes from this series to avoid conflicts with other in-flight series, but has some small conflicts with seen: one semantic conflict with ed/fsmonitor-inotify (correct resolution: include abspath.h in the new file compat/fsmonitor/fsm-path-utils-linux.c) and a few conflicts with sg/parse-options-h-users (which was submitted after my series; correct resolution: just make sure all the headers are included from both sides). The motivating thrust of this series was trying to remove the inclusion of cache.h from strbuf.c. This series doesn't quite achieve that, because I dropped some changes to avoid conflicting with ed/fsmonitor-inotify, and to avoid conflicting with Ævar's series on the_repository cleanup. Overview of the series: * Patch 1: leftover cleanup from en/header-cleanups (now in master) that should have been included in that series * Patches 2-4: cleanups around gettext.h. Without these patches, many gettext inclusions would have to be squashed into later patches and make them harder to review. Also, the implicit dependency on gettext.h was making it much harder to find other dependencies. * Patch 5: move some global var declarations from cache.h into the header associated with where the vars are declared * Patches 6-16: move header declarations from cache.h into a header named the same as the source file containing the actual functions (e.g. move declarations for abspath.c functions from cache.h into abspath.h), and reduce the dependencies on cache.h accordingly. Changes since v1: * rebased on main (to avoid conflict with jk/bundle-use-dash-for-stdfiles; no more conflicts with next!) * cleaned up the wording in patch 4 * added more detail to this cover letter giving an overview of the series, and updating about conflicts Changes not incorporated: * I left patch 1 before patches 2-4, since it's purpose (leftover cleanup from en/header-cleanups) just means it makes more sense there for me. I don't know how to explain it in a way that makes sense if I move it. Notes: * As I mentioned in my last series, Dscho's gitgitgadget is super helpful for a series like this. * As with my previous series (en/header-cleanup), I used the scripts at my header-cleanup-scripts branch [https://github.com/newren/git/commit/db81c8d7fb554b3edf04320e218bddeb98bb6194], and some tweaks thereof, repeatedly while making this series (though they are prone to produce both false positive and false negatives, so if you use them, only use them to generate hints about which files to look at). Elijah Newren (16): treewide: remove unnecessary cache.h inclusion from a few headers treewide: be explicit about dependence on gettext.h treewide: remove unnecessary inclusion of gettext.h treewide: remove unnecessary cache.h inclusion from several sources environment: move comment_line_char from cache.h abspath.h: move absolute path functions from cache.h cache.h: remove expand_user_path() path.h: move function declarations for path.c functions from cache.h wrapper.h: move declarations for wrapper.c functions from cache.h treewide: remove unnecessary includes of cache.h environment.h: move declarations for environment.c functions from cache.h treewide: remove cache.h inclusion due to environment.h changes setup.h: move declarations for setup.c functions from cache.h treewide: remove cache.h inclusion due to setup.h changes write-or-die.h: move declarations for write-or-die.c functions from cache.h csum-file.h: remove unnecessary inclusion of cache.h abspath.c | 4 +- abspath.h | 33 ++ add-interactive.c | 1 + add-patch.c | 2 + apply.c | 5 + archive-tar.c | 2 + archive-zip.c | 2 + archive.c | 4 + attr.c | 3 + bisect.c | 2 + blame.c | 2 + branch.c | 2 + builtin/add.c | 1 + builtin/am.c | 4 + builtin/apply.c | 1 + builtin/archive.c | 1 + builtin/bisect.c | 3 + builtin/blame.c | 4 + builtin/branch.c | 3 + builtin/bugreport.c | 5 +- builtin/bundle.c | 3 + builtin/cat-file.c | 3 + builtin/check-attr.c | 4 + builtin/check-ignore.c | 2 + builtin/check-mailmap.c | 2 + builtin/check-ref-format.c | 1 + builtin/checkout--worker.c | 1 + builtin/checkout-index.c | 2 + builtin/checkout.c | 3 + builtin/clean.c | 3 + builtin/clone.c | 4 + builtin/column.c | 1 + builtin/commit-graph.c | 2 + builtin/commit-tree.c | 1 + builtin/commit.c | 2 + builtin/config.c | 5 + builtin/count-objects.c | 2 + builtin/credential-cache--daemon.c | 2 + builtin/credential-cache.c | 3 + builtin/credential-store.c | 2 + builtin/describe.c | 3 + builtin/diagnose.c | 2 + builtin/diff-index.c | 1 + builtin/diff-tree.c | 1 + builtin/diff.c | 2 + builtin/difftool.c | 5 + builtin/fast-export.c | 1 + builtin/fast-import.c | 4 + builtin/fetch-pack.c | 1 + builtin/fetch.c | 2 + builtin/fmt-merge-msg.c | 2 + builtin/for-each-ref.c | 1 + builtin/for-each-repo.c | 1 + builtin/fsck.c | 1 + builtin/fsmonitor--daemon.c | 3 + builtin/gc.c | 5 + builtin/get-tar-commit-id.c | 1 + builtin/grep.c | 3 + builtin/hash-object.c | 4 + builtin/help.c | 2 + builtin/hook.c | 1 + builtin/index-pack.c | 4 + builtin/init-db.c | 5 + builtin/interpret-trailers.c | 1 + builtin/log.c | 4 + builtin/ls-files.c | 2 + builtin/ls-remote.c | 1 + builtin/ls-tree.c | 1 + builtin/mailinfo.c | 3 + builtin/mailsplit.c | 1 + builtin/merge-base.c | 1 + builtin/merge-file.c | 3 + builtin/merge-recursive.c | 1 + builtin/merge-tree.c | 1 + builtin/merge.c | 4 + builtin/mktag.c | 1 + builtin/mktree.c | 1 + builtin/multi-pack-index.c | 3 + builtin/mv.c | 4 + builtin/name-rev.c | 2 + builtin/notes.c | 2 + builtin/pack-objects.c | 3 + builtin/pack-redundant.c | 1 + builtin/pack-refs.c | 1 + builtin/patch-id.c | 1 + builtin/prune-packed.c | 1 + builtin/prune.c | 2 + builtin/pull.c | 1 + builtin/push.c | 2 + builtin/range-diff.c | 1 + builtin/read-tree.c | 2 + builtin/rebase.c | 4 + builtin/receive-pack.c | 4 + builtin/reflog.c | 1 + builtin/remote.c | 1 + builtin/repack.c | 2 + builtin/replace.c | 2 + builtin/rerere.c | 2 + builtin/reset.c | 3 + builtin/rev-list.c | 2 + builtin/rev-parse.c | 4 + builtin/revert.c | 1 + builtin/rm.c | 2 + builtin/send-pack.c | 1 + builtin/shortlog.c | 3 + builtin/show-branch.c | 2 + builtin/show-index.c | 1 + builtin/show-ref.c | 1 + builtin/sparse-checkout.c | 3 + builtin/stash.c | 4 + builtin/stripspace.c | 3 + builtin/submodule--helper.c | 4 + builtin/symbolic-ref.c | 1 + builtin/tag.c | 3 + builtin/unpack-file.c | 1 + builtin/unpack-objects.c | 2 + builtin/update-index.c | 4 + builtin/update-ref.c | 1 + builtin/update-server-info.c | 1 + builtin/upload-pack.c | 1 + builtin/verify-commit.c | 1 + builtin/verify-pack.c | 1 + builtin/verify-tag.c | 1 + builtin/worktree.c | 4 + builtin/write-tree.c | 2 + bulk-checkin.c | 4 +- bulk-checkin.h | 2 +- bundle-uri.c | 2 + bundle.c | 3 + bundle.h | 1 - cache-tree.c | 3 +- cache.h | 571 ------------------- chdir-notify.c | 1 + chunk-format.c | 2 + color.c | 1 + combine-diff.c | 1 + commit-graph.c | 4 +- commit.c | 3 + common-main.c | 2 + compat/disk.h | 2 + compat/fsmonitor/fsm-health-win32.c | 1 + compat/fsmonitor/fsm-listen-darwin.c | 1 + compat/fsmonitor/fsm-listen-win32.c | 1 + compat/fsmonitor/fsm-path-utils-darwin.c | 1 + compat/fsmonitor/fsm-path-utils-win32.c | 1 + compat/linux/procinfo.c | 2 +- compat/mingw.c | 4 + compat/precompose_utf8.c | 2 + compat/simple-ipc/ipc-shared.c | 2 +- compat/simple-ipc/ipc-unix-socket.c | 1 + compat/simple-ipc/ipc-win32.c | 2 + compat/terminal.c | 4 +- config.c | 7 +- connect.c | 2 + connected.c | 3 +- convert.c | 2 + copy.c | 1 + credential.c | 2 + csum-file.c | 3 +- csum-file.h | 2 +- daemon.c | 4 + date.c | 1 + delta-islands.c | 3 +- diagnose.c | 2 + diff-lib.c | 1 + diff-no-index.c | 2 + diff.c | 5 + dir.c | 5 + editor.c | 3 + entry.c | 3 + environment.c | 5 + environment.h | 220 +++++++ exec-cmd.c | 3 + fetch-pack.c | 5 +- fmt-merge-msg.c | 3 +- fsck.c | 2 +- fsmonitor-ipc.c | 1 + fsmonitor.c | 1 + gettext.c | 4 +- git.c | 3 + gpg-interface.c | 6 +- grep.c | 3 +- http-backend.c | 5 +- http-fetch.c | 2 + http-push.c | 2 + imap-send.c | 3 + line-log.c | 2 +- list-objects-filter-options.h | 1 + list-objects-filter.c | 1 + list-objects.c | 1 + ll-merge.c | 1 + lockfile.c | 4 +- log-tree.c | 2 + ls-refs.c | 4 +- mailmap.c | 2 + merge-ort-wrappers.c | 1 + merge-ort.c | 2 + merge-recursive.c | 3 + merge.c | 1 + midx.c | 4 +- name-hash.c | 2 + negotiator/default.c | 2 +- negotiator/skipping.c | 2 +- notes-cache.c | 2 +- notes-merge.c | 2 + notes-utils.c | 4 +- notes.c | 1 + object-file.c | 7 +- object-name.c | 3 + object.c | 1 + pack-bitmap-write.c | 5 +- pack-bitmap.c | 3 +- pack-check.c | 1 + pack-mtimes.c | 3 +- pack-revindex.c | 1 + pack-write.c | 2 + pack.h | 2 + packfile.c | 5 +- packfile.h | 4 +- parallel-checkout.c | 2 + parse-options-cb.c | 2 + parse-options.c | 3 +- parse-options.h | 2 + patch-ids.c | 2 +- path.c | 6 +- path.h | 62 +- pathspec.c | 4 + pkt-line.c | 3 + preload-index.c | 2 + pretty.c | 2 + progress.c | 1 - promisor-remote.c | 1 + prompt.c | 3 +- protocol.c | 1 + prune-packed.c | 2 + range-diff.c | 2 + reachable.c | 3 +- read-cache.c | 3 + rebase-interactive.c | 3 + ref-filter.c | 4 +- ref-filter.h | 1 + reflog.c | 3 +- refs.c | 6 +- refs.h | 1 - refs/files-backend.c | 7 +- refs/iterator.c | 2 +- refs/packed-backend.c | 4 +- refspec.c | 1 + remote-curl.c | 4 + remote.c | 6 +- replace-object.c | 3 +- repository.c | 2 + rerere.c | 3 + rerere.h | 1 + reset.c | 1 + revision.c | 3 + run-command.c | 2 + scalar.c | 2 + send-pack.c | 4 +- sequencer.c | 4 + server-info.c | 4 +- setup.c | 4 + setup.h | 168 ++++++ shallow.c | 3 +- sideband.c | 2 + sparse-index.c | 2 + split-index.c | 1 + strbuf.c | 6 +- streaming.c | 2 + streaming.h | 4 +- submodule-config.c | 2 + submodule.c | 6 +- symlinks.c | 2 + t/helper/test-advise.c | 2 +- t/helper/test-bitmap.c | 3 +- t/helper/test-bloom.c | 4 +- t/helper/test-bundle-uri.c | 1 + t/helper/test-cache-tree.c | 2 + t/helper/test-config.c | 2 +- t/helper/test-delta.c | 2 +- t/helper/test-dump-cache-tree.c | 2 +- t/helper/test-dump-fsmonitor.c | 1 + t/helper/test-dump-split-index.c | 1 + t/helper/test-dump-untracked-cache.c | 1 + t/helper/test-fast-rebase.c | 5 +- t/helper/test-fsmonitor-client.c | 2 + t/helper/test-hash.c | 1 - t/helper/test-lazy-init-name-hash.c | 2 + t/helper/test-match-trees.c | 1 + t/helper/test-oid-array.c | 1 + t/helper/test-oidmap.c | 1 + t/helper/test-oidtree.c | 1 + t/helper/test-pack-mtimes.c | 2 +- t/helper/test-parse-pathspec-file.c | 1 - t/helper/test-partial-clone.c | 4 +- t/helper/test-path-utils.c | 3 + t/helper/test-pkt-line.c | 3 +- t/helper/test-proc-receive.c | 4 +- t/helper/test-progress.c | 1 - t/helper/test-reach.c | 3 + t/helper/test-read-cache.c | 2 + t/helper/test-read-graph.c | 2 +- t/helper/test-read-midx.c | 1 + t/helper/test-ref-store.c | 2 +- t/helper/test-repository.c | 3 +- t/helper/test-revision-walking.c | 2 +- t/helper/test-run-command.c | 1 - t/helper/test-scrap-cache-tree.c | 1 + t/helper/test-serve-v2.c | 3 +- t/helper/test-submodule-config.c | 1 + t/helper/test-submodule-nested-repo-config.c | 2 +- t/helper/test-submodule.c | 2 +- t/helper/test-subprocess.c | 2 +- t/helper/test-trace2.c | 2 +- t/helper/test-userdiff.c | 2 +- t/helper/test-write-cache.c | 1 + tag.c | 2 + tempfile.c | 4 +- tmp-objdir.c | 2 + trace.c | 4 + trace2/tr2_cfg.c | 4 +- trace2/tr2_cmd_name.c | 3 +- trace2/tr2_dst.c | 4 +- trace2/tr2_sid.c | 3 +- trailer.c | 4 +- transport-helper.c | 3 + transport.c | 4 +- transport.h | 1 - tree-walk.c | 1 + unpack-trees.c | 3 + upload-pack.c | 3 + usage.c | 4 +- walker.c | 1 + worktree.c | 5 + wrapper.c | 3 + wrapper.h | 36 ++ write-or-die.c | 4 +- write-or-die.h | 78 +++ wt-status.c | 3 + xdiff-interface.c | 2 +- 340 files changed, 1311 insertions(+), 669 deletions(-) create mode 100644 abspath.h create mode 100644 setup.h create mode 100644 wrapper.h create mode 100644 write-or-die.h base-commit: e25cabbf6b34e4a6e903d65102d87055cc994778 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1493%2Fnewren%2Fheader-cleanup-2-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1493/newren/header-cleanup-2-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1493 Range-diff vs v1: 1: 88959aba6f1 = 1: a1be63930c4 treewide: remove unnecessary cache.h inclusion from a few headers 2: b771199fae3 = 2: 6e80396e861 treewide: be explicit about dependence on gettext.h 3: a31269a26b3 = 3: 644d305c669 treewide: remove unnecessary inclusion of gettext.h 4: 3c193bef1d0 ! 4: 95b86aecb43 treewide: remove unnecessary cache.h inclusion from several sources @@ Commit message A number of files were apparently including cache.h solely to get gettext.h. By making those files explicitly include gettext.h, we can - already drop the include of cache.h in these files. Remove those now - unnecessary includes. + already drop the include of cache.h in these files. On top of that, + there were some files using cache.h that didn't need to for any reason. + Remove these unnecessary includes. Signed-off-by: Elijah Newren <newren@gmail.com> 5: 43457e44b4e = 5: 417b2ffa999 environment: move comment_line_char from cache.h 6: a0bbd2c9b48 ! 6: 1fbd957047d abspath.h: move absolute path functions from cache.h @@ abspath.h (new) + */ +char *prefix_filename(const char *prefix, const char *path); + ++/* Likewise, but path=="-" always yields "-" */ ++char *prefix_filename_except_for_dash(const char *prefix, const char *path); ++ +static inline int is_absolute_path(const char *path) +{ + return is_dir_sep(path[0]) || has_dos_drive_prefix(path); @@ cache.h: const char *setup_git_directory(void); - * prefix was empty). - */ -char *prefix_filename(const char *prefix, const char *path); +- +-/* Likewise, but path=="-" always yields "-" */ +-char *prefix_filename_except_for_dash(const char *prefix, const char *path); - int check_filename(const char *prefix, const char *name); void verify_filename(const char *prefix, 7: a0fc4aa2470 = 7: 805d0a86951 cache.h: remove expand_user_path() 8: 0f48e561b79 = 8: b0bfbe7f38c path.h: move function declarations for path.c functions from cache.h 9: 66f06731cab = 9: 43a61845240 wrapper.h: move declarations for wrapper.c functions from cache.h 10: f4e15c9e5c2 = 10: a6c7245219a treewide: remove unnecessary includes of cache.h 11: fa7449b5b99 = 11: 5ef1a4f1d6c environment.h: move declarations for environment.c functions from cache.h 12: 4e6a4030b1e = 12: 496d36e4fd0 treewide: remove cache.h inclusion due to environment.h changes 13: 9722988e537 = 13: 1d5cd115161 setup.h: move declarations for setup.c functions from cache.h 14: 0e84202da90 = 14: 30b21691265 treewide: remove cache.h inclusion due to setup.h changes 15: 626d089f10b = 15: 6e40d1d783d write-or-die.h: move declarations for write-or-die.c functions from cache.h 16: 38af06f55bc = 16: 72d58b391a1 csum-file.h: remove unnecessary inclusion of cache.h