threads / discuss / 62334

Modernize the build system v2 problem

Subject: Modernize the build system v2 problem

## tl;dr

4 messages between Oct 14, 2024 and Oct 14, 2024.

replies: 3people: 3as markdown or json

Ramsay Jones· Oct 14, 2024, 16:59 UTC · lore
Hi Patrick,

I took your 'Modernize the build system' v2 series, from 2024-10-09, as patches from the mailing list and put them on top of master@ef8ce8f3d4 ("Start the 2.48 cycle", 2024-10-10). I had to hand edit the 14th patch to change the version number from DEF_VER=v2.47.0 to DEF_VER=v2.47.GIT, because of the change of base. (It would probably have been easier to just base it on v2.47.0, but what would be the fun in that! :) ).

In order to fix the 'dependency loop' error/warning from make, I applied the following change:

    diff --git a/Makefile b/Makefile
    index dc60b2581d..c7b28975ac 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -3219,7 +3219,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(
     
     all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)
     
    -bin-wrappers/%: bin-wrappers/wrap-for-bin.sh
    +$(test_bindir_programs): bin-wrappers/wrap-for-bin.sh
     	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     	     -e 's|@BUILD_DIR@|$(shell pwd)|' \
     	     -e 's|@GIT_TEXTDOMAINDIR@|$(shell pwd)/po/build/locale|' \

There are several ways to fix it, but this seemed like the easiest. I suspect that you have already fixed this.

Having determined that the 'make' build procedure seemed to be unaffected, I now tried the meson build. I had to install meson at this point (ninja came along for the ride). I have never used meson or ninja before.

