aws / aws/s2n-tls

CMake Cleanup

Open
#4,792 0 comments 0 reactions 0 assignees View on GitHub
priority/low s2n-core size/small
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.