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

adding an integration test to show fetch_content can work #2894

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3d68adf
adding an integration test to show fetch_content can work
K20shores Mar 15, 2024
0dc595c
trying to cache deps
K20shores Mar 15, 2024
81b0ca0
removing the caching from windows stuff, didn't know what I was doing
K20shores Mar 15, 2024
a9a8dfb
fetching the h5 stuff
K20shores Mar 15, 2024
96ad9ae
ubuntu needed matrix
K20shores Mar 15, 2024
dfc6031
environment variables
K20shores Mar 15, 2024
9df0730
installing deps on linux
K20shores Mar 15, 2024
cac854f
correcting paths
K20shores Mar 15, 2024
a4aa0f9
path again
K20shores Mar 15, 2024
5b8c273
including documentation for fetch content
K20shores Mar 15, 2024
e6da400
fixing paths
K20shores Mar 15, 2024
70e5f15
debugging the find
K20shores Mar 18, 2024
40866b7
setting cmake path
K20shores Mar 18, 2024
8043ba7
just the one
K20shores Mar 18, 2024
8ae6795
debug
K20shores Mar 18, 2024
479cf1e
wrong one
K20shores Mar 18, 2024
9d6e798
maybe this
K20shores Mar 18, 2024
0d68b33
fixing path
K20shores Mar 18, 2024
6cbff81
verbose serial build
K20shores Mar 18, 2024
150b2c0
and now this
K20shores Mar 18, 2024
487faa9
adding a library?
K20shores Mar 18, 2024
b5f82d3
correct name
K20shores Mar 18, 2024
62bea0c
installing curl?
K20shores Mar 18, 2024
9c7cb02
full name
K20shores Mar 18, 2024
d77206a
maybe curl works now?
K20shores Mar 19, 2024
7d1c71a
removing fetch content example from cygwin and mingw builds
K20shores Mar 22, 2024
ccfcafd
adding appveyor fetch content test
K20shores Mar 22, 2024
bcb3170
trying to correct appveyor
K20shores Mar 22, 2024
c834636
testing the fetch content thing
K20shores Mar 22, 2024
f216dbf
making appveyor pass
K20shores Mar 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .github/workflows/run_tests_osx.yml
Original file line number Diff line number Diff line change
Expand Up @@ -478,3 +478,51 @@ jobs:
cd build
LD_LIBRARY_PATH=${LD_LIBRARY_PATH} ctest -j 12 --rerun-failed --output-on-failure -VV
if: ${{ failure() }}

fetch_content_cmake:
needs: build-deps-osx
runs-on: macos-12
strategy:
matrix:
hdf5: [ 1.12.2, 1.14.3 ]
steps:
- uses: actions/checkout@v3

###
# Set Environmental Variables
###

- run: echo "CMAKE_PREFIX_PATH=${HOME}/environments/${{ matrix.hdf5 }}/" >> $GITHUB_ENV
- run: echo "LD_LIBRARY_PATH=${HOME}/environments/${{ matrix.hdf5 }}/lib" >> $GITHUB_ENV

Choose a reason for hiding this comment

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

Not a fan of these approaches, but I understand the need for them right now. If hdf5 is fetch-contentable, this could be simplified greatly.

Anyway, LD_LIBRARY_PATH should be unnecessary if running from build directory and the dependencies are installed with RPATH. Also, for mac it's DYLD_LIBRARY_PATH


###
# Fetch Cache
###
- name: Fetch HDF Cache
id: cache-hdf5-osx
uses: actions/cache@v3
with:
path: ~/environments/${{ matrix.hdf5 }}
key: hdf5-${{ runner.os }}-${{ matrix.hdf5 }}

- name: Check Cache
shell: bash -l {0}
run: ls ${HOME}/environments && ls ${HOME}/environments/${{ matrix.hdf5 }} && ls ${HOME}/environments/${{ matrix.hdf5}}/lib

###
# Configure and build
###
- name: Run Cmake
run: |
cd integration_tests/fetch_content
cmake -S . -B build

- name: Build
run: |
cd integration_tests/fetch_content
cmake --build build --parallel

