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

[deps] Make OpenSSL build on macos and update #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GregBowyer
Copy link

We change up the openssl build generation to be able to generate builds
for other platforms and systems.

The initial added platforms are for macos. In the process of doing this
the build generation tooling has changed to be a python script that
understands all of the select permutations needed to generate the
stanzas needed for more than one platform.

Alongside this the perl script used to extract out the build details has
been changed. Rather than generating the build rules directly it now
exports the build information needed in the form of JSON outputs.

This has been tested on x86_64 for macos catalina and linux.

This also changes OpenSSL to 1.1.1l due to recent CVEs

Right now there are probably some unixisms left in the code which will
limit its ability to build for windows.

@jsharpe
Copy link
Contributor

jsharpe commented Aug 27, 2021

I wanted to try and pick up this change and see if it would work on linux aarch64 but you appear to have mised adding the config.py file that defines the PLATFORMS variable.

@GregBowyer
Copy link
Author

:( Doh sorry

We change up the openssl build generation to be able to generate builds
for other platforms and systems.

The initial added platforms are for macos. In the process of doing this
the build generation tooling has changed to be a python script that
understands all of the select permutations needed to generate the
stanzas needed for more than one platform.

Alongside this the perl script used to extract out the build details has
been changed. Rather than generating the build rules directly it now
exports the build information needed in the form of JSON outputs.

This has been tested on x86_64 for macos catalina and linux.

Right now there are probably some unixisms left in the code which will
limit its ability to build for windows.
@GregBowyer
Copy link
Author

GregBowyer commented Aug 27, 2021

@jsharpe Config.py pushed

"apps/testrsa.h",
"apps/timeouts.h",
] + glob(["include/internal/*.h"]),
copts = [
Copy link
Author

Choose a reason for hiding this comment

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

This is a bit of a leftover that probably should be in config.py some how, it works but I suspect windows is not going to enjoy it (-I vs /I I think if I remember my MSVC)

"include/crypto/dso_conf.h",
],
copts = copts + defines,
linkopts = [
Copy link
Author

Choose a reason for hiding this comment

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

This is a posixism that probably should be:

  1. Gleaned from the perl logic somehow
  2. Moved to config.py somehow

srcs = libssl_srcs + glob(["ssl/**/*.h"]) + [
"e_os.h",
],
copts = copts + defines,
Copy link
Author

Choose a reason for hiding this comment

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

I suspect defines can become local_defines on the cc_{library,binary} rules. That would take care of Posix -D vs MSVC /D logic

@GregBowyer
Copy link
Author

@jsharpe I have a few tweaks pending w.r.t arm on OSX that you might want

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants