Closed kloczek closed 2 months ago
Here is pytest output:
Can you provide more details on your environment? Did you edit the rpm spec file? Are these errors reproducible in copr/mock? I think the issue is with calling pytests marked as isolated
Can you provide more details on your environment? Did you edit the rpm spec file?
Of course. Usually when I'm building package I'm usually editing spec file.
Are these errors reproducible in copr/mock?
I have no idea. Im not using copr/mock and I'm not using Fedora either.
I think the issue is with calling pytests marked as isolated
I'm not passing such partam to pytest.
Reported issue has nothing to do with rpm.
I have no idea. Im not using copr/mock and I'm not using Fedora either.
Oh what OS are you working on? mock
is basically the equivalent of a container, and it is usually used on the backend for build tools like copr
and packit
. copr
is CI/CD like Github Action but specifically for rpms. It supports all distros that use rpms including open suse, centos, redhat.
In this project we are using packit
to automate the CI/CD using copr and downstream. I think it supports centos as well, and they are happy if there is more people who would want to use it for other downstreams. If you can replicate the issue with packit build in-mock -r /etc/mock/centos-stream-9-x86_64.cfg
or equivalent, that would help a bunch to replicate the issue.
I'm not passing such partam to pytest.
We are passing those arguments though, check: https://github.com/scikit-build/scikit-build-core/blob/cf8b29449f1ff44e453a98c0ced3018811483b7f/.distro/python-scikit-build-core.spec#L51-L53
We don't have a "network" marker in scikit-build-core, but we could (should?) add one. It's currently pretty much synonymous with "isolated" at the moment. Though it does worry me - some of the errors you are seeing are before the network is accessed, during the setup phase. I think I saw the same thing with scikit-build in some cases, and ended up using virtualenv instead of venv there; maybe I should do the same thing here. (I've seen venv broken by default with pypa/build, too).
Aren't they all failing at basically calling pip upgrade
like in:
E subprocess.CalledProcessError: Command '['/tmp/pytest-of-tkloczko/pytest-10/test_abi3_wheel0/venv/bin/python3', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1.
Or am I reading the log wrong?
We don't have a "network" marker in scikit-build-core, but we could (should?) add one. It's currently pretty much synonymous with "isolated" at the moment.
What would be logical difference between isolated
and network
? Presumably not network
is a subset of not isolated
, but what else would the latter contain?
Ahh, yes, I might have missed the --upgrade
, yes, that would access the network. @kloczek, could you try again with -m "not isolated"
? I can add a network
marker, but to start it will be identical to isolated
.
network
mark is more widely used and as looks as those units are accessing to to the public network it make s more sense to use network
than another one.
Here is pyest output with -m "not isolated"
and with -m "isolated"
This is odd, I double-checked and indeed on copr
the internet connection is disabled, but we don't see the same issues.
Let's navigate test_dynamic_metadata.test_pep517_wheel
https://github.com/scikit-build/scikit-build-core/blob/cf8b29449f1ff44e453a98c0ced3018811483b7f/tests/test_dynamic_metadata.py#L226-L229
This one is run on Fedora (reference) and passes, so it should also create the test_fixture virtualenv
which seems to be the problematic part here.
In the VEnv
constructor
https://github.com/scikit-build/scikit-build-core/blob/cf8b29449f1ff44e453a98c0ced3018811483b7f/tests/conftest.py#L83-L84
it calls the EnvBuilder
constructor, which in your Python 3.8 distro seems to be calling self._setup_pip(context)
which in turn calls the problematic pip upgrade
.
Other than that are are other concerning test failures like tests/test_pyproject_pep517.py::test_pep517_sdist_hash
, which I thought we have solved here. test_pyproject_pep660.py::test_pep660_wheel
is another puzzling one.
So can you also output the virtualenv
version that was used there? Probably this is solved in a specific version of virtualenv
that needs to be requested. And we need more information on the OS you are developing to see what is different from the Fedora packaging and figure out if the fix should be on the spec file side or built-in.
We are using venv for testing in scikit-build-core, while I used virtualenv in scikit-build. I might need to move scikit-build-core to virtualenv, too.
I'd expect any use of virtualenv
(the fixture, not the package) to prefer the network due to the upgrade, though it could be that "--upgrade" is a no-op if there's no network, but not on older versions of Python? What version of Python are you (@LecrisUT) testing on?
I think virtualenv doesn't auto-upgrade and instead bundles what it needs.
What version of Python are you (@LecrisUT) testing on?
Python 3.11
. Should be the same on supported fedora coprs. (Surprised there are no other test failures that made us only build for fedora 38 and higher).
We are using venv for testing in scikit-build-core, while I used virtualenv in scikit-build.
Are these different packages (pyenv
vs virtualenv
?). Isn't one of them deprecated in favor of the other? I think the current documentation is to use python3 -m venv
(which is from pyenv
?).
Are these different packages
venv
is part of the Python standard library (sometimes broken out by distro packaging, but it's part of the stdlib), while virtualenv
is a package on PyPI. virtualenv
is faster and up-to-date, since it's not tied to Python's release cycle. It also fixes issues while venv requires a Python update to fix anything.
FYI: I'm working on Linux/Solaris distribution which provides set of packaged python modules. I'm not interested to test anything against downloaded from pypi .whl archives but to test whatever is packaged against exactly that set of modules. I saw many times that something tested using evnv/tox against pypi where working and have been failing on use. So far have packaged +1150 python modules and with few minor exceptions everything is consistent. This is why exactly I'm using ONLY pytest to test anything. In such case using pip to update modules wold cause more harm than positive results.
I've reported fails and set of modules installed in build env during package your module. In such env are installed only components which are NEEDED by build/install/test procedures and nothing more. Using in such condition venv/tox/etc would be waste of time/resources.
Did you try to have a look on list of installed in build env modules and/or try to reproduce that issue? 🤔
In such case using pip to update modules wold cause more harm than positive results.
We do not explicitly call, NOR do we want to call pip upgrade
(see my traceback breakdown). This is a wrong implementation of venv
, somewhere between 3.8 (or earlier) and 3.11, where the constructor itself automatically calls pip upgrade
. Presumably this commit allows us to ignore the actual pip upgrade
, but it looks like it is only introduced in 3.11
(or maybe 3.4?). Maybe this was fixed before that, but it is hard to tell. Again, we are not relying on pip upgrade
for the pytests to succeed, and we are trying to disable downloading/updateing packages see:
(Shouldn't this be moved to the constructor @henryiii? But if virtualenv
can do a better job :thinking:)
Using in such condition venv/tox/etc would be waste of time/resources.
No, because people will want to do pytest
in their local environment outside a CI environment. If a venv
is not created, we would be installing random test packages wherever they are running pytest
, which is absolutely a bad approach. This is a build system, so naturally we are touching the installation process in one test or another.
In such env are installed only components which are NEEDED by build/install/test procedures and nothing more.
Yes, that is normally handled by the pytest
environment, which is evident in that Fedora does not run pip upgrade
(the copr environment does not have internet access and the tests run without an issue for Fedora).
Did you try to have a look on list of installed in build env modules and/or try to reproduce that issue? :thinking:
That is why I was asking for the .spec
file and OS environment so I could reproduce and investigate what differences there are. Solaris is not available in mock
, but probably it uses oracle-linux
as its base?
Again this issue has nothing to with rpm. Here is my spec
# BUG: sphinx warnings https://github.com/scikit-build/scikit-build-core/issues/369
%bcond_with failing_tests # By default skip some failing test units
Summary: Build backend for CMake based projects
Name: python-scikit-build-core
Version: 0.4.5
Release: 2%{?dist}
License: Apache-2.0 (https://spdx.org/licenses/Apache-2.0.html)
URL: https://pypi.org/project/scikit-build-core/
VCS: https://github.com/scikit-build/scikit-build-core/
Source: %{VCS}/archive/v%{version}/%{name}-%{version}.tar.gz
Patch: %{name}-add_module_path_in_conf.py.patch
Patch: %{name}-man3_level.patch
BuildArch: noarch
BuildRequires: cmake
BuildRequires: ninja-build
BuildRequires: python3dist(build)
BuildRequires: python3dist(hatch-vcs)
BuildRequires: python3dist(hatchling)
BuildRequires: python3dist(installer)
BuildRequires: python3dist(myst-parser)
BuildRequires: python3dist(sphinx)
BuildRequires: python3dist(sphinx-copybutton)
BuildRequires: python3dist(sphinx-inline-tabs)
BuildRequires: python3dist(wheel)
# ChcekRequires:
BuildRequires: gcc
BuildRequires: gcc-c++
#BuildRequires: python3dist(cmake)
#BuildRequires: python3dist(ninja)
BuildRequires: python3dist(cattrs)
BuildRequires: python3dist(importlib-resources)
BuildRequires: python3dist(numpy)
BuildRequires: python3dist(pip)
BuildRequires: python3dist(pyproject-metadata)
BuildRequires: python3dist(pytest)
BuildRequires: python3dist(pytest-subprocess)
Obsoletes: python3-scikit-build-core
%description
A next generation Python CMake adaptor and Python API for plugins.
%prep
%autosetup -p1 -n scikit-build-core-%{version}
%build
%pyproject_wheel
%sphinx_build_man
%install
%pyproject_install
%__install -Dm644 build/sphinx/man/*.3 -t %{buildroot}%{_mandir}/man3
%check
%pytest -m "isolated" %{!?with_failing_tests: \
--deselect tests/test_custom_modules.py::test_ep \
--deselect tests/test_dynamic_metadata.py::test_pep517_wheel \
--deselect tests/test_pyproject_abi3.py::test_abi3_wheel \
--deselect tests/test_pyproject_pep517.py::test_pep517_sdist_hash \
--deselect tests/test_pyproject_pep517.py::test_pep517_sdist_time_hash_set_epoch[False] \
--deselect tests/test_pyproject_pep517.py::test_pep517_sdist_time_hash_set_epoch[True] \
--deselect tests/test_pyproject_pep517.py::test_pep517_wheel \
--deselect tests/test_pyproject_pep517.py::test_pep517_wheel_source_dir \
--deselect tests/test_pyproject_pep518.py::test_pep518_pip \
--deselect tests/test_pyproject_pep518.py::test_pep518_rebuild_build_dir[sdist_to_wheel] \
--deselect tests/test_pyproject_pep518.py::test_pep518_rebuild_build_dir[wheel_directly] \
--deselect tests/test_pyproject_pep518.py::test_pep518_sdist \
--deselect tests/test_pyproject_pep518.py::test_pep518_wheel[sdist_to_wheel] \
--deselect tests/test_pyproject_pep518.py::test_pep518_wheel[wheel_directly] \
--deselect tests/test_pyproject_pep660.py::test_pep660_pip_isolated[False] \
--deselect tests/test_pyproject_pep660.py::test_pep660_pip_isolated[True] \
--deselect tests/test_pyproject_pep660.py::test_pep660_wheel \
--deselect tests/test_setuptools_abi3.py::test_abi3_wheel \
--deselect tests/test_setuptools_pep517.py::test_pep517_mixed_wheel \
--deselect tests/test_setuptools_pep517.py::test_pep517_wheel \
--deselect tests/test_setuptools_pep518.py::test_pep518_pip \
--deselect tests/test_setuptools_pep518.py::test_pep518_wheel \
--deselect tests/test_simplest_c.py::test_pep517_wheel \
--deselect tests/test_simplest_c.py::test_pep517_wheel_incexl \
}
%files
%{_mandir}/man3/*
%{python3_sitelib}/scikit_build_core
%{python3_sitelib}/scikit_build_core-*.*-info
I have tried to investigate solaris and it just seems to be a huge rabbit-hole. I get the gist that solaris is a closed-sourced OS that doesn't use rpms. But then I don't understand the build setup you have with RPMBUILD. If you meant oracle-linux, than that one is different and that one follows RHEL distribution and I can reproduce the environment if you can share the oracle-linux version as well.
That information is important to address tests/test_pyproject_pep517.py::test_pep517_sdist_hash
, which I am 90% sure it is fully RPM related. (Check the previous comments about that test for more context)
If you can't share the OS environment, at least share a link to the full build.log
. My suspicion is that your OS is using a different non-standard environment variable to set the build date.
Oracle Solaris is not only Solaris version. Please try to have look on OIpenSolaris derivates like OmniOS.
%pytest -m "isolated"
Shouldn't that be %pytest -m "not isolated"
? The isolated tests require a network to setup (they build from a collection of wheels downloaded from the network). Isolated in this case means PEP 517 build isolation.
Shouldn't that be
%pytest -m "not isolated"
?
I'm not interested to test anything against modules installed out of .whl archives downloaded from pypi.
That’s exactly what isolated does. So don’t you want not isolated
?
On my devel system I'm able to reach public network however prod build system is intentionally cut off from access to public network.
Next release will have a network
marker. But for now, “isolated” refers to setting up an isolated virtual environment and testing modules installed out of .whl
archives downloaded from pypi. So -m isolated
will only run the isolated environment tests. It was called pep518
originally, but using peps for naming is discouraged.
Perfect. Thank you 😄 👍
Just tested 0.4.6 and still I see 3 failing units
On first look it looks like those units are trying to upgrade something.
I think #371 might be needed to fix those last three tests. I just need to figure out why there's sometimes an unfinished process in that PR.
Do you want me to test this PR? 🤔
Just updated to 0.5.1 and looks like I made some progress after add few .whl files to build env and pybind11
cmake module.
However now looks like now some cmake commands are failing because they are expecting pybind11 header files in python sitelib instead in /usr/include where pybind11 installs those files..
Here is update pytest output:
Just found that I'm building pybind11
with -D USE_PYTHON_INCLUDE_DIR=OFF
so it causes that header files are installed in /usr/include however looks like installed pybind11
cmake module is not adapted for that and it expects pybind11
header files in python sitelib.
Will try to open for that issue ticket.
pybind11
on fedora is installed in /usr/include
as well, and so far that issue has not appeared there. Try to reference for probable packaging issues.
For reference latest copr build:
[ 44%] tests/test_pyproject_pep517.py ........s.
From what I can see in the pybind11
main branch, the include directory is from CMAKE_INSTALL_INCLUDEDIR
, so make sure you are installing the cmake project correctly. Make sure there are no cp
/mv
commands after the %cmake_install
. Also for reference pybind11.spec
My spec file for pybind11
uses:
%build
%cmake \
-D PYBIND11_CUDA_TESTS=OFF \
-D PYBIND11_INSTALL=ON \
-D PYBIND11_TEST=ON \
-D PYTHON_EXECUTABLE=%__python3 \
-D USE_PYTHON_INCLUDE_DIR=OFF \
%{nil}
%cmake_build
%pyproject_wheel
%install
%cmake_install
PYBIND11_USE_CMAKE=true \
%pyproject_install
%check
%ctest
Only difference which I see is use pep517 based build for python module (fedora one still uses setuptools setuup.py
).
OK I see what is going on. %files from my spec file
%files
%doc README.*
%attr(755,root,root) %{_bindir}/pybind11-config
%{_datadir}/cmake/*
%{_datadir}/pkgconfig/*.pc
%{_includedir}/pybind11
%{python3_sitelib}/pybind11
%{python3_sitelib}/pybind11-*.*-info
%exclude %{python3_sitelib}/pybind11/include
So in case of fedora it works because they've include %{python3_sitelib}/pybind11/include
symlink.
IMO cmake module should use different paths when is used -D USE_PYTHON_INCLUDE_DIR=OFF
and that symlink should ne be installed.
Interesting, indeed fedora does seem to package apparently symlinks, even though there doesn't seem to be such symlink instructions. I will investigate the files tomorrow to confirm where the files are taken from. Looking at the setup.py
I have a concern that it might overwrite the previous files https://github.com/pybind/pybind11/blob/0a756c0bb2f7e7ac6cf4e2c4de26211409fadbe4/setup.py#L125. Both the fedora spec file and pybind11 upstream could use some modernization? @henryiii are there plans on pybind11 to use scikit-build-core? Anything blocking that?
IMO issue is that looks like pybind11
cmake -D USE_PYTHON_INCLUDE_DIR={ON|OFF}
seems is not fully finished and it works only becase this symlink is added ..
Because I've decided to skip packaging that symlink I was forced me to add kind of pybind11
JFDI patch
--- a/pybind11/commands.py
+++ b/pybind11/commands.py
@@ -4,26 +4,10 @@
def get_include(user: bool = False) -> str: # noqa: ARG001
- """
- Return the path to the pybind11 include directory. The historical "user"
- argument is unused, and may be removed.
- """
- installed_path = os.path.join(DIR, "include")
- source_path = os.path.join(os.path.dirname(DIR), "include")
- return installed_path if os.path.exists(installed_path) else source_path
-
+ return "/usr/include/pybind11"
def get_cmake_dir() -> str:
- """
- Return the path to the pybind11 CMake module directory.
- """
- cmake_installed_path = os.path.join(DIR, "share", "cmake", "pybind11")
- if os.path.exists(cmake_installed_path):
- return cmake_installed_path
-
- msg = "pybind11 not installed, installation required to access the CMake files"
- raise ImportError(msg)
-
+ return "/usr/share/cmake/pybind11"
def get_pkgconfig_dir() -> str:
To return by pybind11/commands.py proper path to header files.
That symling "solves" this issue however it hides as well that on use -D USE_PYTHON_INCLUDE_DIR={ON|OFF}
notjing else is not propagated into cmake module and pybind11/commands.py.
It is only coincidence that as I found that on use -D USE_PYTHON_INCLUDE_DIR=OFF
that symlink theoretically should not be necessary so I've excluded on packaging .. and that exposed that 😋
It is yet another thing in pybind11
generated pybind11.pc file
prefix=/usr
includedir=${prefix}/include
Name: pybind11
Description: Seamless operability between C++11 and Python
Version: 2.11.1
Cflags: -I${includedir}
Here includedir looks like it should be ${prefix}/include/pybind11
.
Theoretically to obtain by cmake pybind11
could uses pkgconfi to obtain CFLAGS (and the same pybind11/commands.py) however as pybind11.pc has incorrect includedir it cannot be used (now).
Indeed you've exposed quite a flaw in their design. My suspicion is that PYBIND11_USE_CMAKE=true
clashes with %cmake_build
. If pybind11 can install only the python files and metadata there could be a workaround for this issue.
Just for curiosity can you invert the cmake and Python commands in the spec file?
Just for curiosity can you invert the cmake and Python commands in the spec file?
No problem 😋 Just send build request with modified build procedure however result may be in 2-3h because build queue is a bit clogged now (will back with result ASP).
Just tested 0.6.1 and now I see that only 3 units are failing
Can you compare the +
commands in the build.log
. Here are the Fedora ones. I think the issue is that your system uses something similar to SOURCE_DATE_EPOCH
that we need to patch out as well.
Unsure if related, but I have an Unknown CMake command "python_add_library"
error.
I noticed the same error in the logs above^1.
After comparing the CMake docs for SWIG & C, my work-around in https://github.com/AMYPAD/CuVec/pull/34 is to replace:
- python_add_library(example MODULE example.c WITH_SOABI)
+ add_library(example MODULE example.c)
+ if(WIN32)
+ set_property(TARGET example PROPERTY SUFFIX ".${Python_SOABI}.pyd")
+ else()
+ set_property(TARGET example PROPERTY SUFFIX ".${Python_SOABI}${CMAKE_SHARED_MODULE_SUFFIX}")
+ endif()
+ target_link_libraries(example PRIVATE Python::Module)
+ # remove shared lib prefix to make importable
+ set_target_properties(example PROPERTIES PREFIX "")
Does that make sense? Should I open a separate issue?
Separate issue, some more details on the OS/python environment would be needed. At first glance, if it's an issue in your CMakeLists.txt
, then it's probably that you did not run find_package(Python)
beforehand, or you had find_package(Python3)
@kloczek Are the 3 unit tests you had still failing with 0.8.0?
Closing. Just tested 0.9.6 and all looks good now
Great!
And pytest will fix the verbose xfail output next release.
I'm packaging your module as an rpm package so I'm using the typical PEP517 based build, install and test cycle used on building packages from non-root account.
python3 -sBm build -w --no-isolation
build
with--no-isolation
I'm using during all processes only locally installed modulescut off from access to the public network
(pytest is executed with-m "not network"
)Here is list of installed modules in build env