Skip to content

Conversation

gilles-peskine-arm
Copy link
Contributor

Fix #10335 and add a bit of testing.

Priority: high despite being unplanned because this is a bug that will affect every future release.

PR checklist

  • changelog provided
  • development PR TODO? Not if we abandon make, but in the meantime we might as well forward-port at least the fix, if not the test.
  • TF-PSA-Crypto PR not required because: make is not supported in TF-PSA-Crypto
  • framework PR provided Mbed-TLS/mbedtls-framework# | not required
  • 3.6 PR here
  • tests provided

Test that `make lib GEN_FILES=` works in a minimal environment (just `${CC}`
and `${AR}`). We promise that in `README.md`.

Non-regression test for Mbed-TLS#10335.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix `make lib GEN_FILES=` attempting to rebuild
`psa_crypto_driver_wrappers*` if the files' timestamps are older than their
dependencies. Turning off `GEN_FILES` is supposed to avoid that.

Fixes Mbed-TLS#10335.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Don't test `make clean` at the toplevel, that would be too much work (we'd
need to support `$(RM)` in all makefiles, and arrange for `find` as well for
`clean_more_on_top`).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Test that `make lib GEN_FILES=` does build the generated files if they're
missing. Also, test that this only requires the expected commands: `$(CC)`,
`$(AR)`, `$(PERL)` and `$(PYTHON)`. For Python (and Perl), we don't test for
reliance on unexpected third-party packages: that would be desirable, but
out of scope of this commit.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm added bug priority-high High priority - will be reviewed soon labels Jul 31, 2025
@gilles-peskine-arm gilles-peskine-arm added size-xs Estimated task size: extra small (a few hours at most) needs-ci Needs to pass CI tests labels Jul 31, 2025
@mpg mpg removed the needs-ci Needs to pass CI tests label Aug 29, 2025
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

Looking pretty good to me, just have one question.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm requested a review from mpg August 29, 2025 11:23
@gilles-peskine-arm gilles-peskine-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 Aug 29, 2025
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 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 priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Projects
Development

Successfully merging this pull request may close these issues.

2 participants