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

P4est version is set to 0.0.0 if built from tarball via CMake #325

Open
jmark opened this issue Nov 18, 2024 · 5 comments
Open

P4est version is set to 0.0.0 if built from tarball via CMake #325

jmark opened this issue Nov 18, 2024 · 5 comments

Comments

@jmark
Copy link

jmark commented Nov 18, 2024

Description

When p4est sources are packaged via a CMake tarball the version is set to 0.0.0.
The result in include/p4est_config.h reads

[...]

/* Define to the full name and version of this package. */
#define P4EST_PACKAGE_STRING "p4est 0.0.0"

/* Define to the one symbol short name of this package. */
#define P4EST_PACKAGE_TARNAME "p4est"

/* Define to the home page for this package. */
#define P4EST_PACKAGE_URL ""

/* Define to the version of this package. */
#define P4EST_PACKAGE_VERSION "0.0.0"

/* Version number of package */
#define P4EST_VERSION "0.0.0"

/* Package major version */
#define P4EST_VERSION_MAJOR 0

/* Package minor version */
#define P4EST_VERSION_MINOR 0

/* Package point version */
#define P4EST_VERSION_POINT 0

To Reproduce

Configure p4est with CMake and then create a tarball distribution via

$ make package_source

Then extract the tarball again and configure it via CMake:

tar xvf ${P4EST_BUILD_DIR}/package/p4est-.tar.gz
cmake
cat include/p4est_config.h

The naming of the tarball is also odd: p4est-.tar.gz.

Additional information

This is related to this issue: DLR-AMR/t8code#1294

In t8code we had the same issue. The problem is that the tarball is not a Git repo anymore
and git-gen version is not available. Thus, the version info has to be retrieved another way.

t8code writes out a version.txt file and reads it in case of a tarball distribution.
See https://github.com/DLR-AMR/t8code/blob/main/cmake/GitProjectVersion.cmake
and https://github.com/DLR-AMR/t8code/blob/f8b7becc597975de218d40fb7804ec9723164e04/cmake/CPackConfig.cmake#L24.

If you find a more elegant solution, I'm all ears!

@jmark
Copy link
Author

jmark commented Nov 18, 2024

The issues applies to libsc as well.

@tim-griesbach
Copy link
Collaborator

Thank you for your report!

I created a draft PR that creates a file similar to your version.txt. However, I create a slightly different file .tarball-version to be closer to the tarball creation with Autotools in p4est. The git-version-gen script can then parse .tarball-version as it does it for the Autotools workflow in p4est.

The naming of the tarball is also odd: p4est-.tar.gz.

I fixed this in the referenced PR.

To Reproduce

Configure p4est with CMake and then create a tarball distribution via

$ make package_source

My PR still requires to call some arbitrary build command before this workflow because otherwise the version file is not created. This was also the case for the workflow in t8code when I tested it.

The standard method for adding a dependency (using add_dependencies) to create the version file did not work for me, as package_source does not appear to be a standard CMake target.

@jmark
Copy link
Author

jmark commented Nov 19, 2024

Thank you very much for addressing this issue, @tim-griesbach !

The standard method for adding a dependency (using add_dependencies) to create the version file did not work for me, as package_source does not appear to be a standard CMake target.

I think you have to include CPack in order get the package_source target. See https://cmake.org/cmake/help/book/mastering-cmake/chapter/Packaging%20With%20CPack.html.

@tim-griesbach
Copy link
Collaborator

I think you have to include CPack in order get the package_source target. See https://cmake.org/cmake/help/book/mastering-cmake/chapter/Packaging%20With%20CPack.html.

Thanks, in install.cmake I tried

# Generate .tarball-version file
add_custom_command(
    OUTPUT ${VERSION_FILE}
    COMMAND ${CMAKE_COMMAND} -E echo "${PROJECT_VERSION}" > ${VERSION_FILE}
    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt
   )

add_custom_target(GenerateVersionFile ALL DEPENDS ${VERSION_FILE})

[...]

include(CPack)
add_dependencies(package_source GenerateVersionFile)

but it results in the error:

CMake Error at cmake/install.cmake:93 (add_dependencies):
  Cannot add target-level dependencies to non-existent target
  "package_source".

Moreover, the code that you linked looks to me like t8code should have the same issue as my PR, i.e. in a fresh repository, that was never used to compile code, calling make package_source does not trigger the creation of the version file. I also tested this in t8code and I could not create version.txt by calling make package_source for a fresh repository. Can you verify this behavior for t8code?

@jmark
Copy link
Author

jmark commented Nov 19, 2024

Moreover, the code that you linked looks to me like t8code should have the same issue as my PR, i.e. in a fresh repository, that was never used to compile code, calling make package_source does not trigger the creation of the version file. I also tested this in t8code and I could not create version.txt by calling make package_source for a fresh repository. Can you verify this behavior for t8code?

I can verify this behavior. @Davknapp What is your take on this?

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

No branches or pull requests

2 participants