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

(#3476) OpenSSL: change default openssldir value on Linux #4742

Merged
merged 1 commit into from
Mar 7, 2021

Conversation

rdeterre
Copy link
Contributor

@rdeterre rdeterre commented Mar 1, 2021

Specify library name and version: openssl/1.x.x

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

This patch modifies the default value of the "openssldir" option on Linux to "/etc/ssl" to solve issue #3476

resolves #3476

@CLAassistant
Copy link

CLAassistant commented Mar 1, 2021

CLA assistant check
All committers have signed the CLA.

@ghost
Copy link

ghost commented Mar 1, 2021

I detected other pull requests that are modifying openssl/1.x.x recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

prince-chrismc
prince-chrismc previously approved these changes Mar 1, 2021
Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

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

LGTM!

@prince-chrismc
Copy link
Contributor

please add resolves in front of the issue so that it closes automatically

@conan-center-bot
Copy link
Collaborator

An unexpected error happened and has been reported. Help is on its way! 🏇

uilianries
uilianries previously approved these changes Mar 1, 2021
@conan-center-bot
Copy link
Collaborator

Some configurations of 'openssl/1.0.2s' failed in build 3 (aea18e8411c3048b561dd10307ea6f04d2b8914a):

mathbunnyru
mathbunnyru previously approved these changes Mar 1, 2021
Copy link
Contributor

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

Great 👍
We had to change the openssldir manually back in days to make it work after conan implementation.

@prince-chrismc
Copy link
Contributor

That's annoying

Cannot create directory /etc/ssl/misc: Permission denied
make: *** [install_sw] Error 13
ERROR: openssl/1.0.2s: Error in package() method, line 736
	self._make_install()
while calling '_make_install', line 658
	self._run_make(targets=["install_sw"], parallel=False)
while calling '_run_make', line 605
	self.run(" ".join(command), win_bash=self._win_bash)
	ConanException: Error 2 while executing /usr/bin/make install_sw -j1

@rdeterre
Copy link
Contributor Author

rdeterre commented Mar 4, 2021

Yes, I'm not sure how this could be solved.

The problem only shows up with OpenSSL v1.0.2x though, all the v1.1+ don't seem to be affected.

Would it be OK to leave the old default openssldir value in place for OpenSSL v1.0.2x and update it only for versions >=1.1?

@prince-chrismc
Copy link
Contributor

CCI stops as soon as it sees a failure.

You can change the order to test that theory but it looks like a permission problem in the build image... and if files are installed there then they can not be installed correctly via Conan

is it just a mkdir call that in the scripts or does it really try to install files?

recipes/openssl/1.x.x/conanfile.py Outdated Show resolved Hide resolved
recipes/openssl/1.x.x/conanfile.py Show resolved Hide resolved
@SSE4
Copy link
Contributor

SSE4 commented Mar 5, 2021

if package tries install files to /etc, it might be a problem. it could pass in docker images, but for many people it will be stopper, as they don't always have write access.

@conan-center-bot
Copy link
Collaborator

All green in build 4 (d1d31e5e47fb8ed4e76c7ea2d25477c738af8efe)! 😊

@rdeterre
Copy link
Contributor Author

rdeterre commented Mar 5, 2021

I tried a few different versions on a test machine with a user without write access to "/etc". Setting openssldir to "/etc/ssl" causes permission issues with OpenSSL v1.0.2s (same error as during continuous integration here), v1.0.2t and v1.0.2u. However, everything works with the v1.1.x versions I tried.

@@ -436,9 +436,14 @@ def _get_env_build(self):
self._env_build = AutoToolsBuildEnvironment(self)
return self._env_build

def _get_default_openssl_dir(self):
if self.settings.os == "Linux" and self._full_version >= "1.1.0":
Copy link
Contributor

@SSE4 SSE4 Mar 5, 2021

Choose a reason for hiding this comment

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

what about MacOSX, FreeBSD, Android, do they need the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very good question. Since I have a mac I just installed Conan there and tried a small test project with OpenSSL 1.1.1i and I have seen no issue. So I don't think we need to update the default "openssldir" variable there.

For FreeBSD and Android, I don't know.

Copy link
Contributor

Choose a reason for hiding this comment

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

This certainly fixes Linux, no other platform is affect so I think we can continue with merging

@mathbunnyru
Copy link
Contributor

@rdeterre small suggestion for the future - please, do not use push-force, it makes it a bit more difficult to review the code and see the changes.
And having commits history is good to see what options you have considered, for example.

@conan-center-bot
Copy link
Collaborator

Failure in build 6 (a14b82484e79699a0382105014122287691ad56c):

  • openssl/1.0.2s@:
    An unexpected error happened and has been reported

  • openssl/1.0.2t@:
    An unexpected error happened and has been reported

  • openssl/1.1.0k@:
    An unexpected error happened and has been reported

  • openssl/1.0.2u@:
    An unexpected error happened and has been reported

  • openssl/1.1.0l@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.1d@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.1e@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.1c@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.1j@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.1g@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.1f@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.1h@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.1i@:
    Didn't run or was cancelled before finishing


Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.

@rdeterre
Copy link
Contributor Author

rdeterre commented Mar 6, 2021

Thanks for the comment for push-force, I'll stop :)

@prince-chrismc
Copy link
Contributor

You'll need to retrigger CI, close the pr wait 10s and then re-open it 🔁

@rdeterre rdeterre closed this Mar 6, 2021
@rdeterre rdeterre reopened this Mar 6, 2021
@conan-center-bot
Copy link
Collaborator

All green in build 8 (a14b82484e79699a0382105014122287691ad56c):

  • openssl/1.0.2t@:
    All packages built successfully! (All logs)

  • openssl/1.0.2s@:
    All packages built successfully! (All logs)

  • openssl/1.0.2u@:
    All packages built successfully! (All logs)

  • openssl/1.1.0l@:
    All packages built successfully! (All logs)

  • openssl/1.1.0k@:
    All packages built successfully! (All logs)

  • openssl/1.1.1c@:
    All packages built successfully! (All logs)

  • openssl/1.1.1d@:
    All packages built successfully! (All logs)

  • openssl/1.1.1g@:
    All packages built successfully! (All logs)

  • openssl/1.1.1e@:
    All packages built successfully! (All logs)

  • openssl/1.1.1f@:
    All packages built successfully! (All logs)

  • openssl/1.1.1h@:
    All packages built successfully! (All logs)

  • openssl/1.1.1i@:
    All packages built successfully! (All logs)

  • openssl/1.1.1j@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 2215209 into conan-io:master Mar 7, 2021
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.

[package] openssl/1.1.1g: Cannot make HTTPS requests using Boost.Beast
7 participants