Open
Conversation
LCOV of commit
|
werwurm
force-pushed
the
werwurm/make_conditional_encodings
branch
from
db7c88a to
c128991
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces build-time feature flags to selectively compile X.509 and/or COSE/CWT certificate encoding support, reducing unnecessary linked code when projects only use one encoding. It also expands CI to validate the supported encoding configurations.
Changes:
- Add
NAT20_WITH_X509/NAT20_WITH_COSECMake options and compile-time defines (N20_WITH_X509,N20_WITH_COSE). - Conditionally compile X.509/COSE-dependent code paths in core functionality and tests.
- Extend the GitHub Actions CMake workflow matrix to test both encodings, x509-only, and cose-only configurations.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/service/test/gnostic.cpp |
Wrap X.509-specific service tests with #ifdef N20_WITH_X509. |
src/core/test/functionality.cpp |
Guard X.509 and COSE test sections behind N20_WITH_X509 / N20_WITH_COSE. |
src/core/functionality.c |
Add conditional compilation blocks for X.509 and COSE implementation/dispatch. |
CMakeLists.txt |
Introduce encoding options and define N20_WITH_X509 / N20_WITH_COSE. |
.github/workflows/cmake-build-and-test.yml |
Add CI matrix jobs for both-encodings, x509-only, and cose-only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Libnat20 provides both x509 and COSE/CWT support in the main functionality function n20_issue_certificate. This means that if a project uses n20_issue_certificate all of the certificate rendering code for x509 and COSE/CWT will be linked in and referenced even if the project never actually uses one or the other. This leads to a lot of dead code which cannot be optimized out by the linker. This patch introduces preprocessor macros to allows for selectively enabling and disabling either encoding for the purpose of building libnat20 as concise as required by the intended use case. Add CI configurations for encoding-only builds. Extend cmake-build-and-test workflow to test all supported certificate encoding configurations: both encodings, X509 only, and COSE only. Changes: - Add x509-only configuration (NAT20_WITH_X509=ON, NAT20_WITH_COSE=OFF) - Add cose-only configuration (NAT20_WITH_X509=OFF, NAT20_WITH_COSE=ON) - Add descriptive job names showing configuration being tested - Matrix now runs 4 jobs: no-tests, both-encodings, x509-only, cose-only This ensures conditional compilation works correctly and prevents regressions when modifying encoding-specific code.
werwurm
force-pushed
the
werwurm/make_conditional_encodings
branch
from
fd7c8d7 to
e570a00
Compare
…rent build systems don't have to add macro definitions to keep the behavior consistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Libnat20 provides both x509 and COSE/CWT support in the main
functionality function n20_issue_certificate. This means that if a
project uses n20_issue_certificate all of the certificate rendering
code for x509 and COSE/CWT will be linked in and referenced even
if the project never actually uses one or the other. This leads
to a lot of dead code which cannot be optimized out by the linker.
This patch introduces preprocessor macros to allows for selectively
enabling and disabling either encoding for the purpose of building
libnat20 as concise as required by the intended use case.
Add CI configurations for encoding-only builds.
Extend cmake-build-and-test workflow to test all supported certificate
encoding configurations: both encodings, X509 only, and COSE only.
Changes:
This ensures conditional compilation works correctly and prevents
regressions when modifying encoding-specific code.