Skip to content

test/recipes/01-test_symbol_presence.t: check for duplicate symbols in static libs#20331

Closed
levitte wants to merge 3 commits intoopenssl:masterfrom
levitte:enhance-test_symbol_presence
Closed

test/recipes/01-test_symbol_presence.t: check for duplicate symbols in static libs#20331
levitte wants to merge 3 commits intoopenssl:masterfrom
levitte:enhance-test_symbol_presence

Conversation

@levitte
Copy link
Member

@levitte levitte commented Feb 18, 2023

This checks that all symbols are unique across all public static libraries.
This includes a bit of refacftoring to avoid repeating code too much.


This is an alternative for #20308

…n static libs

This checks that all symbols are unique across all public static libraries.
This includes a bit of refacftoring to avoid repeating code too much.
@levitte levitte added branch: master Applies to master branch approval: review pending This pull request needs review by a committer approval: otc review pending triaged: feature The issue/pr requests/adds a feature tests: present The PR has suitable tests present labels Feb 18, 2023
@levitte levitte self-assigned this Feb 18, 2023
@levitte levitte requested review from a team February 18, 2023 10:26
@levitte
Copy link
Member Author

levitte commented Feb 21, 2023

Ping! I personally favor this over #20308

@paulidale paulidale 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 22, 2023
@DDvO
Copy link
Contributor

DDvO commented Feb 23, 2023

I personally favor this over #20308

Pleased that you adopted (the essence of) the proposal I gave there, namely to use nm.
I do not have the context to (efficiently) review the new PR in detail, so am glad that others did.

BTW, I took notice of the original PR just because you used (for good reasaon) the strong word 'brutally' in its title :)

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@paulidale
Copy link
Contributor

Merged.

@paulidale paulidale closed this Feb 23, 2023
openssl-machine pushed a commit that referenced this pull request Feb 23, 2023
…n static libs

This checks that all symbols are unique across all public static libraries.
This includes a bit of refacftoring to avoid repeating code too much.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #20331)
@levitte levitte deleted the enhance-test_symbol_presence branch February 24, 2023 07:56
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 Applies to master branch tests: present The PR has suitable tests present triaged: feature The issue/pr requests/adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants