KaveIO / PhiK

Phi_K correlation analyzer library
Other
155 stars 28 forks source link

missing build deps though I have them installed when building with pyproject.toml #50

Closed hubutui closed 2 years ago

hubutui commented 2 years ago

step to build

cd PhiK-0.12.2
python -m build --wheel --no-isolation

logs:

* Getting dependencies for wheel...
running egg_info
creating phik.egg-info
writing phik.egg-info/PKG-INFO
writing dependency_links to phik.egg-info/dependency_links.txt
writing entry points to phik.egg-info/entry_points.txt
writing requirements to phik.egg-info/requires.txt
writing top-level names to phik.egg-info/top_level.txt
writing manifest file 'phik.egg-info/SOURCES.txt'
reading manifest file 'phik.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no previously-included files matching '*.py[cod]' found anywhere in distribution
warning: no previously-included files matching '__pycache__' found anywhere in distribution
warning: no previously-included files matching '*.so' found anywhere in distribution
warning: no previously-included files found matching 'docs'
warning: no previously-included files found matching 'tests'
warning: no previously-included files found matching '.readthedocs.yml'
warning: no previously-included files matching '*.py' found under directory 'tests'
adding license file 'LICENSE'
adding license file 'NOTICE'
writing manifest file 'phik.egg-info/SOURCES.txt'

ERROR Missing dependencies:
        cmake>=3.16
        ninja; platform_system!='Windows'

Also, these files are installed, comparing to version 0.12.0:

/usr/CMakeLists.txt
/usr/LICENSE
/usr/NOTICE
/usr/README.rst
/usr/pyproject.toml
/usr/setup.py

I don't this the pkg should installed these files in this place.

RUrlus commented 2 years ago

hi @hubutui, thanks for reporting.

Your pybind11 version is too low, pybind11>=2.8.1 is the minimum required version. With the correct versions I can't replicate the same issue with python -m build --wheel --no-isolation locally.

The commands above won't install anything but just create wheels, I think those files ended up in /usr/ due to some non-standard way of installing. We certainly don't do anything with setting the installation path

hubutui commented 2 years ago

@RUrlus sorry for the typo, we have pybind11 2.9.1 installed. I'm creating pkg for ArchLinux according to this guide.

# create the wheel
python -m build --wheel --no-isolation
# and install
python -m installer --destdir="$pkgdir" dist/*.whl

the error log complains cmake and ninja, not pybind11.

hubutui commented 2 years ago

It seems the whl file ships some unneeded files. I create a python 3.10 virtual env with conda, and install phik with pip install phik. I found _skbuild dir in the env dir

~/.conda/envs/ENVNAME/_skbuild
RUrlus commented 2 years ago

@hubutui did you install cmake and ninja through pip or through a system manager? I suspect it's a local issue rather than with the pyproject.toml. I can't replicate the behaviour with pip installed ninja/cmake.

I found _skbuild dir in the env dir

Thanks for letting us know, strange it gets included. Fixed by 4a3572f57896c29a459ee3e6e6e3b95d6499b7fb

hubutui commented 2 years ago

what about these files:

~/.conda/envs/ENVNAME/CMakeLists.txt
~/.conda/envs/ENVNAME/LICENSE
~/.conda/envs/ENVNAME/NOTICE
~/.conda/envs/ENVNAME/README.rst
~/.conda/envs/ENVNAME/pyproject.toml
~/.conda/envs/ENVNAME/setup.py

maybe:

~/.conda/envs/ENVNAME/share/licenses/phik/LICENSE
~/.conda/envs/ENVNAME/share/licenses/phik/NOTICE
~/.conda/envs/ENVNAME/share/doc/LICENSE

and setup.py, pyproject.toml, CMakeLists.txt should not be shipped with the pkg.

RUrlus commented 2 years ago

Unfortunately it seems we're running into an upstream issue in Scikit-build. You're right that these files should not be in the data directory.

Did you manage to solve the missing dependency issue with the pyproject.toml?

hubutui commented 2 years ago

No, but I'm ok to build it with:

python -m build --wheel --no-isolation --skip-dependency-check
hubutui commented 2 years ago

Maybe we should not add ninja and cmake to pyproject.toml. It means we need to find a cmake and ninja from PyPI, aka a python package, not cmake and ninja. I don't think people would install cmake from PyPI.

RUrlus commented 2 years ago

@hubutui the fix for the unneeded files is to remove the MANIFEST.in. We'll create a new release soon.

Regarding the pyproject.toml, I disagree. Many people (will) use the pip distributed cmake and ninja. Moreover, building without isolation is non-standard these days. I don't want to require people to install software manually when we can pip install it during the installation.

hubutui commented 2 years ago

ok, I think I'm ok to build it with --skip-dependency-check.