[PATCH] meson: ensure correct version-def.h is used

[PATCH] meson: ensure correct version-def.h is used

[Toon Claes]

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(-)

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,

---

base-commit: fbe8d3079d4a96aeb4e4529cc93cc0043b759a05
change-id: 20250113-toon-fix-meson-version-3d4d33fdabe3

Thanks
--
Toon

Re: [PATCH] meson: ensure correct version-def.h is used

[Patrick Steinhardt]

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

Re: [PATCH] meson: ensure correct version-def.h is used

[Junio C Hamano]

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.

Re: [PATCH] meson: ensure correct version-def.h is used

[Toon Claes]

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

Re: [PATCH] meson: ensure correct version-def.h is used

[Patrick Steinhardt]

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

[PATCH v2] meson: ensure correct version-def.h is used

[Toon Claes]

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 preprocessors 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.

To explicitly tell the preprocessor which `version-def.h` to use, pass
the absolute path of this file as macro GIT_VERSION_H to the
preprocessor using option `-D` and have `version.c` `#include
GIT_VERSION_H`. To remain working with other build systems than Meson,
include "version-def.h" if that macro is not defined.

Co-authored-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Toon Claes <toon@iotcl.com>
---
This is version 2 of the patch trying to ensure the Meson build does not
include `version-def.h` created by Make.
---
Changes in v2:
- Instead of copying `version.c` to the Meson build directory, define a
  macro with the absolute path of `version-def.h` to include.
- Link to v1: https://lore.kernel.org/r/20250113-toon-fix-meson-version-v1-1-9637e2be32e3@iotcl.com
---
Range-diff versus v1:

1:  fad8163ff8 ! 1:  c1ca203451 meson: ensure correct version-def.h is used
    @@ Commit message
         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
    +    with double quotes, some preprocessors 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.
    +    To explicitly tell the preprocessor which `version-def.h` to use, pass
    +    the absolute path of this file as macro GIT_VERSION_H to the
    +    preprocessor using option `-D` and have `version.c` `#include
    +    GIT_VERSION_H`. To remain working with other build systems than Meson,
    +    include "version-def.h" if that macro is not defined.
     
    +    Co-authored-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Toon Claes <toon@iotcl.com>
     
      ## meson.build ##
    -@@ meson.build: version_def_h = custom_target(
    -   env: version_gen_environment,
    +@@ meson.build: 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,
      )
    +
    + ## version.c ##
    +@@
    + #include "git-compat-util.h"
    + #include "version.h"
    +-#include "version-def.h"
    + #include "strbuf.h"
      
    -+# 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')
    ++#ifndef GIT_VERSION_H
    ++# include "version-def.h"
    ++#else
    ++# include GIT_VERSION_H
    ++#endif
     +
    - # 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,
    + const char git_version_string[] = GIT_VERSION;
    + const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;
    + 
---
 meson.build | 4 +++-
 version.c   | 7 ++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 0064eb64f546a6349a8694ce251bd352febda6fe..db27afa99986598aab22ada718f76a7a49238f24 100644
--- a/meson.build
+++ b/meson.build
@@ -1493,7 +1493,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 4d763ab48dd76c0445e5ea390ff4c1f35c1a4b12..4786c4e0a54093ca947da27f8b712bd1ea351203 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;
 

---

base-commit: fbe8d3079d4a96aeb4e4529cc93cc0043b759a05
change-id: 20250113-toon-fix-meson-version-3d4d33fdabe3

Thanks
--
Toon

Re: [PATCH v2] meson: ensure correct version-def.h is used

[Patrick Steinhardt]

On Tue, Jan 14, 2025 at 12:15:23PM +0100, Toon Claes wrote:
> diff --git a/meson.build b/meson.build
> index 0064eb64f546a6349a8694ce251bd352febda6fe..db27afa99986598aab22ada718f76a7a49238f24 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1493,7 +1493,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 4d763ab48dd76c0445e5ea390ff4c1f35c1a4b12..4786c4e0a54093ca947da27f8b712bd1ea351203 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;

Yup, this looks as expected. I'm fine with the solution (well,
obviously, I proposed it), so this looks good to me.

Thanks!

Patrick

Re: [PATCH v2] meson: ensure correct version-def.h is used

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> diff --git a/version.c b/version.c
>> index 4d763ab48dd76c0445e5ea390ff4c1f35c1a4b12..4786c4e0a54093ca947da27f8b712bd1ea351203 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;
>
> Yup, this looks as expected. I'm fine with the solution (well,
> obviously, I proposed it), so this looks good to me.

Excellent.  Thanks, both of you.