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
Fix WARNING: OpenSSL headers and library versions do not match #958
Conversation
Seems it is intentional ignoring
If that is true, |
I disagree strongly. And comments in a homebrew tracker don't speak for the curl project. |
It's worth noting Homebrew builds things in a special environment & comments in Homebrew formulae may not reflect reality outside of that environment. |
|
@@ -33,7 +33,7 @@ AC_DEFUN([CURL_CHECK_DEF], [ | |||
AC_REQUIRE([CURL_CPP_P])dnl | |||
OLDCPPFLAGS=$CPPFLAGS | |||
# CPPPFLAGS comes from CURL_CPP_P | |||
CPPFLAGS="$CPPPFLAGS" | |||
#CPPFLAGS="$CPPPFLAGS" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this effectively disables the CURL_CPP_P
check that we added in ecf9534. If you want to remove the assignment, then remove it. If you want to remove it, then why figure out CPPPFLAGS
to begin with? If you can make the script work without the CPPPFLAGS
logic properly, please elaborate on how and why that works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like I said in my description, this PR is to illustrate the problem and is not a proper fix.
If you want me to come up with a better fix, I will try. If I can't, I hope someone else picks this up and the issue is not ignored. I want to be clear that the openssl header version check is now broken and the problem is easy to reproduce.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made an attempt at a proper fix.
Can you please explain the reasoning behind this patch? I don't understand why it does what it does and the comments seems to be incorrect. Ie the |
Sure, I can explain. Originally, However, I noticed that I modified I hope I haven't added or removed a |
( |
Ah now I get it. Can you then please provide the commits squashed instead of updating an initial commit in a later one within the same pull request? |
17c4177
to
140092b
Compare
@bagder I squashed everything into one commit. I also eliminated the code that changes the warning into an error; let me know if I should reintroduce it. |
Awesome. Thanks a lot for your patch and your patience. Merged now with a slightly modified commit message! |
I noticed that when I specified an OpenSSL dir with
--with-ssl
, the configure script would still find the OpenSSL library version in the system headers, not in my specified library:I looked through config.log and found that the headers check, which is done with the preprocessor (
gcc -E
), was not using the include directories in CPPFLAGS:Here is a fix. I've changed the code that defines
CPPPFLAGS
. Previously,CPPPFLAGS
would contain the contents ofCPPFLAGS
and potentially an added-P
. Now, there is a variableCPPPFLAG
that can either be empty or-P
. This is so-P
can be appended to the current value ofCPPFLAGS
, not whateverCPPFLAGS
was whenCURL_CPP_P
was run.My new output:
EDIT: Made an attempt at a proper fix.