Kozea/tinycss2

Source tarball does not contain setup.py

jameshilliard opened this issue · 20 comments

It looks like --setup-py needs to be passed to flit for this to get generated properly, a lot of tooling still don't work correctly with PEP-517 so this helps a lot with distro packaging.

liZe commented

Hello!

a lot of tooling still don't work correctly with PEP-517 so this helps a lot with distro packaging.

We’ll be happy to help these tools to follow PEP517, as we already did for other distributions. What is your distribution?

What is your distribution?

Buildroot, the python packaging tooling is mostly here.

Oh, and here's a working tinycss build for version 1.1.0(right before setup.py was removed):
https://patchwork.ozlabs.org/project/buildroot/patch/20211127113735.2178776-1-james.hilliard1@gmail.com/

################################################################################
#
# python-tinycss2
#
################################################################################

PYTHON_TINYCSS2_VERSION = 1.1.0
PYTHON_TINYCSS2_SOURCE = tinycss2-$(PYTHON_TINYCSS2_VERSION).tar.gz
PYTHON_TINYCSS2_SITE = https://files.pythonhosted.org/packages/ce/d3/ece7a98d5826bd134e269a3a3030153d30482194fca71d95a3041812aab8
PYTHON_TINYCSS2_SETUP_TYPE = distutils
PYTHON_TINYCSS2_LICENSE = BSD-3-Clause
PYTHON_TINYCSS2_LICENSE_FILES = LICENSE

$(eval $(python-package))

I was researching how to actually port/handle PEP517 based builds but best I can tell it's still somewhat feature incomplete, either that or I'm just not seeing how to make it work here.

liZe commented

I’ve read your discussion on pypa/flit#462, and I agree with @takluyver.

In my opinion, using wheels as the boundary between the build and install steps is the best solution, as wheels are described in a "specification" and don’t depend on specific tools for the install step. That’s what pip does by default now, so you can be pretty sure that it will work for any package on PyPI in the future.

The build step may look strange for you, because you’re building a wheel. Moreover, this build has often already been built and put on PyPI. For Python-only packages, this step is actually useless (in my opinion), and distributions could just reuse the wheel distributed on PyPI. But distributions often like to launch tests, and tests are often only packaged in source packages, and not in wheels (because it’s often useless to install tests).

So… The steps could be:

  • download the source package from PyPI, (already done in buildroot)
  • unzip the source, (already done in buildroot)
  • launch tests (before or after installing it, or don’t launch tests at all, it depends on what distributions like to do),
  • build the wheel,
  • install the wheel.

Some packagers are disturbed by the "install the wheel" part, but as it’s been discussed in pypa/flit#462, I think that it’s actually the "easy" part. The wheels are all the same, and a naive way to install them is "just" to put what’s in them on the filesystem. Of course, distributions will have different habits depending on their specific filesystem tweaks, and it may require some workarounds for some packages. But it’s a much better solution, in my opinion, to adapt the structure of a wheel than to adapt an obscure setup.py (or, even more frightening, to rely on it!). The wheel contains all the files you want to install, the job of the distribution is "just" to copy them into the right folders.

At least, for Python-only packages, it should work easily when you know where to put the python files. For many other packages, it will also work exactly the same way. For some packages, it may require some adjustments, for sure.

The "hard" part is the build step. Using the build package can automatically do that, probably without isolation for you, so that you will use the packages already installed on the system. As you said, it would require bootstrapping for packaging tools, but… you could also directly wheels for these (often Python-only) packages, skip the build step and fully avoid this problem.

In a nutshell, here’s what I would propose:

  • for most packages, use the build (build-based) + install (installer-based?) steps,
  • for packages used for packaging (and actually more, if you wand), only use the wheel and the install step.

The current distutils/setuptools commands used in the current buildroot’s packaging system can still be used for packages where wheels won’t work, but with time I suppose that all the packages will be able to use the "standard" way.

Please let me know if I can write some code to help. I don’t know Makefiles very well and I won’t easily be able to test, but I suppose that I can at least help with the overall organization and the build or installer commands.

The build step may look strange for you, because you’re building a wheel. Moreover, this build has often already been built and put on PyPI. For Python-only packages, this step is actually useless (in my opinion), and distributions could just reuse the wheel distributed on PyPI. But distributions often like to launch tests, and tests are often only packaged in source packages, and not in wheels (because it’s often useless to install tests).

Yeah we probably don't want to be using wheels from pypi, we do run some tests although they are not always the ones in the sdists, using wheels doesn't really help us much at all since we have to support c extensions and such anyways.

The "hard" part is the build step. Using the build package can automatically do that, probably without isolation for you, so that you will use the packages already installed on the system. As you said, it would require bootstrapping for packaging tools, but… you could also directly wheels for these (often Python-only) packages, skip the build step and fully avoid this problem.

IMO it's pretty much pointless to support wheel installs from pypi at all since we know for sure that those will never be usable for packages with c extensions. It would just add complexity to our infrastructure to support both sdist and wheel methods. We also have infrastructure that checks package license files which are usually only in sdists last I checked.

The current distutils/setuptools commands used in the current buildroot’s packaging system can still be used for packages where wheels won’t work, but with time I suppose that all the packages will be able to use the "standard" way.

Well that's not really happened before with python packaging before, heh. The problem is usually that the "standard" doesn't get fully adopted.

Please let me know if I can write some code to help. I don’t know Makefiles very well and I won’t easily be able to test, but I suppose that I can at least help with the overall organization and the build or installer commands.

Well since installer is missing cli support still I didn't bother trying to do that method yet but I did come up with a workaround that seems to get flit packages building, although it's a bit hacky to say the least.

The dev env should be pretty easy to set up, you can just clone my branch and try building with make python-tinycss2.

liZe commented

Well that's not really happened before with python packaging before, heh. The problem is usually that the "standard" doesn't get fully adopted.

The build package should be able to build wheels, even for packages that don’t want to follow the standard.

The idea I propose is, instead of directly using distutils/setuptools, to:

  • build a wheel during the build step,
  • to install the wheel during the install step.

I can try to write a pkg-python.mk file with these steps to show exactly what I have in mind. It will work with tinycss2, and hopefully with a large part of the Python packages. You can then adapt this code and include it into the current pkg-python.mk file if you want.

The dev env should be pretty easy to set up, you can just clone my branch and try building with make python-tinycss2.

I’ve launched make menuconfig to generate a config file (I didn’t change anything), and then launched make python-tinycss2. Unfortunately, it fails while compiling Python 2:

In file included from Include/Python.h:88,
                 from Include/pgenheaders.h:10,
                 from Parser/parsetok.c:4:
Include/unicodeobject.h:575:20: error: unknown type name ‘wchar_t’
  575 |     register const wchar_t *w,  /* wchar_t buffer */
      |                    ^~~~~~~

Maybe there’s an easy fix for that, otherwise I’ll try to find another solution.

The build package should be able to build wheels, even for packages that don’t want to follow the standard.
The idea I propose is, instead of directly using distutils/setuptools, to:

  • build a wheel during the build step,
  • to install the wheel during the install step.

I would maybe start out first with trying to handle non-distutils/non-setuptools pep-517 packages correctly, in my experimental branch the build type selection is controlled via:

PYTHON_TINYCSS2_SETUP_TYPE = flit

Which is currently enabling the setup.py generation but is otherwise using a standard distutils build process.

To make it not use distutils code paths you would want to revert this:

# Distutils/flit
ifneq ($$(filter distutils flit,$$($(2)_SETUP_TYPE)),)

to this:

# Distutils
ifeq ($$($(2)_SETUP_TYPE),distutils)

and then create a flit specific handler like this instead:

# Flit
ifeq ($$($(2)_SETUP_TYPE),flit)

for handling flit+pep517 builds.

I can try to write a pkg-python.mk file with these steps to show exactly what I have in mind. It will work with tinycss2, and hopefully with a large part of the Python packages. You can then adapt this code and include it into the current pkg-python.mk file if you want.

Yeah, that would be helpful, I would try to just modify the existing pkg-python.mk and add flit/pep517 code-paths rather than rewrite it entirely as we will likely need to support multiple build types for a while still, the tricky part I think is ensuring everything gets passed the right flags so that things end up in the right place and get built against the right libraries.

Making sure it works right for packages with c extensions is probably the most difficult part as those need to be built against the target python libraries and not the host python libraries.

Buildroot separates out host and target builds for pretty much all packages but it has to use the host python interpreter to actually build target python extensions while linking against the target python libraries, it's a bit non-trivial to do and at least for distutils/setuptools requires specific flags to be passed.

Maybe there’s an easy fix for that, otherwise I’ll try to find another solution.

Oh, yeah, looks like this doesn't work with default configs due to the toolchain config needing some tweaks, this procedure should work from a fresh clone:

make raspberrypi3_qt5we_defconfig

make menuconfig
Build options -> Advanced -> Use per-package directories (experimental) -> Enable
Target packages -> Interpreter languages and scripting -> python3 -> Enable
Target packages -> Interpreter languages and scripting -> python3 -> External python modules -> python-tinycss2 -> Enable

make python-tinycss2 -j$(nproc)
liZe commented

I have tried to build tinycss2. It didn’t work (mostly because I don’t understand how the host / target work), but I wrote some code that explains what I had in mind: https://github.com/CourtBouillon/buildroot/commit/8beef6927f6659749fda23a0f67b06e4ed1261d2

I’ve added a pep517 setup type. It uses build to build the wheel, and installer to install the wheel. A wheel setup type has been added too (that’s the same as pep517 without the build step).

pep517 can replace both distutils and setuptools, as build is able to handle these tools. wheel can be removed if it’s not required, but it could be useful to install build and the packaging tools (setuptools, flit, others like poetry in the future).

You’ll probably be more able than me to fix this code and use the right destinations and environment variables depending on the different cases. If you’re interested in including some of this code, don’t hesitate to ping me!

I have tried to build tinycss2. It didn’t work (mostly because I don’t understand how the host / target work), but I wrote some code that explains what I had in mind: CourtBouillon/buildroot@8beef69

Ok, I'm seeing if I can clean it up and get it building, and yeah the host/target split is especially complex since we are using a host interpreter to build the package but it needs to have the right env passed so that it actually builds the package for the target.

I’ve added a pep517 setup type. It uses build to build the wheel, and installer to install the wheel. A wheel setup type has been added too (that’s the same as pep517 without the build step).

Yeah, I'm pretty sure upstream doesn't want to ever install from downloaded wheels in general(since they will be missing stuff like license files), but the wheel intermediary build stage may be workable.

pep517 can replace both distutils and setuptools, as build is able to handle these tools. wheel can be removed if it’s not required, but it could be useful to install build and the packaging tools (setuptools, flit, others like poetry in the future).

Hmm, I thought build is just a build frontend while distutils and setuptools are the actual build backends it invokes in the same way flit_core is build backend invoked by build.

You’ll probably be more able than me to fix this code and use the right destinations and environment variables depending on the different cases. If you’re interested in including some of this code, don’t hesitate to ping me!

Yeah, I'll play around with it and see if I can get something that looks clean enough to try and upstream.

liZe commented

Hmm, I thought build is just a build frontend while distutils and setuptools are the actual build backends it invokes in the same way flit_core is build backend invoked by build.

It is. What I mean is that the distutils and setuptools can eventually be removed from the list of the SETUP_TYPE possible values, but the distutils and setuptools modules will of course be needed by build.

Yeah, I'll play around with it and see if I can get something that looks clean enough to try and upstream.

Thanks a lot.

What I mean is that the distutils and setuptools can eventually be removed from the list of the SETUP_TYPE possible values

This doesn't really sounds right to me, since SETUP_TYPE indicates the build backend effectively(it is used both for build system dependency resolution and build invocation control), even if all backends are using a pep517 based frontend we still need to specify SETUP_TYPE otherwise the build system dependency resolution won't work.

but the distutils and setuptools modules will of course be needed by build.

Yeah, so SETUP_TYPE is what adds those dependencies, so we'll need one for flit as well looks like which adds the flit_core backend and appropriate front-ends/installers I think.

It didn’t work

FYI there was a namespace clash with the host-python-build make target for host-python and the package python-build that was causing all sorts of weirdness(it was basically attempting to build the legacy python2.7 interpreter for the host instead of(and/or maybe in addition to) the host python-build package target), I had to rename the package build to python-pypa-build to workaround the namespace clash.

