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

Rework dependencies between config files and build files #17756

Closed
wants to merge 1 commit into from

Conversation

levitte
Copy link
Member

@levitte levitte commented Feb 23, 2022

Before PR #15310, which reworked how build files (Makefile, ...) were
generated, everything was done when configuring, so configdata.pm
could depend on build file templates and we'd get away with it.

However, since building configdata.pm is now independent of the build
file templates, that dependency is unnecessary, and would lead to
surprises of the build file template is updated, with an unexpected
full reconfiguration as a result, when all that's needed is to run
configdata.pm with no flags to get the build file re-generated.

This change is therefore a completion of what was forgotten in #15310.

@levitte levitte added branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch labels Feb 23, 2022
@levitte
Copy link
Member Author

levitte commented Feb 23, 2022

I stumbled upon this when experimenting with an updated build file
template.

This PR is a draft until it's been verified to work properly on a few
platforms.

Before PR openssl#15310, which reworked how build files (Makefile, ...) were
generated, everything was done when configuring, so configdata.pm
could depend on build file templates and we'd get away with it.

However, since building configdata.pm is now independent of the build
file templates, that dependency is unnecessary, and would lead to
surprises of the build file template is updated, with an unexpected
full reconfiguration as a result, when all that's needed is to run
configdata.pm with no flags to get the build file re-generated.

This change is therefore a completion of what was forgotten in openssl#15310.
@levitte levitte marked this pull request as ready for review February 23, 2022 20:55
@levitte levitte added the approval: review pending This pull request needs review by a committer label Feb 23, 2022
@levitte
Copy link
Member Author

levitte commented Feb 23, 2022

Verified to work, so not a draft any more

@levitte
Copy link
Member Author

levitte commented Feb 25, 2022

@openssl/committers, 2nd review needed please

@mspncp mspncp added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Feb 25, 2022
@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

@t8m t8m added the triaged: bug The issue/pr is/fixes a bug label Feb 28, 2022
openssl-machine pushed a commit that referenced this pull request Mar 4, 2022
Before PR #15310, which reworked how build files (Makefile, ...) were
generated, everything was done when configuring, so configdata.pm
could depend on build file templates and we'd get away with it.

However, since building configdata.pm is now independent of the build
file templates, that dependency is unnecessary, and would lead to
surprises of the build file template is updated, with an unexpected
full reconfiguration as a result, when all that's needed is to run
configdata.pm with no flags to get the build file re-generated.

This change is therefore a completion of what was forgotten in #15310.

Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #17756)
openssl-machine pushed a commit that referenced this pull request Mar 4, 2022
Before PR #15310, which reworked how build files (Makefile, ...) were
generated, everything was done when configuring, so configdata.pm
could depend on build file templates and we'd get away with it.

However, since building configdata.pm is now independent of the build
file templates, that dependency is unnecessary, and would lead to
surprises of the build file template is updated, with an unexpected
full reconfiguration as a result, when all that's needed is to run
configdata.pm with no flags to get the build file re-generated.

This change is therefore a completion of what was forgotten in #15310.

Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #17756)

(cherry picked from commit 468d151)
@levitte
Copy link
Member Author

levitte commented Mar 4, 2022

Merged

master:
468d151 Rework dependencies between config files and build files

3.0:
cbb5d2f Rework dependencies between config files and build files

@levitte levitte closed this Mar 4, 2022
@levitte levitte deleted the fix-config-build-20220223 branch March 4, 2022 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants