| Message ID | 20250113-toon-fix-meson-version-v1-1-9637e2be32e3@iotcl.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | meson: ensure correct version-def.h is used | expand |
On Mon, Jan 13, 2025 at 11:28:04AM +0100, Toon Claes wrote: > To build the libgit-version library, Meson first generates > `version-def.h` in the build directory. Then it compiles `version.c` > into a library. During compilation, Meson tells to include both the > build directory and the project root directory. > > However, when the user previously has compiled Git using Make, they will > have a `version-def.h` file in project root directory as well. Because > `version-def.h` is included in `version.c` using the #include directive > with double quotes, some compilers will look for the header file in the > same directory as the source file. This will cause compilation of > `version.c` ran by Meson to include `version-def.h` previously made by > Make, which might be out of date. Makes sense. > Copy `version.c` to the build directory before compiling it to ensure > `version-def.h` from the build directory is used. I was wondering whether there were other solutions that are a bit less intricate. One was to include <version-def.h> instead and then play around with include directories, but that feels even more fragile than the proposed solution. Another alternative would be to inject the full path to the generated header file. For example something like this: diff --git a/meson.build b/meson.build index 3e31648dc1..dbe6e7651f 100644 --- a/meson.build +++ b/meson.build @@ -1543,7 +1543,9 @@ libgit_version_library = static_library('git-version', 'version.c', version_def_h, ], - c_args: libgit_c_args, + c_args: libgit_c_args + [ + '-DGIT_VERSION_H="' + version_def_h.full_path() + '"', + ], dependencies: libgit_dependencies, include_directories: libgit_include_directories, ) diff --git a/version.c b/version.c index 4d763ab48d..4786c4e0a5 100644 --- a/version.c +++ b/version.c @@ -1,8 +1,13 @@ #include "git-compat-util.h" #include "version.h" -#include "version-def.h" #include "strbuf.h" +#ifndef GIT_VERSION_H +# include "version-def.h" +#else +# include GIT_VERSION_H +#endif + const char git_version_string[] = GIT_VERSION; const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT; This feels least fragile and isn't adding a lot of complexity, either. > diff --git a/meson.build b/meson.build > index 0064eb64f546a6349a8694ce251bd352febda6fe..8ecb22c80e4fc3f194e97c14dbf83f541d72b25b 100644 > --- a/meson.build > +++ b/meson.build > @@ -1486,11 +1486,15 @@ version_def_h = custom_target( > env: version_gen_environment, > ) > > +# Because most compilers prefer header files in the same directory as the source > +# file, copy version.c to the build directory. > +version_c = fs.copyfile(meson.current_source_dir() / 'version.c', 'version.c') Unfortunately, `fs.copyfile()` is not supported in Meson v0.61 yet, which is our minimum required version of Meson. You can use a custom target with cp(1) though. I have a patch series pending that starts to generate errors in our CI when warnings are printed to catch such issues going forward. Patrick
Toon Claes <toon@iotcl.com> writes: > have a `version-def.h` file in project root directory as well. Because > `version-def.h` is included in `version.c` using the #include directive > with double quotes, some compilers will look for the header file in the > same directory as the source file. What happens if we use <version-def.h> to include (which is how C standard tells us to do), with an explicit include path specified with -I<directory>? If it solves the issue, that may be a better approach. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > What happens if we use <version-def.h> to include (which is how C > standard tells us to do), with an explicit include path specified > with -I<directory>? If it solves the issue, that may be a better > approach. I don't have a good source, but for example Wikipedia[1] says: Some preprocessors locate the include file differently based on the enclosing delimiters; treating a path in double-quotes as relative to the including file and a path in angle brackets as located in one of the directories of the configured system search path. So behavior seems to depend on the implementation of the compiler. I'm not sure we can trust all architectures to do what we expect. Or, because I don't expect many people to use Make and Meson at the same time, do we not consider this an issue for most anyway? [1]: https://en.wikipedia.org/wiki/Include_directive#C/C++ -- Toon
On Mon, Jan 13, 2025 at 06:24:04PM +0100, Toon Claes wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > What happens if we use <version-def.h> to include (which is how C > > standard tells us to do), with an explicit include path specified > > with -I<directory>? If it solves the issue, that may be a better > > approach. > > I don't have a good source, but for example Wikipedia[1] says: > > Some preprocessors locate the include file differently based on the > enclosing delimiters; treating a path in double-quotes as relative > to the including file and a path in angle brackets as located in one > of the directories of the configured system search path. > > So behavior seems to depend on the implementation of the compiler. I'm > not sure we can trust all architectures to do what we expect. Even if we could it still feels somewhat fragile. The top-level source directory gets added to our include paths, as well, and consequently it may also be found via <version-def.h>. So things would depend on the order of "-I" directives now. Which makes me lean into the direction of my proposed workaround, to optionally inject an absolute path. > Or, because I don't expect many people to use Make and Meson at the > same time, do we not consider this an issue for most anyway? In the current phase I think it's still quite likely that people use both at the same time, so fixing it would be nice. Patrick
diff --git a/meson.build b/meson.build index 0064eb64f546a6349a8694ce251bd352febda6fe..8ecb22c80e4fc3f194e97c14dbf83f541d72b25b 100644 --- a/meson.build +++ b/meson.build @@ -1486,11 +1486,15 @@ version_def_h = custom_target( env: version_gen_environment, ) +# Because most compilers prefer header files in the same directory as the source +# file, copy version.c to the build directory. +version_c = fs.copyfile(meson.current_source_dir() / 'version.c', 'version.c') + # Build a separate library for "version.c" so that we do not have to rebuild # everything when the current Git commit changes. libgit_version_library = static_library('git-version', sources: [ - 'version.c', + version_c, version_def_h, ], c_args: libgit_c_args,
To build the libgit-version library, Meson first generates `version-def.h` in the build directory. Then it compiles `version.c` into a library. During compilation, Meson tells to include both the build directory and the project root directory. However, when the user previously has compiled Git using Make, they will have a `version-def.h` file in project root directory as well. Because `version-def.h` is included in `version.c` using the #include directive with double quotes, some compilers will look for the header file in the same directory as the source file. This will cause compilation of `version.c` ran by Meson to include `version-def.h` previously made by Make, which might be out of date. Copy `version.c` to the build directory before compiling it to ensure `version-def.h` from the build directory is used. Signed-off-by: Toon Claes <toon@iotcl.com> --- --- meson.build | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) --- base-commit: fbe8d3079d4a96aeb4e4529cc93cc0043b759a05 change-id: 20250113-toon-fix-meson-version-3d4d33fdabe3 Thanks -- Toon