public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
From: Eli Schwartz <eschwartz93@gmail.com>
To: gentoo-dev@lists.gentoo.org
Subject: [gentoo-dev] [PATCH 3/3] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly
Date: Mon, 19 Feb 2024 23:26:49 -0500	[thread overview]
Message-ID: <20240220043235.3889132-4-eschwartz93@gmail.com> (raw)
In-Reply-To: <20240220043235.3889132-1-eschwartz93@gmail.com>

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



  parent reply	other threads:[~2024-02-20  4:33 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Eli Schwartz [this message]
2024-02-20  6:35   ` [gentoo-dev] [PATCH 3/3] distutils-r1.eclass: fix src_configure to handle flag-o-matic correctly Michał Górny
2024-02-20  4:41 ` [gentoo-dev] [PATCH 0/3] eclass updates for meson <> distutils <> LTO Sam James

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=20240220043235.3889132-4-eschwartz93@gmail.com \
    --to=eschwartz93@gmail.com \
    --cc=gentoo-dev@lists.gentoo.org \
    /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