refactor: more comments from @YannickJadoul

Author: henryiii

docs/compiling.rst

@@ -127,11 +127,11 @@ Copy manually
 -------------
 
 You can also copy ``setup_helpers.py`` directly to your project; it was
-designed to be usable standalone, like the old example ``setup.py``. The
-``CppExtension`` class is identical to ``Pybind11Extension``, except it does
-not include the pybind11 package headers, so you can use it with git submodules
-and a specific git version. If you use this, you will need to import from a
-local file in ``setup.py`` and ensure the helper file is part of your MANIFEST.
+designed to be usable standalone, like the old example ``setup.py``. You can
+set ``include_pybind11=False`` to skip including the pybind11 package headers,
+so you can use it with git submodules and a specific git version. If you use
+this, you will need to import from a local file in ``setup.py`` and ensure the
+helper file is part of your MANIFEST.
 
 
 .. versionchanged:: 2.6

docs/upgrade.rst

@@ -82,8 +82,8 @@ The Python package now includes the headers as data in the package itself, as
 well as in the "headers" wheel slot. ``pybind11 --includes`` and
 ``pybind11.get_include()`` report the new location, which is always correct
 regardless of how pybind11 was installed, making the old ``user=`` argument
-meaningless. If you are not using the helper, you are encouraged to switch to
-the package location.
+meaningless. If you are not using the function to get the location already, you
+are encouraged to switch to the package location.
 
 
 v2.2

pybind11/setup_helpers.py

@@ -64,32 +64,11 @@
 # directory into your path if it sits beside your setup.py.
 
 
-# Just in case someone clever tries to multithread
-tmp_chdir_lock = threading.Lock()
-cpp_cache_lock = threading.Lock()
-
-
-@contextlib.contextmanager
-def tmp_chdir():
-    "Prepare and enter a temporary directory, cleanup when done"
-
-    # Threadsafe
-    with tmp_chdir_lock:
-        olddir = os.getcwd()
-        try:
-            tmpdir = tempfile.mkdtemp()
-            os.chdir(tmpdir)
-            yield tmpdir
-        finally:
-            os.chdir(olddir)
-            shutil.rmtree(tmpdir)
-
-
-class CppExtension(_Extension):
+class Pybind11Extension(_Extension):
     """
-    Build a C++11+ Extension module. This automatically adds the recommended
-    flags when you init the extension and assumes C++ sources - you can further
-    modify the options yourself.
+    Build a C++11+ Extension module with pybind11. This automatically adds the
+    recommended flags when you init the extension and assumes C++ sources - you
+    can further modify the options yourself.
 
     The customizations are:
 
@@ -105,6 +84,9 @@ class CppExtension(_Extension):
     property more than once, as flags are added when you set it. Set the
     property to None to disable the addition of C++ standard flags.
 
+    If you want to add pybind11 headers manually, for example for an exact
+    git checkout, then set ``include_pybind11=False``.
+
     Warning: do not use property-based access to the instance on Python 2 -
     this is an ugly old-style class due to Distutils.
     """
@@ -127,12 +109,27 @@ def __init__(self, *args, **kwargs):
         if "language" not in kwargs:
             kwargs["language"] = "c++"
 
+        include_pybind11 = kwargs.pop("include_pybind11", True)
+
         # Can't use super here because distutils has old-style classes in
         # Python 2!
         _Extension.__init__(self, *args, **kwargs)
 
+        # Include the installed package pybind11 headers
+        if include_pybind11:
+            # If using setup_requires, this fails the first time - that's okay
+            try:
+                import pybind11
+
+                pyinc = pybind11.get_include()
+
+                if pyinc not in self.include_dirs:
+                    self.include_dirs.append(pyinc)
+            except ImportError:
+                pass
+
         # Have to use the accessor manually to support Python 2 distutils
-        CppExtension.cxx_std.__set__(self, cxx_std)
+        Pybind11Extension.cxx_std.__set__(self, cxx_std)
 
         if WIN:
             self._add_cflags("/EHsc", "/bigobj")
@@ -183,28 +180,25 @@ def cxx_std(self, level):
                 self.extra_compile_args.append("-Wno-deprecated-register")
 
 
-class Pybind11Extension(CppExtension):
-    """
-    A pybind11 Extension subclass. Includes the header directory from the
-    package.
-    """
-
-    def __init__(self, *args, **kwargs):
-        CppExtension.__init__(self, *args, **kwargs)
-
-        # If using setup_requires, this fails the first time - that's okay
-        try:
-            import pybind11
-
-            pyinc = pybind11.get_include()
+# Just in case someone clever tries to multithread
+tmp_chdir_lock = threading.Lock()
+cpp_cache_lock = threading.Lock()
 
-            if pyinc not in self.include_dirs:
-                self.include_dirs.append(pyinc)
-        except ImportError:
-            pass
 
+@contextlib.contextmanager
+def tmp_chdir():
+    "Prepare and enter a temporary directory, cleanup when done"
 
-Pybind11Extension.__doc__ += CppExtension.__doc__
+    # Threadsafe
+    with tmp_chdir_lock:
+        olddir = os.getcwd()
+        try:
+            tmpdir = tempfile.mkdtemp()
+            os.chdir(tmpdir)
+            yield tmpdir
+        finally:
+            os.chdir(olddir)
+            shutil.rmtree(tmpdir)
 
 
 # cf http://bugs.python.org/issue26689

setup.py

@@ -18,10 +18,10 @@
 # PYBIND11_GLOBAL_SDIST will build a different sdist, with the python-headers
 # files, and the sys.prefix files (CMake and headers).
 
-alt_sdist = os.environ.get("PYBIND11_GLOBAL_SDIST", False)
+global_sdist = os.environ.get("PYBIND11_GLOBAL_SDIST", False)
 
 version_py = "pybind11/_version.py"
-setup_py = "tools/setup_global.py.in" if alt_sdist else "tools/setup_main.py.in"
+setup_py = "tools/setup_global.py.in" if global_sdist else "tools/setup_main.py.in"
 extra_cmd = 'cmdclass["sdist"] = SDist\n'
 
 to_src = (

tests/extra_python_package/test_files.py

@@ -159,7 +159,7 @@ def test_build_sdist(monkeypatch, tmpdir):
         assert pyproject_toml == contents
 
 
-def test_build_alt_dist(monkeypatch, tmpdir):
+def test_build_global_dist(monkeypatch, tmpdir):
 
     monkeypatch.chdir(MAIN_DIR)
     monkeypatch.setenv("PYBIND11_GLOBAL_SDIST", "1")
@@ -252,7 +252,7 @@ def tests_build_wheel(monkeypatch, tmpdir):
     assert files == trimmed
 
 
-def tests_build_alt_wheel(monkeypatch, tmpdir):
+def tests_build_global_wheel(monkeypatch, tmpdir):
     monkeypatch.chdir(MAIN_DIR)
     monkeypatch.setenv("PYBIND11_GLOBAL_SDIST", "1")
 

tools/setup_global.py.in

@@ -1,7 +1,8 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
-# Setup script for PyPI; use CMakeFile.txt to build extension modules
+# Setup script for pybind11-global (in the sdist or in tools/setup_global.py in the repository)
+# This package is targeted for easy use from CMake.
 
 import contextlib
 import glob

tools/setup_main.py.in

@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
-# Setup script for PyPI - placed in the sdist
+# Setup script (in the sdist or in tools/setup_main.py in the repository)
 
 from setuptools import setup