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

Fix omissions in providers/common/der/build.info #11906

Closed

Conversation

levitte
Copy link
Member

@levitte levitte commented May 22, 2020

Dependencies on generated files must be declared explicitly. When
refactoring the DER code in providers/common/der, a few of those
dependency declaration were omitted, which may lead to build errors in
a parallel build.

Some cleanup and extensive used of build.info variables is done while
at it, to avoid unnecessary repetition.

Dependencies on generated files must be declared explicitly.  When
refactoring the DER code in providers/common/der, a few of those
dependency declaration were omitted, which may lead to build errors in
a parallel build.

Some cleanup and extensive used of build.info variables is done while
at it, to avoid unnecessary repetition.
@levitte levitte added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels May 22, 2020
@levitte levitte added this to the 3.0.0 milestone May 22, 2020
@levitte
Copy link
Member Author

levitte commented May 22, 2020

The issue was mentioned here: #11868 (comment)

@kroeckx kroeckx added the severity: urgent Fixes an urgent issue (exempt from 24h grace period) label May 22, 2020
@kroeckx
Copy link
Member

kroeckx commented May 22, 2020

I think OSS-Fuzz is actually having problems with this issue, so I've marked it as urgent.

@levitte
Copy link
Member Author

levitte commented May 22, 2020

Okie. Now, all that's missing is an approval

@levitte
Copy link
Member Author

levitte commented May 25, 2020

Ping!

@t8m t8m added this to Reviewer approved in 3.0 New Core + FIPS via automation May 25, 2020
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

Is the $DER_RSA_FIPSABLE separated from $COMMON because it will never go into libcommon.a? It is not clear to me. However approving anyway.

@t8m t8m added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: review pending This pull request needs review by a committer labels May 25, 2020
@levitte
Copy link
Member Author

levitte commented May 25, 2020

Is the $DER_RSA_FIPSABLE separated from $COMMON because it will never go into libcommon.a? It is not clear to me. However approving anyway.

Yes. If you grep for # *ifn?def *FIPS_MODULE, that's the only file in this directory that matches.
I currently can't place the others in libcommon, 'cause the link between that and libcrypto is not as clear as it should be, and that requires some intricate changes in the core of the build system. It's on my mind.

@levitte
Copy link
Member Author

levitte commented May 25, 2020

Merged

3f5ea7d Fix omissions in providers/common/der/build.info

@levitte levitte closed this May 25, 2020
openssl-machine pushed a commit that referenced this pull request May 25, 2020
Dependencies on generated files must be declared explicitly.  When
refactoring the DER code in providers/common/der, a few of those
dependency declaration were omitted, which may lead to build errors in
a parallel build.

Some cleanup and extensive used of build.info variables is done while
at it, to avoid unnecessary repetition.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #11906)
3.0 New Core + FIPS automation moved this from Reviewer approved to Done May 25, 2020
@levitte levitte deleted the fix-der-constants-refactoring-20200522 branch June 6, 2020 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch severity: urgent Fixes an urgent issue (exempt from 24h grace period)
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants