git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Ramsay Jones <ramsay@ramsayjones.plus.com>
Cc: GIT Mailing-list <git@vger.kernel.org>
Subject: Re: Modernize the build system v2 problem
Date: Mon, 14 Oct 2024 19:42:20 +0200	[thread overview]
Message-ID: <Zw1X9-d1OH7Df8Wh@pks.im> (raw)
In-Reply-To: <28e13e74-d4a4-4be5-8555-27a69c5c5787@ramsayjones.plus.com>

On Mon, Oct 14, 2024 at 05:59:03PM +0100, Ramsay Jones wrote:
> 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.

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

>   $ 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

  reply	other threads:[~2024-10-14 17:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-14 16:59 Modernize the build system v2 problem Ramsay Jones
2024-10-14 17:42 ` Patrick Steinhardt [this message]
2024-10-14 19:19 ` Eli Schwartz
2024-10-14 20:18   ` Ramsay Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zw1X9-d1OH7Df8Wh@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=ramsay@ramsayjones.plus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).