Mac: Repair current OpenSSL undefined reference from homebrew to dynamic

[Martii]

• These are the default locations, as of today, for Mac OS X 10.11.4 (El Capitan)

NOTES

• This could really use some environment variables for homebrew root and which SSL version is in use. Not familiar enough yet with recent Qt to do this at this time.
• Symbolic links could also be used but again determining the current homebrew SSL installed is something to be desired e.g. similar to $ openssl version e.g. perhaps a cut of $ brew info openssl?

Refs:

• http://brew.sh/ (homepage)
• https://github.com/Homebrew/brew (repo)
• https://github.com/Homebrew/brew/blob/master/share/doc/homebrew/FAQ.md#why-does-homebrew-prefer-i-install-to-usrlocal
• https://github.com/Homebrew/brew/blob/master/share/doc/homebrew/External-Commands.md#shell-scripts

Historical refs:

• Mac fixes #1888
• QupZilla fails to build OS X 10.11 #1850

---

PR READY AGAIN

[nowrep]

No, on 3 systems I tested the openssl libs were installed in /opt/local/ so this would break it there.
Also it now specifies exactly the version of openssl which is a huge no-no, it will break with new versions.

I will only accept correct way: parsing brew info openssl as you mentioned.

[Martii]

No, on 3 systems I tested the openssl libs were installed in /opt/local/ so this would break it there.

Please specify exactly how you got it there then. This Mac is about 2 weeks old and this is where brew puts it by default... so if you made a change to brew I'd like to know how you did it so I can better accommodate where those systems are putting it in the wrong/different path then. TIA.

Yeah, I have never built master on Mac. It needs some fixing ...

on 3 systems I tested

Elaborate please on your quotes of how you went from this to this.

I will only accept correct way: parsing brew info openssl as you mentioned.

That will only give the version and not the brew home/root directory. This is your project so you have to assist on how you want this done. Some of the code is written well but other parts are just hacked messily together and the documention on BUILDING is absolutely incorrect even though I asked you to clean it up, which you did... brew is a dependency.

Keeping it as is will break it out of the box whereas your alleged "three" are flawed or changed somehow. e.g. please describe.

[nowrep]

I did build master few weeks ago on friend's mac. I also built QtWebKit version couple of times and every time the openssl libraries were at that location. I don't know if they installed it with brew or not, I just assumed that. I asked my friend when I commited the fix and wrote that comment, but he was unsure.

In any case, you should only add the new include/lib paths, not change the current (and working) one.

That will only give the version and not the brew home/root directory.

Well, in that case try something different. If /usr/local/Cellar/ is brew home, then you could use glob (probably from system() qmake command) like /usr/local/Cellar/openssl/*/include

This is your project so you have to assist on how you want this done.

Yes, I reviewed your pull request.

Some of the code is written well but other parts are just hacked messily together and the documention on BUILDING is absolutely incorrect even though I asked you to clean it up, which you did... brew is a dependency.

I accept pull requests, you can improve the code and documentation. Patches are always welcome (but only correct patches, that goes without saying).
You can point out the "hacked messily parts" or even fix them, thanks. Some parts may be hacked messily together, but are still less hacky than this pull request :D

Keeping it as is will break it out of the box whereas your alleged "three" are flawed or changed somehow. e.g. please describe.

It works on your machine as of May 13 2016. When there is new openssl version it won't work anymore.
I'm sorry, but you can't expect me to accept using hardcoded path that contains the exact openssl version (which changes VERY often) and possibly non-default brew path (not sure about it, I don't use brew).

[Martii]

When there is new openssl version it won't work anymore.

As far as I know openssl will never go away... it's core to Linux as well. So this is a bad assumption.

It works on your machine

Not just mine... anything new out of the box. I presume that brew used to incorrectly install it under /opt in the Mac tree (e.g. those machines are bugged)... Mac users are still getting up to Linux speed when it comes to tree handling.

I did build master few weeks ago on friend's mac

Have your friend uninstall openssl and brew and reinstall... or ask him/her how they moved it... but...

$ brew --prefix
/usr/local

... yields something promising (found at b53f0c5/Library/Homebrew/cmd/--prefix.rb). :) ... although it's still missing Cellar

Btw... quote from them

#: Display Homebrew's install path. Default: /usr/local

... at b53f0c5/Library/Homebrew/cmd/--prefix.rb#L2

So your friends machine is booged for absolute sure now. :) ;)

Yes, I reviewed your pull request.

Until I asked for more information... I appreciate the cooperation now.

were installed in /opt/local/ so this would break it there

So this is absolutely the wrong path now.

I accept pull requests, you can improve the code and documentation.

Of course... that's why I am here to fix some bugs. :) However you need to take the initiative as well to get others started out of the box... I bought a $1000.00 Macintosh as a test machine and I've resurrected my understanding of it and Qt a bit just in a few weeks.

on friend's mac.

You may want to look into a Hackintosh but you didn't hear that from me. ;)

[nowrep]

As far as I know openssl will never go away... it's core to Linux as well. So this is a bad assumption.

It won't go away, at least not in the foreseeable future. What it will, on the other hand, is get a new release. Once it is 1.0.2i instead of 1.0.2h your patch stops working. That's the main issue I have with this change.

[Martii]

Here we go. Please ask your friend to send you a dump of:

$ brew config
HOMEBREW_VERSION: 0.9.9
ORIGIN: https://github.com/Homebrew/brew
HEAD: 5f06312916e8ae518e8a9f212f7e683c49ceee67
Last commit: 3 days ago
Core tap ORIGIN: https://github.com/Homebrew/homebrew-core
Core tap HEAD: 410ff750e698a95e6f9dccdf4f2098ab008a8509
Core tap last commit: 2 days ago
HOMEBREW_PREFIX: /usr/local
HOMEBREW_REPOSITORY: /usr/local
HOMEBREW_CELLAR: /usr/local/Cellar
HOMEBREW_BOTTLE_DOMAIN: https://homebrew.bintray.com
CPU: quad-core 64-bit haswell
Clang: 7.3 build 703
Perl: /usr/bin/perl
Python: /usr/bin/python
Ruby: /usr/bin/ruby => /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/bin/ruby
Java: N/A
System Ruby: 2.0.0-p648
OS X: 10.11.4-x86_64
Xcode: 7.3.1
CLT: 7.3.1.0.1.1461711523
X11: N/A

... if he/she has HOMEBREW_CELLAR then this project should be able to read that for the home/root. I'll see what I can do for gleaning the path for openssl... and of course figuring out how to do this in the .pro syntax.

[nowrep]

You can call any system command (including your own shell scripts) with system( ... ). Take a look at how pkg-config is used in gnomekeyringpasswords plugin.

[Martii]

Thanks. Will look into that after I figure out the specifics of brew a little more... this might be better ...if it wasn't bugged too :

$ brew --prefix openssl
/usr/local/opt/openssl

... seems they have an artifact laying around of the old, incorrect, tree. :\ EDIT: Looks like it's a symlink mentioned in this PR... so not bugged.

[Martii]

@nowrep
So what do you think the odds of the $ brew info openssl are going to be constant for the fourth line and a signature of installationPath (### files, size) are going to be? e.g. it's probably going to break down the road again just like it did before. Scraping stdout isn't always the best method unless it's an exact API to return just that... which is why I did #1958 with git.

---

$ brew info openssl | head -n4 | tail -n1 | cut -f1 -d' ' (scrape method)

[nowrep]

Can you please post output of brew info openssl ?

[Martii]

Can you please post output of brew info openssl ?

Sure...

$ brew info openssl
openssl: stable 1.0.2h (bottled) [keg-only]
SSL/TLS cryptography library
https://openssl.org/
/usr/local/Cellar/openssl/1.0.2h (1,691 files, 12M)
  Poured from bottle on 2016-05-11 at 12:48:11
From: https://github.com/Homebrew/homebrew-core/blob/master/Formula/openssl.rb
==> Dependencies
Build: makedepend ✘
==> Options
--universal
    Build a universal binary
--without-test
    Skip build-time tests (not recommended)
==> Caveats
A CA file has been bootstrapped using certificates from the system
keychain. To add additional certificates, place .pem files in
  /usr/local/etc/openssl/certs

and run
  /usr/local/opt/openssl/bin/c_rehash

This formula is keg-only, which means it was not symlinked into /usr/local.

Apple has deprecated use of OpenSSL in favor of its own TLS and crypto libraries

Generally there are no consequences of this for you. If you build your
own software and it requires this formula, you'll need to add to your
build variables:

    LDFLAGS:  -L/usr/local/opt/openssl/lib
    CPPFLAGS: -I/usr/local/opt/openssl/include

[Martii]

btw that last commit compiles and installs peachy. :)

[MikeMcQuaid]

Homebrew maintainer here. @Martii's Homebrew installation PATH is the default and using brew --prefix openssl is the right solution here.

@Martii In future if you CC me to an issue it would be good to provide me with more context and spend a little more time verifying if something is a bug or not (and if it is: submit it to the Homebrew bugtracker).

Thanks!

[ghost]

@Martii, the following will not works. It was correct before your change:
https://github.com/QupZilla/qupzilla/blob/master/BUILDING.md
Please do check before make a chenge!

[Martii]

@MikeMcQuaid

provide me with more context

Glad you are in a good mood today. You're being perceived as being funny and facetious with this portion. ;)

a little more time verifying if something is a bug

That's why you were striked out. Discovery is all about knowing when to approach someone instead of blindly being led. Apologies for the 12 hours today and 3 days of investigation which led to a Cc and not a To... just thought it was about time to get an outside opinion on this contextual issue in the PR. Thanks so much for dropping in.

and if it is: submit it to the Homebrew bugtracker

You guys will just have to wait your turn. ;) :) As Yoda would say... "There, there is another" mentioned in here and it'll be mirrored to your issue tracker eventually.

@nowrep
So is there anything else you would like to have done to wrap this up? Are all the eyes dotted and tees crossed?

@cranes-bill
What?

[nowrep]

These changes break newlines, the double space at the end of line must stay.

[Martii]

They might indeed... however my editor is permanently set to strip trailing spaces and is done automatically. The MD could use some fine tuning for sure outside of this issue.

[nowrep]

This is not an issue, those newlines are intended (double space in md = newline).

[Martii]

What a pain.. this isn't quite what I was expecting... and yes I did test it in the GH editor before committing... so GH has a bug with their preview too... Give me a minute and I'll go find that option. Eventually this should be phased out.

[Martii]

Btw who's spec is that? I've never seen that in CommonMark nor Daring Fireballs... but since this is a new discovery I may have missed it. Is it GH specific?

[nowrep]

Not sure, it may indeed be GitHub extension.

[r-a-v-a-s]

When you do want to insert a <br /> break tag using Markdown, you end a line with two or more spaces, then type return.

http://daringfireball.net/projects/markdown/syntax#p

[nowrep]

@cranes-bill Renaming the BUILDING to BUILDING.md is fine, as long as all links are changed too.

[Martii]

not working: https://github.com/QupZilla/qupzilla/blob/master/BUILDING.md

It won't work until it's merged ... click the View button in the Files changed way up top.

[Martii]

as long as all links are changed too.

They are in this PR... I did a grep for BUILD in the project.

[Martii]

@nowrep
Do you really want those spaces? I don't see it in any of the specs yet. It's something I have to turn off if I contribute here... if it's a defined spec then I can accomodate but otherwise a unordered list might bring more attention to those items or even a table with markdown.

[nowrep]

Yes, I need the newlines in readme. When I last tried to remove the spaces, the rendered document had no newlines. Not sure if it is still needed (github preview indicates that it is not, so we may try it without).

[Martii]

I've literally never heard of this before... so what did you want me to do?

[nowrep]

https://github.com/Martii/qupzilla/blob/edbb1eff723409b169b2e201568ff25b64a8ad05/README.md it is needed, so keep the double spaces (for newlines) as they were.

It's good idea to configure your text editor to not fix whitespaces automatically, because no one likes patches full of whitespace changes.

[Martii]

because no one likes patches full of whitespace changes.

That's why you do this:

• https://github.com/QupZilla/qupzilla/pull/1971/files?w=1 (also done in this Userscript for the UI)

It's good idea to configure your text editor to not fix whitespaces automatically

It's actually a terrible idea to not fix whitespace.

[nowrep]

It's actually a terrible idea to not fix whitespace.

It's not, it makes it harder to find the right commit in history. And if you really want to fix the whitespaces, do it in separate commit.

[Martii]

It's not, it makes it harder to find the right commit in history. And if you really want to fix the whitespaces, do it in separate commit.

I'll put them back in for this but for future reference I will not be able to tell until it's up and they'll have to be backed out. You have no choice in this matter unless you try defining an editor config profile to override this... which is why it should be a separate issue.

Btw it's very simple to make all commits merged into one now so it doesn't muddy up the commit history here if that's what you are worried about. Use the UI.

[nowrep]

Btw it's very simple to make all commits merged into one now so it doesn't muddy up the commit history here if that's what you are worried about. Use the UI.

I'm not worried about this, there is an option to squash and merge. The problem is when you git blame and see the whitespace change commit which you really were not looking for.

[Martii]

Let me check to see if that did it.

[Martii]

Looks like I got the commit in before you merged... confusing here... GH went all wacky too.

[Martii]

@nowrep

The problem is when you git blame and see the whitespace change commit which you really were not looking for.

Try using:

$ git blame -w

So it's definitely a non-issue... not to mention this is the least feature of git that I would ever think of using. I do see some incorrect articles out there promoting what you are saying though... hopefully by better education everyone will learn.