CMake Cleanup
- Dominant language
- C
- Stars
- 4.8k
- Forks
- 802
- Avg merge
- 5d 16h
- Merged PRs (30d)
- 27
Description
### Problem:
- [ ] refactor to multiple files
- [ ] common cflags
#### refactor to multiple files
Our CMakeLists.txt is nearly 750 lines long. Given that CMake syntax is quite difficult to read and parse (at least for me) we should consider splitting this into multiple CMakeLists.txt files.
My suggestion is that we add four new files
- `s2n-tls/tests/CMakeLists.txt`: responsible for defining common test configuration. E.g. ASAN is used by both unit and fuzz tests.
- `s2n-tls/tests/testlib/CMakeLists.txt`: responsible for defining the "testss2n" target
- `s2n-tls/tests/unit/CMakeLists.txt`: responsible for defining all of the unit tests
- `s2n-tls/tests/fuzz/CMakeLists.txt`: responsible for defining the fuzz tests
#### common cflags
We have a large collection of compile options that we use for libs2n.
https://github.com/aws/s2n-tls/blob/24674164b0353662fd02f792f0c53a786b7720bd/CMakeLists.txt#L138-L141
These are general hygiene options that should be applied everywhere, but we don't have a good mechanism for doing that.
My suggestion is that we collect all of the compilation variables into `S2N_COMMON_COMPILE_OPTIONS` or something like that. Then we'd change the S2N_WERROR_ALL section
https://github.com/aws/s2n-tls/blob/24674164b0353662fd02f792f0c53a786b7720bd/CMakeLists.txt#L143-L147
```CMake
if (S2N_WERROR_ALL)
# because the options are public, all attached targets will inherit all of the compile options.
# this includes unit and fuzz tests and the testlib
target_compile_options(${PROJECT_NAME} PUBLIC -Werror ${S2N_COMMON_COMPILE_OPTIONS})
elseif (UNSAFE_TREAT_WARNINGS_AS_ERRORS)
target_compile_options(${PROJECT_NAME} PRIVATE -Werror ${S2N_COMMON_COMPILE_OPTIONS})
endif ()
```
[//]: # (NOTE: If you believe this might be a security issue, please email aws-security@amazon.com instead of creating a GitHub issue. For more details, see the AWS Vulnerability Reporting Guide: https://aws.amazon.com/security/vulnerability-reporting/ )
Contributor guide
Research direction
Start with the root CMakeLists.txt, especially the test configuration and compile-option sections linked in the issue. Trace how tests, testlib, and fuzz targets are defined before separating the suggested files and consolidating common compile options. Done means the build and test targets retain their behavior while shared options apply as intended.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, cmake
- Domain
- build-system
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100