ecmwf / ecbuild

A CMake-based build system, consisting of a collection of CMake macros and functions that ease the managing of software build systems
https://ecbuild.readthedocs.io
Apache License 2.0
26 stars 25 forks source link

Capability to download data files from given url #18

Closed mer-a-o closed 4 years ago

mer-a-o commented 4 years ago

This PR adds more capabilities to cmake/ecbuild_get_test_data.cmake. It also adds cmake/ecbuild_check_url.cmake which will check whether a given URL is available.

FussyDuck commented 4 years ago

CLA assistant check
All committers have signed the CLA.

tlmquintino commented 4 years ago

I agree with @wdeconinck that this should use full URL's instead of NAMES and DIRHOST

tlmquintino commented 4 years ago

I don't see the need for 2 macros. I would only provide externally the ecbuild_check_multiurls that would use a list as input. And then rename to ecbuild_check_urls() (note the plural).

mer-a-o commented 4 years ago

@wdeconinck and @tlmquintino thanks for reviewing these changes. I address your comments and all tests pass for me.

mer-a-o commented 4 years ago

@oiffrig, thanks for your review. I addressed them in my latest commit.

oiffrig commented 4 years ago

Thanks @mer-a-o for the changes. In the current state this does not work if neither curl nor wget is found, and despite being working CMake code, I would prefer not having to use interleaved if / foreach. What I would suggest doing is:

find_program( CURL_PROGRAM curl )
if( NOT CURL_PROGRAM )
  find_program( WGET_PROGRAM wget )
  if( NOT WGET_PROGRAM )
    # warn and exit
  endif()
endif()

foreach( NAME ${_p_NAMES} )
  if( CURL_PROGRAM )
    # download with curl
  else()
    # download with wget
  endif()
endforeach()
mer-a-o commented 4 years ago

@benjaminmenetrier, can you please check the new changes in ecbuild_check_urls.cmake ?