awslabs / aws-c-common

Core c99 package for AWS SDK for C. Includes cross-platform primitives, configuration, data structures, and error handling.
Apache License 2.0
263 stars 159 forks source link

[fix] prebuild set CMAKE_PREFIX_PATH properly #1154

Closed TingDaoK closed 1 month ago

TingDaoK commented 1 month ago

Issue #, if available:

cmakeCommand /usr/bin/cmake;-S;/root/aws-crt-cpp/crt/s2n; -DCMAKE_BUILD_TYPE=RelWithDebInfo; -DCMAKE_PREFIX_PATH=/root/aws-crt-cpp/build/aws-crt-cpp/deps/AWSLC/install/;/root/aws-crt-cpp/build/install; -DCMAKE_INSTALL_PREFIX=/root/aws-crt-cpp/build/aws-crt-cpp/deps/S2N/install; -DCMAKE_INSTALL_RPATH=; -DBUILD_SHARED_LIBS=; -DUNSAFE_TREAT_WARNINGS_AS_ERRORS=OFF; -DBUILD_TESTING=OFF



### After more digging
- It's not random, it's from some other `CMAKE_PREFIX_PATH`
- We add the new install path to the `CMAKE_PREFIX_PATH`, so the `CMAKE_PREFIX_PATH` is now a list
- We add `CMAKE_PREFIX_PATH` variable into the list of args to call for Cmake `execute_process`, it seems like based on the version of CMAKE, cmake sometimes confuses about those values. And treat the second value in the `CMAKE_PREFIX_PATH` list as a separate args for cmake to take.
- You can see **-DCMAKE_PREFIX_PATH=/root/aws-crt-cpp/build/aws-crt-cpp/deps/AWSLC/install/;/root/aws-crt-cpp/build/install;** I assume some version of cmake just took it as `-DCMAKE_PREFIX_PATH=/root/aws-crt-cpp/build/aws-crt-cpp/deps/AWSLC/install/` and `/root/aws-crt-cpp/build/install`
- I tried a lot different way to set the `CMAKE_PREFIX_PATH` properly, and MOST failed.
- I have to set the environment variable of CMAKE_PREFIX_PATH instead of passing it to as the commend line args for Cmake, which seems to work.

*Description of changes:*
- Set the CMAKE_PREFIX_PATH as environment variables incase of multiple cmake prefix path needs to be set,
- With the fix, https://github.com/awslabs/aws-crt-cpp/actions/runs/11283228222/job/31382255479?pr=665 works
- Updated to use the escape for the string to pass in

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
codecov-commenter commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.84%. Comparing base (6978946) to head (7310e92).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1154 +/- ## ======================================= Coverage 83.84% 83.84% ======================================= Files 57 57 Lines 5992 5993 +1 ======================================= + Hits 5024 5025 +1 Misses 968 968 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.