Fix OpenSSL versions 1.1.1 & newer for tvOS #1586

faithfracture · 2020-05-08T21:12:30Z

fork() is unavailable on tvOS. This adds and applies a patch file when
building for tvOS and sets an argument for the compiler.

Specify library name and version: lib/1.0

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

`fork()` is unavailable on tvOS. This adds and applies a patch file when building for tvOS and sets an argument for the compiler.

CLAassistant · 2020-05-08T21:12:35Z

All committers have signed the CLA.

conan-center-bot · 2020-05-08T21:12:42Z

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

madebr · 2020-05-11T13:56:59Z

@faithfracture
Have you tried adding -DNO_FORK conditionally to the cflags list in the _create_targets method instead of patching?

faithfracture · 2020-05-11T14:05:27Z

Yes - that doesn't work. See openssl/openssl#7607 (sorry, I should have added this to the original PR.)

faithfracture · 2020-05-14T22:19:56Z

What needs to happen to move this forward? Looks like the 3 review approvals have happened. What's next?

madebr · 2020-05-14T22:46:59Z

recipes/openssl/1.x.x/conanfile.py

@@ -413,6 +414,8 @@ def _configure_args(self):
        if self._full_version >= "1.1.0":
            args.append("--debug" if self.settings.build_type == "Debug" else "--release")

+        if str(self.settings.os) == "tvOS":


Suggested change

if str(self.settings.os) == "tvOS":

if self.settings.os == "tvOS":

If this is changed, then the rest of the if self.settings.os == statements that follow should also be changed. I didn't notice it before, but the usage of this is horribly inconsistent though the entire recipe. Is there a standardization somewhere that says one should be used over the other? I typically don't do "standardization" type changes for commits for things like this, but I'm happy to do it if you'd prefer it.

By dropping the str, conan is able to check whether tvOS is a valid os.
That way typos can be catched.
If the code is equivalent, I'ld say 'why not?'

Yeah, it doesn't matter to me one way or the other - I suck at Python & honestly don't know the difference.

Just because I'm curious and would like to know - how is it that Conan can check if it's a valid OS by doing self.settings.os rather than str(self.settings.os)? If self.settings.os is a string, then using str() really just turns an existing string into a string, right?

Maybe a better way to put it is - how is it that dropping str() makes it so Conan can check if tvOS is a valid OS?

When you compare against self.settings.os, you're comparing against a SettingsItem object.
This class checks whether strings to which it is compared against would also be a valid setting (ie it is present in your settings.yml file).
By doing str, you compare against a string without an additional check.

madebr · 2020-05-14T22:49:22Z

recipes/openssl/1.x.x/conanfile.py

+                    if self.settings.os == "tvOS":
+                        tools.patch(patch_file=os.path.join("patches", "1.1.1-tvos.patch"),
+                                    base_path=self._source_subfolder)


I don't like the conditional patching.
Can the patch be rewritten so it only changes behavior for WatchOS?
Maybe by checking for the existence for a certain preprocessor variable that can be set in the conanfile.py?

Considering this is the patch that is being used as the fix for this in the official OpenSSL repository, I don't think that makes sense. Furthermore, since even if you pass -DNO_FORK, it gets undefined in apps/ocsp.c, that is a bug in OpenSSL itself that is not specific to tvOS, so I would argue that it shouldn't be made specific to tvOS.

Ok. Good point.

This allows Conan to check against valid OSes, rather than just doing a basic string comparison.

conan-center-bot · 2020-05-15T12:07:46Z

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

conan-center-bot · 2020-05-18T10:04:43Z

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

conan-center-bot · 2020-05-18T10:06:17Z

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

faithfracture · 2020-05-23T14:35:55Z

bump

danimtb · 2020-05-25T08:51:45Z

@faithfracture it seems that you did not request access to EAP in #4

faithfracture · 2020-05-25T12:46:30Z

Oh - it wasn’t clear to me that I had to run the pr-merge build. It just says that I don’t have access to EAP. I just assumed that someone who did have access to it would run the build. I’ll request access.

Also, the PR still says I don’t have all the required approvals. (1 of 3)

uilianries · 2020-05-25T14:26:35Z

@faithfracture It's documented in the Wiki: https://github.com/conan-io/conan-center-index/wiki#adding-packages-to-conancenter

One of required approvals is the EAP. Once approved, your recipe will be checked by the CCI.

faithfracture · 2020-05-27T20:09:54Z

Ok - I requested EAP access.

conan-center-bot · 2020-06-08T21:52:11Z

All green in build 5 (83fba722eb54b2ba7d0dff9116d242d126ae75c9)! 😊

openssl/1.0.2s: Generated 82 packages. All logs here
openssl/1.0.2t: Generated 82 packages. All logs here
openssl/1.0.2u: Generated 82 packages. All logs here
openssl/1.1.0k: Generated 82 packages. All logs here
openssl/1.1.0l: Generated 82 packages. All logs here
openssl/1.1.1c: Generated 82 packages. All logs here
openssl/1.1.1d: Generated 82 packages. All logs here
openssl/1.1.1e: Generated 82 packages. All logs here
openssl/1.1.1f: Generated 82 packages. All logs here
openssl/1.1.1g: Generated 82 packages. All logs here

faithfracture · 2020-06-08T21:53:18Z

@SSE4 looks like this is waiting on you for the final review

$faithfracture$

$@faithfracture$

Fix OpenSSL versions 1.1.1 & newer for tvOS

1393de5

`fork()` is unavailable on tvOS. This adds and applies a patch file when building for tvOS and sets an argument for the compiler.

conan-center-bot added the No Beta user label May 8, 2020

uilianries previously approved these changes May 11, 2020

View reviewed changes

uilianries requested review from SSE4, danimtb and jgsogo May 11, 2020 13:33

danimtb previously approved these changes May 11, 2020

View reviewed changes

jgsogo previously approved these changes May 12, 2020

View reviewed changes

madebr reviewed May 14, 2020

View reviewed changes

$@faithfracture$

Perform OS checks against SettingsItem

83fba72

This allows Conan to check against valid OSes, rather than just doing a basic string comparison.

$@faithfracture$ faithfracture dismissed stale reviews from jgsogo, danimtb, and uilianries via 83fba72 May 15, 2020 12:07

danimtb removed the No Beta user label May 18, 2020

conan-center-bot added the No Beta user label May 18, 2020

uilianries approved these changes May 18, 2020

View reviewed changes

jgsogo approved these changes May 29, 2020

View reviewed changes

danimtb removed the No Beta user label Jun 8, 2020

SSE4 approved these changes Jun 10, 2020

View reviewed changes

SSE4 merged commit 2c175a8 into conan-io:master Jun 10, 2020

Croydon mentioned this pull request Jun 26, 2020

[WIP] add openssl/3.0.0 #2054

Closed

10 tasks

Croydon mentioned this pull request Sep 9, 2021

add openssl/3.0.0 #7229

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix OpenSSL versions 1.1.1 & newer for tvOS #1586

Fix OpenSSL versions 1.1.1 & newer for tvOS #1586

$@faithfracture$ faithfracture commented May 8, 2020

CLAassistant commented May 8, 2020 •

edited

Loading

conan-center-bot commented May 8, 2020

madebr commented May 11, 2020

faithfracture commented May 11, 2020

faithfracture commented May 14, 2020

madebr May 14, 2020

$@faithfracture$ faithfracture May 14, 2020

madebr May 14, 2020

$@faithfracture$ faithfracture May 15, 2020

madebr May 15, 2020 •

edited

Loading

madebr May 14, 2020

$@faithfracture$ faithfracture May 14, 2020

madebr May 14, 2020

conan-center-bot commented May 15, 2020

conan-center-bot commented May 18, 2020

conan-center-bot commented May 18, 2020

faithfracture commented May 23, 2020

danimtb commented May 25, 2020

faithfracture commented May 25, 2020

uilianries commented May 25, 2020

faithfracture commented May 27, 2020

conan-center-bot commented Jun 8, 2020

faithfracture commented Jun 8, 2020

	if str(self.settings.os) == "tvOS":
	if self.settings.os == "tvOS":

Fix OpenSSL versions 1.1.1 & newer for tvOS #1586

Fix OpenSSL versions 1.1.1 & newer for tvOS #1586

Conversation

faithfracture commented May 8, 2020

CLAassistant commented May 8, 2020 • edited Loading

conan-center-bot commented May 8, 2020

madebr commented May 11, 2020

faithfracture commented May 11, 2020

faithfracture commented May 14, 2020

madebr May 14, 2020

Choose a reason for hiding this comment

faithfracture May 14, 2020

Choose a reason for hiding this comment

madebr May 14, 2020

Choose a reason for hiding this comment

faithfracture May 15, 2020

Choose a reason for hiding this comment

madebr May 15, 2020 • edited Loading

Choose a reason for hiding this comment

madebr May 14, 2020

Choose a reason for hiding this comment

faithfracture May 14, 2020

Choose a reason for hiding this comment

madebr May 14, 2020

Choose a reason for hiding this comment

conan-center-bot commented May 15, 2020

conan-center-bot commented May 18, 2020

conan-center-bot commented May 18, 2020

faithfracture commented May 23, 2020

danimtb commented May 25, 2020

faithfracture commented May 25, 2020

uilianries commented May 25, 2020

faithfracture commented May 27, 2020

conan-center-bot commented Jun 8, 2020

faithfracture commented Jun 8, 2020

$@faithfracture$ faithfracture commented May 8, 2020

CLAassistant commented May 8, 2020 •

edited

Loading

$@faithfracture$ faithfracture May 14, 2020

$@faithfracture$ faithfracture May 15, 2020

madebr May 15, 2020 •

edited

Loading

$@faithfracture$ faithfracture May 14, 2020