- name: Run tests
run: |
cd integration_tests/fetch_content/build
ctest --rerun-failed --output-on-failure . --verbose
53 changes: 53 additions & 0 deletions .github/workflows/run_tests_ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -888,3 +888,56 @@ jobs:
cd build
LD_LIBRARY_PATH=${LD_LIBRARY_PATH} ctest -j 12 --rerun-failed --output-on-failure -VV
if: ${{ failure() }}

fetch_content_cmake:
needs: build-deps-serial
runs-on: ubuntu-latest
strategy:
matrix:
hdf5: [ 1.14.3 ]
steps:
- uses: actions/checkout@v3

- name: Install System dependencies
shell: bash -l {0}
run: sudo apt update && sudo apt install -y libaec-dev zlib1g-dev automake autoconf libcurl4-openssl-dev libjpeg-dev wget curl bzip2 m4 flex bison cmake libzip-dev mpich libmpich-dev

###
# Set Environmental Variables
###

- run: echo "CMAKE_PREFIX_PATH=${HOME}/environments/${{ matrix.hdf5 }}/" >> $GITHUB_ENV
- run: echo "LD_LIBRARY_PATH=${HOME}/environments/${{ matrix.hdf5 }}/lib" >> $GITHUB_ENV

###
# Fetch Cache
###

- name: Fetch HDF Cache
id: cache-hdf5
uses: actions/cache@v3
with:
path: ~/environments/${{ matrix.hdf5 }}
key: hdf5-${{ runner.os }}-${{ matrix.hdf5 }}

- name: Check Cache
shell: bash -l {0}
run: ls ${HOME}/environments/${{ matrix.hdf5 }} && ls ${HOME}/environments/${{ matrix.hdf5}}/lib

###
# Configure and build
###
- name: Run Cmake
run: |
cd integration_tests/fetch_content
cmake -S . -B build

- name: Build
run: |
cd integration_tests/fetch_content
cmake --build build --parallel

Choose a reason for hiding this comment

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

Preferably these tests are run as part of ctest using ctest --build-and-test with a FETCHCONTENT_SOURCE_DIR_<uppercaseName> option. That way you are checking the current version of the project is working with fetchcontent.

For an example: https://github.com/LecrisUT/CMake-Template/blob/a4f73dbd53d8cb98e6325296c2905b2ed40dcaa3/test/CMakeLists.txt#L114-L131

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh neat! I'll give that a try tomorrow!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LecrisUT I actually don't really understand what that test is doing and how it works. The stuff I added builds a small example project. From what I understand of what you sent, it seems like it would only build the netcdf project but not actually link to and use it. I feel like the integration test is a complete test. What am I missing here?

Copy link

@LecrisUT LecrisUT Mar 23, 2024

Choose a reason for hiding this comment

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

The first link there is the general framework for an arbitrary (regression) test that runs a cmake project as if it's used by a user from tge ground up. In the template it's ueed for packaging, example and regular regression test.

For an example of a packaging test, see this CMakeLists.txt. There is a bit of fluff to allow testing in the build tree and outside, but if you look closely it's very minimal, FetchContent + link libraries + simple smoke test. See the parent folder for how the CMakeLists project is registered as a test.

In the framework link I've sent before, notice the --test-command ${CMAKE_CTEST_COMMAND}. That instructs that the test does ctest (after configure -> build) of the cmake project being pointed to. That way you xan be as thorough as you want, e.g. checking that project version matches library, checking features are enabled in the library, etc. This gives more navigable tests since it's just a ctest output run from a ctest test.

Compared to your approach, it's more or less identical, but this adds the ability to be called within the ctest itself, which will make it much easier to keep track of active cmake changes that break the test (it uses the current source as the "git commit" to FetchContent). Your current approach only tests main at any given point.

Basically what I suggest here is to move the execution outside of the dedicated Github Workflow and inside the project's test itself.


Sorry for being too wordy in my explanations most of the times, hope it doesn't put you off

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries! I like detailed explanations and yours makes sense to me. Thanks for the input!


