{"thread":{"id":"62795","subject":"[PATCH] meson: ensure correct version-def.h is used","startedAt":"2025-01-13T10:29:05Z","lastAt":"2025-01-16T16:02:18Z","messageCount":8,"participants":["Toon Claes","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"510409","messageId":"20250113-toon-fix-meson-version-v1-1-9637e2be32e3@iotcl.com","threadId":"62795","inReplyTo":null,"subject":"[PATCH] meson: ensure correct version-def.h is used","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-01-13T10:28:04Z","receivedAt":"2025-01-13T10:29:05Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"To build the libgit-version library, Meson first generates\n`version-def.h` in the build directory. Then it compiles `version.c`\ninto a library. During compilation, Meson tells to include both the\nbuild directory and the project root directory.\n\nHowever, when the user previously has compiled Git using Make, they will\nhave a `version-def.h` file in project root directory as well. Because\n`version-def.h` is included in `version.c` using the #include directive\nwith double quotes, some compilers will look for the header file in the\nsame directory as the source file. This will cause compilation of\n`version.c` ran by Meson to include `version-def.h` previously made by\nMake, which might be out of date.\n\nCopy `version.c` to the build directory before compiling it to ensure\n`version-def.h` from the build directory is used.\n\nSigned-off-by: Toon Claes \n---\n---\n meson.build | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/meson.build b/meson.build\nindex 0064eb64f546a6349a8694ce251bd352febda6fe..8ecb22c80e4fc3f194e97c14dbf83f541d72b25b 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1486,11 +1486,15 @@ version_def_h = custom_target(\n env: version_gen_environment,\n )\n \n+# Because most compilers prefer header files in the same directory as the source\n+# file, copy version.c to the build directory.\n+version_c = fs.copyfile(meson.current_source_dir() / 'version.c', 'version.c')\n+\n # Build a separate library for \"version.c\" so that we do not have to rebuild\n # everything when the current Git commit changes.\n libgit_version_library = static_library('git-version',\n sources: [\n- 'version.c',\n+ version_c,\n version_def_h,\n ],\n c_args: libgit_c_args,\n\n---\n\nbase-commit: fbe8d3079d4a96aeb4e4529cc93cc0043b759a05\nchange-id: 20250113-toon-fix-meson-version-3d4d33fdabe3\n\nThanks\n--\nToon\n\n"},{"id":"510410","messageId":"Z4Tv37SXzKrPwd_M@pks.im","threadId":"62795","inReplyTo":"20250113-toon-fix-meson-version-v1-1-9637e2be32e3@iotcl.com","subject":"Re: [PATCH] meson: ensure correct version-def.h is used","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-13T10:50:13Z","receivedAt":"2025-01-13T10:50:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jan 13, 2025 at 11:28:04AM +0100, Toon Claes wrote:\n> To build the libgit-version library, Meson first generates\n> `version-def.h` in the build directory. Then it compiles `version.c`\n> into a library. During compilation, Meson tells to include both the\n> build directory and the project root directory.\n> \n> However, when the user previously has compiled Git using Make, they will\n> have a `version-def.h` file in project root directory as well. Because\n> `version-def.h` is included in `version.c` using the #include directive\n> with double quotes, some compilers will look for the header file in the\n> same directory as the source file. This will cause compilation of\n> `version.c` ran by Meson to include `version-def.h` previously made by\n> Make, which might be out of date.\n\nMakes sense.\n\n> Copy `version.c` to the build directory before compiling it to ensure\n> `version-def.h` from the build directory is used.\n\nI was wondering whether there were other solutions that are a bit\nless intricate. One was to include instead and then play\naround with include directories, but that feels even more fragile than\nthe proposed solution.\n\nAnother alternative would be to inject the full path to the generated\nheader file. For example something like this:\n\ndiff --git a/meson.build b/meson.build\nindex 3e31648dc1..dbe6e7651f 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1543,7 +1543,9 @@ libgit_version_library = static_library('git-version',\n 'version.c',\n version_def_h,\n ],\n- c_args: libgit_c_args,\n+ c_args: libgit_c_args + [\n+ '-DGIT_VERSION_H=\"' + version_def_h.full_path() + '\"',\n+ ],\n dependencies: libgit_dependencies,\n include_directories: libgit_include_directories,\n )\ndiff --git a/version.c b/version.c\nindex 4d763ab48d..4786c4e0a5 100644\n--- a/version.c\n+++ b/version.c\n@@ -1,8 +1,13 @@\n #include \"git-compat-util.h\"\n #include \"version.h\"\n-#include \"version-def.h\"\n #include \"strbuf.h\"\n \n+#ifndef GIT_VERSION_H\n+# include \"version-def.h\"\n+#else\n+# include GIT_VERSION_H\n+#endif\n+\n const char git_version_string[] = GIT_VERSION;\n const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;\n\nThis feels least fragile and isn't adding a lot of complexity, either.\n\n> diff --git a/meson.build b/meson.build\n> index 0064eb64f546a6349a8694ce251bd352febda6fe..8ecb22c80e4fc3f194e97c14dbf83f541d72b25b 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1486,11 +1486,15 @@ version_def_h = custom_target(\n> env: version_gen_environment,\n> )\n> \n> +# Because most compilers prefer header files in the same directory as the source\n> +# file, copy version.c to the build directory.\n> +version_c = fs.copyfile(meson.current_source_dir() / 'version.c', 'version.c')\n\nUnfortunately, `fs.copyfile()` is not supported in Meson v0.61 yet,\nwhich is our minimum required version of Meson. You can use a custom\ntarget with cp(1) though. I have a patch series pending that starts to\ngenerate errors in our CI when warnings are printed to catch such issues\ngoing forward.\n\nPatrick\n"},{"id":"510433","messageId":"xmqqr056abry.fsf@gitster.g","threadId":"62795","inReplyTo":"20250113-toon-fix-meson-version-v1-1-9637e2be32e3@iotcl.com","subject":"Re: [PATCH] meson: ensure correct version-def.h is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-13T17:01:21Z","receivedAt":"2025-01-13T17:01:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> have a `version-def.h` file in project root directory as well. Because\n> `version-def.h` is included in `version.c` using the #include directive\n> with double quotes, some compilers will look for the header file in the\n> same directory as the source file.\n\nWhat happens if we use to include (which is how C\nstandard tells us to do), with an explicit include path specified\nwith -I? If it solves the issue, that may be a better\napproach.\n\nThanks.\n\n"},{"id":"510438","messageId":"87bjwak4p7.fsf@iotcl.com","threadId":"62795","inReplyTo":"xmqqr056abry.fsf@gitster.g","subject":"Re: [PATCH] meson: ensure correct version-def.h is used","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-01-13T17:24:04Z","receivedAt":"2025-01-13T17:24:23Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Junio C Hamano writes:\n\n> What happens if we use to include (which is how C\n> standard tells us to do), with an explicit include path specified\n> with -I? If it solves the issue, that may be a better\n> approach.\n\nI don't have a good source, but for example Wikipedia[1] says:\n\n Some preprocessors locate the include file differently based on the\n enclosing delimiters; treating a path in double-quotes as relative\n to the including file and a path in angle brackets as located in one\n of the directories of the configured system search path.\n\nSo behavior seems to depend on the implementation of the compiler. I'm\nnot sure we can trust all architectures to do what we expect. Or,\nbecause I don't expect many people to use Make and Meson at the same\ntime, do we not consider this an issue for most anyway?\n\n[1]: https://en.wikipedia.org/wiki/Include_directive#C/C++\n\n\n--\nToon\n"},{"id":"510480","messageId":"Z4YJkUg_DB0yyARs@pks.im","threadId":"62795","inReplyTo":"87bjwak4p7.fsf@iotcl.com","subject":"Re: [PATCH] meson: ensure correct version-def.h is used","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-14T06:52:07Z","receivedAt":"2025-01-14T06:52:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jan 13, 2025 at 06:24:04PM +0100, Toon Claes wrote:\n> Junio C Hamano writes:\n> \n> > What happens if we use to include (which is how C\n> > standard tells us to do), with an explicit include path specified\n> > with -I? If it solves the issue, that may be a better\n> > approach.\n> \n> I don't have a good source, but for example Wikipedia[1] says:\n> \n> Some preprocessors locate the include file differently based on the\n> enclosing delimiters; treating a path in double-quotes as relative\n> to the including file and a path in angle brackets as located in one\n> of the directories of the configured system search path.\n> \n> So behavior seems to depend on the implementation of the compiler. I'm\n> not sure we can trust all architectures to do what we expect.\n\nEven if we could it still feels somewhat fragile. The top-level source\ndirectory gets added to our include paths, as well, and consequently it\nmay also be found via . So things would depend on the\norder of \"-I\" directives now. Which makes me lean into the direction of\nmy proposed workaround, to optionally inject an absolute path.\n\n> Or, because I don't expect many people to use Make and Meson at the\n> same time, do we not consider this an issue for most anyway?\n\nIn the current phase I think it's still quite likely that people use\nboth at the same time, so fixing it would be nice.\n\nPatrick\n"},{"id":"510495","messageId":"20250114-toon-fix-meson-version-v2-1-66ddb1a82c28@iotcl.com","threadId":"62795","inReplyTo":"20250113-toon-fix-meson-version-v1-1-9637e2be32e3@iotcl.com","subject":"[PATCH v2] meson: ensure correct version-def.h is used","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-01-14T11:15:23Z","receivedAt":"2025-01-14T11:15:42Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"To build the libgit-version library, Meson first generates\n`version-def.h` in the build directory. Then it compiles `version.c`\ninto a library. During compilation, Meson tells to include both the\nbuild directory and the project root directory.\n\nHowever, when the user previously has compiled Git using Make, they will\nhave a `version-def.h` file in project root directory as well. Because\n`version-def.h` is included in `version.c` using the #include directive\nwith double quotes, some preprocessors will look for the header file in\nthe same directory as the source file. This will cause compilation of\n`version.c` ran by Meson to include `version-def.h` previously made by\nMake, which might be out of date.\n\nTo explicitly tell the preprocessor which `version-def.h` to use, pass\nthe absolute path of this file as macro GIT_VERSION_H to the\npreprocessor using option `-D` and have `version.c` `#include\nGIT_VERSION_H`. To remain working with other build systems than Meson,\ninclude \"version-def.h\" if that macro is not defined.\n\nCo-authored-by: Patrick Steinhardt \nSigned-off-by: Toon Claes \n---\nThis is version 2 of the patch trying to ensure the Meson build does not\ninclude `version-def.h` created by Make.\n---\nChanges in v2:\n- Instead of copying `version.c` to the Meson build directory, define a\n macro with the absolute path of `version-def.h` to include.\n- Link to v1: https://lore.kernel.org/r/20250113-toon-fix-meson-version-v1-1-9637e2be32e3@iotcl.com\n---\nRange-diff versus v1:\n\n1: fad8163ff8 ! 1: c1ca203451 meson: ensure correct version-def.h is used\n @@ Commit message\n However, when the user previously has compiled Git using Make, they will\n have a `version-def.h` file in project root directory as well. Because\n `version-def.h` is included in `version.c` using the #include directive\n - with double quotes, some compilers will look for the header file in the\n - same directory as the source file. This will cause compilation of\n + with double quotes, some preprocessors will look for the header file in\n + the same directory as the source file. This will cause compilation of\n `version.c` ran by Meson to include `version-def.h` previously made by\n Make, which might be out of date.\n \n - Copy `version.c` to the build directory before compiling it to ensure\n - `version-def.h` from the build directory is used.\n + To explicitly tell the preprocessor which `version-def.h` to use, pass\n + the absolute path of this file as macro GIT_VERSION_H to the\n + preprocessor using option `-D` and have `version.c` `#include\n + GIT_VERSION_H`. To remain working with other build systems than Meson,\n + include \"version-def.h\" if that macro is not defined.\n \n + Co-authored-by: Patrick Steinhardt \n Signed-off-by: Toon Claes \n \n ## meson.build ##\n -@@ meson.build: version_def_h = custom_target(\n - env: version_gen_environment,\n +@@ meson.build: libgit_version_library = static_library('git-version',\n + 'version.c',\n + version_def_h,\n + ],\n +- c_args: libgit_c_args,\n ++ c_args: libgit_c_args + [\n ++ '-DGIT_VERSION_H=\"' + version_def_h.full_path() + '\"',\n ++ ],\n + dependencies: libgit_dependencies,\n + include_directories: libgit_include_directories,\n )\n +\n + ## version.c ##\n +@@\n + #include \"git-compat-util.h\"\n + #include \"version.h\"\n +-#include \"version-def.h\"\n + #include \"strbuf.h\"\n \n -+# Because most compilers prefer header files in the same directory as the source\n -+# file, copy version.c to the build directory.\n -+version_c = fs.copyfile(meson.current_source_dir() / 'version.c', 'version.c')\n ++#ifndef GIT_VERSION_H\n ++# include \"version-def.h\"\n ++#else\n ++# include GIT_VERSION_H\n ++#endif\n +\n - # Build a separate library for \"version.c\" so that we do not have to rebuild\n - # everything when the current Git commit changes.\n - libgit_version_library = static_library('git-version',\n - sources: [\n -- 'version.c',\n -+ version_c,\n - version_def_h,\n - ],\n - c_args: libgit_c_args,\n + const char git_version_string[] = GIT_VERSION;\n + const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;\n + \n---\n meson.build | 4 +++-\n version.c | 7 ++++++-\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex 0064eb64f546a6349a8694ce251bd352febda6fe..db27afa99986598aab22ada718f76a7a49238f24 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1493,7 +1493,9 @@ libgit_version_library = static_library('git-version',\n 'version.c',\n version_def_h,\n ],\n- c_args: libgit_c_args,\n+ c_args: libgit_c_args + [\n+ '-DGIT_VERSION_H=\"' + version_def_h.full_path() + '\"',\n+ ],\n dependencies: libgit_dependencies,\n include_directories: libgit_include_directories,\n )\ndiff --git a/version.c b/version.c\nindex 4d763ab48dd76c0445e5ea390ff4c1f35c1a4b12..4786c4e0a54093ca947da27f8b712bd1ea351203 100644\n--- a/version.c\n+++ b/version.c\n@@ -1,8 +1,13 @@\n #include \"git-compat-util.h\"\n #include \"version.h\"\n-#include \"version-def.h\"\n #include \"strbuf.h\"\n \n+#ifndef GIT_VERSION_H\n+# include \"version-def.h\"\n+#else\n+# include GIT_VERSION_H\n+#endif\n+\n const char git_version_string[] = GIT_VERSION;\n const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;\n \n\n---\n\nbase-commit: fbe8d3079d4a96aeb4e4529cc93cc0043b759a05\nchange-id: 20250113-toon-fix-meson-version-3d4d33fdabe3\n\nThanks\n--\nToon\n\n"},{"id":"510652","messageId":"Z4jUsd77AFih24OW@pks.im","threadId":"62795","inReplyTo":"20250114-toon-fix-meson-version-v2-1-66ddb1a82c28@iotcl.com","subject":"Re: [PATCH v2] meson: ensure correct version-def.h is used","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-16T09:43:20Z","receivedAt":"2025-01-16T09:43:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Jan 14, 2025 at 12:15:23PM +0100, Toon Claes wrote:\n> diff --git a/meson.build b/meson.build\n> index 0064eb64f546a6349a8694ce251bd352febda6fe..db27afa99986598aab22ada718f76a7a49238f24 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1493,7 +1493,9 @@ libgit_version_library = static_library('git-version',\n> 'version.c',\n> version_def_h,\n> ],\n> - c_args: libgit_c_args,\n> + c_args: libgit_c_args + [\n> + '-DGIT_VERSION_H=\"' + version_def_h.full_path() + '\"',\n> + ],\n> dependencies: libgit_dependencies,\n> include_directories: libgit_include_directories,\n> )\n> diff --git a/version.c b/version.c\n> index 4d763ab48dd76c0445e5ea390ff4c1f35c1a4b12..4786c4e0a54093ca947da27f8b712bd1ea351203 100644\n> --- a/version.c\n> +++ b/version.c\n> @@ -1,8 +1,13 @@\n> #include \"git-compat-util.h\"\n> #include \"version.h\"\n> -#include \"version-def.h\"\n> #include \"strbuf.h\"\n> \n> +#ifndef GIT_VERSION_H\n> +# include \"version-def.h\"\n> +#else\n> +# include GIT_VERSION_H\n> +#endif\n> +\n> const char git_version_string[] = GIT_VERSION;\n> const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;\n\nYup, this looks as expected. I'm fine with the solution (well,\nobviously, I proposed it), so this looks good to me.\n\nThanks!\n\nPatrick\n"},{"id":"510709","messageId":"xmqqsepiu4yj.fsf@gitster.g","threadId":"62795","inReplyTo":"Z4jUsd77AFih24OW@pks.im","subject":"Re: [PATCH v2] meson: ensure correct version-def.h is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-16T15:57:24Z","receivedAt":"2025-01-16T16:02:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n>> diff --git a/version.c b/version.c\n>> index 4d763ab48dd76c0445e5ea390ff4c1f35c1a4b12..4786c4e0a54093ca947da27f8b712bd1ea351203 100644\n>> --- a/version.c\n>> +++ b/version.c\n>> @@ -1,8 +1,13 @@\n>> #include \"git-compat-util.h\"\n>> #include \"version.h\"\n>> -#include \"version-def.h\"\n>> #include \"strbuf.h\"\n>> \n>> +#ifndef GIT_VERSION_H\n>> +# include \"version-def.h\"\n>> +#else\n>> +# include GIT_VERSION_H\n>> +#endif\n>> +\n>> const char git_version_string[] = GIT_VERSION;\n>> const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;\n>\n> Yup, this looks as expected. I'm fine with the solution (well,\n> obviously, I proposed it), so this looks good to me.\n\nExcellent. Thanks, both of you.\n"}]}