At this point I had to fix another fallout from changing the base:
    diff --git a/meson.build b/meson.build
    index 338d472bc6..54557eee03 100644
    --- a/meson.build
    +++ b/meson.build
    @@ -194,7 +194,6 @@ libgit_sources = [
       'reftable/block.c',
       'reftable/blocksource.c',
       'reftable/iter.c',
    -  'reftable/publicbasics.c',
       'reftable/merged.c',
       'reftable/pq.c',
       'reftable/reader.c',

Everything seemed to go without a hitch after that, as far as the build is concerned, but when I did a 'ninja test' I ended up with three failures:

  Summary of Failures:
  
   979/1028 t9500-gitweb-standalone-no-errors              FAIL           12.36s   exit status 1
   980/1028 t9501-gitweb-standalone-http-status            FAIL            2.19s   exit status 1
   981/1028 t9502-gitweb-standalone-parse-output           FAIL            2.22s   exit status 1
  
  Ok:                 1025
  Expected Fail:      0   
  Fail:               3   
  Unexpected Pass:    0   
  Skipped:            0   
  Timeout:            0   
  
  Full log written to /home/ramsay/git/build/meson-logs/testlog.txt
  FAILED: meson-internal__test 
  /usr/bin/meson test --no-rebuild --print-errorlogs
  ninja: build stopped: subcommand failed.

The failure is caused by an (apparently) mangled 'gitweb.cgi' file. Since I still had the make build file, I could directly compare the files:

  $ diff ../gitweb/gitweb.cgi gitweb/gitweb.cgi | wc -l
  160
  $ 
I won't bore you with the whole diff, but it begins like so:
  $ diff ../gitweb/gitweb.cgi gitweb/gitweb.cgi
  83c83
  < our $GIT = "/home/ramsay/bin/git";
  ---
  > our $GIT = "/usr/local/bin/git";
  91c91
  < our $project_maxdepth = 2007;
  ---
  > our $project_maxdepth = "2007";
  2497c2497
  < 		{ regexp => qr/^\@\@{$num_sign} /, class => "chunk_header"},
  ---
  > 		{ regexp => qr/^@@{$num_sign} /, class => "chunk_header"},
  2521c2521
  < 		$line =~ m/^\@{2} (-(\d+)(?:,(\d+))?) (\+(\d+)(?:,(\d+))?) \@{2}(.*)$/;
  ---
  > 		$line =~ m/^@{2} (-(\d+)(?:,(\d+))?) (\+(\d+)(?:,(\d+))?) @{2}(.*)$/;
  ...
  $ 

Note that, after the 'template variables' have been substituted, many (all?) character pairs \@ are replaced with @ (ie the backslashes have gone walkabout). This results in compilation errors in the 'gitweb.log' file, for example the log file for the t9500-*.sh test, looks like:

  $ cat gitweb.log
  [Mon Oct 14 15:12:33 2024] gitweb.cgi: Possible unintended interpolation of @2 in string at /home/ramsay/git/build/gitweb/gitweb.cgi line 2521.
  [Mon Oct 14 15:12:33 2024] gitweb.cgi: Possible unintended interpolation of @3 in string at /home/ramsay/git/build/gitweb/gitweb.cgi line 2593.
  [Mon Oct 14 15:12:33 2024] gitweb.cgi: Possible unintended interpolation of @vrfy in string at /home/ramsay/git/build/gitweb/gitweb.cgi line 4212.
  [Mon Oct 14 15:12:33 2024] gitweb.cgi: Global symbol "@vrfy" requires explicit package name (did you forget to declare "my @vrfy"?) at /home/ramsay/git/build/gitweb/gitweb.cgi line 4212.
  [Mon Oct 14 15:12:33 2024] gitweb.cgi: Execution of /home/ramsay/git/build/gitweb/gitweb.cgi aborted due to compilation errors.
  $ 
 
So, keeping in mind that I know absolutely nothing about meson, it seems that
the 'configure_file' function is mangling the 'gitweb.perl' file. I assume
that you are not seeing this, so I suspect that you are using a newer (fixed)
version than me. :(
  $ meson --version
  1.3.2
  $ ninja --version
  1.11.1
  $ 
This is on Linux Mint 22.1, which is based on Ubuntu LTS, so not that old!

I am about to try converting the Makefile 'procedure' into a shell script to use in both the Makefile and in the meson.build file (I see that the 'configure_file' procedure can take a 'command' to generate the file).

Note that '$project_maxdepth' is a snowflake in the make procedure! :)
Any thoughts?
Thanks.

ATB, Ramsay Jones

 
Patrick Steinhardt· Oct 14, 2024, 17:42 UTC · re: Ramsay Jones · lore

Re: Modernize the build system v2 problem

On Mon, Oct 14, 2024 at 05:59:03PM +0100, Ramsay Jones wrote:
Show 25 quoted lines
> Hi Patrick,
> 
> I took your 'Modernize the build system' v2 series, from 2024-10-09, as patches
> from the mailing list and put them on top of master@ef8ce8f3d4 ("Start the 2.48
> cycle", 2024-10-10). I had to hand edit the 14th patch to change the version
> number from DEF_VER=v2.47.0 to DEF_VER=v2.47.GIT, because of the change of base.
> (It would probably have been easier to just base it on v2.47.0, but what would
> be the fun in that! :) ).
> 
> In order to fix the 'dependency loop' error/warning from make, I applied the
> following change:
> 
>     diff --git a/Makefile b/Makefile
>     index dc60b2581d..c7b28975ac 100644
>     --- a/Makefile
>     +++ b/Makefile
>     @@ -3219,7 +3219,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(
>      
>      all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)
>      
>     -bin-wrappers/%: bin-wrappers/wrap-for-bin.sh
>     +$(test_bindir_programs): bin-wrappers/wrap-for-bin.sh
>      	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
>      	     -e 's|@BUILD_DIR@|$(shell pwd)|' \
>      	     -e 's|@GIT_TEXTDOMAINDIR@|$(shell pwd)/po/build/locale|' \
Yup, I've already got this exact change pending in v3.
Show 21 quoted lines
> There are several ways to fix it, but this seemed like the easiest. I suspect
> that you have already fixed this.
> 
> Having determined that the 'make' build procedure seemed to be unaffected,
> I now tried the meson build. I had to install meson at this point (ninja
> came along for the ride). I have never used meson or ninja before.
> 
> At this point I had to fix another fallout from changing the base:
> 
>     diff --git a/meson.build b/meson.build
>     index 338d472bc6..54557eee03 100644
>     --- a/meson.build
>     +++ b/meson.build
>     @@ -194,7 +194,6 @@ libgit_sources = [
>        'reftable/block.c',
>        'reftable/blocksource.c',
>        'reftable/iter.c',
>     -  'reftable/publicbasics.c',
>        'reftable/merged.c',
>        'reftable/pq.c',
>        'reftable/reader.c',
This one, as well.
[snip]
> So, keeping in mind that I know absolutely nothing about meson, it seems that
> the 'configure_file' function is mangling the 'gitweb.perl' file. I assume
> that you are not seeing this, so I suspect that you are using a newer (fixed)
> version than me. :(
I didn't, no, so this is quite helpful to me.
Show 15 quoted lines
>   $ meson --version
>   1.3.2
>   $ ninja --version
>   1.11.1
>   $ 
> 
> This is on Linux Mint 22.1, which is based on Ubuntu LTS, so not that old!
> 
> I am about to try converting the Makefile 'procedure' into a shell script
> to use in both the Makefile and in the meson.build file (I see that the
> 'configure_file' procedure can take a 'command' to generate the file).
> 
> Note that '$project_maxdepth' is a snowflake in the make procedure! :)
> 
> Any thoughts?

I'll investigate tomorrow and come up with a fix. I'd prefer to not have to script our way around this, as eventually I would like to get rid of shellscripts in the build system altogether. But that is something for the future anyway, and for now I'll do whatever it takes to fix issues.

In any case, I'll try to reproduce the reported issues tomorrow and will then come up with a fix. Thanks a lot for testing things!

Patrick
Eli Schwartz· Oct 14, 2024, 19:19 UTC · re: Ramsay Jones · lore

Re: Modernize the build system v2 problem

On 10/14/24 12:59 PM, Ramsay Jones wrote:
Show 75 quoted lines
> Everything seemed to go without a hitch after that, as far as the build is
> concerned, but when I did a 'ninja test' I ended up with three failures:
> 
>   Summary of Failures:
>   
>    979/1028 t9500-gitweb-standalone-no-errors              FAIL           12.36s   exit status 1
>    980/1028 t9501-gitweb-standalone-http-status            FAIL            2.19s   exit status 1
>    981/1028 t9502-gitweb-standalone-parse-output           FAIL            2.22s   exit status 1
>   
>   Ok:                 1025
>   Expected Fail:      0   
>   Fail:               3   
>   Unexpected Pass:    0   
>   Skipped:            0   
>   Timeout:            0   
>   
>   Full log written to /home/ramsay/git/build/meson-logs/testlog.txt
>   FAILED: meson-internal__test 
>   /usr/bin/meson test --no-rebuild --print-errorlogs
>   ninja: build stopped: subcommand failed.
> 
> The failure is caused by an (apparently) mangled 'gitweb.cgi' file. Since I
> still had the make build file, I could directly compare the files:
> 
>   $ diff ../gitweb/gitweb.cgi gitweb/gitweb.cgi | wc -l
>   160
>   $ 
> 
> I won't bore you with the whole diff, but it begins like so:
> 
>   $ diff ../gitweb/gitweb.cgi gitweb/gitweb.cgi
>   83c83
>   < our $GIT = "/home/ramsay/bin/git";
>   ---
>   > our $GIT = "/usr/local/bin/git";
>   91c91
>   < our $project_maxdepth = 2007;
>   ---
>   > our $project_maxdepth = "2007";
>   2497c2497
>   < 		{ regexp => qr/^\@\@{$num_sign} /, class => "chunk_header"},
>   ---
>   > 		{ regexp => qr/^@@{$num_sign} /, class => "chunk_header"},
>   2521c2521
>   < 		$line =~ m/^\@{2} (-(\d+)(?:,(\d+))?) (\+(\d+)(?:,(\d+))?) \@{2}(.*)$/;
>   ---
>   > 		$line =~ m/^@{2} (-(\d+)(?:,(\d+))?) (\+(\d+)(?:,(\d+))?) @{2}(.*)$/;
> 
>   ...
> 
>   $ 
> 
> Note that, after the 'template variables' have been substituted, many (all?)
> character pairs \@ are replaced with @ (ie the backslashes have gone walkabout).
> This results in compilation errors in the 'gitweb.log' file, for example the
> log file for the t9500-*.sh test, looks like:
> 
>   $ cat gitweb.log
>   [Mon Oct 14 15:12:33 2024] gitweb.cgi: Possible unintended interpolation of @2 in string at /home/ramsay/git/build/gitweb/gitweb.cgi line 2521.
>   [Mon Oct 14 15:12:33 2024] gitweb.cgi: Possible unintended interpolation of @3 in string at /home/ramsay/git/build/gitweb/gitweb.cgi line 2593.
>   [Mon Oct 14 15:12:33 2024] gitweb.cgi: Possible unintended interpolation of @vrfy in string at /home/ramsay/git/build/gitweb/gitweb.cgi line 4212.
>   [Mon Oct 14 15:12:33 2024] gitweb.cgi: Global symbol "@vrfy" requires explicit package name (did you forget to declare "my @vrfy"?) at /home/ramsay/git/build/gitweb/gitweb.cgi line 4212.
>   [Mon Oct 14 15:12:33 2024] gitweb.cgi: Execution of /home/ramsay/git/build/gitweb/gitweb.cgi aborted due to compilation errors.
>   $ 
>  
> So, keeping in mind that I know absolutely nothing about meson, it seems that
> the 'configure_file' function is mangling the 'gitweb.perl' file. I assume
> that you are not seeing this, so I suspect that you are using a newer (fixed)
> version than me. :(
> 
>   $ meson --version
>   1.3.2
>   $ ninja --version
>   1.11.1
>   $ 
I recognize this: https://github.com/mesonbuild/meson/pull/13302

Note that for files which can change semi-regularly as part of development it may be better to avoid configure_file() and create something like edit-files.sh.in which then produces build rules, not configure-time changes. This would sidestep the problem entirely as you'd then process gitweb.cgi via e.g. a sed script.

(The main reason though is because it avoids reconfiguring meson when the gitweb.cgi script is modified via a patch / git pull.)

-- 
Eli Schwartz
Ramsay Jones· Oct 14, 2024, 20:18 UTC · re: Eli Schwartz · lore

Re: Modernize the build system v2 problem

On 14/10/2024 20:19, Eli Schwartz wrote: [snip]

Show 13 quoted lines
>> So, keeping in mind that I know absolutely nothing about meson, it seems that
>> the 'configure_file' function is mangling the 'gitweb.perl' file. I assume
>> that you are not seeing this, so I suspect that you are using a newer (fixed)
>> version than me. :(
>>
>>   $ meson --version
>>   1.3.2
>>   $ ninja --version
>>   1.11.1
>>   $ 
> 
> 
> I recognize this: https://github.com/mesonbuild/meson/pull/13302
Yep, that looks pretty much on point. :)

Having said that, after squinting at the patch text for about ten minutes, I'm still not sure it would leave all valid perl syntax alone. So, ...

Show 6 quoted lines
> 
> Note that for files which can change semi-regularly as part of
> development it may be better to avoid configure_file() and create
> something like edit-files.sh.in which then produces build rules, not
> configure-time changes. This would sidestep the problem entirely as
> you'd then process gitweb.cgi via e.g. a sed script.

Showing ignorance of meson, I don't quite follow the 'edit-files.sh.in' suggestion, but I had intended (before Patrick's email) to use a script to generate 'gitweb.cgi' as the 'command' in the 'configure_file'. It feels like you are suggesting something else.

> (The main reason though is because it avoids reconfiguring meson when
> the gitweb.cgi script is modified via a patch / git pull.)
Thanks.

ATB, Ramsay Jones

← back to recent threads