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

CMake improvement #9

Merged
merged 1 commit into from
Oct 22, 2023
Merged

Conversation

topazus
Copy link
Contributor

@topazus topazus commented Oct 22, 2023

close #8

write_basic_package_version_file(<filename>
  [VERSION <major.minor.patch>]
  COMPATIBILITY <AnyNewerVersion|SameMajorVersion|SameMinorVersion|ExactVersion>
  [ARCH_INDEPENDENT] )

There are some extra questions.

  1. which COMPATIBILITY mode will be appropriate to use? AnyNewerVersion, SameMajorVersion, SameMinorVersion and ExactVersion .
  2. ARCH_INDEPENDENT in write_basic_package_version_file can be used when CMake version >= 3.14 since boxed-cpp is a header-only package and architecture-independent.
# https://github.com/xtensor-stack/xtl/blob/908958eb8edebc4fdb0c5cd9b86912bca3eca5c9/CMakeLists.txt#L129-L144
if(CMAKE_VERSION VERSION_LESS 3.14)
    set(BOXED_CPP_CMAKE_SIZEOF_VOID_P ${CMAKE_SIZEOF_VOID_P})
    unset(CMAKE_SIZEOF_VOID_P)
    write_basic_package_version_file(
        ${CMAKE_CURRENT_BINARY_DIR}/boxed-cpp-config-version.cmake
        VERSION ${PROJECT_VERSION}
        COMPATIBILITY SameMajorVersion
    )
    set(CMAKE_SIZEOF_VOID_P ${BOXED_CPP_CMAKE_SIZEOF_VOID_P})
else()
    write_basic_package_version_file(
        ${CMAKE_CURRENT_BINARY_DIR}/boxed-cpp-config-version.cmake
        VERSION ${PROJECT_VERSION}
        COMPATIBILITY SameMajorVersion
        ARCH_INDEPENDENT
    )
endif()

Or just update the CMake minimum version restriction to version 3.14.

Ref:

@christianparpart
Copy link
Member

oh many thanks @topazus. I'll have a look ASAP! :)

Copy link
Member

@christianparpart christianparpart left a comment

Choose a reason for hiding this comment

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

i wonder how we can make use of this in Contour 🤔

include(ThirdParties)
include(ClangTidy)
include(PedanticCompiler)

set(boxed_cpp_HEADERS
include/boxed-cpp/boxed.hpp
${PROJECT_SOURCE_DIR}/include/boxed-cpp/boxed.hpp
Copy link
Member

Choose a reason for hiding this comment

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

Should we adapt this for our other libraries as well? Especially libunicode? What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is fine if you want to use ${PROJECT_SOURCE_DIR} in this situation for libunicode.

@christianparpart christianparpart merged commit 783cb74 into contour-terminal:master Oct 22, 2023
@christianparpart
Copy link
Member

p.s.: SameMajorVersion is fine :)

@topazus topazus deleted the fix-cmake branch October 23, 2023 03:38
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.

CMake improvement
2 participants