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.

Sign up for GitHub

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

Jump to bottom

Add support for BSD-ppc, BSD-ppc64 and BSD-ppc64le configs #17090

Closed
wants to merge 1 commit into from
Closed

Add support for BSD-ppc, BSD-ppc64 and BSD-ppc64le configs #17090

wants to merge 1 commit into from

Conversation

pkubaj
Copy link
Contributor

@pkubaj pkubaj commented Nov 20, 2021 •
edited

While OpenSSL currently works on FreeBSD/powerpc platforms, it uses a generic configuration without AltiVec / VSX.
CLA: trivial

@openssl-machine openssl-machine added hold: cla required The contributor needs to submit a license agreement and removed hold: cla required The contributor needs to submit a license agreement labels Nov 20, 2021
@pkubaj pkubaj changed the title Add support for BSD-ppc64 config Add support for BSD-ppc64 and BSD-ppc64le configs Nov 20, 2021
@pkubaj pkubaj changed the title Add support for BSD-ppc64 and BSD-ppc64le configs Add support for BSD-ppc, BSD-ppc64 and BSD-ppc64le configs Nov 20, 2021
@mattcaswell
Copy link
Member

I don't have access to these platforms to test them. But from code inspection alone these changes seem reasonable.

I would point out our (recently updated) platform policy here:
https://www.openssl.org/policies/platformpolicy.html

Which says:

Support for a new platform should only be added if it is being adopted as a primary, secondary or community platform.

Since this PR effectively adds new platforms, we would at least need to find someone willing to be the community platform maintainer. @pkubaj is this something you are willing to take on as a result of this PR? Or do you know someone who would be willing to take on that role? From the platform policy I linked to above - the responsibility of the maintainer is:

If defects are raised that are specific to a community platform then the community maintainer may be contacted to help find a resolution. If a community maintainer is unresponsive, or unable to provide fixes then the platform may be moved to "unadopted".

@pkubaj
Copy link
Contributor Author

pkubaj commented Nov 22, 2021

I'm fine with being a maintainer.

mattcaswell
mattcaswell approved these changes Nov 22, 2021
View reviewed changes
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Approved, subject to https://github.com/openssl/web/blob/master/policies/platformpolicy.html being updated to reflect @pkubaj as being the community maintainer for these platforms

@mattcaswell
Copy link
Member

I'm fine with being a maintainer.

Cool!

@pkubaj
Copy link
Contributor Author

pkubaj commented Nov 22, 2021

Am I supposed to make a PR to update platformpolicy.html?

@mattcaswell
Copy link
Member

Am I supposed to make a PR to update platformpolicy.html?

If you are willing to do that, then that would be great!

@pkubaj pkubaj mentioned this pull request Nov 22, 2021
Closed
@paulidale paulidale added approval: done This pull request has the required number of approvals branch: master Merge to master branch labels Nov 23, 2021
@pkubaj
Copy link
Contributor Author

pkubaj commented Nov 23, 2021

Would it be possible to also merge to 1.1.1?

paulidale
paulidale approved these changes Nov 24, 2021
View reviewed changes
Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

I don't think this is suitable for back porting to 1.1.1 or 3.0. It doesn't seem like a bug fix and the changes go beyond only being configuration additions.

@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.

@mattcaswell
Copy link
Member

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.

Not sure why the bot thinks this since there haven't been any updates. Setting the label manually.

@mattcaswell mattcaswell added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Nov 24, 2021
@mattcaswell
Copy link
Member

Ah.I just came to merge this and noticed the "CLA: trivial" line in the commit message. Unfortunately I think this goes beyond our definition of trivial and so I think a CLA is required. Could you provide one?

https://www.openssl.org/policies/cla.html

@mattcaswell mattcaswell added the hold: cla required The contributor needs to submit a license agreement label Nov 24, 2021
@paulidale
Copy link
Contributor

Agreed, well beyond trivial.

@mattcaswell
Copy link
Member

@pkubaj - looks like this is still holding waiting for a CLA. Will you be able to submit one?

@pkubaj
Copy link
Contributor Author

pkubaj commented Dec 3, 2021

Yes, I have it printed out, but I don't have a scanner at home, so will need to scan it at work next week.

@mattcaswell
Copy link
Member

@pkubaj - CLA has now been received - thanks! It looks like this PR now needs a rebase - could you do that?

@mattcaswell mattcaswell removed the hold: cla required The contributor needs to submit a license agreement label Dec 7, 2021
@mattcaswell
Copy link
Member

Ah...unfortunately you added a merge commit. You need to do a rebase instead (we don't allow merge commits).

@pkubaj
Add support for BSD-ppc, BSD-ppc64 and BSD-ppc64le configurations
9e70ba2 
OpenSSL assumes AT_HWCAP = 16 (as on Linux), but on FreeBSD AT_HWCAP = 25
Switch to using AT_HWCAP, and setting it to 16 if it is not defined.

CLA: trivial
@mattcaswell
Copy link
Member

This looks good now. Thanks.

@paulidale - can you reconfirm your approval?

@mattcaswell mattcaswell added approval: review pending This pull request needs review by a committer and removed approval: ready to merge The 24 hour grace period has passed, ready to merge labels Dec 7, 2021
@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 Dec 8, 2021
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Dec 9, 2021
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@paulidale
Copy link
Contributor

Merged, thanks for the contribution.

@paulidale paulidale closed this Dec 9, 2021
openssl-machine pushed a commit that referenced this pull request Dec 9, 2021
@pkubaj @paulidale
Add support for BSD-ppc, BSD-ppc64 and BSD-ppc64le configurations
f5485b9 
OpenSSL assumes AT_HWCAP = 16 (as on Linux), but on FreeBSD AT_HWCAP = 25
Switch to using AT_HWCAP, and setting it to 16 if it is not defined.

CLA: trivial

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #17090)
@pkubaj pkubaj mentioned this pull request Aug 1, 2022
Closed
2 tasks
t8m pushed a commit to t8m/openssl that referenced this pull request Nov 2, 2022
@pkubaj @t8m
Add support for BSD-ppc, BSD-ppc64 and BSD-ppc64le configurations
a4412fe 
OpenSSL assumes AT_HWCAP = 16 (as on Linux), but on FreeBSD AT_HWCAP = 25
Switch to using AT_HWCAP, and setting it to 16 if it is not defined.

CLA: trivial

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#17090)

(cherry picked from commit f5485b9)
openssl-machine pushed a commit that referenced this pull request Nov 9, 2022
@pkubaj @t8m
Add support for BSD-ppc, BSD-ppc64 and BSD-ppc64le configurations
923140b 
OpenSSL assumes AT_HWCAP = 16 (as on Linux), but on FreeBSD AT_HWCAP = 25
Switch to using AT_HWCAP, and setting it to 16 if it is not defined.

CLA: trivial

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #17090)

(cherry picked from commit f5485b9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattcaswell mattcaswell mattcaswell approved these changes

@paulidale paulidale paulidale approved these changes

Assignees
No one assigned
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pkubaj @mattcaswell @openssl-machine @paulidale