Hi Patrick
On 12/11/2024 17:03, Patrick Steinhardt wrote:
I had a read through to see what the meson syntax was like and it
seems to be quite nice. I've left a couple of comments below. It's
an impressive amount of work to get to this stage, it's a bit
daunting that replicating everything in our current Makefile is
even more work!
+# # Set up a build directory with 'address' and 'undefined' sanitizers
+# # using Clang.
+# $ CC=clang meson setup -Db_sanitize=address,undefined
Unfortunately when building with this the tests fail with
==217115==ASan runtime does not come first in initial library list; you should either link runtime to your application or manually preload it with LD_PRELOAD.
./test-lib.sh: line 1080: 217115 Aborted (core dumped) git switch -C primary
error: last command exited with $?=134
because we don't wire up -Db_sanitize to SANITIZE_ADDRESS in
GIT-BUILD-OPTIONS. The Makefile sets a few other build options as well
with SANATIZE=address which we should replicate here. I was suprised
meson wont allow -Db_sanitize=address,leak.
I assume that it would be possible to set some default CFLAGS to match
the Makefile and have something like DEVELOPER and DEVOPTS if we adopt
this? As a contributor it's really convenient to be able to set
DEVELOPER=1 and have a sane set of compiler flags. It's really good for
the project as it makes it easy for everyone to compile their code with
a standard set of warnings enabled.
+if compiler.compiles('''
+ #include <inttypes.h>
+
+ void func(void)
+ {
+ uintmax_t x = 0;
+ }
+''', name: 'uintmax_t')
+ libgit_c_args += '-DNO_UINTMAX_T'
+endif
I think the logic is backwards here - shouldn't we be defining
NO_UINTMAX_T if it does not compile? uintptr_t is optional so we
should be checking for that and defining NO_UINTPTR_T if it's missing
as well I think.
+if compiler.run('''
+ #include <stdio.h>
+
+ int main(int argc, const char **argv)
+ {
+ FILE *f = fopen(".", "r");
+ return f ? 0 : 1;
+ }
+''', name: 'fread reads directories').returncode() == 0
+ libgit_c_args += '-DFREAD_READS_DIRECTORIES'
+ libgit_sources += 'compat/fopen.c'
+endif
This checks a property of the build host so should it be guarded with
if not meson.is_cross_build() as below?
+# Build a separate library for "version.c" so that we do not have to rebuild
+# everything when the current Git commit changes. TODO: this only gets set up
+# at configuration time, so we do not notice version changes unless the build
+# instructions get regenerated. We should refactor the source file such that we
+# can substitute tags in the file via `vcs_tag()`.
There are three version dependent strings GIT_VERSION,
GIT_BUILT_FROM_COMMIT and GIT_USER_AGENT - can vcs_tag() handle all
three or do we want a script that writes a header?
Best Wishes
Phillip
