-
Notifications
You must be signed in to change notification settings - Fork 2k
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
makefiles: select default prng _after_ recursive dependency resolution #21060
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
If you would reorder the commits (first the fix than the test) every commit would paas the CI, which is nice for bisecting. But that is purely optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
If you would reorder the commits (first the fix than the test) every commit would paas the CI, which is nice for bisecting. But that is purely optional.
71fbb66
to
d5783d7
Compare
Reordered the commits as proposed :) |
d5783d7
to
9f2755a
Compare
Thank you! |
Contribution description
The PRNG is supposed to be set only if there is none selected at the end of the recursive dependency resolution. However, this is currently not the case: a transitive dependency setting the PRNG (encountered with
psa_crypto
which selectsprng_sha256prng
) will lead to a user-facing dependency clash.The first commit extends
tests/build_system/external_module
to showcase the issue and the second one fixes it by moving the automatic prng selection todefaultmodules_deps.inc.mk
.Disclaimer: I'm not sure if this is the right approach to follow, but it seems to work. Any suggestions from people with more insights on the build system are welcome.
Testing procedure
with only the first commit:
with both commits: