public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO
@ 2024-02-20  4:26 Eli Schwartz
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options Eli Schwartz
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  4:26 UTC (permalink / raw)
  To: gentoo-dev

Mainly motivated by some upstream work I did with dev-python/scipy,
which has some ancient code that will never be updated to work with LTO,
and some nice new code that works great.

The first patch is a nice improvement on its own. The second one makes
limited sense without the third one.

Eli Schwartz (3):
  meson.eclass: wire up LTO support directly into the meson options
  distutils-r1.eclass: wire up meson-python to meson.eclass
  distutils-r1.eclass: fix src_configure to handle flag-o-matic
    correctly

 eclass/distutils-r1.eclass | 11 +++++++----
 eclass/meson.eclass        | 36 ++++++++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 8 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options
  2024-02-20  4:26 [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO Eli Schwartz
@ 2024-02-20  4:26 ` Eli Schwartz
  2024-02-20  5:58   ` Mike Gilbert
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass Eli Schwartz
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  4:26 UTC (permalink / raw)
  To: gentoo-dev

meson's builtin LTO support allows meson to introspect whether LTO is
enabled and do some fancy things, such as forcing LTO off for a single
target that is known to be special(ly bad) and not support LTO.

Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---
 eclass/meson.eclass | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/eclass/meson.eclass b/eclass/meson.eclass
index d8bd93082ea5..c2ae289019e9 100644
--- a/eclass/meson.eclass
+++ b/eclass/meson.eclass
@@ -41,7 +41,7 @@ esac
 if [[ -z ${_MESON_ECLASS} ]]; then
 _MESON_ECLASS=1
 
-inherit multiprocessing ninja-utils python-utils-r1 toolchain-funcs
+inherit flag-o-matic multiprocessing ninja-utils python-utils-r1 toolchain-funcs
 
 BDEPEND=">=dev-build/meson-1.2.1
 	${NINJA_DEPEND}
@@ -286,6 +286,36 @@ meson_src_configure() {
 
 	[[ -n "${NINJA_DEPEND}" ]] || ewarn "Unknown value '${NINJA}' for \${NINJA}"
 
+	local ltoflags=()
+	if tc-is-lto; then
+		# We want to connect -flto in *FLAGS to the dedicated meson option,
+		# to ensure that meson has visibility into what the user set. Although
+		# it is unlikely projects will check `get_option('b_lto')` and change
+		# their behavior, individual targets which are broken with LTO can
+		# disable it per target. Injecting via *FLAGS means that meson cannot
+		# strip -flto from that target.
+		ltoflags+=( -Db_lto=true )
+
+		# respect -flto value, e.g. -flto=8, -flto=thin
+		local v=$(get-flag flto)
+		case ${v} in
+			thin)
+				ltoflags+=( -Db_lto_mode=thin )
+				;;
+			''|*[!0-9]*)
+				;;
+			*) ltoflags+=( -Db_lto_threads=${v} )
+		esac
+		# finally, remove it from *FLAGS to avoid passing it:
+		# - twice, with potentially different values
+		# - on excluded targets
+		filter-lto
+	else
+		# Prevent projects from enabling LTO by default.  In Gentoo, LTO is
+		# enabled via setting *FLAGS appropriately.
+		ltoflags+=( -Db_lto=false )
+	fi
+
 	local BUILD_CFLAGS=${BUILD_CFLAGS}
 	local BUILD_CPPFLAGS=${BUILD_CPPFLAGS}
 	local BUILD_CXXFLAGS=${BUILD_CXXFLAGS}
@@ -335,9 +365,7 @@ meson_src_configure() {
 		# an upstream development matter. bug #754279.
 		-Dwerror=false
 
-		# Prevent projects from enabling LTO by default.  In Gentoo, LTO is
-		# enabled via setting *FLAGS appropriately.
-		-Db_lto=false
+		"${ltoflags[@]}"
 	)
 
 	if [[ -n ${EMESON_BUILDTYPE} ]]; then
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass
  2024-02-20  4:26 [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO Eli Schwartz
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options Eli Schwartz
@ 2024-02-20  4:26 ` Eli Schwartz
  2024-02-20  4:42   ` Sam James
  2024-02-20  6:28   ` [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass Michał Górny
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 3/3] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly Eli Schwartz
  2024-02-20  4:41 ` [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO Sam James
  3 siblings, 2 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  4:26 UTC (permalink / raw)
  To: gentoo-dev

The meson-python build backend -- as the name suggests -- uses meson
under the hood. We have a meson eclass which does lots of useful things
pertinent to meson. Make sure it gets invoked.

Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---
 eclass/distutils-r1.eclass | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
index c0d1992ccce0..35825d4c3aa6 100644
--- a/eclass/distutils-r1.eclass
+++ b/eclass/distutils-r1.eclass
@@ -197,6 +197,10 @@ _DISTUTILS_R1_ECLASS=1
 inherit flag-o-matic
 inherit multibuild multilib multiprocessing ninja-utils toolchain-funcs
 
+if [[ ${DISTUTILS_USE_PEP517} = meson-python ]]; then
+	inherit meson
+fi
+
 if [[ ! ${DISTUTILS_SINGLE_IMPL} ]]; then
 	inherit python-r1
 else
@@ -1386,6 +1390,7 @@ distutils_pep517_install() {
 			)
 			;;
 		meson-python)
+			meson_src_configure "${DISTUTILS_ARGS[@]}"
 			local -x NINJAOPTS=$(get_NINJAOPTS)
 			config_settings=$(
 				"${EPYTHON}" - "${DISTUTILS_ARGS[@]}" <<-EOF || die
@@ -1397,7 +1402,6 @@ distutils_pep517_install() {
 					ninjaopts = shlex.split(os.environ["NINJAOPTS"])
 					print(json.dumps({
 						"builddir": "${BUILD_DIR}",
-						"setup-args": sys.argv[1:],
 						"compile-args": ["-v"] + ninjaopts,
 					}))
 				EOF
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [gentoo-dev] [PATCH 3/3] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly
  2024-02-20  4:26 [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO Eli Schwartz
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options Eli Schwartz
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass Eli Schwartz
@ 2024-02-20  4:26 ` Eli Schwartz
  2024-02-20  6:35   ` Michał Górny
  2024-02-20  4:41 ` [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO Sam James
  3 siblings, 1 reply; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  4:26 UTC (permalink / raw)
  To: gentoo-dev

Use of python_configure_all is a bit broken, because distutils-r1 is a
bit broken. It has the intriguing value-claim that *FLAGS shall be
modified with local -x as part of run-phase, which means that all
modifications are reset as soon as any given phase exits. Including
sub-phases. If you make changes in python_configure or
python_configure_all, as you are expected to do instead of defining
src_configure and then running the eclass phases manually, your changes
inherently get erased before you actually *get* to running builds (i.e.
reading the variables).

For an example of how this works, consider dev-python/scipy. It builds
with filter-lto, for the standard reasons. However, filter-lto gets run
inside python_configure_all, so it gets erased and ignored.

Note: it "worked" anyways prior to reworking meson.eclass to pass
-Db_lto based on `is-flagq flto`. This is because filter-lto removed it
from LDFLAGS, and since distutils-r1 doesn't localize that variable, the
changes stuck around. As a result:

- the previous fix to scipy caused scipy to be compiled with LTO, but
  not linked with LTO
- meson.eclass changes caused scipy to be built with -Db_lto=true, which
  affects both compiling and linking

It's absolutely unsafe to have flag-o-matic be ignored like this, and it
is fascinating to end up with LTO restored into compile flags while
still being filtered from LDFLAGS... simply as a side effect of
distutils-r1 not modifying the latter.

- this patch causes scipy to be built with -Db_lto=false

Consequences of the change make no difference to standard dev-python/
packages, but affect packages that use both distutils-r1 and other
packaging infrastructure:

- global variables for tools are exported. This is supposed to be a
  safe, albeit unnecessary for better-than-setuptools build systems,
  option.

- the "debug" option applies the DEBUG preprocessor macro to more than
  just python code. This was already the case for python projects that
  built non-CPython API C/C++ code and then linked them with thin python
  wrappers, so projects with python bindings shouldn't be any different.
  Also, it is a "debug" use flag so it's pretty silly if it only toggles
  debug on python bindings but not the rest of the package, so just...
  deal with it I guess?

- any project that uses cython, now ignores the Modern C Porting flag
  "incompatible-pointer-types". This is terrible, because code which
  should trigger that error is terrible. But it's also terrible for
  python projects with mixed Cython and handwritten C, to squelch that
  error just because one file happens to use Cython. Yet, we do it
  because we don't have a way to make extremely specific files built
  with different CFLAGS compared to the rest of the project. There's no
  actual reason to treat handwritten C python modules different from
  non-distutils phases.

Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---
 eclass/distutils-r1.eclass | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
index 35825d4c3aa6..873421bbcee8 100644
--- a/eclass/distutils-r1.eclass
+++ b/eclass/distutils-r1.eclass
@@ -1813,16 +1813,15 @@ distutils-r1_run_phase() {
 	fi
 
 	# Set up build environment, bug #513664.
-	local -x AR=${AR} CC=${CC} CPP=${CPP} CXX=${CXX}
 	tc-export AR CC CPP CXX
 
 	if [[ ${DISTUTILS_EXT} ]]; then
 		if [[ ${BDEPEND} == *dev-python/cython* ]] ; then
 			# Workaround for https://github.com/cython/cython/issues/2747 (bug #918983)
-			local -x CFLAGS="${CFLAGS} $(test-flags-CC -Wno-error=incompatible-pointer-types)"
+			append-cflags $(test-flags-CC -Wno-error=incompatible-pointer-types)
 		fi
 
-		local -x CPPFLAGS="${CPPFLAGS} $(usex debug '-UNDEBUG' '-DNDEBUG')"
+		append-cppflags $(usex debug '-UNDEBUG' '-DNDEBUG')
 		# always generate .c files from .pyx files to ensure we get latest
 		# bug fixes from Cython (this works only when setup.py is using
 		# cythonize() but it's better than nothing)
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO
  2024-02-20  4:26 [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO Eli Schwartz
                   ` (2 preceding siblings ...)
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 3/3] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly Eli Schwartz
@ 2024-02-20  4:41 ` Sam James
  3 siblings, 0 replies; 24+ messages in thread
From: Sam James @ 2024-02-20  4:41 UTC (permalink / raw)
  To: gentoo-dev

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]


Eli Schwartz <eschwartz93@gmail.com> writes:

> Mainly motivated by some upstream work I did with dev-python/scipy,
> which has some ancient code that will never be updated to work with LTO,
> and some nice new code that works great.
>
> The first patch is a nice improvement on its own. The second one makes
> limited sense without the third one.
>

The lot LGTM.

> Eli Schwartz (3):
>   meson.eclass: wire up LTO support directly into the meson options
>   distutils-r1.eclass: wire up meson-python to meson.eclass
>   distutils-r1.eclass: fix src_configure to handle flag-o-matic
>     correctly
>
>  eclass/distutils-r1.eclass | 11 +++++++----
>  eclass/meson.eclass        | 36 ++++++++++++++++++++++++++++++++----
>  2 files changed, 39 insertions(+), 8 deletions(-)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass Eli Schwartz
@ 2024-02-20  4:42   ` Sam James
  2024-02-20  5:12     ` Eli Schwartz
  2024-02-20  6:28   ` [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass Michał Górny
  1 sibling, 1 reply; 24+ messages in thread
From: Sam James @ 2024-02-20  4:42 UTC (permalink / raw)
  To: gentoo-dev

[-- Attachment #1: Type: text/plain, Size: 1562 bytes --]


Eli Schwartz <eschwartz93@gmail.com> writes:

> The meson-python build backend -- as the name suggests -- uses meson
> under the hood. We have a meson eclass which does lots of useful things
> pertinent to meson. Make sure it gets invoked.
>

Maybe check a sample (or ideally all) of the meson-python reverse
dependencies?

> Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
> ---
>  eclass/distutils-r1.eclass | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
> index c0d1992ccce0..35825d4c3aa6 100644
> --- a/eclass/distutils-r1.eclass
> +++ b/eclass/distutils-r1.eclass
> @@ -197,6 +197,10 @@ _DISTUTILS_R1_ECLASS=1
>  inherit flag-o-matic
>  inherit multibuild multilib multiprocessing ninja-utils toolchain-funcs
>  
> +if [[ ${DISTUTILS_USE_PEP517} = meson-python ]]; then
> +	inherit meson
> +fi
> +
>  if [[ ! ${DISTUTILS_SINGLE_IMPL} ]]; then
>  	inherit python-r1
>  else
> @@ -1386,6 +1390,7 @@ distutils_pep517_install() {
>  			)
>  			;;
>  		meson-python)
> +			meson_src_configure "${DISTUTILS_ARGS[@]}"
>  			local -x NINJAOPTS=$(get_NINJAOPTS)
>  			config_settings=$(
>  				"${EPYTHON}" - "${DISTUTILS_ARGS[@]}" <<-EOF || die
> @@ -1397,7 +1402,6 @@ distutils_pep517_install() {
>  					ninjaopts = shlex.split(os.environ["NINJAOPTS"])
>  					print(json.dumps({
>  						"builddir": "${BUILD_DIR}",
> -						"setup-args": sys.argv[1:],
>  						"compile-args": ["-v"] + ninjaopts,
>  					}))
>  				EOF


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass
  2024-02-20  4:42   ` Sam James
@ 2024-02-20  5:12     ` Eli Schwartz
  2024-02-20  6:14       ` [gentoo-dev] [PATCH v2 0/5] eclass updates for meson <> distutils <> LTO Eli Schwartz
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  5:12 UTC (permalink / raw)
  To: gentoo-dev


[-- Attachment #1.1.1: Type: text/plain, Size: 2181 bytes --]

On 2/19/24 11:42 PM, Sam James wrote:
> 
> Eli Schwartz <eschwartz93@gmail.com> writes:
> 
>> The meson-python build backend -- as the name suggests -- uses meson
>> under the hood. We have a meson eclass which does lots of useful things
>> pertinent to meson. Make sure it gets invoked.
>>
> 
> Maybe check a sample (or ideally all) of the meson-python reverse
> dependencies?


Actually it works great for scipy but fails for numpy since numpy is
using a localized testing branch of meson... so meson_src_configure
cannot parse the new meson module they are adding.

This is caused by the fact that I ran meson_src_configure followed by
gpep517 build-wheel. It is doubling up the configure runs, which isn't
great, but it wasn't obvious to me how to solve this:

- how to get at the accumulated mesonargs?
- BOOST_INCLUDEDIR / BOOST_LIBRARYDIR should be made global?


>> Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
>> ---
>>  eclass/distutils-r1.eclass | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
>> index c0d1992ccce0..35825d4c3aa6 100644
>> --- a/eclass/distutils-r1.eclass
>> +++ b/eclass/distutils-r1.eclass
>> @@ -197,6 +197,10 @@ _DISTUTILS_R1_ECLASS=1
>>  inherit flag-o-matic
>>  inherit multibuild multilib multiprocessing ninja-utils toolchain-funcs
>>  
>> +if [[ ${DISTUTILS_USE_PEP517} = meson-python ]]; then
>> +	inherit meson
>> +fi
>> +
>>  if [[ ! ${DISTUTILS_SINGLE_IMPL} ]]; then
>>  	inherit python-r1
>>  else
>> @@ -1386,6 +1390,7 @@ distutils_pep517_install() {
>>  			)
>>  			;;
>>  		meson-python)
>> +			meson_src_configure "${DISTUTILS_ARGS[@]}"
>>  			local -x NINJAOPTS=$(get_NINJAOPTS)
>>  			config_settings=$(
>>  				"${EPYTHON}" - "${DISTUTILS_ARGS[@]}" <<-EOF || die
>> @@ -1397,7 +1402,6 @@ distutils_pep517_install() {
>>  					ninjaopts = shlex.split(os.environ["NINJAOPTS"])
>>  					print(json.dumps({
>>  						"builddir": "${BUILD_DIR}",
>> -						"setup-args": sys.argv[1:],
>>  						"compile-args": ["-v"] + ninjaopts,
>>  					}))
>>  				EOF
> 

-- 
Eli Schwartz

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 18399 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options Eli Schwartz
@ 2024-02-20  5:58   ` Mike Gilbert
  2024-02-20  6:09     ` Eli Schwartz
  0 siblings, 1 reply; 24+ messages in thread
From: Mike Gilbert @ 2024-02-20  5:58 UTC (permalink / raw)
  To: gentoo-dev

On Mon, Feb 19, 2024 at 11:26 PM Eli Schwartz <eschwartz93@gmail.com> wrote:
>
> meson's builtin LTO support allows meson to introspect whether LTO is
> enabled and do some fancy things, such as forcing LTO off for a single
> target that is known to be special(ly bad) and not support LTO.

Please make sure to test this change with a multilib-enabled ebuild
with multiple ABIs enabled. I suspect the filter-lto call will cause
differing results for the ABIs after the first.

If that is the case, we may need to declare the relevant FLAGS
variables with "local -x".


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options
  2024-02-20  5:58   ` Mike Gilbert
@ 2024-02-20  6:09     ` Eli Schwartz
  2024-02-20  6:24       ` Mike Gilbert
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:09 UTC (permalink / raw)
  To: gentoo-dev


[-- Attachment #1.1.1: Type: text/plain, Size: 2992 bytes --]

On 2/20/24 12:58 AM, Mike Gilbert wrote:
> On Mon, Feb 19, 2024 at 11:26 PM Eli Schwartz <eschwartz93@gmail.com> wrote:
>>
>> meson's builtin LTO support allows meson to introspect whether LTO is
>> enabled and do some fancy things, such as forcing LTO off for a single
>> target that is known to be special(ly bad) and not support LTO.
> 
> Please make sure to test this change with a multilib-enabled ebuild
> with multiple ABIs enabled. I suspect the filter-lto call will cause
> differing results for the ABIs after the first.
> 
> If that is the case, we may need to declare the relevant FLAGS
> variables with "local -x".


>>> Configuring source in
/var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson ...
 * abi_x86_32.x86: running multilib-minimal_abi_src_configure
meson setup --libdir lib --localstatedir /var/lib --prefix /usr
--sysconfdir /etc --wrap-mode nodownload --build.pkg-config-path
/usr/share/pkgconfig:/usr/share/pkgconfig --pkg-config-path
/usr/share/pkgconfig:/usr/share/pkgconfig --native-file
/var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini
-Db_pch=false -Dwerror=false -Db_lto=true -Db_lto_threads=4
-Dbuildtype=plain -Ddefault_library=shared -Dbin_programs=false
-Dbin_contrib=false -Dbin_tests=false -Dzlib=disabled -Dlzma=disabled
-Dlz4=disabled --native-file
/var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini.local
/var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson
/var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_32.x86
[...]
    Native files         :
/var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini

/var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini.local
    b_lto                : true
    b_lto_threads        : 4




 * abi_x86_64.amd64: running multilib-minimal_abi_src_configure
meson setup --libdir lib64 --localstatedir /var/lib --prefix /usr
--sysconfdir /etc --wrap-mode nodownload --build.pkg-config-path
/usr/share/pkgconfig --pkg-config-path /usr/share/pkgconfig
--native-file
/var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini
-Db_pch=false -Dwerror=false -Db_lto=true -Db_lto_threads=4
-Dbuildtype=plain -Ddefault_library=shared -Dbin_programs=true
-Dbin_contrib=true -Dbin_tests=false -Dzlib=enabled -Dlzma=enabled
-Dlz4=disabled --native-file
/var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini.local
/var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson
/var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_64.amd64
[...]
    Native files         :
/var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini

/var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini.local
    b_lto                : true
    b_lto_threads        : 4





-- 
Eli Schwartz

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 18399 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [gentoo-dev] [PATCH v2 0/5] eclass updates for meson <> distutils <> LTO
  2024-02-20  5:12     ` Eli Schwartz
@ 2024-02-20  6:14       ` Eli Schwartz
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 1/5] meson.eclass: wire up LTO support directly into the meson options Eli Schwartz
                           ` (4 more replies)
  0 siblings, 5 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:14 UTC (permalink / raw)
  To: gentoo-dev

v2 answers the question: how to deal with accumulated mesonargs without
actually *running* meson_src_configure before gpep517

Eli Schwartz (5):
  meson.eclass: wire up LTO support directly into the meson options
  meson.eclass: prefer -D buildtype instead of --buildtype
  meson.eclass: refactor src_configure into a setter function
  distutils-r1.eclass: wire up meson-python to meson.eclass
  distutils-r1.eclass: fix src_configure to handle flag-o-matic
    correctly

 eclass/distutils-r1.eclass | 13 ++++--
 eclass/meson.eclass        | 85 +++++++++++++++++++++++++++-----------
 2 files changed, 70 insertions(+), 28 deletions(-)

Range-diff against v1:
1:  aac9d1516675 = 1:  aac9d1516675 meson.eclass: wire up LTO support directly into the meson options
-:  ------------ > 2:  cf98596d9bd1 meson.eclass: prefer -D buildtype instead of --buildtype
-:  ------------ > 3:  7ac90f1b2d88 meson.eclass: refactor src_configure into a setter function
2:  bcbec23f5c76 ! 4:  e8387e8dec72 distutils-r1.eclass: wire up meson-python to meson.eclass
    @@ Commit message
     
         The meson-python build backend -- as the name suggests -- uses meson
         under the hood. We have a meson eclass which does lots of useful things
    -    pertinent to meson. Make sure it gets invoked.
    +    pertinent to meson. Make sure it gets invoked, by prying out the options
    +    that meson_src_configure would use and setting passing them as our seed
    +    values for gpep517.
     
         Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
     
    @@ eclass/distutils-r1.eclass: distutils_pep517_install() {
      			)
      			;;
      		meson-python)
    -+			meson_src_configure "${DISTUTILS_ARGS[@]}"
    ++			local mesonargs=()
    ++			setup_meson_src_configure "${DISTUTILS_ARGS[@]}"
      			local -x NINJAOPTS=$(get_NINJAOPTS)
      			config_settings=$(
    - 				"${EPYTHON}" - "${DISTUTILS_ARGS[@]}" <<-EOF || die
    -@@ eclass/distutils-r1.eclass: distutils_pep517_install() {
    - 					ninjaopts = shlex.split(os.environ["NINJAOPTS"])
    - 					print(json.dumps({
    - 						"builddir": "${BUILD_DIR}",
    --						"setup-args": sys.argv[1:],
    - 						"compile-args": ["-v"] + ninjaopts,
    - 					}))
    - 				EOF
    +-				"${EPYTHON}" - "${DISTUTILS_ARGS[@]}" <<-EOF || die
    ++				"${EPYTHON}" - "${mesonargs[@]}" <<-EOF || die
    + 					import json
    + 					import os
    + 					import shlex
-:  ------------ > 5:  a30c280f4573 distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly
-- 
2.43.0



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [gentoo-dev] [PATCH v2 1/5] meson.eclass: wire up LTO support directly into the meson options
  2024-02-20  6:14       ` [gentoo-dev] [PATCH v2 0/5] eclass updates for meson <> distutils <> LTO Eli Schwartz
@ 2024-02-20  6:14         ` Eli Schwartz
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 2/5] meson.eclass: prefer -D buildtype instead of --buildtype Eli Schwartz
                           ` (3 subsequent siblings)
  4 siblings, 0 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:14 UTC (permalink / raw)
  To: gentoo-dev

meson's builtin LTO support allows meson to introspect whether LTO is
enabled and do some fancy things, such as forcing LTO off for a single
target that is known to be special(ly bad) and not support LTO.

Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---

no change

 eclass/meson.eclass | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/eclass/meson.eclass b/eclass/meson.eclass
index d8bd93082ea5..c2ae289019e9 100644
--- a/eclass/meson.eclass
+++ b/eclass/meson.eclass
@@ -41,7 +41,7 @@ esac
 if [[ -z ${_MESON_ECLASS} ]]; then
 _MESON_ECLASS=1
 
-inherit multiprocessing ninja-utils python-utils-r1 toolchain-funcs
+inherit flag-o-matic multiprocessing ninja-utils python-utils-r1 toolchain-funcs
 
 BDEPEND=">=dev-build/meson-1.2.1
 	${NINJA_DEPEND}
@@ -286,6 +286,36 @@ meson_src_configure() {
 
 	[[ -n "${NINJA_DEPEND}" ]] || ewarn "Unknown value '${NINJA}' for \${NINJA}"
 
+	local ltoflags=()
+	if tc-is-lto; then
+		# We want to connect -flto in *FLAGS to the dedicated meson option,
+		# to ensure that meson has visibility into what the user set. Although
+		# it is unlikely projects will check `get_option('b_lto')` and change
+		# their behavior, individual targets which are broken with LTO can
+		# disable it per target. Injecting via *FLAGS means that meson cannot
+		# strip -flto from that target.
+		ltoflags+=( -Db_lto=true )
+
+		# respect -flto value, e.g. -flto=8, -flto=thin
+		local v=$(get-flag flto)
+		case ${v} in
+			thin)
+				ltoflags+=( -Db_lto_mode=thin )
+				;;
+			''|*[!0-9]*)
+				;;
+			*) ltoflags+=( -Db_lto_threads=${v} )
+		esac
+		# finally, remove it from *FLAGS to avoid passing it:
+		# - twice, with potentially different values
+		# - on excluded targets
+		filter-lto
+	else
+		# Prevent projects from enabling LTO by default.  In Gentoo, LTO is
+		# enabled via setting *FLAGS appropriately.
+		ltoflags+=( -Db_lto=false )
+	fi
+
 	local BUILD_CFLAGS=${BUILD_CFLAGS}
 	local BUILD_CPPFLAGS=${BUILD_CPPFLAGS}
 	local BUILD_CXXFLAGS=${BUILD_CXXFLAGS}
@@ -335,9 +365,7 @@ meson_src_configure() {
 		# an upstream development matter. bug #754279.
 		-Dwerror=false
 
-		# Prevent projects from enabling LTO by default.  In Gentoo, LTO is
-		# enabled via setting *FLAGS appropriately.
-		-Db_lto=false
+		"${ltoflags[@]}"
 	)
 
 	if [[ -n ${EMESON_BUILDTYPE} ]]; then
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [gentoo-dev] [PATCH v2 2/5] meson.eclass: prefer -D buildtype instead of --buildtype
  2024-02-20  6:14       ` [gentoo-dev] [PATCH v2 0/5] eclass updates for meson <> distutils <> LTO Eli Schwartz
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 1/5] meson.eclass: wire up LTO support directly into the meson options Eli Schwartz
@ 2024-02-20  6:14         ` Eli Schwartz
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 3/5] meson.eclass: refactor src_configure into a setter function Eli Schwartz
                           ` (2 subsequent siblings)
  4 siblings, 0 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:14 UTC (permalink / raw)
  To: gentoo-dev

Because that is the logic which meson-python hardcodes, and meson needs
to match calling convention.

Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---

v2: new patch

 eclass/meson.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/meson.eclass b/eclass/meson.eclass
index c2ae289019e9..e1963e552710 100644
--- a/eclass/meson.eclass
+++ b/eclass/meson.eclass
@@ -369,7 +369,7 @@ meson_src_configure() {
 	)
 
 	if [[ -n ${EMESON_BUILDTYPE} ]]; then
-		mesonargs+=( --buildtype "${EMESON_BUILDTYPE}" )
+		mesonargs+=( -Dbuildtype="${EMESON_BUILDTYPE}" )
 	fi
 
 	if tc-is-cross-compiler; then
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [gentoo-dev] [PATCH v2 3/5] meson.eclass: refactor src_configure into a setter function
  2024-02-20  6:14       ` [gentoo-dev] [PATCH v2 0/5] eclass updates for meson <> distutils <> LTO Eli Schwartz
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 1/5] meson.eclass: wire up LTO support directly into the meson options Eli Schwartz
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 2/5] meson.eclass: prefer -D buildtype instead of --buildtype Eli Schwartz
@ 2024-02-20  6:14         ` Eli Schwartz
  2024-02-20  6:42           ` [gentoo-dev] " Eli Schwartz
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 4/5] distutils-r1.eclass: wire up meson-python to meson.eclass Eli Schwartz
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 5/5] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly Eli Schwartz
  4 siblings, 1 reply; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:14 UTC (permalink / raw)
  To: gentoo-dev

