{"thread":{"id":"62673","subject":"[PATCH] meson: skip gitweb build when Perl is disabled","startedAt":"2024-12-20T07:26:52Z","lastAt":"2024-12-20T16:30:59Z","messageCount":5,"participants":["Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"509360","messageId":"20241220-b4-pks-meson-fix-gitweb-wo-perl-v1-1-41039ad8d8d4@pks.im","threadId":"62673","inReplyTo":null,"subject":"[PATCH] meson: skip gitweb build when Perl is disabled","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T07:26:30Z","receivedAt":"2024-12-20T07:26:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"It is possible to configure a Git build without Perl when disabling both\nour test suite and all Perl-based features. In Meson, this can be\nachieved with `meson setup -Dperl=disabled -Dtests=false`.\n\nIt was reported by a user that this breaks the Meson build because\ngitweb gets built even if Perl was not discovered in such a build:\n\n    $ meson setup .. -Dtests=false -Dperl=disabled\n    ...\n    ../gitweb/meson.build:2:43: ERROR: Unable to get the path of a not-found external program\n\nFix this issue by introducing a new feature-option that allows the user\nto configure whether or not to build Gitweb. The feature is set to\n'auto' by default and will be disabled automatically in case Perl was\nnot found on the system.\n\nReported-by: Daniel Engberg <daniel.engberg.lists@pyret.net>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\nHi,\n\nI received an off-list mail from a user interested in the new Meson\nbuild system who has done a bit of testing of it on FreeBSD. They found\nan issue when configuring the build without Perl enabled, which can be\nachieved by both disabling tests and Perl-based features. This patch\nhere fixes the issue.\n\nThanks!\n\nPatrick\n---\n meson.build       | 13 +++++++++++--\n meson_options.txt |  2 ++\n 2 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex 0dccebcdf16b07650d943e53643f0e09e2975cc9..8e34a895dca80da77d6809a6fe90fe7661f142a1 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -740,7 +740,7 @@ endif\n # features. It is optional if you want to neither execute tests nor use any of\n # these optional features.\n perl_required = get_option('perl')\n-if get_option('tests')\n+if get_option('tests') or get_option('gitweb').enabled()\n   perl_required = true\n endif\n \n@@ -1874,7 +1874,15 @@ if intl.found()\n   subdir('po')\n endif\n subdir('contrib')\n-subdir('gitweb')\n+\n+# Gitweb requires Perl, so we disable the auto-feature if Perl was not found.\n+# We make sure further up that Perl is required in case the gitweb option is\n+# enabled.\n+gitweb_option = get_option('gitweb').disable_auto_if(not perl.found())\n+if gitweb_option.enabled()\n+  subdir('gitweb')\n+endif\n+\n subdir('templates')\n \n # Everything but the bin-wrappers need to come before this target such that we\n@@ -1893,6 +1901,7 @@ summary({\n   'curl': curl.found(),\n   'expat': expat.found(),\n   'gettext': intl.found(),\n+  'gitweb': gitweb_option.enabled(),\n   'https': https_backend,\n   'iconv': iconv.found(),\n   'pcre2': pcre2.found(),\ndiff --git a/meson_options.txt b/meson_options.txt\nindex 32a72139bae870745d9131cc9086a4594826be91..1b38d9b716ee2d06ac1484a744529547f7ed4acd 100644\n--- a/meson_options.txt\n+++ b/meson_options.txt\n@@ -23,6 +23,8 @@ option('expat', type: 'feature', value: 'enabled',\n   description: 'Build helpers used to push to remotes with the HTTP transport.')\n option('gettext', type: 'feature', value: 'auto',\n   description: 'Build translation files.')\n+option('gitweb', type: 'feature', value: 'auto',\n+  description: 'Build Git web interface. Required Perl.')\n option('iconv', type: 'feature', value: 'auto',\n   description: 'Support reencoding strings with different encodings.')\n option('pcre2', type: 'feature', value: 'enabled',\n\n---\nbase-commit: d882f382b3d939d90cfa58d17b17802338f05d66\nchange-id: 20241218-b4-pks-meson-fix-gitweb-wo-perl-93379dd0ceed\n\n"},{"id":"509393","messageId":"xmqq4j2ygzwk.fsf@gitster.g","threadId":"62673","inReplyTo":"20241220-b4-pks-meson-fix-gitweb-wo-perl-v1-1-41039ad8d8d4@pks.im","subject":"Re: [PATCH] meson: skip gitweb build when Perl is disabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-20T15:03:23Z","receivedAt":"2024-12-20T15:03:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> It is possible to configure a Git build without Perl when disabling both\n> our test suite and all Perl-based features. In Meson, this can be\n> achieved with `meson setup -Dperl=disabled -Dtests=false`.\n>\n> It was reported by a user that this breaks the Meson build because\n> gitweb gets built even if Perl was not discovered in such a build:\n>\n>     $ meson setup .. -Dtests=false -Dperl=disabled\n>     ...\n>     ../gitweb/meson.build:2:43: ERROR: Unable to get the path of a not-found external program\n>\n> Fix this issue by introducing a new feature-option that allows the user\n> to configure whether or not to build Gitweb. The feature is set to\n> 'auto' by default and will be disabled automatically in case Perl was\n> not found on the system.\n>\n> Reported-by: Daniel Engberg <daniel.engberg.lists@pyret.net>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n> Hi,\n>\n> I received an off-list mail from a user interested in the new Meson\n> build system who has done a bit of testing of it on FreeBSD. They found\n> an issue when configuring the build without Perl enabled, which can be\n> achieved by both disabling tests and Perl-based features. This patch\n> here fixes the issue.\n>\n> Thanks!\n\nThanks, Patrick and Daniel.\n\n> -if get_option('tests')\n> +if get_option('tests') or get_option('gitweb').enabled()\n>    perl_required = true\n>  endif\n\nOK, so we use \"perl_required\" to keep track of the fact that\nsomething else wants Perl to be usable.\n\n> +# Gitweb requires Perl, so we disable the auto-feature if Perl was not found.\n> +# We make sure further up that Perl is required in case the gitweb option is\n> +# enabled.\n> +gitweb_option = get_option('gitweb').disable_auto_if(not perl.found())\n\nHopefully before we reach this point, we would have figured out if\nperl is avialable, to allow us do this.\n\nThere seem to be too many \"perl\" related variables, interaction\namong which smells way too complex for their worth.  For example,\n\n    perl = find_program('perl', ..., required: perl_required);\n    perl_features_enabled = perl.found() and get_option('perl').allowed()\n\nand only when the latter is true, we'd do further configuration to\nmake perl usable.  Does that mean the condition you wrote above to\nautomatically disable gitweb somewhat incorrect?  Even if we may\nhave found perl, the builder may deliberately have disallowed use of\nit, in which case we just know perl is there without figuring out\nwhat other things (like where the localedir is) that are needed to\nuse it correctly.\n\n> +if gitweb_option.enabled()\n> +  subdir('gitweb')\n> +endif\n> +\n>  subdir('templates')\n\nOK, we used to do the subdir() thing unconditionally, but now, if we\ndecide that we shouldn't or cannot do gitweb, we do not do\nthat,which sounds good.\n\n> +option('gitweb', type: 'feature', value: 'auto',\n> +  description: 'Build Git web interface. Required Perl.')\n\nI do not know the convention in the meson world, but to me, this\nwould sound more natural with \"Required\" -> \"Requires\".\n\n>  option('iconv', type: 'feature', value: 'auto',\n>    description: 'Support reencoding strings with different encodings.')\n>  option('pcre2', type: 'feature', value: 'enabled',\n>\n> ---\n> base-commit: d882f382b3d939d90cfa58d17b17802338f05d66\n> change-id: 20241218-b4-pks-meson-fix-gitweb-wo-perl-93379dd0ceed\n"},{"id":"509395","messageId":"Z2WKnAh2BVWr063E@pks.im","threadId":"62673","inReplyTo":"xmqq4j2ygzwk.fsf@gitster.g","subject":"Re: [PATCH] meson: skip gitweb build when Perl is disabled","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T15:18:01Z","receivedAt":"2024-12-20T15:19:43Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 07:03:23AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > -if get_option('tests')\n> > +if get_option('tests') or get_option('gitweb').enabled()\n> >    perl_required = true\n> >  endif\n> \n> OK, so we use \"perl_required\" to keep track of the fact that\n> something else wants Perl to be usable.\n> \n> > +# Gitweb requires Perl, so we disable the auto-feature if Perl was not found.\n> > +# We make sure further up that Perl is required in case the gitweb option is\n> > +# enabled.\n> > +gitweb_option = get_option('gitweb').disable_auto_if(not perl.found())\n> \n> Hopefully before we reach this point, we would have figured out if\n> perl is avialable, to allow us do this.\n\nYup.\n\n> There seem to be too many \"perl\" related variables, interaction\n> among which smells way too complex for their worth.  For example,\n> \n>     perl = find_program('perl', ..., required: perl_required);\n>     perl_features_enabled = perl.found() and get_option('perl').allowed()\n> \n> and only when the latter is true, we'd do further configuration to\n> make perl usable.  Does that mean the condition you wrote above to\n> automatically disable gitweb somewhat incorrect?  Even if we may\n> have found perl, the builder may deliberately have disallowed use of\n> it, in which case we just know perl is there without figuring out\n> what other things (like where the localedir is) that are needed to\n> use it correctly.\n\nI don't think it's incorrect. The Perl features are those that the Git\ndistribution itself uses, including all the Perl modules in \"perl/\". The\ngitweb subsystem, as far as I can see, uses none of those functions and\nthus the data we configure in case `perl_features_enabled` is true is\nirrelevant for gitweb.\n\nNow I don't disagree with your statement that there are too many\nPerl-related variables, but it simply reflects the fact that Git uses it\nfor multiple different things: testing, git-instaweb, the Perl library,\nseveral Perl-based commands and gitweb. And I think it's not entirely\nunreasonable to let users configure these independently of one another,\nand that brings some complexity with it.\n\nThat being said I'd also be okay to not introduce the separate \"gitweb\"\noption and instead just use the existing \"perl\" option for it. I don't\nmind it all that much.\n\n> > +option('gitweb', type: 'feature', value: 'auto',\n> > +  description: 'Build Git web interface. Required Perl.')\n> \n> I do not know the convention in the meson world, but to me, this\n> would sound more natural with \"Required\" -> \"Requires\".\n\nOh, yes, indeed. I'll wait a bit for your opinion on the above before\nsending a simple typo fix for this.\n\nPatrick\n"},{"id":"509398","messageId":"xmqq4j2yfj5b.fsf@gitster.g","threadId":"62673","inReplyTo":"Z2WKnAh2BVWr063E@pks.im","subject":"Re: [PATCH] meson: skip gitweb build when Perl is disabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-20T15:50:40Z","receivedAt":"2024-12-20T15:50:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> I don't think it's incorrect. The Perl features are those that the Git\n> distribution itself uses, including all the Perl modules in \"perl/\". The\n> gitweb subsystem, as far as I can see, uses none of those functions and\n> thus the data we configure in case `perl_features_enabled` is true is\n> irrelevant for gitweb.\n\nAh, now that makes sense to me why these things are laid out that\nway.  Thanks.\n"},{"id":"509424","messageId":"20241220-b4-pks-meson-fix-gitweb-wo-perl-v2-1-18ab23dd6446@pks.im","threadId":"62673","inReplyTo":"20241220-b4-pks-meson-fix-gitweb-wo-perl-v1-1-41039ad8d8d4@pks.im","subject":"[PATCH v2] meson: skip gitweb build when Perl is disabled","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T16:30:35Z","receivedAt":"2024-12-20T16:30:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"It is possible to configure a Git build without Perl when disabling both\nour test suite and all Perl-based features. In Meson, this can be\nachieved with `meson setup -Dperl=disabled -Dtests=false`.\n\nIt was reported by a user that this breaks the Meson build because\ngitweb gets built even if Perl was not discovered in such a build:\n\n    $ meson setup .. -Dtests=false -Dperl=disabled\n    ...\n    ../gitweb/meson.build:2:43: ERROR: Unable to get the path of a not-found external program\n\nFix this issue by introducing a new feature-option that allows the user\nto configure whether or not to build Gitweb. The feature is set to\n'auto' by default and will be disabled automatically in case Perl was\nnot found on the system.\n\nReported-by: Daniel Engberg <daniel.engberg.lists@pyret.net>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\nChanges in v2:\n\n  - Fix awkward description of the new \"gitweb\" option.\n  - Link to v1: https://lore.kernel.org/r/20241220-b4-pks-meson-fix-gitweb-wo-perl-v1-1-41039ad8d8d4@pks.im\n\nThanks!\n\nPatrick\n---\n meson.build       | 13 +++++++++++--\n meson_options.txt |  2 ++\n 2 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex 0dccebcdf16b07650d943e53643f0e09e2975cc9..8e34a895dca80da77d6809a6fe90fe7661f142a1 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -740,7 +740,7 @@ endif\n # features. It is optional if you want to neither execute tests nor use any of\n # these optional features.\n perl_required = get_option('perl')\n-if get_option('tests')\n+if get_option('tests') or get_option('gitweb').enabled()\n   perl_required = true\n endif\n \n@@ -1874,7 +1874,15 @@ if intl.found()\n   subdir('po')\n endif\n subdir('contrib')\n-subdir('gitweb')\n+\n+# Gitweb requires Perl, so we disable the auto-feature if Perl was not found.\n+# We make sure further up that Perl is required in case the gitweb option is\n+# enabled.\n+gitweb_option = get_option('gitweb').disable_auto_if(not perl.found())\n+if gitweb_option.enabled()\n+  subdir('gitweb')\n+endif\n+\n subdir('templates')\n \n # Everything but the bin-wrappers need to come before this target such that we\n@@ -1893,6 +1901,7 @@ summary({\n   'curl': curl.found(),\n   'expat': expat.found(),\n   'gettext': intl.found(),\n+  'gitweb': gitweb_option.enabled(),\n   'https': https_backend,\n   'iconv': iconv.found(),\n   'pcre2': pcre2.found(),\ndiff --git a/meson_options.txt b/meson_options.txt\nindex 32a72139bae870745d9131cc9086a4594826be91..c374345b1a2e5bd9101fec07e9640b7726a623fb 100644\n--- a/meson_options.txt\n+++ b/meson_options.txt\n@@ -23,6 +23,8 @@ option('expat', type: 'feature', value: 'enabled',\n   description: 'Build helpers used to push to remotes with the HTTP transport.')\n option('gettext', type: 'feature', value: 'auto',\n   description: 'Build translation files.')\n+option('gitweb', type: 'feature', value: 'auto',\n+  description: 'Build Git web interface. Requires Perl.')\n option('iconv', type: 'feature', value: 'auto',\n   description: 'Support reencoding strings with different encodings.')\n option('pcre2', type: 'feature', value: 'enabled',\n\n---\nbase-commit: d882f382b3d939d90cfa58d17b17802338f05d66\nchange-id: 20241218-b4-pks-meson-fix-gitweb-wo-perl-93379dd0ceed\n\n"}]}