From b62cc7b361eff60b72f16530d484fa47d5700937 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Tue, 12 Nov 2024 14:50:57 -0800 Subject: [PATCH] meson: prepare for include rearrangement Context: we have include paths that are "types.hh" and similarly common names. We currently have these compatibly available as "lix/libutil/types.hh" externally but *not yet internally*. This is because we don't have any way for the src directory to appear as `"lix/"` from inside of Lix: the lix/ include directory is created by the install process. The goal of this whole thing is to make it clearer which component of Lix that files are a part of, which should hopefully help at least a little bit to new developers. One disadvantage of un-mixing these is that it will cause some API changes if we ever move a file between libraries, but that is not very common, and we don't care that much about external API users. This was planned for a while and is why we have a FixIncludes check to begin with. Personally I don't see a great benefit in rearranging our source code, and in fact, it would probably be counterproductive: - Moving the includes into a separate `include/` directory would just make developers have to deal with more directories, when we can already generate the desired layout through the build process. - This would also decouple the .cc and .hh files which currently conventionally have each others' definitions and declarations respectively, right next to each other, making it easier for them to feel decoupled and diverge. Content: Add ../include as an include directory so that lix/ in include paths will resolve to src/ within Lix itself, just as it does externally today. This prepares for a further series of commits applying the actual change to each library one-by-one by accepting both include versions at once. This could have been done with ../ and a symlink called lix, but we would like to not accept libexpr/foo.hh internally for it would be broken externally, so we need an otherwise empty directory for the include. Change-Id: Ideac17faadae2bcea2dffbab34eb27c582ede399 --- doc/manual/rl-next/include-rearrangement.md | 20 ++++++++++++++++++++ src/include/lix | 1 + src/libcmd/meson.build | 5 ++++- src/libexpr/meson.build | 6 ++++-- src/libfetchers/meson.build | 5 +++-- src/libmain/meson.build | 3 ++- src/libstore/meson.build | 6 ++++-- src/libutil/meson.build | 6 ++++-- subprojects/lix-clang-tidy/FixIncludes.cc | 4 +++- 9 files changed, 45 insertions(+), 11 deletions(-) create mode 100644 doc/manual/rl-next/include-rearrangement.md create mode 120000 src/include/lix diff --git a/doc/manual/rl-next/include-rearrangement.md b/doc/manual/rl-next/include-rearrangement.md new file mode 100644 index 000000000..370c40450 --- /dev/null +++ b/doc/manual/rl-next/include-rearrangement.md @@ -0,0 +1,20 @@ +--- +synopsis: "Includes are now qualified with library name" +category: Development +cls: [2178] +credits: jade +--- + +The Lix includes have all been rearranged to be of the form `"lix/libexpr/foo.hh"` instead of `"foo.hh"`. +This was already supported externally for a migration period, but it is now being applied to all the internal usages within Lix itself. +The goal of this change is to both clarify where a file is from and to avoid polluting global include paths with things like `config.h` that might conflict with other projects. + +For other details, see the release notes of Lix 2.90.0, under "Rename all the libraries" in Breaking Changes. + +To fix an external project with sources in `src` which has a separate build directory (such that headers are in `../src` relative to where the compiler is running): + +``` +lix_root=$HOME/lix +(cd $lix_root && nix develop -c 'meson setup build && ninja -C build subprojects/lix-clang-tidy/liblix-clang-tidy.so') +run-clang-tidy -checks='-*,lix-fixincludes' -load=$lix_root/build/subprojects/lix-clang-tidy/liblix-clang-tidy.so -p build/ -header-filter '\.\./src/.*\.h' -fix src +``` diff --git a/src/include/lix b/src/include/lix new file mode 120000 index 000000000..a96aa0ea9 --- /dev/null +++ b/src/include/lix @@ -0,0 +1 @@ +.. \ No newline at end of file diff --git a/src/libcmd/meson.build b/src/libcmd/meson.build index 82071b3b2..dd66cf3fa 100644 --- a/src/libcmd/meson.build +++ b/src/libcmd/meson.build @@ -53,6 +53,8 @@ libcmd = library( nlohmann_json, liblix_doc, ], + # '../include' for self references like "lix/libcmd/*.hh" + include_directories : [ '../include' ], cpp_pch : cpp_pch, install : true, # FIXME(Qyriad): is this right? @@ -69,7 +71,8 @@ custom_target( ) liblixcmd = declare_dependency( - include_directories : '.', + # FIXME(jade): remove old '.' include directory + include_directories : include_directories('.', '../include'), link_with : libcmd, ) diff --git a/src/libexpr/meson.build b/src/libexpr/meson.build index 02a335cbd..3efdca022 100644 --- a/src/libexpr/meson.build +++ b/src/libexpr/meson.build @@ -278,6 +278,7 @@ libexpr = library( ], # for shared.hh include_directories : [ + '../', '../libmain', ], cpp_pch : cpp_pch, @@ -293,7 +294,8 @@ install_headers( ) liblixexpr = declare_dependency( - include_directories : include_directories('.'), + # FIXME(jade): remove old '.' include directory + include_directories : include_directories('.', '../include'), sources : libexpr_settings_header, link_with : libexpr, ) @@ -301,7 +303,7 @@ liblixexpr = declare_dependency( # FIXME: remove when https://git.lix.systems/lix-project/lix/issues/359 is fixed. if is_static liblixexpr_mstatic = declare_dependency( - include_directories : include_directories('.'), + include_directories : include_directories('.', '../include'), sources : libexpr_settings_header, link_whole : libexpr, ) diff --git a/src/libfetchers/meson.build b/src/libfetchers/meson.build index e388e5498..3d06eebd4 100644 --- a/src/libfetchers/meson.build +++ b/src/libfetchers/meson.build @@ -55,6 +55,7 @@ libfetchers = library( liblixutil, nlohmann_json, ], + include_directories : [ '../include' ], cpp_pch : cpp_pch, install : true, # FIXME(Qyriad): is this right? @@ -78,7 +79,7 @@ configure_file( ) liblixfetchers = declare_dependency( - include_directories : include_directories('.'), + include_directories : include_directories('.', '../include'), sources : libfetchers_settings_header, link_with : libfetchers, ) @@ -86,7 +87,7 @@ liblixfetchers = declare_dependency( # FIXME: remove when https://git.lix.systems/lix-project/lix/issues/359 is fixed. if is_static liblixfetchers_mstatic = declare_dependency( - include_directories : include_directories('.'), + include_directories : include_directories('.', '../include'), sources : libfetchers_settings_header, link_whole : libfetchers, ) diff --git a/src/libmain/meson.build b/src/libmain/meson.build index a1a888c16..35633b611 100644 --- a/src/libmain/meson.build +++ b/src/libmain/meson.build @@ -22,6 +22,7 @@ libmain = library( liblixutil, liblixstore, ], + include_directories : [ '../include' ], cpp_pch : cpp_pch, install : true, # FIXME(Qyriad): is this right? @@ -31,7 +32,7 @@ libmain = library( install_headers(libmain_headers, subdir : 'lix/libmain', preserve_path : true) liblixmain = declare_dependency( - include_directories : include_directories('.'), + include_directories : include_directories('.', '../include'), link_with : libmain, ) diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 04bdf70dd..4c9ce5c63 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -349,6 +349,7 @@ libstore = library( libstore_sources, libstore_settings_headers, libstore_generated_headers, + include_directories : [ '../include' ], dependencies : dependencies, cpp_args : cpp_args, cpp_pch : cpp_pch, @@ -361,7 +362,8 @@ install_headers(libstore_headers, subdir : 'lix/libstore', preserve_path : true) # Used by libfetchers. liblixstore = declare_dependency( - include_directories : include_directories('.'), + # FIXME(jade): remove old '.' include directory + include_directories : include_directories('.', '../include'), sources : libstore_settings_headers, link_with : libstore, ) @@ -369,7 +371,7 @@ liblixstore = declare_dependency( # FIXME: remove when https://git.lix.systems/lix-project/lix/issues/359 is fixed. if is_static liblixstore_mstatic = declare_dependency( - include_directories : include_directories('.'), + include_directories : include_directories('.', '../include'), sources : libstore_settings_headers, link_whole : libstore, ) diff --git a/src/libutil/meson.build b/src/libutil/meson.build index 5aec64416..cd46f512a 100644 --- a/src/libutil/meson.build +++ b/src/libutil/meson.build @@ -292,6 +292,7 @@ libutil = library( openssl, nlohmann_json, ], + include_directories : [ '../include' ], cpp_pch : cpp_pch, implicit_include_directories : true, install : true, @@ -315,7 +316,8 @@ configure_file( # Used by libstore and libfetchers. liblixutil = declare_dependency( - include_directories : include_directories('.'), + # FIXME(jade): remove old "." include-dir + include_directories : include_directories('.', '../include'), sources : [ experimental_features_header, deprecated_features_header, @@ -327,7 +329,7 @@ liblixutil = declare_dependency( # FIXME: remove when https://git.lix.systems/lix-project/lix/issues/359 is fixed. if is_static liblixutil_mstatic = declare_dependency( - include_directories : include_directories('.'), + include_directories : include_directories('.', '../include'), sources : [ experimental_features_header, deprecated_features_header, diff --git a/subprojects/lix-clang-tidy/FixIncludes.cc b/subprojects/lix-clang-tidy/FixIncludes.cc index 602d3d355..2ebeaacdf 100644 --- a/subprojects/lix-clang-tidy/FixIncludes.cc +++ b/subprojects/lix-clang-tidy/FixIncludes.cc @@ -58,7 +58,9 @@ void FixIncludesCallbacks::InclusionDirective( }; for (const auto &SourceDir : SourceDirs) { - const bool IsAlreadyFixed = FileName.starts_with("lix/lib"); + // Ignore generated files, since they are often only used internally within + // a library anyhow and they are not in the normal source dir. + const bool IsAlreadyFixed = FileName.starts_with("lix/lib") || FileName.contains(".gen.") || FileName.ends_with(".md"); if (File && File->getNameAsRequested().contains(SourceDir) && !IsAlreadyFixed) { StringRef Name = File->getNameAsRequested();