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

Fetch ecbuild if not downloaded before #241

Merged
merged 3 commits into from
Sep 7, 2024
Merged

Conversation

foxtran
Copy link
Contributor

@foxtran foxtran commented Sep 1, 2024

This patch makes possible to clone and build this repo without additional download steps

@FussyDuck
Copy link

FussyDuck commented Sep 1, 2024

CLA assistant check
All committers have signed the CLA.

@shahramn shahramn self-assigned this Sep 1, 2024
@shahramn shahramn added the approved-for-ci Approved to run CI on ECMWF machines label Sep 1, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.83%. Comparing base (ff5ab61) to head (f25cedf).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #241   +/-   ##
========================================
  Coverage    87.83%   87.83%           
========================================
  Files          776      776           
  Lines        62454    62454           
  Branches     11025    11025           
========================================
  Hits         54858    54858           
  Misses        7596     7596           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shahramn
Copy link
Collaborator

shahramn commented Sep 2, 2024

Thank you. But I spotted a problem
Building ecCodes works fine but when you run the tests, many fail. Specifically those of ecbuild itself

@shahramn
Copy link
Collaborator

shahramn commented Sep 2, 2024

One solution would be to suppress or disable the tests of ecbuild in this case. Normally one can do this by:
cmake /path/to/git/ecbuild -D ENABLE_TESTS=0

@foxtran
Copy link
Contributor Author

foxtran commented Sep 2, 2024

I did not try to run tests. Probably, I should try to figure out how to fix them

@github-actions github-actions bot removed the approved-for-ci Approved to run CI on ECMWF machines label Sep 5, 2024
@foxtran
Copy link
Contributor Author

foxtran commented Sep 5, 2024

@shahramn, thank you! I've fixed issue with tests.

100% tests passed, 0 tests failed out of 98

Label Time Summary:
eccodes       = 102.18 sec*proc (98 tests)
executable    =   0.11 sec*proc (1 test)
sanity        = 102.07 sec*proc (97 tests)
script        = 102.07 sec*proc (97 tests)

@shahramn
Copy link
Collaborator

shahramn commented Sep 6, 2024

Many thanks. Now I cannot reproduce the errors I got before! Very odd
Now it all works and all tests pass (tested it on Linux and MacOS).
My apologies; I don't know what I did wrong before!!

@shahramn
Copy link
Collaborator

shahramn commented Sep 6, 2024

A couple of points:
When I run this on my MacBook with cmake 3.30.3, I get this warning:

-- Fetching ecbuild...
CMake Warning (dev) at /opt/homebrew/Cellar/cmake/3.30.3/share/cmake/Modules/FetchContent.cmake:1953 (message):
  Calling FetchContent_Populate(ecbuild) is deprecated, call
  FetchContent_MakeAvailable(ecbuild) instead.  Policy CMP0169 can be set to
  OLD to allow FetchContent_Populate(ecbuild) to be called directly for now,
  but the ability to call it with declared details will be removed completely
  in a future version.
Call Stack (most recent call first):
  CMakeLists.txt:31 (FetchContent_Populate)
This warning is for project developers.  Use -Wno-dev to suppress it.

@shahramn shahramn added the approved-for-ci Approved to run CI on ECMWF machines label Sep 6, 2024
@github-actions github-actions bot removed the approved-for-ci Approved to run CI on ECMWF machines label Sep 7, 2024
@foxtran
Copy link
Contributor Author

foxtran commented Sep 7, 2024

@shahramn, the warning should disappear now :)

@shahramn shahramn added the approved-for-ci Approved to run CI on ECMWF machines label Sep 7, 2024
@shahramn
Copy link
Collaborator

shahramn commented Sep 7, 2024

It looks a lot better now. The messages are much more informative:

-- Could NOT find ecbuild (missing: ecbuild_DIR)
-- Fetching ecbuild...
-- Populating ecbuild
-- Configuring done (0.0s)
-- Generating done (0.0s)
-- Build files have been written to: /tmp/bld/ecbuild-subbuild
[ 11%] Creating directories for 'ecbuild-populate'
[ 22%] Performing download step (git clone) for 'ecbuild-populate'
Cloning into 'ecbuild-src'...
HEAD is now at a07c0a5 Merge branch 'hotfix/3.8.5'
[ 33%] Performing update step for 'ecbuild-populate'
-- Fetching latest from the remote origin
HEAD is now at a07c0a5 Merge branch 'hotfix/3.8.5'
[ 44%] No patch step for 'ecbuild-populate'
[ 55%] No configure step for 'ecbuild-populate'
[ 66%] No build step for 'ecbuild-populate'
[ 77%] No install step for 'ecbuild-populate'
[ 88%] No test step for 'ecbuild-populate'
[100%] Completed 'ecbuild-populate'
[100%] Built target ecbuild-populate

@shahramn shahramn merged commit 40747f8 into ecmwf:develop Sep 7, 2024
208 checks passed
@shahramn
Copy link
Collaborator

shahramn commented Sep 7, 2024

Thank you so much

@foxtran foxtran deleted the patch-1 branch September 7, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved-for-ci Approved to run CI on ECMWF machines contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants