Skip to content

Conversation

@gabor-mezei-arm
Copy link
Contributor

@gabor-mezei-arm gabor-mezei-arm commented Oct 1, 2024

Description

In depends.py use PSA macros for the pkalgs domain.

Resolve #9145
Depends on #9292 (merged)

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.

  • changelog not required because: test related changes
  • development PR provided this is
  • framework PR not required
  • 3.6 PR not required because: 4.0 related changes
  • 2.28 PR not required because: 4.0 related changes
  • tests provided

@gabor-mezei-arm gabor-mezei-arm added enhancement needs-ci Needs to pass CI tests size-s Estimated task size: small (~2d) component-test Test framework and CI scripts priority-high High priority - will be reviewed soon labels Oct 1, 2024
@gabor-mezei-arm gabor-mezei-arm self-assigned this Oct 1, 2024
@minosgalanakis minosgalanakis self-requested a review January 13, 2025 15:17
@gabor-mezei-arm gabor-mezei-arm force-pushed the 9145_update_depends.py_pkalgs_domain branch from 7c4a029 to 174d116 Compare January 14, 2025 10:16
@gabor-mezei-arm gabor-mezei-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Jan 14, 2025
@gabor-mezei-arm gabor-mezei-arm force-pushed the 9145_update_depends.py_pkalgs_domain branch from 174d116 to 7acd70b Compare January 14, 2025 10:28
@ronald-cron-arm ronald-cron-arm self-requested a review January 15, 2025 08:17
@ronald-cron-arm ronald-cron-arm removed needs-reviewer This PR needs someone to pick it up for review needs-ci Needs to pass CI tests labels Jan 15, 2025
minosgalanakis
minosgalanakis previously approved these changes Jan 15, 2025
Copy link
Contributor

@minosgalanakis minosgalanakis left a comment

Choose a reason for hiding this comment

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

LGTM

@mpg
Copy link
Contributor

mpg commented Apr 25, 2025

@gabor-mezei-arm This has a conflict.

@ronald-cron-arm Kind reminder that this is waiting for your review :)

@mpg
Copy link
Contributor

mpg commented May 7, 2025

@ronald-cron-arm Are you still planning to review this? We're trying to close off our DI task and we have one that depends on this.

@ronald-cron-arm
Copy link
Contributor

ronald-cron-arm commented May 9, 2025

@mpg I can see that you already pinged me about the review of this PR but I somehow missed it, sorry about that. I looked at this PR sometimes ago, I can finalize my review of it.

@ronald-cron-arm
Copy link
Contributor

@gabor-mezei-arm could you rebase to resolve the conflict? Thanks.

@gabor-mezei-arm gabor-mezei-arm force-pushed the 9145_update_depends.py_pkalgs_domain branch from 9c3e433 to f13fd1e Compare May 9, 2025 12:52
@mpg
Copy link
Contributor

mpg commented May 13, 2025

@mpg I can see that you already pinged me about the review of this PR but I somehow missed it, sorry about that.

No worries. Quite ironically, I missed the notification about your reply, and only saw it today while checking the status of the PR :)

I looked at this PR sometimes ago, I can finalize my review of it.

Great, thanks!

@mpg mpg requested a review from minosgalanakis May 13, 2025 07:58
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

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

This PR changes the definitions of the pkalgs complementaty domains to be based on PSA_WANT_ symbols instead of MBEDTLS_ ones, in preparation of the removal of the MBEDTLS_ symbols as configuration options. The MbedTLS configurations that are tested as part of the pkalgs complementary domain remain the same. Thus this can be merged as it is to me.

After that step, we would probably want to adapt the definition of the pkalgs domain to the specifics of the PSA_WANT_ symbols compared to the MBEDTLS_ ones. For example, having a test where only PSA_WANT_ALG_RSA_OAEP is disabled and another one where PSA_WANT_ALG_RSA_PSS is disabled, whereas currently we only have one test that disables both. We would also probably want to keep the test where both are disabled and to have the three cases we need to add support for grouping as discussed in #9634.

@mpg
Copy link
Contributor

mpg commented May 13, 2025

I'm not super familiar with the concept of grouping as discussed in 9634. @gabor-mezei-arm can you create (an) issue(s) to track the follow-up(s) that Ronald is suggesting? Then add it/them to the 4.0 board, probably as SHOULDs in the "PSA Crypto always on" topic? Thanks in advance.

Copy link
Contributor

@minosgalanakis minosgalanakis left a comment

Choose a reason for hiding this comment

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

LGTM

@github-project-automation github-project-automation bot moved this from In Development to Has Approval in Roadmap pull requests (new board) May 14, 2025
@mpg mpg added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels May 14, 2025
@mpg mpg added this pull request to the merge queue May 14, 2025
Merged via the queue into Mbed-TLS:development with commit 7769c16 May 14, 2025
6 checks passed
@github-project-automation github-project-automation bot moved this from Has Approval to Done in Roadmap pull requests (new board) May 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Design and code approved - may be waiting for CI or backports component-test Test framework and CI scripts enhancement priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)

Development

Successfully merging this pull request may close these issues.

Update the pkalgs domain to use PSA macros in depends.py

4 participants