Library Targets Include Directories Broken

Abierto
#1,016 1 comentario 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
45/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Estancado
Stack tecnológico
cmake, cpp
Área
build-system

Línea de trabajo

Comienza en cmake/draco_targets.cmake, en la definición de draco_add_library y en las llamadas a target_include_directories alrededor de las líneas 226, 319 y 323. Revisa cómo se genera draco/draco_features.h y cómo se consumen los targets como subproyectos o bibliotecas instaladas. El trabajo estará terminado cuando las rutas de inclusión funcionen correctamente tanto durante la compilación como durante la instalación, y las macros de características estén disponibles para los consumidores de la biblioteca.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

bug cmake

Hi folks, when using the draco library as a subproject, or with CMake's FetchContent/ExternalProject machinery, the draco includes are broken. This stems from the definition of the draco_add_library macro. Specifically, on line 226, the library target has the INSTALL_INTERFACE set at the include directory of the install folder. This works as intended. However, the BUILD_INTERFACE is not set at any point. Instead on lines 319 and 323 target_include_directories is called again to provide private and public directories, those these are not restricted to build time.

Now, my CMake experience does not include using macro wrappers around the typical target oriented machinery. Instead I am used to the modern CMake style of generator expressions. I worry that producing a large pull request reworking the CMake would not be accepted as it would be difficult to review.

So here is how I would address the issue: All of the library targets use the same build and install include directories. I would replace the referenced lines by the singular block:

target_include_directories(${lib_NAME} PUBLIC
    $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/src/>
    $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/>
)

Now this doesn't address the generated draco/draco_features.h header, at least at build time. This CMake generated header defines a set of macros describing which features are supported. Instead this should be handled using the target_compile_definitions machinery. By setting these values publicly on the targets, both consumers of the installed library and the build time library will have those macros defined. The header can still be generated for backwards compatibility purposes as well.

Lenguaje dominante
C++
Estrellas
7.5k
Forks
1.1k
Merge medio
23 h 57 min
PR fusionados (30 d)
3

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de google/draco

Todos los issues de google/draco

Issues similares

Más issues de C++

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.