I sent off a preliminary series with the pep-517 dependencies(well the parts that don't have strange bootstrapping requirements like flit at least) for now.

liZe commented

This doesn't really sounds right to me, since SETUP_TYPE indicates the build backend effectively(it is used both for build system dependency resolution and build invocation control), even if all backends are using a pep517 based frontend we still need to specify SETUP_TYPE otherwise the build system dependency resolution won't work.

Yeah, so SETUP_TYPE is what adds those dependencies, so we'll need one for flit as well looks like which adds the flit_core backend and appropriate front-ends/installers I think.

I added BUILD_TYPE for that, but that was probably awkward.

FYI there was a namespace clash with the host-python-build make target for host-python and the package python-build that was causing all sorts of weirdness(it was basically attempting to build the legacy python2.7 interpreter for the host instead of(and/or maybe in addition to) the host python-build package target), I had to rename the package build to python-pypa-build to workaround the namespace clash.

It explains why I had to build Python 2, even if I desperately tried hard to avoid this 😁.

I sent off a preliminary series with the pep-517 dependencies(well the parts that don't have strange bootstrapping requirements like flit at least) for now.

👍

I added BUILD_TYPE for that, but that was probably awkward.

Yeah, I'm assuming if the build frontend can be changed over for distutils and setuptools then we will just use SETUP_TYPE for setting the backend.

It explains why I had to build Python 2, even if I desperately tried hard to avoid this 😁.

Yeah, I caught it since I was splitting things up into a series(while incrementally testing each patch) and noticed it went haywire when I added and tested python-build.

So right now I have it so that the pep517 SETUP_TYPE is used for in-tree pep517 backends like flit-core while flit SETUP_TYPE is used for flit backend packages like tinycss2, this seems to work reasonably well, will probably end up getting cleaned up/refactored further once it goes through code review.

So I refactored and cleaned things up a bit more and now have things so that it's now able to build and install tinycss2 with make python-tinycss2, only problem is I don't know how to get installer to actually install wheels into the target site-packages instead of the host site-packages.

For setuptools the install path override seems to be done likes this I guess:

# Target setuptools-based packages
PKG_PYTHON_SETUPTOOLS_ENV = \
	_PYTHON_SYSCONFIGDATA_NAME="$(PKG_PYTHON_SYSCONFIGDATA_NAME)" \
	PATH=$(BR_PATH) \
	$(TARGET_CONFIGURE_OPTS) \
	PYTHONPATH="$(if $(BR2_PACKAGE_PYTHON3),$(PYTHON3_PATH),$(PYTHON_PATH))" \
	PYTHONNOUSERSITE=1 \
	_python_sysroot=$(STAGING_DIR) \
	_python_prefix=/usr \
	_python_exec_prefix=/usr

PKG_PYTHON_SETUPTOOLS_INSTALL_TARGET_OPTS = \
	--prefix=/usr \
	--executable=/usr/bin/python \
	--single-version-externally-managed \
	--root=$(TARGET_DIR)

The define for the target installation stage that is broken should be define $(2)_INSTALL_TARGET_CMDS

Current testing branch is here.

Any idea how I'm supposed to actually override that?

liZe commented

Any idea how I'm supposed to actually override that?

You can explicitly define where packages are installed by defining the scheme variable. For a simple installation, the folders are the ones returned by sysconfig.get_paths(), but you can override them. You can either define the mapping manually:

{
  'stdlib': '$(TARGET_DIR)/xxx',
  'platstdlib': '$(TARGET_DIR)/yyy',
  …
}

or use a dict-comprehension (less code but more complicated):

{key: os.path.join('$(TARGET_DIR)', value[len('$(HOST_DIR)'):]) for key, value in sysconfig.get_paths().items()}

Hmm, I'm wondering if that's actually the best approach, mostly since I don't see the setuptools/distutils infrastructure actually setting those params directly.

liZe commented

Hmm, I'm wondering if that's actually the best approach, mostly since I don't see the setuptools/distutils infrastructure actually setting those params directly.

One of the goals of PEP 517 is actually to be sure that generated wheels follow the standard, and that they can be installed independently from the build system. So, as long as the library you’re packaging is able to generate a wheel (and it should, as it’s the default way packages are installed using pip), you can safely set the scheme and let installer take care of everything for you.

Before that, if you wanted to use non-standard prefixes to build and to install your packages, you had to change some environment variables, and cross the fingers hoping that setup.py was following these variables instead of using static absolute paths. That’s probably why you had to change the _python_* environment variables. Some of these environment variables (those needed to change the installation folders) are thus probably useless with PEP 517.

Before that, if you wanted to use non-standard prefixes to build and to install your packages, you had to change some environment variables, and cross the fingers hoping that setup.py was following these variables instead of using static absolute paths.

It looks like the specific feature being used to redirect the install is this by way of the --root=$(TARGET_DIR) flag. Does installer have an equivalent of that which doesn't require manually setting the sysconfig variables?

Some of these environment variables (those needed to change the installation folders) are thus probably useless with PEP 517.

I think those env variables are not changing the installation folder but rather redirecting the sysconfig to the target sysconfig so that target c libraries are built against the target interpreter.

liZe commented

It looks like the specific feature being used to redirect the install is this by way of the --root=$(TARGET_DIR) flag. Does installer have an equivalent of that which doesn't require manually setting the sysconfig variables?

No. There’s an issue open about that, but it doesn’t look like it will be fixed soon.

I think those env variables are not changing the installation folder but rather redirecting the sysconfig to the target sysconfig so that target c libraries are built against the target interpreter.

By changing the sysconfig, I assume that it also changes the default installation folders, but I may be wrong.