resdata icon indicating copy to clipboard operation
resdata copied to clipboard

Update CMakeLists.txt

Open billcxx opened this issue 2 years ago • 5 comments

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

billcxx avatar Oct 03 '22 04:10 billcxx

Can one of the admins verify this patch?

ertomatic avatar Oct 03 '22 04:10 ertomatic

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 ?

lars-petter-hauge avatar Jan 05 '23 15:01 lars-petter-hauge

Jenkins test this please

pinkwah avatar Jan 17 '23 08:01 pinkwah

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. :)

pinkwah avatar Jan 17 '23 09:01 pinkwah

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. :)

billcxx avatar Jan 17 '23 16:01 billcxx