feat: clean up and update to pybind11 2.6.0

[henryiii]

Updating CI and cleaning up the example. CI work mostly copied from python_example - after all, most packages should behave "nicely" under python Packaging and not need significant customizations/workarounds. Adds the full GitHub Actions testing, including conda and cibuildwheel.

Discovery is not perfect - if you build on windows-latest in GitHub Actions, for example, the conda example breaks because it's looking for MSVC 2017 instead of 2019 - while it works with the python_example. Something for a later followup. I'd like to try (again) to use Ninja everywhere in the future; the problem is communicating the correct MSVC to Ninja.

I took some inspiration from @HDembinski's work on iMinuit and @jpivarski's work on Awkward-1; this helped solve an issue with 32/64 bits and an issue with Conda (thank you!) (closes #25). Also closes #7.

This is still using the submodule method, rather than PyPI method. It's probably best to have a mix - python_example uses PyPI, so I don't think this one should change. But we might eventually want to note that somewhere.

Obviously needs a squash and merge since I left a mess in the history. I'll likely be referring to it later, though, so that's why I left it.

[henryiii]

One thing that bothers me, by the way, is that we are just running from the build directory; we never run the install step. CMake may not setup RPaths correctly, for example (unless you mess with CMAKE_BUILD_RPATH_USE_ORIGIN). There may be other things, too. Not for this PR though, and it's no worse / not changed from before.

[henryiii]

I think all review points are now addressed.

We still use Ninja by default (since it comes with a wheel, while make does not, along with being faster and better), but I now allow CMAKE_GENERATOR to force a generator on all platforms.

I also put in support of self.parallel, though as far as I can tell it was a disaster and is practically never used - pip & PyPI build have no support for it, NumPy designed something totally different anyway, and it doesn't parallelize file builds within extensions, but only extensions so it only helps if you have multiple Extensions, and it's Python 3 only. Of course, because we are properly parallelizing with it, it's more useful for us, but the lack of support in the ecosystem makes it rather useless, as far as I can tell. I don't think there's no harm in supporting it, as it defaults to None, but don't know if it fits in a "simple" example. It's not really settable using python -m build or python -m pip wheel ., so it's not something that can control the number of threads on CI, where it might matter (due to memory constraints).

PS: Any PEP 517 system cannot directly interact with setup.py commands. Python is moving away from "setup.py" being required or used by users. A general hook for parallelization would need to be added. They are still working on an "editable mode" hook, so this is probably very low priority, especially since setuptools parallelization is so bad.

[HDembinski]

You don't need make to build on windows, you just use cmake --build bla. I still think ninja should be dropped from a minimal example.

[HDembinski]

Setting -j works just fine when you do python setup.py build_ext -i -j 8 for instance. That pip cannot use it is no reason to not support it, it exists in the command-line interface. Not connecting this existing cmd-line flag with this functionality would be weird.

[henryiii]

You must supply make or ninja in your pyproject.toml, otherwise you may not be able to build. You can't supply make, so that leaves ninja. There is no promise that make is available in UNIX. Usually, sure, but not always. Windows doesn't matter, but downloading ninja doesn't hurt (and I'd rather use it there, I just haven't figured out how to broadcast the setuptools compiler discovery to Ninja). I could use an environment marker to disable it on windows, I believe, but I think it's better simple (and not 100% about that).

python setup.py build_ext -i -j 8

And I did add support for that. But my point is that python setup.py STUFF is being discouraged in Python. No normal user downloads every package they want to install then runs python setup.py in each one. So if there's a difference between pip install and python setup.py, the pip install version is the correct version. And there can be differences, such as pip always making an SDist first, coping to a build directory, and such. If you know that those exist and build your package to work in both cases, great. But you always run a risk when your development environment is different than a user install.

Also, one place you may need to limit the number of threads is when building wheels in CI (such as I have to do on ARM emulation for boost-histogram, more than one thread crashes without enough memory), then you can't do that if you can't access the parallel compiler control when cibuildwheel runs the pip wheel command (soon to also allow python -m build).

But you have convinced me that it should be supported too. :) I just felt that it might not be needed in a "simple" example since the only way to use it requires knowledge of setting up a "fast and dirty" development environment.

[henryiii]

Looks like pyproject.toml just implements standard environment markers, PEP 518 just points at PEP 508 (which is good!). I can turn off ninja being downloaded needlessly (for now) for Windows then.

[henryiii]

Also, I think an example should cover in as many places as possible. A user can always remove the ninja part if they think make will always be available. It almost always is if you have a compiler. For a library, I might be willing to risk it, but I don't think a general example should.

[henryiii]

Let's simplify and mostly drop the custom thread handing, since CMAKE_BUILD_PARALLEL_LEVEL will set the default value of -j across all generators anyway. If users want more, they can add more. I'll leave the handling of self.parallel, but otherwise, CMAKE_BUILD_PARALLEL_LEVEL should be used, otherwise you get the generator default. For ninja, of course, that's nicer than make, but if we decide to go with make, then we should go with all defaults.

Trying to use multiprocessing.cpu_count means we have to add a try/catch, and we need to not override CMAKE_BUILD_PARALLEL_LEVEL, etc...

[YannickJadoul]

You must supply make or ninja in your pyproject.toml, otherwise you may not be able to build. You can't supply make, so that leaves ninja.

You still need compilers as well, and I think it's safe to assume that any Unix system with compilers has (or should have?) make installed.

Also, I think an example should cover in as many places as possible.

