cookie icon indicating copy to clipboard operation
cookie copied to clipboard

built wheels install CMakeLists.txt and License in env's lib folder

Open 2bndy5 opened this issue 3 years ago • 1 comments

This is related to scikit-build option that I chose when asked by cookiecutter. I think the problem is that the MANIFEST.in explicitly includes CMakeLists.txt and LICENSE for sdist, but this really doesn't apply to bdist_wheels.

  1. Binary wheels don't need to include the CMakeLists.txt that was used to build it, so I think that should be excluded by default.
  2. The License file should be relative the pkg installed, not in the env's lib folder. In fact, the License is already included in the pkg's corresponding -info installed path.

I was able to solve this by removing https://github.com/scikit-hep/cookie/blob/eed6d2600ffa9d1517a09abf240b4a4b7dd38e6e/%7B%7Bcookiecutter.project_name%7D%7D/setup-skbuild.py#L16

But, there's likely a more elegant solution (probably using exclude_package_data). I'm not sure if this issue was introduced by trying to satisfy installation for different implementations of Python (like PyPy).

2bndy5 avatar Aug 14 '22 22:08 2bndy5

You do very much need the CMakeLists.txt & LICENSE for the SDist, those are part of the source. I thought include_package_data would only include things inside packages, but I know it's sometimes problematic. It could also be something we are muddling with scikit-build's package modifications (which I really dislike and hope to remove in the future).

henryiii avatar Aug 15 '22 03:08 henryiii

Yeah, include_package_data is basically useless and breaks things. The correct thing to do is to explicitly list package_data. Most other backends are smart enough to to know automatic package data needs to be inside the package…

henryiii avatar Sep 07 '22 04:09 henryiii

The correct thing to do is to explicitly list package_data.

Thank goodness this cookiecutter has jinja at its disposal.

Since discovering this, I have moved away from using scikit-build for my pybind11 projects. I wouldn't blame you if you wanted to remove that build backend option from the cookiecutter.

2bndy5 avatar Sep 07 '22 05:09 2bndy5

I'm very much invested in making scikit-build work. Though that includes rewriting the whole thing to avoid setuptools entirely. 😵‍💫

I am wondering if it's an issue with scikit-build confusing the package directory - I'm thinking the places where include_package_data has broken in this out-of-folder way usually involves scikit-build (though setuptools itself discourages the use of include_package_data these days).

henryiii avatar Sep 07 '22 14:09 henryiii

I think their resulting location is because of their location in the src (relative to package_dir). I'm guessing you already knew that though.

A while ago, I had to use globs for package_data in a sphinx theme, and I haven't had to monkey with it since.

2bndy5 avatar Sep 07 '22 14:09 2bndy5