pybind11 icon indicating copy to clipboard operation
pybind11 copied to clipboard

[BUG]: CMake invalid include guard in pybind11Common.cmake

Open petersteneteg opened this issue 1 year ago • 0 comments

Required prerequisites

  • [X] Make sure you've read the documentation. Your issue may be addressed there.
  • [X] Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
  • [X] Consider asking first in the Gitter chat room or in a Discussion.

What version (or hash if on master) of pybind11 are you using?

2.13.6

Problem description

pybind11Common.cmake defines a set of imported targets pybind11::pybind11 etc, They are guarded behind a global include guard. But the defined targets only have directory scope. Hence if one runs find_package(pybind11) from different subdirectories, the first one will run and pybind11Common.cmake and define the targets, the second one will bail at the global include guard, but the targets are not available since they where defined in a different directory and are not global.

This will need to an hard error, since the needed targets are now missing in the second subdirectory.

Changing the guard to "DIRECTORY" solves the issues. But one might still run into issues if one subdirectory would promote one or more targets to global scope. Generally I think each targets should be guarded by a if(NOT TARGET ..)That seems safer to me. Generally I would prefer of handle it more in line with how the autogenerated pybind11Targets.cmake handle it. Basically it has a list of all the expected targets, and checks if all are defined, then it can return early, or if none is defined, then it can add them. Everything else is a fatal error.

The relevant line, https://github.com/pybind/pybind11/blob/7e418f49243bb7d13fa92cf2634af1eeac386465/tools/pybind11Common.cmake#L21

which was introduced in a recent change. https://github.com/pybind/pybind11/commit/28dbce4157c89219705801b06e5a94b612d611a5

Reproducible example code

No response

Is this a regression? Put the last known working version here if it is.

2.13.5

petersteneteg avatar Sep 30 '24 18:09 petersteneteg