Probably, yes. Though in this case, this example is meant as a minimal example (that's sturdy enough to build on and extend), right?

That all being said, Henry's approach seems like giving a good and safe example for users, and, while I've not really used it myself, Ninja isn't all too new or radically different from make, and doesn't add too much overhead. So I don't strongly feel it nééds to be this way, but from my perspective, I think it's perfectly fine like this. If anything, you could add some pointers to what users could change, based on their assumptions. But actually, things are already pretty well documented, so ...

[henryiii]

In my opinion, a minimal example should work as many places as possible (such as in docker images that don't have make), but it doesn't need to be full featured (say, having multithreaded builds).

Again, this for me is a good starting point to attempt removing MSBuild and using pure Ninja, which would both simplify the if statements, and fix the current issue that MSVC isn't picked up correctly from setuptools. python_example works just fine on all cibuildwheel images, and with conda on the latest GitHub Actions image, but here we can't enable cibuildwheel yet and the latest GitHub Actions image, because those break due to CMake saying it can't find MSVC when setuptools is happy.

@HDembinski , is this a reasonable enough compromise for you to sign off on it too? I plan to refine with a PR in the near future to fix the MSVC discovery somehow. I could instead make-only PR if Ninja doesn't help solve that issue and I am able to fix the current Ninja on Unix, MSBuild on Windows structure on Windows.

I think anything is better than the current broken-on-all-CI state... :)

[HDembinski]

Like @YannickJadoul, I have never seen a UNIX-y image that has a compiler but comes without make, so I don't see why we need to prepare for cases where make is missing.

iminuit builds with my own version of this example (before @henryiii made these changes) just fine on Windows, MacOS and Linux without ninja on the images from cibuildwheels. So there is apparently no need to add it. Likewise, I don't see the need to customize the generator with an environment variable. A minimal example does not need that. Removing ninja and generator customization does not remove any useful functionality from this example.

This is a template to give users a good start for customization, it is not a library solution that has to offer all possible features and has to cover all possible use cases. That's why I am so insistent on having it minimal. We want this to be easy to read and uncluttered.

I will not argue further for my position since there is nothing else to say.

[henryiii]

docker run -it --rm ubuntu
apt update && apt install python3-dev python3-venv g++ git
python3 -m venv venv
. ./venv/bin/activate
pip install git+https://github.com/scikit-hep/iminuit.git

Collecting git+https://github.com/scikit-hep/iminuit.git
  Cloning https://github.com/scikit-hep/iminuit.git to /tmp/pip-req-build-186q9zxn
  Running command git clone -q https://github.com/scikit-hep/iminuit.git /tmp/pip-req-build-186q9zxn
  Running command git submodule update --init --recursive -q
  From https://github.com/root-project/root
   * branch                  d69de80125476ab6c7b330df90bc8282341637b6 -> FETCH_HEAD
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Collecting numpy
  Downloading numpy-1.19.4-cp38-cp38-manylinux2010_x86_64.whl (14.5 MB)
     |████████████████████████████████| 14.5 MB 9.8 MB/s
Building wheels for collected packages: iminuit
  Building wheel for iminuit (PEP 517) ... error
  ERROR: Command errored out with exit status 1:
   command: /vevn/bin/python3 /tmp/tmpx1n63x__ build_wheel /tmp/tmphl8v7s95
       cwd: /tmp/pip-req-build-186q9zxn
  Complete output (69 lines):
  running bdist_wheel
  running build
  running build_py
  creating build
  creating build/lib.linux-x86_64-3.8
  creating build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/repr_html.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/cost.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/color.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/repr_text.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/version.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/__init__.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/_minuit.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/latex.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/_minimize.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/pdg_format.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/util.py -> build/lib.linux-x86_64-3.8/iminuit
  copying src/iminuit/_deprecated.py -> build/lib.linux-x86_64-3.8/iminuit
  running build_ext
  CMake Error: CMake was unable to find a build program corresponding to "Unix Makefiles".  CMAKE_MAKE_PROGRAM is not set.  You probably need to select a different build tool.

(I've stopped here, it continues to spew errors that rather hide the first one here, making look more like it can't find a compiler)

---

pip install git+https://github.com/henryiii/cmake_example.git@feat/260

Collecting git+https://github.com/henryiii/cmake_example.git@feat/260
  Cloning https://github.com/henryiii/cmake_example.git (to revision feat/260) to /tmp/pip-req-build-63aop3tp
  Running command git clone -q https://github.com/henryiii/cmake_example.git /tmp/pip-req-build-63aop3tp
  Running command git checkout -b feat/260 --track origin/feat/260
  Switched to a new branch 'feat/260'
  Branch 'feat/260' set up to track remote branch 'feat/260' from 'origin'.
  Running command git submodule update --init --recursive -q
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Building wheels for collected packages: cmake-example
  Building wheel for cmake-example (PEP 517) ... done
  Created wheel for cmake-example: filename=cmake_example-0.0.1-cp38-cp38-linux_x86_64.whl size=45239 sha256=339379130b5e9d04dcd6f801257e0dc75561b2e8bbc640abc7571eccc6d213e2
  Stored in directory: /tmp/pip-ephem-wheel-cache-x5odwbhf/wheels/11/1a/71/7188b24487333331edd5d60a0deb1bfdac50d2ddf6cadd2a81
Successfully built cmake-example
Installing collected packages: cmake-example
Successfully installed cmake-example-0.0.1

[henryiii]

Opps, sorry, it was only conda that's having a problem, I do have cibuildwheel enabled. Great news about iMinuit using cibuildwheel!