{"thread":{"id":"62334","subject":"Modernize the build system v2 problem","startedAt":"2024-10-14T17:02:14Z","lastAt":"2024-10-14T20:21:21Z","messageCount":4,"participants":["Ramsay Jones","Patrick Steinhardt","Eli Schwartz"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"505039","messageId":"28e13e74-d4a4-4be5-8555-27a69c5c5787@ramsayjones.plus.com","threadId":"62334","inReplyTo":null,"subject":"Modernize the build system v2 problem","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2024-10-14T16:59:03Z","receivedAt":"2024-10-14T17:02:14Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Hi Patrick,\n\nI took your 'Modernize the build system' v2 series, from 2024-10-09, as patches\nfrom the mailing list and put them on top of master@ef8ce8f3d4 (\"Start the 2.48\ncycle\", 2024-10-10). I had to hand edit the 14th patch to change the version\nnumber from DEF_VER=v2.47.0 to DEF_VER=v2.47.GIT, because of the change of base.\n(It would probably have been easier to just base it on v2.47.0, but what would\nbe the fun in that! :) ).\n\nIn order to fix the 'dependency loop' error/warning from make, I applied the\nfollowing change:\n\n    diff --git a/Makefile b/Makefile\n    index dc60b2581d..c7b28975ac 100644\n    --- a/Makefile\n    +++ b/Makefile\n    @@ -3219,7 +3219,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(\n     \n     all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)\n     \n    -bin-wrappers/%: bin-wrappers/wrap-for-bin.sh\n    +$(test_bindir_programs): bin-wrappers/wrap-for-bin.sh\n     \t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n     \t     -e 's|@BUILD_DIR@|$(shell pwd)|' \\\n     \t     -e 's|@GIT_TEXTDOMAINDIR@|$(shell pwd)/po/build/locale|' \\\n\nThere are several ways to fix it, but this seemed like the easiest. I suspect\nthat you have already fixed this.\n\nHaving determined that the 'make' build procedure seemed to be unaffected,\nI now tried the meson build. I had to install meson at this point (ninja\ncame along for the ride). I have never used meson or ninja before.\n\nAt this point I had to fix another fallout from changing the base:\n\n    diff --git a/meson.build b/meson.build\n    index 338d472bc6..54557eee03 100644\n    --- a/meson.build\n    +++ b/meson.build\n    @@ -194,7 +194,6 @@ libgit_sources = [\n       'reftable/block.c',\n       'reftable/blocksource.c',\n       'reftable/iter.c',\n    -  'reftable/publicbasics.c',\n       'reftable/merged.c',\n       'reftable/pq.c',\n       'reftable/reader.c',\n\nEverything seemed to go without a hitch after that, as far as the build is\nconcerned, but when I did a 'ninja test' I ended up with three failures:\n\n  Summary of Failures:\n  \n   979/1028 t9500-gitweb-standalone-no-errors              FAIL           12.36s   exit status 1\n   980/1028 t9501-gitweb-standalone-http-status            FAIL            2.19s   exit status 1\n   981/1028 t9502-gitweb-standalone-parse-output           FAIL            2.22s   exit status 1\n  \n  Ok:                 1025\n  Expected Fail:      0   \n  Fail:               3   \n  Unexpected Pass:    0   \n  Skipped:            0   \n  Timeout:            0   \n  \n  Full log written to /home/ramsay/git/build/meson-logs/testlog.txt\n  FAILED: meson-internal__test \n  /usr/bin/meson test --no-rebuild --print-errorlogs\n  ninja: build stopped: subcommand failed.\n\nThe failure is caused by an (apparently) mangled 'gitweb.cgi' file. Since I\nstill had the make build file, I could directly compare the files:\n\n  $ diff ../gitweb/gitweb.cgi gitweb/gitweb.cgi | wc -l\n  160\n  $ \n\nI won't bore you with the whole diff, but it begins like so:\n\n  $ diff ../gitweb/gitweb.cgi gitweb/gitweb.cgi\n  83c83\n  < our $GIT = \"/home/ramsay/bin/git\";\n  ---\n  > our $GIT = \"/usr/local/bin/git\";\n  91c91\n  < our $project_maxdepth = 2007;\n  ---\n  > our $project_maxdepth = \"2007\";\n  2497c2497\n  < \t\t{ regexp => qr/^\\@\\@{$num_sign} /, class => \"chunk_header\"},\n  ---\n  > \t\t{ regexp => qr/^@@{$num_sign} /, class => \"chunk_header\"},\n  2521c2521\n  < \t\t$line =~ m/^\\@{2} (-(\\d+)(?:,(\\d+))?) (\\+(\\d+)(?:,(\\d+))?) \\@{2}(.*)$/;\n  ---\n  > \t\t$line =~ m/^@{2} (-(\\d+)(?:,(\\d+))?) (\\+(\\d+)(?:,(\\d+))?) @{2}(.*)$/;\n\n  ...\n\n  $ \n\nNote that, after the 'template variables' have been substituted, many (all?)\ncharacter pairs \\@ are replaced with @ (ie the backslashes have gone walkabout).\nThis results in compilation errors in the 'gitweb.log' file, for example the\nlog file for the t9500-*.sh test, looks like:\n\n  $ cat gitweb.log\n  [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.\n  [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.\n  [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.\n  [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.\n  [Mon Oct 14 15:12:33 2024] gitweb.cgi: Execution of /home/ramsay/git/build/gitweb/gitweb.cgi aborted due to compilation errors.\n  $ \n \nSo, keeping in mind that I know absolutely nothing about meson, it seems that\nthe 'configure_file' function is mangling the 'gitweb.perl' file. I assume\nthat you are not seeing this, so I suspect that you are using a newer (fixed)\nversion than me. :(\n\n  $ meson --version\n  1.3.2\n  $ ninja --version\n  1.11.1\n  $ \n\nThis is on Linux Mint 22.1, which is based on Ubuntu LTS, so not that old!\n\nI am about to try converting the Makefile 'procedure' into a shell script\nto use in both the Makefile and in the meson.build file (I see that the\n'configure_file' procedure can take a 'command' to generate the file).\n\nNote that '$project_maxdepth' is a snowflake in the make procedure! :)\n\nAny thoughts?\n\nThanks.\n\nATB,\nRamsay Jones\n\n \n"},{"id":"505040","messageId":"Zw1X9-d1OH7Df8Wh@pks.im","threadId":"62334","inReplyTo":"28e13e74-d4a4-4be5-8555-27a69c5c5787@ramsayjones.plus.com","subject":"Re: Modernize the build system v2 problem","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-14T17:42:20Z","receivedAt":"2024-10-14T17:43:00Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 14, 2024 at 05:59:03PM +0100, Ramsay Jones wrote:\n> Hi Patrick,\n> \n> I took your 'Modernize the build system' v2 series, from 2024-10-09, as patches\n> from the mailing list and put them on top of master@ef8ce8f3d4 (\"Start the 2.48\n> cycle\", 2024-10-10). I had to hand edit the 14th patch to change the version\n> number from DEF_VER=v2.47.0 to DEF_VER=v2.47.GIT, because of the change of base.\n> (It would probably have been easier to just base it on v2.47.0, but what would\n> be the fun in that! :) ).\n> \n> In order to fix the 'dependency loop' error/warning from make, I applied the\n> following change:\n> \n>     diff --git a/Makefile b/Makefile\n>     index dc60b2581d..c7b28975ac 100644\n>     --- a/Makefile\n>     +++ b/Makefile\n>     @@ -3219,7 +3219,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(\n>      \n>      all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)\n>      \n>     -bin-wrappers/%: bin-wrappers/wrap-for-bin.sh\n>     +$(test_bindir_programs): bin-wrappers/wrap-for-bin.sh\n>      \t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n>      \t     -e 's|@BUILD_DIR@|$(shell pwd)|' \\\n>      \t     -e 's|@GIT_TEXTDOMAINDIR@|$(shell pwd)/po/build/locale|' \\\n\nYup, I've already got this exact change pending in v3.\n\n> There are several ways to fix it, but this seemed like the easiest. I suspect\n> that you have already fixed this.\n> \n> Having determined that the 'make' build procedure seemed to be unaffected,\n> I now tried the meson build. I had to install meson at this point (ninja\n> came along for the ride). I have never used meson or ninja before.\n> \n> At this point I had to fix another fallout from changing the base:\n> \n>     diff --git a/meson.build b/meson.build\n>     index 338d472bc6..54557eee03 100644\n>     --- a/meson.build\n>     +++ b/meson.build\n>     @@ -194,7 +194,6 @@ libgit_sources = [\n>        'reftable/block.c',\n>        'reftable/blocksource.c',\n>        'reftable/iter.c',\n>     -  'reftable/publicbasics.c',\n>        'reftable/merged.c',\n>        'reftable/pq.c',\n>        'reftable/reader.c',\n\nThis one, as well.\n\n[snip]\n> So, keeping in mind that I know absolutely nothing about meson, it seems that\n> the 'configure_file' function is mangling the 'gitweb.perl' file. I assume\n> that you are not seeing this, so I suspect that you are using a newer (fixed)\n> version than me. :(\n\nI didn't, no, so this is quite helpful to me.\n\n>   $ meson --version\n>   1.3.2\n>   $ ninja --version\n>   1.11.1\n>   $ \n> \n> This is on Linux Mint 22.1, which is based on Ubuntu LTS, so not that old!\n> \n> I am about to try converting the Makefile 'procedure' into a shell script\n> to use in both the Makefile and in the meson.build file (I see that the\n> 'configure_file' procedure can take a 'command' to generate the file).\n> \n> Note that '$project_maxdepth' is a snowflake in the make procedure! :)\n> \n> Any thoughts?\n\nI'll investigate tomorrow and come up with a fix. I'd prefer to not have\nto script our way around this, as eventually I would like to get rid of\nshellscripts in the build system altogether. But that is something for\nthe future anyway, and for now I'll do whatever it takes to fix issues.\n\nIn any case, I'll try to reproduce the reported issues tomorrow and will\nthen come up with a fix. Thanks a lot for testing things!\n\nPatrick\n"},{"id":"505045","messageId":"cd118725-f820-494c-8d10-81aba32e4064@gmail.com","threadId":"62334","inReplyTo":"28e13e74-d4a4-4be5-8555-27a69c5c5787@ramsayjones.plus.com","subject":"Re: Modernize the build system v2 problem","fromName":"Eli Schwartz","fromEmail":"eschwartz93@gmail.com","sentAt":"2024-10-14T19:19:31Z","receivedAt":"2024-10-14T19:19:34Z","isPatch":false,"sender":{"key":"eschwartz93@gmail.com","avatar":"https://gravatar.com/avatar/80b459bb75c0edb5e116884705adb9095fbd01cbbf91cb29a9ed23577fff7d34?d=mp&s=160"},"body":"On 10/14/24 12:59 PM, Ramsay Jones wrote:\n> Everything seemed to go without a hitch after that, as far as the build is\n> concerned, but when I did a 'ninja test' I ended up with three failures:\n> \n>   Summary of Failures:\n>   \n>    979/1028 t9500-gitweb-standalone-no-errors              FAIL           12.36s   exit status 1\n>    980/1028 t9501-gitweb-standalone-http-status            FAIL            2.19s   exit status 1\n>    981/1028 t9502-gitweb-standalone-parse-output           FAIL            2.22s   exit status 1\n>   \n>   Ok:                 1025\n>   Expected Fail:      0   \n>   Fail:               3   \n>   Unexpected Pass:    0   \n>   Skipped:            0   \n>   Timeout:            0   \n>   \n>   Full log written to /home/ramsay/git/build/meson-logs/testlog.txt\n>   FAILED: meson-internal__test \n>   /usr/bin/meson test --no-rebuild --print-errorlogs\n>   ninja: build stopped: subcommand failed.\n> \n> The failure is caused by an (apparently) mangled 'gitweb.cgi' file. Since I\n> still had the make build file, I could directly compare the files:\n> \n>   $ diff ../gitweb/gitweb.cgi gitweb/gitweb.cgi | wc -l\n>   160\n>   $ \n> \n> I won't bore you with the whole diff, but it begins like so:\n> \n>   $ diff ../gitweb/gitweb.cgi gitweb/gitweb.cgi\n>   83c83\n>   < our $GIT = \"/home/ramsay/bin/git\";\n>   ---\n>   > our $GIT = \"/usr/local/bin/git\";\n>   91c91\n>   < our $project_maxdepth = 2007;\n>   ---\n>   > our $project_maxdepth = \"2007\";\n>   2497c2497\n>   < \t\t{ regexp => qr/^\\@\\@{$num_sign} /, class => \"chunk_header\"},\n>   ---\n>   > \t\t{ regexp => qr/^@@{$num_sign} /, class => \"chunk_header\"},\n>   2521c2521\n>   < \t\t$line =~ m/^\\@{2} (-(\\d+)(?:,(\\d+))?) (\\+(\\d+)(?:,(\\d+))?) \\@{2}(.*)$/;\n>   ---\n>   > \t\t$line =~ m/^@{2} (-(\\d+)(?:,(\\d+))?) (\\+(\\d+)(?:,(\\d+))?) @{2}(.*)$/;\n> \n>   ...\n> \n>   $ \n> \n> Note that, after the 'template variables' have been substituted, many (all?)\n> character pairs \\@ are replaced with @ (ie the backslashes have gone walkabout).\n> This results in compilation errors in the 'gitweb.log' file, for example the\n> log file for the t9500-*.sh test, looks like:\n> \n>   $ cat gitweb.log\n>   [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.\n>   [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.\n>   [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.\n>   [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.\n>   [Mon Oct 14 15:12:33 2024] gitweb.cgi: Execution of /home/ramsay/git/build/gitweb/gitweb.cgi aborted due to compilation errors.\n>   $ \n>  \n> So, keeping in mind that I know absolutely nothing about meson, it seems that\n> the 'configure_file' function is mangling the 'gitweb.perl' file. I assume\n> that you are not seeing this, so I suspect that you are using a newer (fixed)\n> version than me. :(\n> \n>   $ meson --version\n>   1.3.2\n>   $ ninja --version\n>   1.11.1\n>   $ \n\n\nI recognize this: https://github.com/mesonbuild/meson/pull/13302\n\nNote that for files which can change semi-regularly as part of\ndevelopment it may be better to avoid configure_file() and create\nsomething like edit-files.sh.in which then produces build rules, not\nconfigure-time changes. This would sidestep the problem entirely as\nyou'd then process gitweb.cgi via e.g. a sed script.\n\n(The main reason though is because it avoids reconfiguring meson when\nthe gitweb.cgi script is modified via a patch / git pull.)\n\n\n-- \nEli Schwartz\n"},{"id":"505049","messageId":"4dd62151-38d8-48ac-b5ad-d8e2d00fa820@ramsayjones.plus.com","threadId":"62334","inReplyTo":"cd118725-f820-494c-8d10-81aba32e4064@gmail.com","subject":"Re: Modernize the build system v2 problem","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2024-10-14T20:18:10Z","receivedAt":"2024-10-14T20:21:21Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 14/10/2024 20:19, Eli Schwartz wrote:\n[snip]\n\n>> So, keeping in mind that I know absolutely nothing about meson, it seems that\n>> the 'configure_file' function is mangling the 'gitweb.perl' file. I assume\n>> that you are not seeing this, so I suspect that you are using a newer (fixed)\n>> version than me. :(\n>>\n>>   $ meson --version\n>>   1.3.2\n>>   $ ninja --version\n>>   1.11.1\n>>   $ \n> \n> \n> I recognize this: https://github.com/mesonbuild/meson/pull/13302\n\nYep, that looks pretty much on point. :)\n\nHaving said that, after squinting at the patch text for about ten\nminutes, I'm still not sure it would leave all valid perl syntax\nalone. So, ...\n\n> \n> Note that for files which can change semi-regularly as part of\n> development it may be better to avoid configure_file() and create\n> something like edit-files.sh.in which then produces build rules, not\n> configure-time changes. This would sidestep the problem entirely as\n> you'd then process gitweb.cgi via e.g. a sed script.\n\nShowing ignorance of meson, I don't quite follow the 'edit-files.sh.in'\nsuggestion, but I had intended (before Patrick's email) to use a script\nto generate 'gitweb.cgi' as the 'command' in the 'configure_file'.\nIt feels like you are suggesting something else.\n\n> (The main reason though is because it avoids reconfiguring meson when\n> the gitweb.cgi script is modified via a patch / git pull.)\n\nThanks.\n\nATB,\nRamsay Jones\n\n"}]}