-
Notifications
You must be signed in to change notification settings - Fork 167
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
Raise an error when the specified OpenSSL library directory doesn't exist. #618
Conversation
94da3c0
to
8f7e326
Compare
8f7e326
to
fc2ddf5
Compare
Rebasing on the latest master branch. I also updated the commit message too. |
I see one warning in macOS truffleruby-head case. https://github.com/ruby/openssl/actions/runs/5580792815/jobs/10198185535#step:8:94
|
@rhenium what do you think? Would you like reviewing? I plan to merge it if it looks good to you. |
This is a common pitfall and a check can be certainly useful: if the lib directory derived from However, I'm unsure whether suggesting the use of While using
|
Thanks for your comment.
I agree with the decision of using lib or lib64 (or any other name) should be made by the system admin, or distros. And I agree it should be consistent in the system. That means that users don't need to use the And it seems that the admin or distros can configure the search directories that is used as
I don't understand this part. You meant that recompiling "Ruby" OpenSSL binding (ruby/openssl) with Ruby library directory such as
I agree with this, as ruby/openssl can be used as a standard library of the Ruby. Considering your comment, how can I change this PR or the commit message or I don't need to change the PR? |
I meant OpenSSL (openssl/openssl) and Ruby (ruby/ruby) need to agree on the library directory name. From On second thought,
#477 (comment) is another example with MacPorts, which should have been obvious with the warning added by this PR. However, the commit message doesn't match. If custom-built OpenSSL is using the different library directory name from the rest of the system, the correct solution is to recompile OpenSSL with I started to think we should expand CONTRIBUTING.md or the GitHub wiki pages. CONTRIBUTING.md uses |
Thanks for explaining it.
I still don't understand this. The ruby/ruby doesn't have the
Okay. I understand that both your Arch Linux and also MacPorts cases explain showing both By the way, you showed the
That sounds good! |
I start to think perhaps the following way that is to explain the actual ways in a document page. Because we don't want to modify the What do you think?
|
fc2ddf5
to
c428080
Compare
I rebased this PR on the latest master branch, updating the commit message too. I intent I replace the
|
All right. Now I understood the Ruby configuring case above.
Then the
|
…xist. OpenSSL built from the source creates the library directory to the `/path/to/openssl_dir/lib64` as a default. In the case, the `bundle exec rake compile -- --with-openssl-dir=<openssl_dir>` cannot compile with the lib64 directory, and may compile with system OpenSSL's libraries unintentionally. This commit is to check this case to avoid linking with an unintentional library directory.
c428080
to
ca54087
Compare
I rebased again without mentioning Now the error message is like this. It seems the
I referred to the following code for the style of the openssl/ext/openssl/extconf.rb Lines 109 to 111 in ee03210
|
You're right, I must have been confused with another project. |
Looking at the history of |
All right! Thanks for explaining it. Do you have any other comments about this PR? Perhaps can we merge it? |
Sorry for the slow turn around. It looks good to me. Let's merge it! |
It's all right. Thank you for checking and merging this PR! |
This pull-request fixes #617. At least it gives users an instruction in the case of
rake compile -- --with-openssl-dir=/path/to/openssl
below.Commit message
Below is the commit message.
Add a logic to check a OpenSSL library directory existence on the given
--with-openssl-dir=<dir>
option.OpenSSL creates the library directory to the
/path/to/openssl_dir/lib64
as a default, when it is built from the source as follow.OpenSSL
In the case of the following command, the
dir_config("openssl")
returns["/path/to/openssl/include", "/path/to/openssl/lib"]
. And this causes the Ruby OpenSSL binding is unintentionally built with the system OpenSSL's library directory, because the/path/to/openssl/lib
doesn't exist. The logic to check the OpenSSL library directory's existence is to avoid building in this case.ruby/openssl
In the case of the following command, the
dir_config("openssl")
returns["/path/to/openssl/include", "/path/to/openssl/lib64:/path/to/openssl/lib64:/path/to/openssl/lib"]
. The returned library directory string is a set of the directories with the path separator ":".