git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org,  Daniel Engberg <daniel.engberg.lists@pyret.net>
Subject: Re: [PATCH] meson: skip gitweb build when Perl is disabled
Date: Fri, 20 Dec 2024 07:03:23 -0800	[thread overview]
Message-ID: <xmqq4j2ygzwk.fsf@gitster.g> (raw)
In-Reply-To: <20241220-b4-pks-meson-fix-gitweb-wo-perl-v1-1-41039ad8d8d4@pks.im> (Patrick Steinhardt's message of "Fri, 20 Dec 2024 08:26:30 +0100")

Patrick Steinhardt <ps@pks.im> writes:

> It is possible to configure a Git build without Perl when disabling both
> our test suite and all Perl-based features. In Meson, this can be
> achieved with `meson setup -Dperl=disabled -Dtests=false`.
>
> It was reported by a user that this breaks the Meson build because
> gitweb gets built even if Perl was not discovered in such a build:
>
>     $ meson setup .. -Dtests=false -Dperl=disabled
>     ...
>     ../gitweb/meson.build:2:43: ERROR: Unable to get the path of a not-found external program
>
> Fix this issue by introducing a new feature-option that allows the user
> to configure whether or not to build Gitweb. The feature is set to
> 'auto' by default and will be disabled automatically in case Perl was
> not found on the system.
>
> Reported-by: Daniel Engberg <daniel.engberg.lists@pyret.net>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Hi,
>
> I received an off-list mail from a user interested in the new Meson
> build system who has done a bit of testing of it on FreeBSD. They found
> an issue when configuring the build without Perl enabled, which can be
> achieved by both disabling tests and Perl-based features. This patch
> here fixes the issue.
>
> Thanks!

Thanks, Patrick and Daniel.

> -if get_option('tests')
> +if get_option('tests') or get_option('gitweb').enabled()
>    perl_required = true
>  endif

OK, so we use "perl_required" to keep track of the fact that
something else wants Perl to be usable.

> +# Gitweb requires Perl, so we disable the auto-feature if Perl was not found.
> +# We make sure further up that Perl is required in case the gitweb option is
> +# enabled.
> +gitweb_option = get_option('gitweb').disable_auto_if(not perl.found())

Hopefully before we reach this point, we would have figured out if
perl is avialable, to allow us do this.

There seem to be too many "perl" related variables, interaction
among which smells way too complex for their worth.  For example,

    perl = find_program('perl', ..., required: perl_required);
    perl_features_enabled = perl.found() and get_option('perl').allowed()

and only when the latter is true, we'd do further configuration to
make perl usable.  Does that mean the condition you wrote above to
automatically disable gitweb somewhat incorrect?  Even if we may
have found perl, the builder may deliberately have disallowed use of
it, in which case we just know perl is there without figuring out
what other things (like where the localedir is) that are needed to
use it correctly.

> +if gitweb_option.enabled()
> +  subdir('gitweb')
> +endif
> +
>  subdir('templates')

OK, we used to do the subdir() thing unconditionally, but now, if we
decide that we shouldn't or cannot do gitweb, we do not do
that,which sounds good.

> +option('gitweb', type: 'feature', value: 'auto',
> +  description: 'Build Git web interface. Required Perl.')

I do not know the convention in the meson world, but to me, this
would sound more natural with "Required" -> "Requires".

>  option('iconv', type: 'feature', value: 'auto',
>    description: 'Support reencoding strings with different encodings.')
>  option('pcre2', type: 'feature', value: 'enabled',
>
> ---
> base-commit: d882f382b3d939d90cfa58d17b17802338f05d66
> change-id: 20241218-b4-pks-meson-fix-gitweb-wo-perl-93379dd0ceed

  reply	other threads:[~2024-12-20 15:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-20  7:26 [PATCH] meson: skip gitweb build when Perl is disabled Patrick Steinhardt
2024-12-20 15:03 ` Junio C Hamano [this message]
2024-12-20 15:18   ` Patrick Steinhardt
2024-12-20 15:50     ` Junio C Hamano
2024-12-20 16:30 ` [PATCH v2] " Patrick Steinhardt

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=xmqq4j2ygzwk.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=daniel.engberg.lists@pyret.net \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    /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).