This is necessary in order to get at the implementation of `meson setup`
from other eclasses, which do not simply call meson_src_configure. The
intended use case is distutils-r1, where a python build backend wraps
meson and needs its arguments while calling meson on its own.

This allows distutils-r1 to invoke `setup_meson_src_configure` followed
by gpep517, and get access to:
- the preparation which needs to be done, including setting up the
  environment
- the array of setup arguments

Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---

v2: new patch

 eclass/meson.eclass | 49 +++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/eclass/meson.eclass b/eclass/meson.eclass
index e1963e552710..620381ed7079 100644
--- a/eclass/meson.eclass
+++ b/eclass/meson.eclass
@@ -277,15 +277,12 @@ meson_feature() {
 	usex "$1" "-D${2-$1}=enabled" "-D${2-$1}=disabled"
 }
 
-# @FUNCTION: meson_src_configure
-# @USAGE: [extra meson arguments]
+# @FUNCTION: setup_meson_src_configure
 # @DESCRIPTION:
-# This is the meson_src_configure function.
-meson_src_configure() {
-	debug-print-function ${FUNCNAME} "$@"
-
-	[[ -n "${NINJA_DEPEND}" ]] || ewarn "Unknown value '${NINJA}' for \${NINJA}"
-
+# Calculate the command line which meson should use, and other relevant
+# variables. Invoke via "${mesonargs[@]}" in the calling environment.
+# This function is called from meson_src_configure.
+setup_meson_src_configure() {
 	local ltoflags=()
 	if tc-is-lto; then
 		# We want to connect -flto in *FLAGS to the dedicated meson option,
@@ -344,8 +341,7 @@ meson_src_configure() {
 		: "${BUILD_PKG_CONFIG_PATH:=${PKG_CONFIG_PATH}}"
 	fi
 
-	local mesonargs=(
-		meson setup
+	mesonargs=(
 		--libdir "$(get_libdir)"
 		--localstatedir "${EPREFIX}/var/lib"
 		--prefix "${EPREFIX}/usr"
@@ -390,12 +386,6 @@ meson_src_configure() {
 
 		# Arguments from user
 		"${MYMESONARGS[@]}"
-
-		# Source directory
-		"${EMESON_SOURCE:-${S}}"
-
-		# Build directory
-		"${BUILD_DIR}"
 	)
 
 	# Used by symbolextractor.py
@@ -407,13 +397,32 @@ meson_src_configure() {
 	python_export_utf8_locale
 
 	# https://bugs.gentoo.org/721786
-	local -x BOOST_INCLUDEDIR="${BOOST_INCLUDEDIR-${EPREFIX}/usr/include}"
-	local -x BOOST_LIBRARYDIR="${BOOST_LIBRARYDIR-${EPREFIX}/usr/$(get_libdir)}"
+	export BOOST_INCLUDEDIR="${BOOST_INCLUDEDIR-${EPREFIX}/usr/include}"
+	export BOOST_LIBRARYDIR="${BOOST_LIBRARYDIR-${EPREFIX}/usr/$(get_libdir)}"
+}
+
+# @FUNCTION: meson_src_configure
+# @USAGE: [extra meson arguments]
+# @DESCRIPTION:
+# This is the meson_src_configure function.
+meson_src_configure() {
+	debug-print-function ${FUNCNAME} "$@"
+
+	[[ -n "${NINJA_DEPEND}" ]] || ewarn "Unknown value '${NINJA}' for \${NINJA}"
 
 	(
+		setup_meson_src_configure "$@"
+		mesonargs+=(
+			# Source directory
+			"${EMESON_SOURCE:-${S}}"
+
+			# Build directory
+			"${BUILD_DIR}"
+		)
+
 		export -n {C,CPP,CXX,F,OBJC,OBJCXX,LD}FLAGS PKG_CONFIG_{LIBDIR,PATH}
-		echo "${mesonargs[@]}" >&2
-		"${mesonargs[@]}"
+		echo meson setup "${mesonargs[@]}" >&2
+		meson setup "${mesonargs[@]}"
 	) || die
 }
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [gentoo-dev] [PATCH v2 4/5] distutils-r1.eclass: wire up meson-python to meson.eclass
  2024-02-20  6:14       ` [gentoo-dev] [PATCH v2 0/5] eclass updates for meson <> distutils <> LTO Eli Schwartz
                           ` (2 preceding siblings ...)
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 3/5] meson.eclass: refactor src_configure into a setter function Eli Schwartz
@ 2024-02-20  6:14         ` Eli Schwartz
  2024-02-20  6:55           ` Michał Górny
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 5/5] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly Eli Schwartz
  4 siblings, 1 reply; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:14 UTC (permalink / raw)
  To: gentoo-dev

The meson-python build backend -- as the name suggests -- uses meson
under the hood. We have a meson eclass which does lots of useful things
pertinent to meson. Make sure it gets invoked, by prying out the options
that meson_src_configure would use and setting passing them as our seed
values for gpep517.

Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---

v2: call setup_meson_src_configure instead of meson_src_configure. This
avoids running `meson setup` twice, and guarantees we use whatever
settings the PEP517 backend requires. In particular, it respects numpy's
vendored meson fork with experimental new features.

 eclass/distutils-r1.eclass | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
index c0d1992ccce0..a42adc182ed9 100644
--- a/eclass/distutils-r1.eclass
+++ b/eclass/distutils-r1.eclass
@@ -197,6 +197,10 @@ _DISTUTILS_R1_ECLASS=1
 inherit flag-o-matic
 inherit multibuild multilib multiprocessing ninja-utils toolchain-funcs
 
+if [[ ${DISTUTILS_USE_PEP517} = meson-python ]]; then
+	inherit meson
+fi
+
 if [[ ! ${DISTUTILS_SINGLE_IMPL} ]]; then
 	inherit python-r1
 else
@@ -1386,9 +1390,11 @@ distutils_pep517_install() {
 			)
 			;;
 		meson-python)
+			local mesonargs=()
+			setup_meson_src_configure "${DISTUTILS_ARGS[@]}"
 			local -x NINJAOPTS=$(get_NINJAOPTS)
 			config_settings=$(
-				"${EPYTHON}" - "${DISTUTILS_ARGS[@]}" <<-EOF || die
+				"${EPYTHON}" - "${mesonargs[@]}" <<-EOF || die
 					import json
 					import os
 					import shlex
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [gentoo-dev] [PATCH v2 5/5] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly
  2024-02-20  6:14       ` [gentoo-dev] [PATCH v2 0/5] eclass updates for meson <> distutils <> LTO Eli Schwartz
                           ` (3 preceding siblings ...)
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 4/5] distutils-r1.eclass: wire up meson-python to meson.eclass Eli Schwartz
@ 2024-02-20  6:14         ` Eli Schwartz
  4 siblings, 0 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:14 UTC (permalink / raw)
  To: gentoo-dev

Use of python_configure_all is a bit broken, because distutils-r1 is a
bit broken. It has the intriguing value-claim that *FLAGS shall be
modified with local -x as part of run-phase, which means that all
modifications are reset as soon as any given phase exits. Including
sub-phases. If you make changes in python_configure or
python_configure_all, as you are expected to do instead of defining
src_configure and then running the eclass phases manually, your changes
inherently get erased before you actually *get* to running builds (i.e.
reading the variables).

For an example of how this works, consider dev-python/scipy. It builds
with filter-lto, for the standard reasons. However, filter-lto gets run
inside python_configure_all, so it gets erased and ignored.

Note: it "worked" anyways prior to reworking meson.eclass to pass
-Db_lto based on `is-flagq flto`. This is because filter-lto removed it
from LDFLAGS, and since distutils-r1 doesn't localize that variable, the
changes stuck around. As a result:

- the previous fix to scipy caused scipy to be compiled with LTO, but
  not linked with LTO
- meson.eclass changes caused scipy to be built with -Db_lto=true, which
  affects both compiling and linking

It's absolutely unsafe to have flag-o-matic be ignored like this, and it
is fascinating to end up with LTO restored into compile flags while
still being filtered from LDFLAGS... simply as a side effect of
distutils-r1 not modifying the latter.

- this patch causes scipy to be built with -Db_lto=false

Consequences of the change make no difference to standard dev-python/
packages, but affect packages that use both distutils-r1 and other
packaging infrastructure:

- global variables for tools are exported. This is supposed to be a
  safe, albeit unnecessary for better-than-setuptools build systems,
  option.

- the "debug" option applies the DEBUG preprocessor macro to more than
  just python code. This was already the case for python projects that
  built non-CPython API C/C++ code and then linked them with thin python
  wrappers, so projects with python bindings shouldn't be any different.
  Also, it is a "debug" use flag so it's pretty silly if it only toggles
  debug on python bindings but not the rest of the package, so just...
  deal with it I guess?

- any project that uses cython, now ignores the Modern C Porting flag
  "incompatible-pointer-types". This is terrible, because code which
  should trigger that error is terrible. But it's also terrible for
  python projects with mixed Cython and handwritten C, to squelch that
  error just because one file happens to use Cython. Yet, we do it
  because we don't have a way to make extremely specific files built
  with different CFLAGS compared to the rest of the project. There's no
  actual reason to treat handwritten C python modules different from
  non-distutils phases.

Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---

no change

 eclass/distutils-r1.eclass | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
index a42adc182ed9..841fe2df5405 100644
--- a/eclass/distutils-r1.eclass
+++ b/eclass/distutils-r1.eclass
@@ -1815,16 +1815,15 @@ distutils-r1_run_phase() {
 	fi
 
 	# Set up build environment, bug #513664.
-	local -x AR=${AR} CC=${CC} CPP=${CPP} CXX=${CXX}
 	tc-export AR CC CPP CXX
 
 	if [[ ${DISTUTILS_EXT} ]]; then
 		if [[ ${BDEPEND} == *dev-python/cython* ]] ; then
 			# Workaround for https://github.com/cython/cython/issues/2747 (bug #918983)
-			local -x CFLAGS="${CFLAGS} $(test-flags-CC -Wno-error=incompatible-pointer-types)"
+			append-cflags $(test-flags-CC -Wno-error=incompatible-pointer-types)
 		fi
 
-		local -x CPPFLAGS="${CPPFLAGS} $(usex debug '-UNDEBUG' '-DNDEBUG')"
+		append-cppflags $(usex debug '-UNDEBUG' '-DNDEBUG')
 		# always generate .c files from .pyx files to ensure we get latest
 		# bug fixes from Cython (this works only when setup.py is using
 		# cythonize() but it's better than nothing)
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options
  2024-02-20  6:09     ` Eli Schwartz
@ 2024-02-20  6:24       ` Mike Gilbert
  2024-02-20  6:33         ` Eli Schwartz
  0 siblings, 1 reply; 24+ messages in thread
From: Mike Gilbert @ 2024-02-20  6:24 UTC (permalink / raw)
  To: gentoo-dev

[-- Attachment #1: Type: text/plain, Size: 3415 bytes --]

On Tue, Feb 20, 2024 at 1:09 AM Eli Schwartz <eschwartz93@gmail.com> wrote:
>
> On 2/20/24 12:58 AM, Mike Gilbert wrote:
> > On Mon, Feb 19, 2024 at 11:26 PM Eli Schwartz <eschwartz93@gmail.com> wrote:
> >>
> >> meson's builtin LTO support allows meson to introspect whether LTO is
> >> enabled and do some fancy things, such as forcing LTO off for a single
> >> target that is known to be special(ly bad) and not support LTO.
> >
> > Please make sure to test this change with a multilib-enabled ebuild
> > with multiple ABIs enabled. I suspect the filter-lto call will cause
> > differing results for the ABIs after the first.
> >
> > If that is the case, we may need to declare the relevant FLAGS
> > variables with "local -x".
>
>
> >>> Configuring source in
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson ...
>  * abi_x86_32.x86: running multilib-minimal_abi_src_configure
> meson setup --libdir lib --localstatedir /var/lib --prefix /usr
> --sysconfdir /etc --wrap-mode nodownload --build.pkg-config-path
> /usr/share/pkgconfig:/usr/share/pkgconfig --pkg-config-path
> /usr/share/pkgconfig:/usr/share/pkgconfig --native-file
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini
> -Db_pch=false -Dwerror=false -Db_lto=true -Db_lto_threads=4
> -Dbuildtype=plain -Ddefault_library=shared -Dbin_programs=false
> -Dbin_contrib=false -Dbin_tests=false -Dzlib=disabled -Dlzma=disabled
> -Dlz4=disabled --native-file
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini.local
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_32.x86
> [...]
>     Native files         :
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini
>
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini.local
>     b_lto                : true
>     b_lto_threads        : 4
>
>
>
>
>  * abi_x86_64.amd64: running multilib-minimal_abi_src_configure
> meson setup --libdir lib64 --localstatedir /var/lib --prefix /usr
> --sysconfdir /etc --wrap-mode nodownload --build.pkg-config-path
> /usr/share/pkgconfig --pkg-config-path /usr/share/pkgconfig
> --native-file
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini
> -Db_pch=false -Dwerror=false -Db_lto=true -Db_lto_threads=4
> -Dbuildtype=plain -Ddefault_library=shared -Dbin_programs=true
> -Dbin_contrib=true -Dbin_tests=false -Dzlib=enabled -Dlzma=enabled
> -Dlz4=disabled --native-file
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini.local
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_64.amd64
> [...]
>     Native files         :
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini
>
> /var/tmp/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini.local
>     b_lto                : true
>     b_lto_threads        : 4

I'm afraid I get different results. Build log attached. Happy to help
figure this out tomorrow.

To test, I applied this patch and ran this:

ABI_X86="32 x32 64" CFLAGS="-O2 -pipe -march=amdfam10 -flto=2" ebuild
zstd-1.5.5-r1.ebuild clean configure

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: app-arch:zstd-1.5.5-r1:20240220-061809.log --]
[-- Type: text/x-log; charset="x-binaryenc";  name="app-arch:zstd-1.5.5-r1:20240220-061809.log", Size: 11952 bytes --]

^[[32m * ^[[39;49;00mPackage:    app-arch/zstd-1.5.5-r1:0/1
^[[32m * ^[[39;49;00mRepository: gentoo
^[[32m * ^[[39;49;00mMaintainer: base-system@gentoo.org
^[[32m * ^[[39;49;00mUSE:        abi_x86_32 abi_x86_64 abi_x86_x32 amd64 elibc_glibc kernel_linux lzma zlib
^[[32m * ^[[39;49;00mFEATURES:   ccache network-sandbox preserve-libs sandbox userpriv usersandbox
^[[32m * ^[[39;49;00mPackage:    app-arch/zstd-1.5.5-r1:0/1
^[[32m * ^[[39;49;00mRepository: gentoo
^[[32m * ^[[39;49;00mMaintainer: base-system@gentoo.org
^[[32m * ^[[39;49;00mUSE:        abi_x86_32 abi_x86_64 abi_x86_x32 amd64 elibc_glibc kernel_linux lzma zlib
^[[32m * ^[[39;49;00mFEATURES:   ccache network-sandbox preserve-libs sandbox userpriv usersandbox
>>> Unpacking source...
>>> Unpacking zstd-1.5.5.tar.gz to /x/portage/app-arch/zstd-1.5.5-r1/work
>>> Source unpacked in /x/portage/app-arch/zstd-1.5.5-r1/work
>>> Preparing source in /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson ...
 ^[[32m*^[[0m Applying zstd-1.5.4-no-find-valgrind.patch ...
^[[A^[[92C ^[[34;01m[ ^[[32;01mok^[[34;01m ]^[[0m
>>> Source prepared.
>>> Configuring source in /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson ...
 ^[[32m*^[[0m abi_x86_32.x86: running multilib-minimal_abi_src_configure
meson setup --libdir lib --localstatedir /var/lib --prefix /usr --sysconfdir /etc --wrap-mode nodownload --build.pkg-config-path /usr/share/pkgconfig:/usr/share/pkgconfig --pkg-config-path /usr/share/pkgconfig:/usr/share/pkgconfig --native-file /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini -Db_pch=false -Dwerror=false -Db_lto=true -Db_lto_threads=2 --buildtype plain -Ddefault_library=shared -Dbin_programs=false -Dbin_contrib=false -Dbin_tests=false -Dzlib=disabled -Dlzma=disabled -Dlz4=disabled --native-file /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini.local /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_32.x86
The Meson build system
Version: 1.3.1
Source dir: /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson
Build dir: /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_32.x86
Build type: native build
Program GetZstdLibraryVersion.py found: YES (/usr/bin/python3.12 /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson/GetZstdLibraryVersion.py)
Project name: zstd
Project version: 1.5.5
C compiler for the host machine: x86_64-pc-linux-gnu-gcc -m32 -mfpmath=sse (gcc 13.2.1 "x86_64-pc-linux-gnu-gcc (Gentoo 13.2.1_p20240113-r1 p12) 13.2.1 20240113")
C linker for the host machine: x86_64-pc-linux-gnu-gcc -m32 -mfpmath=sse ld.bfd 2.41
C++ compiler for the host machine: x86_64-pc-linux-gnu-g++ -m32 -mfpmath=sse (gcc 13.2.1 "x86_64-pc-linux-gnu-g++ (Gentoo 13.2.1_p20240113-r1 p12) 13.2.1 20240113")
C++ linker for the host machine: x86_64-pc-linux-gnu-g++ -m32 -mfpmath=sse ld.bfd 2.41
Host machine cpu family: x86
Host machine cpu: i686
Library m found: YES
Run-time dependency threads found: YES
Dependency zlib skipped: feature zlib disabled
Dependency liblzma skipped: feature lzma disabled
Dependency liblz4 skipped: feature lz4 disabled
Compiler for C supports arguments -Wundef: YES 
Compiler for C supports arguments -Wshadow: YES 
Compiler for C supports arguments -Wcast-align: YES 
Compiler for C supports arguments -Wcast-qual: YES 
Compiler for C supports arguments -Wstrict-prototypes: YES 
Compiler for C++ supports arguments -Wundef: YES 
Compiler for C++ supports arguments -Wshadow: YES 
Compiler for C++ supports arguments -Wcast-align: YES 
Compiler for C++ supports arguments -Wcast-qual: YES 
Message: Enable legacy support back to version 0.5
Message: Enable multi-threading support
Found pkg-config: YES (/usr/bin/x86_64-pc-linux-gnu-pkg-config) 2.1.0
Build targets in project: 2

zstd 1.5.5

  User defined options
    Native files         : /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini
                           /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.i686-pc-linux-gnu.x86.ini.local
    build.pkg_config_path: /usr/share/pkgconfig:/usr/share/pkgconfig
    buildtype            : plain
    default_library      : shared
    libdir               : lib
    localstatedir        : /var/lib
    pkg_config_path      : /usr/share/pkgconfig:/usr/share/pkgconfig
    prefix               : /usr
    sysconfdir           : /etc
    werror               : false
    wrap_mode            : nodownload
    b_lto                : true
    b_lto_threads        : 2
    b_pch                : false
    bin_contrib          : false
    bin_programs         : false
    bin_tests            : false
    lz4                  : disabled
    lzma                 : disabled
    zlib                 : disabled

Found ninja-1.11.1 at /usr/bin/ninja
 ^[[32m*^[[0m abi_x86_x32.x32: running multilib-minimal_abi_src_configure
meson setup --libdir libx32 --localstatedir /var/lib --prefix /usr --sysconfdir /etc --wrap-mode nodownload --build.pkg-config-path /usr/share/pkgconfig:/usr/share/pkgconfig --pkg-config-path /usr/share/pkgconfig:/usr/share/pkgconfig --native-file /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnux32.x32.ini -Db_pch=false -Dwerror=false -Db_lto=false --buildtype plain -Ddefault_library=shared -Dbin_programs=false -Dbin_contrib=false -Dbin_tests=false -Dzlib=disabled -Dlzma=disabled -Dlz4=disabled --native-file /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnux32.x32.ini.local /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_x32.x32
The Meson build system
Version: 1.3.1
Source dir: /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson
Build dir: /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_x32.x32
Build type: native build
Program GetZstdLibraryVersion.py found: YES (/usr/bin/python3.12 /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson/GetZstdLibraryVersion.py)
Project name: zstd
Project version: 1.5.5
C compiler for the host machine: x86_64-pc-linux-gnu-gcc -mx32 (gcc 13.2.1 "x86_64-pc-linux-gnu-gcc (Gentoo 13.2.1_p20240113-r1 p12) 13.2.1 20240113")
C linker for the host machine: x86_64-pc-linux-gnu-gcc -mx32 ld.bfd 2.41
C++ compiler for the host machine: x86_64-pc-linux-gnu-g++ -mx32 (gcc 13.2.1 "x86_64-pc-linux-gnu-g++ (Gentoo 13.2.1_p20240113-r1 p12) 13.2.1 20240113")
C++ linker for the host machine: x86_64-pc-linux-gnu-g++ -mx32 ld.bfd 2.41
Host machine cpu family: x86_64
Host machine cpu: x86_64
Library m found: YES
Run-time dependency threads found: YES
Dependency zlib skipped: feature zlib disabled
Dependency liblzma skipped: feature lzma disabled
Dependency liblz4 skipped: feature lz4 disabled
Compiler for C supports arguments -Wundef: YES 
Compiler for C supports arguments -Wshadow: YES 
Compiler for C supports arguments -Wcast-align: YES 
Compiler for C supports arguments -Wcast-qual: YES 
Compiler for C supports arguments -Wstrict-prototypes: YES 
Compiler for C++ supports arguments -Wundef: YES 
Compiler for C++ supports arguments -Wshadow: YES 
Compiler for C++ supports arguments -Wcast-align: YES 
Compiler for C++ supports arguments -Wcast-qual: YES 
Message: Enable legacy support back to version 0.5
Message: Enable multi-threading support
Found pkg-config: YES (/usr/bin/x86_64-pc-linux-gnu-pkg-config) 2.1.0
Build targets in project: 2

zstd 1.5.5

  User defined options
    Native files         : /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnux32.x32.ini
                           /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnux32.x32.ini.local
    build.pkg_config_path: /usr/share/pkgconfig:/usr/share/pkgconfig
    buildtype            : plain
    default_library      : shared
    libdir               : libx32
    localstatedir        : /var/lib
    pkg_config_path      : /usr/share/pkgconfig:/usr/share/pkgconfig
    prefix               : /usr
    sysconfdir           : /etc
    werror               : false
    wrap_mode            : nodownload
    b_lto                : false
    b_pch                : false
    bin_contrib          : false
    bin_programs         : false
    bin_tests            : false
    lz4                  : disabled
    lzma                 : disabled
    zlib                 : disabled

Found ninja-1.11.1 at /usr/bin/ninja
 ^[[32m*^[[0m abi_x86_64.amd64: running multilib-minimal_abi_src_configure
meson setup --libdir lib64 --localstatedir /var/lib --prefix /usr --sysconfdir /etc --wrap-mode nodownload --build.pkg-config-path /usr/share/pkgconfig --pkg-config-path /usr/share/pkgconfig --native-file /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini -Db_pch=false -Dwerror=false -Db_lto=false --buildtype plain -Ddefault_library=shared -Dbin_programs=true -Dbin_contrib=true -Dbin_tests=false -Dzlib=enabled -Dlzma=enabled -Dlz4=disabled --native-file /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini.local /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_64.amd64
The Meson build system
Version: 1.3.1
Source dir: /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson
Build dir: /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson-abi_x86_64.amd64
Build type: native build
Program GetZstdLibraryVersion.py found: YES (/usr/bin/python3.12 /x/portage/app-arch/zstd-1.5.5-r1/work/zstd-1.5.5/build/meson/GetZstdLibraryVersion.py)
Project name: zstd
Project version: 1.5.5
C compiler for the host machine: x86_64-pc-linux-gnu-gcc (gcc 13.2.1 "x86_64-pc-linux-gnu-gcc (Gentoo 13.2.1_p20240113-r1 p12) 13.2.1 20240113")
C linker for the host machine: x86_64-pc-linux-gnu-gcc ld.bfd 2.41
C++ compiler for the host machine: x86_64-pc-linux-gnu-g++ (gcc 13.2.1 "x86_64-pc-linux-gnu-g++ (Gentoo 13.2.1_p20240113-r1 p12) 13.2.1 20240113")
C++ linker for the host machine: x86_64-pc-linux-gnu-g++ ld.bfd 2.41
Host machine cpu family: x86_64
Host machine cpu: x86_64
Library m found: YES
Run-time dependency threads found: YES
Found pkg-config: YES (/usr/bin/x86_64-pc-linux-gnu-pkg-config) 2.1.0
Run-time dependency zlib found: YES 1.3.1
Run-time dependency liblzma found: YES 5.4.6
Dependency liblz4 skipped: feature lz4 disabled
Compiler for C supports arguments -Wundef: YES 
Compiler for C supports arguments -Wshadow: YES 
Compiler for C supports arguments -Wcast-align: YES 
Compiler for C supports arguments -Wcast-qual: YES 
Compiler for C supports arguments -Wstrict-prototypes: YES 
Compiler for C++ supports arguments -Wundef: YES 
Compiler for C++ supports arguments -Wshadow: YES 
Compiler for C++ supports arguments -Wcast-align: YES 
Compiler for C++ supports arguments -Wcast-qual: YES 
Message: Enable legacy support back to version 0.5
Message: Enable multi-threading support
Has header "execinfo.h" skipped: feature backtrace disabled
Build targets in project: 7

zstd 1.5.5

  User defined options
    Native files         : /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini
                           /x/portage/app-arch/zstd-1.5.5-r1/temp/meson.x86_64-pc-linux-gnu.amd64.ini.local
    build.pkg_config_path: /usr/share/pkgconfig
    buildtype            : plain
    default_library      : shared
    libdir               : lib64
    localstatedir        : /var/lib
    pkg_config_path      : /usr/share/pkgconfig
    prefix               : /usr
    sysconfdir           : /etc
    werror               : false
    wrap_mode            : nodownload
    b_lto                : false
    b_pch                : false
    bin_contrib          : true
    bin_programs         : true
    bin_tests            : false
    lz4                  : disabled
    lzma                 : enabled
    zlib                 : enabled

Found ninja-1.11.1 at /usr/bin/ninja
>>> Source configured.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass Eli Schwartz
  2024-02-20  4:42   ` Sam James
@ 2024-02-20  6:28   ` Michał Górny
  1 sibling, 0 replies; 24+ messages in thread
From: Michał Górny @ 2024-02-20  6:28 UTC (permalink / raw)
  To: gentoo-dev

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]

On Mon, 2024-02-19 at 23:26 -0500, Eli Schwartz wrote:
> The meson-python build backend -- as the name suggests -- uses meson
> under the hood. We have a meson eclass which does lots of useful things
> pertinent to meson. Make sure it gets invoked.
> 
> Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
> ---
>  eclass/distutils-r1.eclass | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
> index c0d1992ccce0..35825d4c3aa6 100644
> --- a/eclass/distutils-r1.eclass
> +++ b/eclass/distutils-r1.eclass
> @@ -197,6 +197,10 @@ _DISTUTILS_R1_ECLASS=1
>  inherit flag-o-matic
>  inherit multibuild multilib multiprocessing ninja-utils toolchain-funcs
>  
> +if [[ ${DISTUTILS_USE_PEP517} = meson-python ]]; then
> +	inherit meson
> +fi
> +
>  if [[ ! ${DISTUTILS_SINGLE_IMPL} ]]; then
>  	inherit python-r1
>  else
> @@ -1386,6 +1390,7 @@ distutils_pep517_install() {
>  			)
>  			;;
>  		meson-python)
> +			meson_src_configure "${DISTUTILS_ARGS[@]}"

NAK.  You're calling configure twice, possibly with disjoint results. 
This is bound to be a mess, as you discovered yourself.

>  			local -x NINJAOPTS=$(get_NINJAOPTS)
>  			config_settings=$(
>  				"${EPYTHON}" - "${DISTUTILS_ARGS[@]}" <<-EOF || die
> @@ -1397,7 +1402,6 @@ distutils_pep517_install() {
>  					ninjaopts = shlex.split(os.environ["NINJAOPTS"])
>  					print(json.dumps({
>  						"builddir": "${BUILD_DIR}",
> -						"setup-args": sys.argv[1:],
>  						"compile-args": ["-v"] + ninjaopts,
>  					}))
>  				EOF

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options
  2024-02-20  6:24       ` Mike Gilbert
@ 2024-02-20  6:33         ` Eli Schwartz
  2024-02-20  6:37           ` Eli Schwartz
  2024-02-20  6:40           ` Sam James
  0 siblings, 2 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:33 UTC (permalink / raw)
  To: gentoo-dev


[-- Attachment #1.1.1: Type: text/plain, Size: 721 bytes --]

On 2/20/24 1:24 AM, Mike Gilbert wrote:
> I'm afraid I get different results. Build log attached. Happy to help
> figure this out tomorrow.
> 
> To test, I applied this patch and ran this:
> 
> ABI_X86="32 x32 64" CFLAGS="-O2 -pipe -march=amdfam10 -flto=2" ebuild
> zstd-1.5.5-r1.ebuild clean configure


Yikes. I wonder if this is also a problem for ffmpeg:

multilib_src_configure() {

[...]

# LTO support, bug #566282, bug #754654, bug #772854
if [[ ${ABI} != x86 ]] && tc-is-lto; then
        # Respect -flto value, e.g -flto=thin
        local v="$(get-flag flto)"
        [[ -n ${v} ]] && myconf+=( "--enable-lto=${v}" ) || myconf+=(
"--enable-lto" )
fi
filter-lto



-- 
Eli Schwartz

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 18399 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 3/3] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly
  2024-02-20  4:26 ` [gentoo-dev] [PATCH 3/3] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly Eli Schwartz
@ 2024-02-20  6:35   ` Michał Górny
  0 siblings, 0 replies; 24+ messages in thread
From: Michał Górny @ 2024-02-20  6:35 UTC (permalink / raw)
  To: gentoo-dev

[-- Attachment #1: Type: text/plain, Size: 4628 bytes --]

On Mon, 2024-02-19 at 23:26 -0500, Eli Schwartz wrote:
> Use of python_configure_all is a bit broken, because distutils-r1 is a
> bit broken. It has the intriguing value-claim that *FLAGS shall be
> modified with local -x as part of run-phase, which means that all
> modifications are reset as soon as any given phase exits. Including
> sub-phases. If you make changes in python_configure or
> python_configure_all, as you are expected to do instead of defining
> src_configure and then running the eclass phases manually, your changes
> inherently get erased before you actually *get* to running builds (i.e.
> reading the variables).
> 
> For an example of how this works, consider dev-python/scipy. It builds
> with filter-lto, for the standard reasons. However, filter-lto gets run
> inside python_configure_all, so it gets erased and ignored.
> 
> Note: it "worked" anyways prior to reworking meson.eclass to pass
> -Db_lto based on `is-flagq flto`. This is because filter-lto removed it
> from LDFLAGS, and since distutils-r1 doesn't localize that variable, the
> changes stuck around. As a result:
> 
> - the previous fix to scipy caused scipy to be compiled with LTO, but
>   not linked with LTO
> - meson.eclass changes caused scipy to be built with -Db_lto=true, which
>   affects both compiling and linking
> 
> It's absolutely unsafe to have flag-o-matic be ignored like this, and it
> is fascinating to end up with LTO restored into compile flags while
> still being filtered from LDFLAGS... simply as a side effect of
> distutils-r1 not modifying the latter.
> 
> - this patch causes scipy to be built with -Db_lto=false
> 
> Consequences of the change make no difference to standard dev-python/
> packages, but affect packages that use both distutils-r1 and other
> packaging infrastructure:
> 
> - global variables for tools are exported. This is supposed to be a
>   safe, albeit unnecessary for better-than-setuptools build systems,
>   option.
> 
> - the "debug" option applies the DEBUG preprocessor macro to more than
>   just python code. This was already the case for python projects that
>   built non-CPython API C/C++ code and then linked them with thin python
>   wrappers, so projects with python bindings shouldn't be any different.
>   Also, it is a "debug" use flag so it's pretty silly if it only toggles
>   debug on python bindings but not the rest of the package, so just...
>   deal with it I guess?
> 
> - any project that uses cython, now ignores the Modern C Porting flag
>   "incompatible-pointer-types". This is terrible, because code which
>   should trigger that error is terrible. But it's also terrible for
>   python projects with mixed Cython and handwritten C, to squelch that
>   error just because one file happens to use Cython. Yet, we do it
>   because we don't have a way to make extremely specific files built
>   with different CFLAGS compared to the rest of the project. There's no
>   actual reason to treat handwritten C python modules different from
>   non-distutils phases.
> 
> Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
> ---
>  eclass/distutils-r1.eclass | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
> index 35825d4c3aa6..873421bbcee8 100644
> --- a/eclass/distutils-r1.eclass
> +++ b/eclass/distutils-r1.eclass
> @@ -1813,16 +1813,15 @@ distutils-r1_run_phase() {
>  	fi
>  
>  	# Set up build environment, bug #513664.
> -	local -x AR=${AR} CC=${CC} CPP=${CPP} CXX=${CXX}
>  	tc-export AR CC CPP CXX

Sigh.  While I see your point, this feels like simultaneously breaking
and half-assed change.  If we were to remove it, I'd rather move
the logic somewhere where it is actually called once.

>  
>  	if [[ ${DISTUTILS_EXT} ]]; then
>  		if [[ ${BDEPEND} == *dev-python/cython* ]] ; then
>  			# Workaround for https://github.com/cython/cython/issues/2747 (bug #918983)
> -			local -x CFLAGS="${CFLAGS} $(test-flags-CC -Wno-error=incompatible-pointer-types)"
> +			append-cflags $(test-flags-CC -Wno-error=incompatible-pointer-types)

Sounds like it will be appended multiple times.

>  		fi
>  
> -		local -x CPPFLAGS="${CPPFLAGS} $(usex debug '-UNDEBUG' '-DNDEBUG')"
> +		append-cppflags $(usex debug '-UNDEBUG' '-DNDEBUG')
>  		# always generate .c files from .pyx files to ensure we get latest
>  		# bug fixes from Cython (this works only when setup.py is using
>  		# cythonize() but it's better than nothing)

Likewise.

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options
  2024-02-20  6:33         ` Eli Schwartz
@ 2024-02-20  6:37           ` Eli Schwartz
  2024-02-20  6:40           ` Sam James
  1 sibling, 0 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:37 UTC (permalink / raw)
  To: gentoo-dev


[-- Attachment #1.1.1: Type: text/plain, Size: 688 bytes --]

On 2/20/24 1:33 AM, Eli Schwartz wrote:
> On 2/20/24 1:24 AM, Mike Gilbert wrote:
>> I'm afraid I get different results. Build log attached. Happy to help
>> figure this out tomorrow.
>>
>> To test, I applied this patch and ran this:
>>
>> ABI_X86="32 x32 64" CFLAGS="-O2 -pipe -march=amdfam10 -flto=2" ebuild
>> zstd-1.5.5-r1.ebuild clean configure
> 
> 
> Yikes. I wonder if this is also a problem for ffmpeg:


(That being said, I actually tested this with the more thorough v2 patch
that fixes numpy by not running twice.)

Putting it out there because it's more interesting to look at than v1,
not because I think it's necessarily ready. :)


-- 
Eli Schwartz

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 18399 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options
  2024-02-20  6:33         ` Eli Schwartz
  2024-02-20  6:37           ` Eli Schwartz
@ 2024-02-20  6:40           ` Sam James
  1 sibling, 0 replies; 24+ messages in thread
From: Sam James @ 2024-02-20  6:40 UTC (permalink / raw)
  To: gentoo-dev

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]


Eli Schwartz <eschwartz93@gmail.com> writes:

> [[PGP Signed Part:Undecided]]
> On 2/20/24 1:24 AM, Mike Gilbert wrote:
>> I'm afraid I get different results. Build log attached. Happy to help
>> figure this out tomorrow.
>> 
>> To test, I applied this patch and ran this:
>> 
>> ABI_X86="32 x32 64" CFLAGS="-O2 -pipe -march=amdfam10 -flto=2" ebuild
>> zstd-1.5.5-r1.ebuild clean configure
>
>
> Yikes. I wonder if this is also a problem for ffmpeg:
>
> multilib_src_configure() {
>
> [...]
>
> # LTO support, bug #566282, bug #754654, bug #772854
> if [[ ${ABI} != x86 ]] && tc-is-lto; then
>         # Respect -flto value, e.g -flto=thin
>         local v="$(get-flag flto)"
>         [[ -n ${v} ]] && myconf+=( "--enable-lto=${v}" ) || myconf+=(
> "--enable-lto" )
> fi
> filter-lto

It is indeed.. https://bugs.gentoo.org/923491.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [gentoo-dev] Re: [PATCH v2 3/5] meson.eclass: refactor src_configure into a setter function
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 3/5] meson.eclass: refactor src_configure into a setter function Eli Schwartz
@ 2024-02-20  6:42           ` Eli Schwartz
  0 siblings, 0 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20  6:42 UTC (permalink / raw)
  To: gentoo-dev


[-- Attachment #1.1.1: Type: text/plain, Size: 389 bytes --]

On 2/20/24 1:14 AM, Eli Schwartz wrote:

> +# Calculate the command line which meson should use, and other relevant
> +# variables. Invoke via "${mesonargs[@]}" in the calling environment.
> +# This function is called from meson_src_configure.


I'm sure someone probably has a better name than "${mesonargs[@]}". If
you do I will be eternally grateful. :)


-- 
Eli Schwartz

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 18399 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH v2 4/5] distutils-r1.eclass: wire up meson-python to meson.eclass
  2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 4/5] distutils-r1.eclass: wire up meson-python to meson.eclass Eli Schwartz
@ 2024-02-20  6:55           ` Michał Górny
  2024-02-20 20:45             ` Eli Schwartz
  0 siblings, 1 reply; 24+ messages in thread
From: Michał Górny @ 2024-02-20  6:55 UTC (permalink / raw)
  To: gentoo-dev

[-- Attachment #1: Type: text/plain, Size: 1824 bytes --]

On Tue, 2024-02-20 at 01:14 -0500, Eli Schwartz wrote:
> The meson-python build backend -- as the name suggests -- uses meson
> under the hood. We have a meson eclass which does lots of useful things
> pertinent to meson. Make sure it gets invoked, by prying out the options
> that meson_src_configure would use and setting passing them as our seed
> values for gpep517.
> 
> Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
> ---
> 
> v2: call setup_meson_src_configure instead of meson_src_configure. This
> avoids running `meson setup` twice, and guarantees we use whatever
> settings the PEP517 backend requires. In particular, it respects numpy's
> vendored meson fork with experimental new features.
> 
>  eclass/distutils-r1.eclass | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
> index c0d1992ccce0..a42adc182ed9 100644
> --- a/eclass/distutils-r1.eclass
> +++ b/eclass/distutils-r1.eclass
> @@ -197,6 +197,10 @@ _DISTUTILS_R1_ECLASS=1
>  inherit flag-o-matic
>  inherit multibuild multilib multiprocessing ninja-utils toolchain-funcs
>  
> +if [[ ${DISTUTILS_USE_PEP517} = meson-python ]]; then

We use '==' throughout.

> +	inherit meson
> +fi
> +
>  if [[ ! ${DISTUTILS_SINGLE_IMPL} ]]; then
>  	inherit python-r1
>  else
> @@ -1386,9 +1390,11 @@ distutils_pep517_install() {
>  			)
>  			;;
>  		meson-python)
> +			local mesonargs=()
> +			setup_meson_src_configure "${DISTUTILS_ARGS[@]}"
>  			local -x NINJAOPTS=$(get_NINJAOPTS)
>  			config_settings=$(
> -				"${EPYTHON}" - "${DISTUTILS_ARGS[@]}" <<-EOF || die
> +				"${EPYTHON}" - "${mesonargs[@]}" <<-EOF || die
>  					import json
>  					import os
>  					import shlex

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [gentoo-dev] [PATCH v2 4/5] distutils-r1.eclass: wire up meson-python to meson.eclass
  2024-02-20  6:55           ` Michał Górny
@ 2024-02-20 20:45             ` Eli Schwartz
  0 siblings, 0 replies; 24+ messages in thread
From: Eli Schwartz @ 2024-02-20 20:45 UTC (permalink / raw)
  To: gentoo-dev


[-- Attachment #1.1.1: Type: text/plain, Size: 862 bytes --]

On 2/20/24 1:55 AM, Michał Górny wrote:
>> +if [[ ${DISTUTILS_USE_PEP517} = meson-python ]]; then
> 
> We use '==' throughout.


I'm sorry to hear that. Would you like a patch to remove it everywhere
else? :)

Shell is a complex language to get consistently right already. The
double equals is almost-offensively redundant and pointless -- its
implementation is to be an exact alias for a single equals.

It causes muscle memory to be more likely to accidentally use this
bashism in #!/bin/sh scripts, it provides zero benefit, and the cherry
on top is that it takes up possibly-valuable real estate from your
screen width and causes (uncompressed) scripts to be (trivially) larger.

I wish Chet would repent of having added it and make bash issue a
bash-level warning on stderr if it encounters one in your scripts.


-- 
Eli Schwartz

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 18399 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2024-02-20 20:45 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-20  4:26 [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO Eli Schwartz
2024-02-20  4:26 ` [gentoo-dev] [PATCH 1/3] meson.eclass: wire up LTO support directly into the meson options Eli Schwartz
2024-02-20  5:58   ` Mike Gilbert
2024-02-20  6:09     ` Eli Schwartz
2024-02-20  6:24       ` Mike Gilbert
2024-02-20  6:33         ` Eli Schwartz
2024-02-20  6:37           ` Eli Schwartz
2024-02-20  6:40           ` Sam James
2024-02-20  4:26 ` [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass Eli Schwartz
2024-02-20  4:42   ` Sam James
2024-02-20  5:12     ` Eli Schwartz
2024-02-20  6:14       ` [gentoo-dev] [PATCH v2 0/5] eclass updates for meson <> distutils <> LTO Eli Schwartz
2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 1/5] meson.eclass: wire up LTO support directly into the meson options Eli Schwartz
2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 2/5] meson.eclass: prefer -D buildtype instead of --buildtype Eli Schwartz
2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 3/5] meson.eclass: refactor src_configure into a setter function Eli Schwartz
2024-02-20  6:42           ` [gentoo-dev] " Eli Schwartz
2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 4/5] distutils-r1.eclass: wire up meson-python to meson.eclass Eli Schwartz
2024-02-20  6:55           ` Michał Górny
2024-02-20 20:45             ` Eli Schwartz
2024-02-20  6:14         ` [gentoo-dev] [PATCH v2 5/5] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly Eli Schwartz
2024-02-20  6:28   ` [gentoo-dev] [PATCH 2/3] distutils-r1.eclass: wire up meson-python to meson.eclass Michał Górny
2024-02-20  4:26 ` [gentoo-dev] [PATCH 3/3] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly Eli Schwartz
2024-02-20  6:35   ` Michał Górny
2024-02-20  4:41 ` [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO Sam James

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox