Skip to content

Port: Use a single configuration file for internal and external headers #782

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.

Sign up for GitHub

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

Jump to bottom
Merged
hanno-becker merged 7 commits into from
Dec 10, 2025
Merged

Port: Use a single configuration file for internal and external headers #782

hanno-becker merged 7 commits into from
Dec 10, 2025

Conversation

@mkannwischer
Copy link
Contributor

@mkannwischer mkannwischer commented Dec 10, 2025

@mkannwischer
Move mldsa/src/config -> mldsa/mldsa_native_config.h
7a57acf 
Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
@mkannwischer
Adjust mentions of config.h->mldsa_native_config.h in comments
6fe7c1e 
Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
@mkannwischer
config-variations CI: avoid config include via relative path
278760c 
Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
@mkannwischer mkannwischer force-pushed the config_cleanup_api_compatible branch 8 times, most recently from dc3b495 to 9e73d34 Compare December 10, 2025 05:52
@mkannwischer mkannwischer marked this pull request as ready for review December 10, 2025 06:09
@mkannwischer mkannwischer requested a review from a team as a code owner December 10, 2025 06:09
hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
hanno-becker
hanno-becker requested changes Dec 10, 2025
View reviewed changes
Copy link
Contributor

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, thanks a lot @mkannwischer for working through this. There are still a few typos left, plus some minor issues which however we can defer if we have issues for them.

@hanno-becker hanno-becker mentioned this pull request Dec 10, 2025
Merged
@mkannwischer
Derive mldsa_native.h configuration from config file
9a28f6a 
Previously, mldsa_native.h required users to set MLD_CONFIG_API_* macros
before including it, separate from the MLD_CONFIG_* macros used for the
build configuration. This distinction between internal and external
configuration it confusing.

Now, mldsa_native.h derives all API settings from the same
build configuration file used internally.

- The legacy configuration via MLD_CONFIG_API_XXX still works for
  backwards compatibility, but will be removed in v2.

- New Config Options:
  * MLD_CONFIG_MULTILEVEL_BUILD: Explicit flag for multi-level builds

    This makes it simpler to get headers for multi-level builds.

    In the common case, the configs used in a multi-level build are
    identical except for the setting of MLD_CONFIG_PARAMETER_SET and
    MLD_CONFIG_MULTILEVEL_{NO,WITH}_SHARED. However, the latter only
    affect the build, not the API.

    With MLD_CONFIG_MULTILEVEL_BUILD exposed as a separate option,
    mldsa_native.h can use it to determine whether to suffix the
    namespace prefix with the parameter set (44/65/87) or not,
    so no adaptation of MLD_CONFIG_MULTILEVEL_{NO,WITH}_SHARED is
    needed. Instead, a multi-level header simply becomes:

    ```c
    #define MLD_CONFIG_PARAMETER_SET 44
    #include "mldsa_native.h"
    #undef MLD_CONFIG_PARAMETER_SET

    #define MLD_CONFIG_PARAMETER_SET 65
    #include "mldsa_native.h"
    #undef MLD_CONFIG_PARAMETER_SET

    #define MLD_CONFIG_PARAMETER_SET 87
    #include "mldsa_native.h"
    #undef MLD_CONFIG_PARAMETER_SET
    ```

    For backwards compatibility, MLD_CONFIG_MULTILEVEL_BUILD is not
    used in the build, which continues to detect multilevel builds
    via the existing options MLD_CONFIG_MULTILEVEL_{WITH,NO}_SHARED.

  * MLD_CONFIG_NO_SUPERCOP: We had this before in mldsa_native.h as
    MLD_CONFIG_API_NO_SUPERCOP; it's now in the config and documented.

  * MLD_CONFIG_CONSTANTS_ONLY: We had this before in mldsa_native.h as
    MLD_CONFIG_API_CONSTANTS_ONLY; it's now in the config and documented.

- Build-Internal vs API Config Separation: Config file now guards
  build-only options with #if defined(MLD_BUILD_INTERNAL).

Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
@mkannwischer
Restructure examples
45c6838 
- Example Directory Restructuring: All examples refactored with mldsa/
  subdirectories renamed to mldsa_native/ and CFLAGS configuration
  replaced by custom configs.

Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
@mkannwischer
params.h: Rename CRYPTO_XXX to MLDSA_CRYPTO_XXX
9e1ee6a 
To avoid clashing with the symbols defined in mldsa_native.h, this commits
aligns with mlkem-native, by using different symbol names (MLDSA_CRYPTO_XXX)

Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
@mkannwischer mkannwischer force-pushed the config_cleanup_api_compatible branch from 9e73d34 to 426a813 Compare December 10, 2025 09:59
@mkannwischer
Copy link
Contributor Author

mkannwischer commented Dec 10, 2025

Mostly looks good, thanks a lot @mkannwischer for working through this. There are still a few typos left, plus some minor issues which however we can defer if we have issues for them.

Thanks for catching those numerous typos. Should all be fixed now.

hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
hanno-becker
hanno-becker reviewed Dec 10, 2025
View reviewed changes
@mkannwischer mkannwischer force-pushed the config_cleanup_api_compatible branch 2 times, most recently from 75ca4d5 to 816961b Compare December 10, 2025 10:19
@mkannwischer
Expand and unify example READMEs
ae19b92 
Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
@mkannwischer mkannwischer force-pushed the config_cleanup_api_compatible branch from 816961b to ae19b92 Compare December 10, 2025 10:20
hanno-becker
hanno-becker approved these changes Dec 10, 2025
View reviewed changes
Copy link
Contributor

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's hope CI agrees. Thank you @mkannwischer, not a fun change.

@hanno-becker hanno-becker merged commit 2227d0b into main Dec 10, 2025
328 checks passed
@hanno-becker hanno-becker deleted the config_cleanup_api_compatible branch December 10, 2025 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hanno-becker hanno-becker hanno-becker approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Port: Use a single configuration file for internal and external headers

3 participants

@mkannwischer @hanno-becker