Skip to content

Conversation

@billcxx
Copy link

@billcxx billcxx commented Oct 3, 2022

Use CMAKE_CURRENT_SOURCE_DIR when looking for tag, this will prevent potential problems if the project is grabbed by fetchContent in other project

Issue
Resolves #874

Approach
Should use CMAKE_CURRENT_SOURCE_DIR instead of CMAKE_SOURCE_DIR when looking for tags

Use CMAKE_CURRENT_SOURCE_DIR when looking for tag, this will prevent potential problems if the project is grabbed by fetchContent in other project
@ertomatic
Copy link
Collaborator

Can one of the admins verify this patch?

@lars-petter-hauge
Copy link
Contributor

Hi,

Thanks for the contribution and sorry for the very delayed response.

I suppose the discrepancy using CMAKE_SOURCE_DIR when building ecl as part of a different project will be relevant for all usages of CMAKE_SOURCE_DIR and not only when fetching tags.

Similar using CMAKE_BINARY_DIR vs CMAKE_CURRENT_BINARY_DIR will be affected.

Care to weigh in @pinkwah ?

@pinkwah
Copy link
Collaborator

pinkwah commented Jan 17, 2023

Jenkins test this please

@pinkwah pinkwah closed this Jan 17, 2023
@pinkwah pinkwah reopened this Jan 17, 2023
@pinkwah
Copy link
Collaborator

pinkwah commented Jan 17, 2023

Thanks for your contribution @billcxx!

It seems that the CMake code styling check failed. Would you mind running cmake-format -i CMakeLists.txt (after installing the pip package cmake-format) to satisfy it? I'll merge when that is done. :)

@billcxx
Copy link
Author

billcxx commented Jan 17, 2023

Hi,

Thanks for the contribution and sorry for the very delayed response.

I suppose the discrepancy using CMAKE_SOURCE_DIR when building ecl as part of a different project will be relevant for all usages of CMAKE_SOURCE_DIR and not only when fetching tags.

Similar using CMAKE_BINARY_DIR vs CMAKE_CURRENT_BINARY_DIR will be affected.

Care to weigh in @pinkwah ?

I changed all cmake_source_dir and cmake_binary_dir.
Tested with a simple

mkdir build; cd build
cmake ..
make

Then also format the CMakeLists.txt with cmake-format.

Thanks for your contribution @billcxx!

It seems that the CMake code styling check failed. Would you mind running cmake-format -i CMakeLists.txt (after installing the pip package cmake-format) to satisfy it? I'll merge when that is done. :)

@eivindjahren eivindjahren added the christmas-review Issues and PRs for Christmas review label Dec 13, 2024
@eivindjahren eivindjahren removed the christmas-review Issues and PRs for Christmas review label Dec 20, 2024
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.

Cannot build the project with cmake fetchContent

5 participants