Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libxml2: fix include dirs in CMake config file #24649

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

fpoirotte
Copy link
Contributor

Summary

Changes to recipe: libxml2/2.12.7

Motivation

The recipe creates its own CMake config file through the _create_cmake_module_variables() method.

However, the method contains a typo. when the LibXml2_INCLUDE_DIRS variable is not defined but libxml2_INCLUDE_DIRS is, it used its value to set LIBXML2_LIBRARIES & LIBXML2_LIBRARY. This is incorrect, as the two settings are not related (one is used by the preprocessor while the other is used by the linker).

This is further evidenced by the fact that the variables LIBXML2_LIBRARIES & LIBXML2_LIBRARY are later overwritten using the value in LibXml2_LIBRARIES or libxml2_LIBRARIES (in order of precedence).

Details

The PR uses the value of libxml2_INCLUDE_DIRS to set LIBXML2_INCLUDE_DIR / LIBXML2_INCLUDE_DIRS instead of LIBXML2_LIBRARIES / LIBXML2_LIBRARY.


If libxml2_INCLUDE_DIRS is defined, it should be used to set
the variables related to the preprocessor's include directories,
rather than the variables related to the linker.
@CLAassistant
Copy link

CLAassistant commented Jul 17, 2024

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@valgur
Copy link
Contributor

valgur commented Jul 17, 2024

The custom CMake module can be made considerably simpler altogether and copied from the recipe dir instead of generating it:

set(LibXml2_FOUND TRUE)
set(LIBXML2_FOUND TRUE)
set(LIBXML2_INCLUDE_DIR "${${CMAKE_FIND_PACKAGE_NAME}_INCLUDE_DIRS}")
set(LIBXML2_INCLUDE_DIRS "${${CMAKE_FIND_PACKAGE_NAME}_INCLUDE_DIRS}")
set(LIBXML2_LIBRARIES "${${CMAKE_FIND_PACKAGE_NAME}_INCLUDE_DIRS}")
set(LIBXML2_LIBRARY "${${CMAKE_FIND_PACKAGE_NAME}_INCLUDE_DIRS}")
set(LIBXML2_DEFINITIONS "${${CMAKE_FIND_PACKAGE_NAME}_DEFINITIONS}")
set(LIBXML2_VERSION_STRING "${${CMAKE_FIND_PACKAGE_NAME}_VERSION}")

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 8 (11a4e28497aa02be4b07a638fc627c27e2e14843):

  • libxml2/2.12.5:
    All packages built successfully! (All logs)

  • libxml2/2.12.4:
    All packages built successfully! (All logs)

  • libxml2/2.12.6:
    All packages built successfully! (All logs)

  • libxml2/2.12.7:
    All packages built successfully! (All logs)

  • libxml2/2.11.8:
    All packages built successfully! (All logs)

  • libxml2/2.10.3:
    All packages built successfully! (All logs)

  • libxml2/2.10.4:
    All packages built successfully! (All logs)

  • libxml2/2.11.4:
    All packages built successfully! (All logs)

  • libxml2/2.12.3:
    All packages built successfully! (All logs)

  • libxml2/2.9.10:
    All packages built successfully! (All logs)

  • libxml2/2.11.7:
    All packages built successfully! (All logs)

  • libxml2/2.9.12:
    All packages built successfully! (All logs)

  • libxml2/2.11.6:
    All packages built successfully! (All logs)

  • libxml2/2.9.14:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 7 (11a4e28497aa02be4b07a638fc627c27e2e14843):

  • libxml2/2.12.6:
    All packages built successfully! (All logs)

  • libxml2/2.12.5:
    All packages built successfully! (All logs)

  • libxml2/2.12.7:
    All packages built successfully! (All logs)

  • libxml2/2.12.4:
    All packages built successfully! (All logs)

  • libxml2/2.11.6:
    All packages built successfully! (All logs)

  • libxml2/2.12.3:
    All packages built successfully! (All logs)

  • libxml2/2.11.8:
    All packages built successfully! (All logs)

  • libxml2/2.9.14:
    All packages built successfully! (All logs)

  • libxml2/2.10.3:
    All packages built successfully! (All logs)

  • libxml2/2.10.4:
    All packages built successfully! (All logs)

  • libxml2/2.9.12:
    All packages built successfully! (All logs)

  • libxml2/2.11.4:
    All packages built successfully! (All logs)

  • libxml2/2.9.10:
    All packages built successfully! (All logs)

  • libxml2/2.11.7:
    All packages built successfully! (All logs)

Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@conan-center-bot conan-center-bot merged commit 29d1173 into conan-io:master Jul 24, 2024
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants