cmake: adapt to cache.h being no more
#1544
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…che-h-part-3 * en/header-split-cache-h-part-2: (22 commits) reftable: ensure git-compat-util.h is the first (indirect) include diff.h: reduce unnecessary includes object-store.h: reduce unnecessary includes commit.h: reduce unnecessary includes fsmonitor: reduce includes of cache.h cache.h: remove unnecessary headers treewide: remove cache.h inclusion due to previous changes cache,tree: move basic name compare functions from read-cache to tree cache,tree: move cmp_cache_name_compare from tree.[ch] to read-cache.c hash-ll.h: split out of hash.h to remove dependency on repository.h tree-diff.c: move S_DIFFTREE_IFXMIN_NEQ define from cache.h dir.h: move DTYPE defines from cache.h versioncmp.h: move declarations for versioncmp.c functions from cache.h ws.h: move declarations for ws.c functions from cache.h match-trees.h: move declarations for match-trees.c functions from cache.h pkt-line.h: move declarations for pkt-line.c functions from cache.h base85.h: move declarations for base85.c functions from cache.h copy.h: move declarations for copy.c functions from cache.h server-info.h: move declarations for server-info.c functions from cache.h packfile.h: move pack_window and pack_entry from cache.h ...
The comments in create_default_files() talks about reading config from
the config file in the specified `--templates` directory, which leads to
the question of whether core.bare could be set in such a config file and
thus whether the code is doing the right thing. It turns out, that it
doesn't; it unconditionally ignores core.bare in the config file in any
--templates directory. It is not clear to me that fixing it can be done
within this function; it seems to occur too late:
* create_default_files() is called by init_db()
* init_db() is called by both builtin/{clone.c,init-db.c}
* both callers of init_db() call set_git_work_tree() before init_db()
and in order to actual affect whether a repository is bear, we'd need to
somewhere reset these values, not just the is_bare_repository_cfg
setting.
I do not want to open this can of worms at this time; I'm trying to
clean up some headers, for which I need to move some functions, for
which I need to clean up some globals, and that's far enough down the
rabbit hole. So, simply document the issue with a careful TODO comment
and a few testcases.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit was prompted by a desire to move the functions which builtin/init-db.c and builtin/clone.c share out of the former file and into setup.c. One issue that made it difficult was the init_is_bare_repository global variable. init_is_bare_repository's sole use in life it to cache a value in init_db(), and then be used in create_default_files(). This is a bit odd since init_db() directly calls create_default_files(), and is the only caller of that function. Convert the global to a simple function parameter instead. (Of course, this doesn't fix the fact that this value is then ignored by create_default_files(), as noted in a big TODO comment in that function, but it at least includes no behavioral change other than getting rid of a very questionable global variable.) Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Much like the parent commit, this commit was prompted by a desire to move the functions which builtin/init-db.c and builtin/clone.c share out of the former file and into setup.c. A secondary issue that made it difficult was the init_shared_repository global variable; replace it with a simple parameter that is passed to the relevant functions. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The functions init_db() and initialize_repository_version() were shared by builtin/init-db.c and builtin/clone.c, and declared in cache.h. Move these functions, plus their several helpers only used by these functions, to setup.[ch]. Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function overlay_tree_on_index(), plus associated helper functions, were defined in builtin/ls-files.c, but also shared with builtin/commit.c. Move these shared functions to read-cache.c. Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function add_files_to_cache() is used by all three of builtin/{add,
checkout, commit}.c. That suggests this is common library code, and
should be moved somewhere else, like read-cache.c. However, the
function and its helpers made use of two global variables that made
straight code movement difficult:
* the_index
* include_sparse
The latter was perhaps more problematic since it was only accessible in
builtin/add.c but was still affecting builtin/checkout.c and
builtin/commit.c without this fact being very clear from the code. I'm
not sure if the other two callers would want to add a `--sparse` flag
similar to add.c to get non-default behavior, but exposing this
dependence will help if we ever decide we do want to add such a flag.
Modify add_files_to_cache() and its helpers to accept the necessary
arguments instead of relying on globals.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function add_files_to_cache(), plus associated helper functions, were defined in builtin/add.c, but also shared with builtin/checkout.c and builtin/commit.c. Move these shared functions to read-cache.c. Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
These functions do not depend upon struct cache_entry or struct index_state in any way, and it seems more logical to break them out into this file, especially since statinfo.h already has the struct stat_data declaration. Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Note in particular that this reverses the decision made in 118a2e8 ("cache: move ensure_full_index() to cache.h", 2021-04-01). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We already have a preload-index.c file; move the declarations for the functions in that file into a new preload-index.h. These were previously split between cache.h and repository.h. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
the_index is a global variable defined in repository.c; as such, its declaration feels better suited living in repository.h rather than cache.h. Move it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
For the functions defined in read-cache.c, move their declarations from cache.h to a new header, read-cache-ll.h. Also move some related inline functions from cache.h to read-cache.h. The purpose of the read-cache-ll.h/read-cache.h split is that about 70% of the sites don't need the inline functions and the extra headers they include. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since this header showed up in some places besides just #include statements, update/clean-up/remove those other places as well. Note that compat/fsmonitor/fsm-path-utils-darwin.c previously got away with violating the rule that all files must start with an include of git-compat-util.h (or a short-list of alternate headers that happen to include it first). This change exposed the violation and caused it to stop building correctly; fix it by having it include git-compat-util.h first, as per policy. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This also made it clear that several .c files that depended upon path.h were missing a #include for it; add the missing includes while at it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This also made it clear that several .c files depended upon various things that oidset included, but had omitted the direct #include for those headers. Add those now. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This also made it clear that a few .c files under builtin/ were depending upon some headers but had forgotten to #include them. Add the missing direct includes while at it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The include of wildmatch.h in git-compat-util.h was added in cebcab1 (Makefile: add USE_WILDMATCH to use wildmatch as fnmatch, 2013-01-01) as a way to be able to compile-time force any calls to fnmatch() to instead invoke wildmatch(). The defines and inline function were removed in 70a8fc9 (stop using fnmatch (either native or compat), 2014-02-15), and this include in git-compat-util.h has been unnecessary ever since. Remove the include from git-compat-util.h, but add it to the .c files that had omitted the direct #include they needed. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A long term (but rather minor) pet-peeve of mine was the name ll-merge.[ch]. I thought it made it harder to realize what stuff was related to merging when I was working on the merge machinery and trying to improve it. Further, back in d1cbe1e ("hash-ll.h: split out of hash.h to remove dependency on repository.h", 2023-04-22), we have split the portions of hash.h that do not depend upon repository.h into a "hash-ll.h" (due to the recommendation to use "ll" for "low-level" in its name[1], but which I used as a suffix precisely because of my distaste for "ll-merge"). When we discussed adding additional "*-ll.h" files, a request was made that we use "ll" consistently as either a prefix or a suffix. Since it is already in use as both a prefix and a suffix, the only way to do so is to rename some files. Besides my distaste for the ll-merge.[ch] name, let me also note that the files ll-fsmonitor.h, ll-hash.h, ll-merge.h, ll-object-store.h, ll-read-cache.h would have essentially nothing to do with each other and make no sense to group. But giving them the common "ll-" prefix would group them. Using "-ll" as a suffix thus seems just much more logical to me. Rename ll-merge.[ch] to merge-ll.[ch] to achieve this consistency, and to ensure we get a more logical grouping of files. [1] https://lore.kernel.org/git/kl6lsfcu1g8w.fsf@chooglen-macbookpro.roam.corp.google.com/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
khash.h lets you instantiate custom hash types that map between two
types. These are defined as a struct, as you might expect, and khash
typedef's that to kh_foo_t. But it declares the struct anonymously,
which doesn't give a name to the struct type itself; there is no
"struct kh_foo". This has two small downsides:
- when using khash, we declare "kh_foo_t *the_foo". This is
unlike our usual naming style, which is "struct kh_foo *the_foo".
- you can't forward-declare a typedef of an unnamed struct type in
C. So we might do something like this in a header file:
struct kh_foo;
struct bar {
struct kh_foo *the_foo;
};
to avoid having to include the header that defines the real
kh_foo. But that doesn't work with the typedef'd name. Without the
"struct" keyword, the compiler doesn't know we mean that kh_foo is
a type.
So let's always give khash structs the name that matches our
conventions ("struct kh_foo" to match "kh_foo_t"). We'll keep doing
the typedef to retain compatibility with existing callers.
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The vast majority of files including object-store.h did not need dir.h
nor khash.h. Split the header into two files, and let most just depend
upon object-store-ll.h, while letting the two callers that need it
depend on the full object-store.h.
After this patch:
$ git grep -h include..object-store | sort | uniq -c
2 #include "object-store.h"
129 #include "object-store-ll.h"
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
oidhash() was used by both hashmap and khash, which makes sense. However, the location of this function in hashmap.[ch] meant that khash.h had to depend upon hashmap.h, making people unfamiliar with khash think that it was built upon hashmap. (Or at least, I personally was confused for a while about this in the past.) Move this function to hash-ll, so that khash.h can stop depending upon hashmap.h. This has another benefit as well: it allows us to remove hashmap.h's dependency on hash-ll.h. While some callers of hashmap.h were making use of oidhash, most were not, so this change provides another way to reduce the number of includes. Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This creates a new fsmonitor-ll.h with most of the functions from fsmonitor.h, though it leaves three inline functions where they were. Two-thirds of the files that previously included fsmonitor.h did not need those three inline functions or the six extra includes those inline functions required, so this allows them to only include the lower level header. Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a companion to 4275a22 (cache.h: remove this no-longer-used header, 2023-05-16). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
/submit |
|
Submitted as pull.1544.git.1686844054022.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> This is a companion to 4275a22e169c (cache.h: remove this no-longer-used
> header, 2023-05-16).
Thanks.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> cmake: adapt to cache.h being no more
>
> This is a companion to 4275a22e169c (cache.h: remove this no-longer-used
> header, 2023-05-16), based on en/header-split-cache-h-part-3.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1544%2Fdscho%2Fheader-split-cache-h-cmake-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1544/dscho/header-split-cache-h-cmake-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1544
>
> contrib/buildsystems/CMakeLists.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 2f6e0197ffa..4faa419cc3d 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -227,7 +227,7 @@ add_compile_definitions(GIT_HOST_CPU="${CMAKE_SYSTEM_PROCESSOR}")
> add_compile_definitions(SHA256_BLK INTERNAL_QSORT RUNTIME_PREFIX)
> add_compile_definitions(NO_OPENSSL SHA1_DC SHA1DC_NO_STANDARD_INCLUDES
> SHA1DC_INIT_SAFE_HASH_DEFAULT=0
> - SHA1DC_CUSTOM_INCLUDE_SHA1_C="cache.h"
> + SHA1DC_CUSTOM_INCLUDE_SHA1_C="git-compat-util.h"
> SHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="git-compat-util.h" )
> list(APPEND compat_SOURCES sha1dc_git.c sha1dc/sha1.c sha1dc/ubc_check.c block-sha1/sha1.c sha256/block/sha256.c compat/qsort_s.c)
>
>
> base-commit: d677f7e76abb837502c589d17889cf8ad591dc09 |
|
This branch is now known as |
|
This patch series was integrated into seen via git@3276934. |
|
This patch series was integrated into next via git@b138273. |
|
This patch series was integrated into seen via git@caf6f78. |
|
This patch series was integrated into seen via git@9fc101b. |
|
This patch series was integrated into seen via git@fe3d8e5. |
|
There was a status update in the "New Topics" section about the branch Build fix in en/header-split-cache-h-part-3 topic. Will merge to 'master'. source: <pull.1525.v3.git.1684218848.gitgitgadget@gmail.com> |
dscho
force-pushed
the
en/header-split-cache-h-part-3
branch
from
d677f7e
to
68d6864
Compare
|
There was a status update in the "Discarded" section about the branch Build fix in en/header-split-cache-h-part-3 topic. Ejected out of 'next' and made into a part of en/header-split-cache-h-part-3 source: <pull.1525.v3.git.1684218848.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@b138273. |
|
There was a status update in the "Discarded" section about the branch Build fix in en/header-split-cache-h-part-3 topic. Ejected out of 'next' and made into a part of en/header-split-cache-h-part-3 source: <pull.1525.v3.git.1684218848.gitgitgadget@gmail.com> |
This is part of bc5c5ec0446, which made it into Git's main branch in a1264a0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a companion to 4275a22 (cache.h: remove this no-longer-used header, 2023-05-16), based on
en/header-split-cache-h-part-3.