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

fixes for MacOS and new CI to prevent future breaks #164

Merged
merged 31 commits into from
Dec 18, 2023

Conversation

jeanbez
Copy link
Member

@jeanbez jeanbez commented Oct 30, 2023

Fixes for MacOS by correctly handle finding UUID_LIBRARY (the current code was mixing the library provided by the system, but no the headers, and getting partially overwritten by a local user installation). Some tests were also breaking due to wrong memory allocation. For MacOS we also need to run mercury with sockets instead of tcp.

We also include a new CI check where we compile PDC with MacOS, but we do not run tests due to some issues in Github provided runners (actions/runner-images#3885)

@jeanbez jeanbez added type: bug Something isn't working type: CI Continuous Integration in GitHub or NERSC labels Oct 30, 2023
@jeanbez jeanbez self-assigned this Oct 30, 2023
@jeanbez jeanbez temporarily deployed to external October 30, 2023 17:22 — with GitHub Actions Inactive
.github/workflows/dependencies-macos.sh Outdated Show resolved Hide resolved
.github/workflows/dependencies-macos.sh Outdated Show resolved Hide resolved
src/commons/utils/include/pdc_malloc.h Outdated Show resolved Hide resolved
@jeanbez jeanbez temporarily deployed to external October 31, 2023 19:32 — with GitHub Actions Inactive
@jeanbez jeanbez temporarily deployed to external October 31, 2023 19:36 — with GitHub Actions Inactive
@zhangwei217245
Copy link
Collaborator

zhangwei217245 commented Nov 1, 2023

Tested on my Mac with the guide provided by Jean Luca, no issue with compilation. the first 66 ctest ran smoothly, and like the situation in a linux PC, the 67th test takes a long time.

some flags used for installing libfabric and mercury on MacOS are different, I suggest we should open a dedicated subsection in the installation guide document to provide clear commands for Mac users.

Below are the flags provided by Jean Luca and they are working on my Mac:

libfabric: ./configure --prefix=<install_path> CFLAG=-O2 --enable-sockets=yes --enable-tcp=yes --enable-udp=yes --enable-rxm=yes
Mercury: -DBUILD_SHARED_LIBS=ON -DBUILD_TESTING=ON -DNA_USE_OFI=ON -DNA_USE_SM=OFF -DNA_OFI_TESTING_PROTOCOL=sockets
PDC: -DBUILD_MPI_TESTING=ON -DBUILD_SHARED_LIBS=ON -DBUILD_TESTING=ON -DCMAKE_INSTALL_PREFIX=$PDC_DIR -DPDC_ENABLE_MPI=ON -DMERCURY_DIR=$MERCURY_DIR -DCMAKE_C_COMPILER=mpicc -DMPI_RUN_CMD=mpiexec ../
and also export HG_TRANSPORT="sockets" (before ctest)

Also, documentation should be added about authorizing all mercury and PDC ctests in MacOS to avoid dialog box being popped continuously.

@zhangwei217245
Copy link
Collaborator

Test #67 and later just timed out.

image

Would it be possible if we can setup a shorter timeout limit for these ctests?

@jeanbez
Copy link
Member Author

jeanbez commented Nov 7, 2023

Would it be possible if we can setup a shorter timeout limit for these ctests?

There is a global way to set the timeout directly in CMake, we can either set the timeout for each test or you can change the way you call ctest to ctest --timeout 60 for a 60 seconds timeout. I believe the second approach would be better than manually adding a timeout for each of the >100 tests we have.

@jeanbez
Copy link
Member Author

jeanbez commented Dec 13, 2023

Updated documentation and the CI can now run in macOS runners after GitHub's latest updates on their support.

@jeanbez jeanbez merged commit b02a970 into hpc-io:develop Dec 18, 2023
6 checks passed
jeanbez added a commit that referenced this pull request Jan 9, 2024
* include MacOS in CI
* include dependencies
* replace TCP with sockets
* add libuuid
* fix find UUID to correctly locate files in Ubuntu and MacOS
* fix random segfault in strdup + malloc due to wrong allocation
* ensure consistent use of defined variables
* change transport for MacOS tests
* update env
* update dependencies-macos.sh
* replace found to TRUE/FALSE
* update documentation with timeout and MacOS specifics
* fix git link to avoid authentication
* change transport for tests
* configure network for MacOS tests

---------

Co-authored-by: github-actions <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working type: CI Continuous Integration in GitHub or NERSC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants