Skip to content
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.

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

Switch generate_psa_test.py to automatic dependencies for positive test cases #9841

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Dec 11, 2024

Mostly happening in the framework, see Mbed-TLS/mbedtls-framework#83.

I simplified the history to have a single submodule update. As a result,
f3014ad breaks make generated_files, which is fixed by the subsequent submodule update.

We expect one set of complaints from the interface stability check: many storage test cases have disappeared, namely the never-executed test cases that are no longer getting generated.

PR checklist

@gilles-peskine-arm gilles-peskine-arm added component-crypto Crypto primitives and low-level interfaces component-platform Portability layer and build scripts needs-ci Needs to pass CI tests priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels Dec 11, 2024
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-positive-dev branch from a0ea101 to 981be3f Compare December 11, 2024 13:00
@gilles-peskine-arm
Copy link
Contributor Author

gilles-peskine-arm commented Dec 11, 2024

There's a conflict on the framework. This PR changes the framework twice because of the interactions between the consuming branch and the framework:

  1. Framework: create a Python module.
  2. Consumer: add this Python module as a dependency in build scripts that are not (yet?) in the framework.
  3. Framework: use this Python module.
  4. Consumer: framework update to get the latest stuff.

I propose to resolve the conflict by doing a merge commit at the end, which hopefully will only need to reconcile the framework to its latest version resulting from the merge of the framework PR.

@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-ci Needs to pass CI tests labels Dec 17, 2024
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-positive-dev branch from 981be3f to b4aa9f6 Compare December 17, 2024 18:04
@gilles-peskine-arm gilles-peskine-arm added needs-ci Needs to pass CI tests 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 and removed needs-work labels Dec 17, 2024
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-positive-dev branch from b4aa9f6 to 80e8618 Compare December 18, 2024 10:51
@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, needs-ci Needs to pass CI tests needs-reviewer This PR needs someone to pick it up for review labels Dec 24, 2024
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-positive-dev branch from 80e8618 to a6c8107 Compare December 24, 2024 19:09
@gilles-peskine-arm gilles-peskine-arm added needs-ci Needs to pass CI tests and removed needs-work labels Dec 24, 2024
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-positive-dev branch from a6c8107 to 198e7db Compare December 25, 2024 19:52
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-positive-dev branch 2 times, most recently from 1ed8ac3 to 64bd0c8 Compare January 2, 2025 12:56
@gilles-peskine-arm gilles-peskine-arm removed the needs-ci Needs to pass CI tests label Jan 2, 2025
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Remove all code guarded by `PSA_WANT_ECC_SECP_K1_224`, which is not and will
not be implemented. (It would be K1_225 anyway, but we don't intend to
implement it anyway.)

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-positive-dev branch from 14d18da to dc941ab Compare January 8, 2025 15:51
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

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

Rebase LGTM

@waleed-elmelegy-arm waleed-elmelegy-arm 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 Jan 8, 2025
@gilles-peskine-arm gilles-peskine-arm added needs-preceding-pr Requires another PR to be merged first and removed approved Design and code approved - may be waiting for CI or backports labels Jan 8, 2025
@gilles-peskine-arm
Copy link
Contributor Author

This will need one final submodule update once the crypto PR Mbed-TLS/TF-PSA-Crypto#122 is merged.

Update TF-PSA-Crypto to have the latest framework with
Mbed-TLS/mbedtls-framework#83 .

Update the framework to match.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-positive-dev branch from dc941ab to 49e48ef Compare January 9, 2025 09:42
@gilles-peskine-arm
Copy link
Contributor Author

I've updated the submodule-updating commit to have the crypto update that replaces the head of the crypto PR by the merge result.

@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, and removed needs-ci Needs to pass CI tests needs-preceding-pr Requires another PR to be merged first labels Jan 9, 2025
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

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

Rebase LGTM

@davidhorstmann-arm davidhorstmann-arm added the needs-ci Needs to pass CI tests label Jan 9, 2025
@waleed-elmelegy-arm waleed-elmelegy-arm 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 Jan 9, 2025
@gilles-peskine-arm gilles-peskine-arm added this pull request to the merge queue Jan 9, 2025
Merged via the queue into Mbed-TLS:development with commit eef2a2e Jan 9, 2025
4 of 6 checks passed
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-crypto Crypto primitives and low-level interfaces component-platform Portability layer and build scripts needs-ci Needs to pass CI tests priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Development

Successfully merging this pull request may close these issues.

3 participants