- name: Run tests
run: |
cd integration_tests/fetch_content/build
ctest --rerun-failed --output-on-failure . --verbose
2 changes: 1 addition & 1 deletion .github/workflows/run_tests_win_cygwin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,4 @@ jobs:
- name: (Autotools) Build and run tests
timeout-minutes: 30
run: |
make check -j8 SHELL=/bin/dash
make check -j8 SHELL=/bin/dash
5 changes: 2 additions & 3 deletions .github/workflows/run_tests_win_mingw.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ concurrency:
cancel-in-progress: true

jobs:

build-and-test-autotools:

runs-on: windows-latest
Expand All @@ -27,12 +26,12 @@ jobs:
steps:

- uses: actions/checkout@v3

- uses: msys2/setup-msys2@v2
with:
msystem: MINGW64
update: true
install: git mingw-w64-x86_64-toolchain automake libtool autoconf make cmake mingw-w64-x86_64-hdf5 unzip mingw-w64-x86_64-libxml2 mingw-w64-x86_64-zlib

###
# Configure and build
###
Expand Down Expand Up @@ -72,4 +71,4 @@ jobs:
name: mingw-autotools-test-logs
path: |
*/*.log
*/*.trs
*/*.trs
9 changes: 9 additions & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ build_script:
- cmd: cmake .. -G "%CMAKE_GENERATOR%" -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=%INSTALL_LOC% -DENABLE_BASH_SCRIPT_TESTING=OFF -DENABLE_FILTER_TESTING=OFF -DENABLE_BYTERANGE=ON
- cmd: if errorlevel 1 exit 1
- cmd: cmake --build . --config Release -- /maxcpucount:4
# Check fetch content integration
- cmd: cd ../integration_tests/fetch_content
- cmd: mkdir build
- cmd: cd build
- cmd: cmake ..
- cmd: cmake --build . --config Release -- /maxcpucount:4
# go back to the first build directory
- cmd: cd ../../../build

test_script:
- cmd: cmake --build . --config Release --target install -- /maxcpucount:4

26 changes: 26 additions & 0 deletions docs/building-with-cmake.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,32 @@ or

> $ cmake --build [Build Directory] --target install

## Including netcdf with cmake's FetchContent

Some projects may wish to have cmake handle including their project dependencies with [FetchContent](https://cmake.org/cmake/help/latest/module/FetchContent.html). netCDF's cmake file's also support this. An example cmake setup
that downloads a specific tag of netCDF and links it to some cmake target shows how this can be done.

```
include(FetchContent)

# Fetch NetCDF
FetchContent_Declare(
netcdf
GIT_REPOSITORY https://github.com/Unidata/netcdf-c
GIT_TAG main
)

# disable netCDF tests
set(ENABLE_TESTS OFF CACHE BOOL "" FORCE)

FetchContent_MakeAvailable(netcdf)

add_executable(netcdf_example main.c)

# Link against NetCDF
target_link_libraries(netcdf_example PUBLIC netcdf)
```

# See Also {#cmake_see_also}

For further information regarding NetCDF and CMake, see \ref cmake_faq
25 changes: 25 additions & 0 deletions integration_tests/fetch_content/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
cmake_minimum_required(VERSION 3.11)
project(NetCDFExample)

include(FetchContent)

# Fetch NetCDF
FetchContent_Declare(
netcdf
GIT_REPOSITORY https://github.com/Unidata/netcdf-c
GIT_TAG main
)

set(ENABLE_TESTS OFF CACHE BOOL "" FORCE)

FetchContent_MakeAvailable(netcdf)

# Add executable
add_executable(netcdf_example main.c)

# Link against NetCDF
target_link_libraries(netcdf_example PUBLIC netcdf)

# Add test
enable_testing()
add_test(NAME netcdf_example_test COMMAND netcdf_example)
7 changes: 7 additions & 0 deletions integration_tests/fetch_content/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <stdio.h>
#include <netcdf.h>

int main() {
printf("NetCDF library version: %s\n", nc_inq_libvers());
return 0;
}
Loading