Skip to content

Comments

Engine code removal#28548

Closed
mbroz wants to merge 7 commits intoopenssl:feature/engineremovalfrom
mbroz:engine-code-removal
Closed

Engine code removal#28548
mbroz wants to merge 7 commits intoopenssl:feature/engineremovalfrom
mbroz:engine-code-removal

Conversation

@mbroz
Copy link
Member

@mbroz mbroz commented Sep 15, 2025 •
edited
Loading

This is a follow-up to #28384 that removes the rest of the Engine code.

[updated as it is required for follow-up header changes]

This PR

  • removes engines and crypto/engine directories
  • removes engine headers (engine.h) and related error codes
  • removes symbols from API and documentation
  • Fixes CI as some options disappeared
  • add a workaround to test that still requires headers
  • Add stub definitions
  • Add deprecated macro with own message (as I found no no better alternative)
  • Add stubs for other removed engine-related symbols

https://github.com/openssl/private/issues/840

Checklist
  • documentation is added or updated
  • tests are added or updated

@mbroz mbroz linked an issue Sep 15, 2025 that may be closed by this pull request
Remove the engines/ directory openssl/project#1367
Closed
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Sep 15, 2025
@mbroz mbroz added the severity: ABI change This pull request contains ABI changes label Sep 15, 2025
@beldmit
Copy link
Member

beldmit commented Sep 15, 2025

@mbroz could you please adjust code-style (the newly added failures)? LGTM otherwise

@mbroz mbroz force-pushed the engine-code-removal branch from 85fda4a to b8a897c Compare September 15, 2025 07:41
@mbroz
Copy link
Member Author

mbroz commented Sep 15, 2025

@mbroz could you please adjust code-style (the newly added failures)? LGTM otherwise

done, but it was just copy & paste #if from the existing code (to be removed later anyway :)
The rest are warnings from #28384

@github-actions github-actions bot removed the severity: ABI change This pull request contains ABI changes label Sep 15, 2025
@mbroz mbroz force-pushed the engine-code-removal branch from b8a897c to 530627d Compare September 15, 2025 10:38
This was linked to issues Sep 15, 2025
Remove ENGINE afalg openssl/project#1356
Closed
Remove ENGINE capi openssl/project#1357
Closed
Remove ENGINE devcrypto openssl/project#1359
Closed
Remove ENGINE padlock openssl/project#1360
Closed
Remove ENGINE loader_attic openssl/project#1422
Closed
@mbroz mbroz mentioned this pull request Sep 15, 2025
Closed
@mbroz mbroz linked an issue Sep 15, 2025 that may be closed by this pull request
Remove dasync ENGINE openssl/project#1358
Closed
@mbroz mbroz mentioned this pull request Sep 15, 2025
Closed
2 tasks
@mbroz mbroz force-pushed the engine-code-removal branch from 530627d to 2d88359 Compare September 15, 2025 12:19
jogme
jogme requested changes Sep 16, 2025
View reviewed changes
@t-j-h
Copy link
Member

t-j-h commented Sep 16, 2025

header files will need to remain - so that existing code that includes them doesn't break - i.e. the concept is for the implementation to be gone - but anything that doesn't actually meaningfully use engines actually still works unchanged

@beldmit
Copy link
Member

beldmit commented Sep 16, 2025

We provide empty engine.h in Red Hat to avoid compilation issues

@mbroz
Copy link
Member Author

mbroz commented Sep 16, 2025

header files will need to remain - so that existing code that includes them doesn't break - i.e. the concept is for the implementation to be gone - but anything that doesn't actually meaningfully use engines actually still works unchanged

The same can be said about error codes (generated openssl/engineerr.h) - someone can just use them in some code (even if never used). So we need to discuss how to do it. I think some trivial compilation changes (remove include line) would be acceptable, but I do not know the details. Full include removal was a nice test that our code is clean now, I can add an empty header file now.

@jogme jogme linked an issue Sep 16, 2025 that may be closed by this pull request
Deprecation of ENGINE_get_id openssl/project#1439
Closed
2 tasks
This was referenced Sep 16, 2025
Closed
Closed
@jogme jogme linked an issue Sep 16, 2025 that may be closed by this pull request
Deprecation of ENGINE_get_name openssl/project#1440
Closed
2 tasks
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 1, 2025
@mbroz @nhorman
Define stubs for other ENGINE-related legacy API and revert legacy er…
fe2839a 
…rors definitions.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 1, 2025
@mbroz @nhorman
Ignore unused arguments warnings in engine.h stubs.
02a4d4f 
This add pragma setting for gcc an clang compilers.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 1, 2025
@mbroz @nhorman
Remove top-level engines dir.
82e399b 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 1, 2025
@mbroz @nhorman
Remove crypto/engine dir.
9af054e 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 1, 2025
@mbroz @nhorman
Remove ENGINE symbols from API.
94a72d1 
Note, that engine.h now does not contain any real forward declarations,
so it should be excluded from the symbols parsing.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 1, 2025
@mbroz @nhorman
Temporary disable ENGINE_* docs tests.
6158031 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 1, 2025
@mbroz @nhorman
Use stub declarations in engine.h
5527ddf 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 1, 2025
@mbroz @nhorman
Define stubs for other ENGINE-related legacy API and revert legacy er…
5b1e97e 
…rors definitions.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 1, 2025
@mbroz @nhorman
Ignore unused arguments warnings in engine.h stubs.
abfd70c 
This add pragma setting for gcc an clang compilers.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Remove top-level engines dir.
d5dd969 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Remove crypto/engine dir.
67f795d 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Remove ENGINE symbols from API.
ffd1037 
Note, that engine.h now does not contain any real forward declarations,
so it should be excluded from the symbols parsing.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Temporary disable ENGINE_* docs tests.
884c51a 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Use stub declarations in engine.h
0935bed 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Define stubs for other ENGINE-related legacy API and revert legacy er…
0f6e931 
…rors definitions.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Ignore unused arguments warnings in engine.h stubs.
4c78b3f 
This add pragma setting for gcc an clang compilers.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 4, 2025
@mbroz @nhorman
Remove top-level engines dir.
6980a2a 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 4, 2025
@mbroz @nhorman
Remove crypto/engine dir.
6e4f67b 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 4, 2025
@mbroz @nhorman
Remove ENGINE symbols from API.
ec426ff 
Note, that engine.h now does not contain any real forward declarations,
so it should be excluded from the symbols parsing.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 4, 2025
@mbroz @nhorman
Temporary disable ENGINE_* docs tests.
f4f2168 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 4, 2025
@mbroz @nhorman
Use stub declarations in engine.h
fc08d32 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 4, 2025
@mbroz @nhorman
Define stubs for other ENGINE-related legacy API and revert legacy er…
39412f3 
…rors definitions.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
nhorman pushed a commit to nhorman/openssl that referenced this pull request Dec 4, 2025
@mbroz @nhorman
Ignore unused arguments warnings in engine.h stubs.
db6d9a5 
This add pragma setting for gcc an clang compilers.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Remove top-level engines dir.
e12acb0 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Remove crypto/engine dir.
2ca17dc 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Remove ENGINE symbols from API.
f180a8f 
Note, that engine.h now does not contain any real forward declarations,
so it should be excluded from the symbols parsing.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Temporary disable ENGINE_* docs tests.
1471043 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Use stub declarations in engine.h
64d2b8a 
Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Define stubs for other ENGINE-related legacy API and revert legacy er…
1c1e960 
…rors definitions.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
mbroz added a commit to mbroz/openssl that referenced this pull request Dec 4, 2025
@mbroz
Ignore unused arguments warnings in engine.h stubs.
5c7ab73 
This add pragma setting for gcc an clang compilers.

Signed-off-by: Milan Broz <[email protected]>

Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#28548)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Sashan Sashan Sashan left review comments

@jogme jogme jogme requested changes

@nhorman nhorman nhorman approved these changes

@t8m t8m t8m approved these changes

@mattcaswell mattcaswell mattcaswell approved these changes

@npajkovsky npajkovsky npajkovsky approved these changes

@beldmit beldmit Awaiting requested review from beldmit

@levitte levitte Awaiting requested review from levitte

@quarckster quarckster Awaiting requested review from quarckster

Assignees

No one assigned

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: feature The issue or PR is relevant only to one of the feature branches. severity: fips change The pull request changes FIPS provider sources tests: exempted The PR is exempt from requirements for testing triaged: feature The issue/pr requests/adds a feature

Projects

None yet

Milestone

No milestone

10 participants

@mbroz @beldmit @t-j-h @nhorman @openssl-machine @t8m @Sashan @mattcaswell @jogme @npajkovsky