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 'no-deprecated' #13706

Closed
wants to merge 3 commits into from
Closed

Conversation

@levitte
Copy link
Member

@levitte levitte commented Dec 18, 2020

Some of the handling of no-deprecated stuff wasn't quite complete, or
even plain wrong.

This restores i2d_PublicKey() to be able to handle EVP_PKEYs with
legacy internal keys.

This also refactors the DSA key tests in test/evp_extra_test.c to use
EVP functionality entirely.

@levitte
Copy link
Member Author

@levitte levitte commented Dec 18, 2020

It might be interesting to have a separate CI job that only does no-deprecated... `minimal does that, but it also disables many of the disabled algorithms, which means we don't get notified of deprecation logic misses...

I could add that as part of this PR, if that's desirable

@levitte
Copy link
Member Author

@levitte levitte commented Dec 18, 2020

Cis are happy

@t8m
Copy link
Member

@t8m t8m commented Dec 18, 2020

It might be interesting to have a separate CI job that only does no-deprecated... `minimal does that, but it also disables many of the disabled algorithms, which means we don't get notified of deprecation logic misses...

I could add that as part of this PR, if that's desirable

It would make sense to have such job. And given the run times of the jobs are now very reasonable I think it should not give any problems to have it.

It is actually questionable whether the minimal build should include no-deprecated or not. Probably not if we have this new no-deprecated build.

@levitte
Copy link
Member Author

@levitte levitte commented Dec 18, 2020

It is actually questionable whether the minimal build should include no-deprecated or not. Probably not if we have this new no-deprecated build.

I agree, since they serve two different purposes

@levitte
Copy link
Member Author

@levitte levitte commented Dec 18, 2020

It is actually questionable whether the minimal build should include no-deprecated or not. Probably not if we have this new no-deprecated build.

Commit added

@levitte
Copy link
Member Author

@levitte levitte commented Dec 18, 2020

... that broke the minimal job. Fix committed

@t8m
t8m approved these changes Dec 18, 2020
@levitte
Copy link
Member Author

@levitte levitte commented Dec 18, 2020

(I so wanna call this "urgent", so I can rebase #13700 without making it ugly 😉)

levitte added 3 commits Dec 18, 2020
Some of the handling of no-deprecated stuff wasn't quite complete, or
even plain wrong.

This restores i2d_PublicKey() to be able to handle EVP_PKEYs with
legacy internal keys.

This also refactors the DSA key tests in test/evp_extra_test.c to use
EVP functionality entirely.
@levitte levitte force-pushed the levitte:fix-no-deprecated-20201217 branch from 7e89006 to d395569 Dec 19, 2020
@t8m
Copy link
Member

@t8m t8m commented Dec 19, 2020

Still approved.

@openssl-machine
Copy link

@openssl-machine openssl-machine commented Dec 19, 2020

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.

openssl-machine pushed a commit that referenced this pull request Dec 19, 2020
Some of the handling of no-deprecated stuff wasn't quite complete, or
even plain wrong.

This restores i2d_PublicKey() to be able to handle EVP_PKEYs with
legacy internal keys.

This also refactors the DSA key tests in test/evp_extra_test.c to use
EVP functionality entirely.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #13706)
openssl-machine pushed a commit that referenced this pull request Dec 19, 2020
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #13706)
@levitte
Copy link
Member Author

@levitte levitte commented Dec 19, 2020

Merged

6ed4022 Fix 'no-deprecated'
e3577ad GitHub CI: Separate no-deprecated job from minimal job

@levitte levitte closed this Dec 19, 2020
@levitte levitte deleted the levitte:fix-no-deprecated-20201217 branch Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants