Tarsnap / tarsnap

Command-line client code for Tarsnap.
https://tarsnap.com
Other
864 stars 60 forks source link

Fix OpenSSL detection on Apple silicon #583

Closed shawwn closed 10 months ago

shawwn commented 11 months ago

Currently, when macOS users clone tarsnap and try to run ./configure, M1 and M2 macs will run into an error saying they're missing OpenSSL:

~/ml/tarsnap $ ./configure
checking build system type... aarch64-apple-darwin22.5.0
checking host system type... aarch64-apple-darwin22.5.0
checking target system type... aarch64-apple-darwin22.5.0
...
checking for openssl/rsa.h... no
configure: error: *** OpenSSL header files missing ***

This PR detects whether the system is running arm64, and if so, searches /opt/homebrew instead of /usr/local.

The effect is that all macOS users (whether on Intel or Apple silicon) can clone tarsnap, run autoreconf && ./configure && make -j12 and the build process will work, assuming they've run brew install openssl.

gperciva commented 11 months ago

Hmm. Is the directory change actually related to arm64, or does it depend on the version of homebrew? Also, does MacPorts still exist? The wikipedia page suggests that it's still active.

I definitely agree that there's a problem, but I'm not convinced that this is the right way to fix it. I'm going to continue investigating.

gperciva commented 11 months ago

Ah, found it: homebrew defaults to /opt/homebrew for arm64, and /usr/local for intel. https://docs.brew.sh/Installation

So yes, this PR makes complete sense.

It does raise the question of "why doesn't our check_darwin_paths.m4 support macports, which installs to /opt/local?", but that's a separate issue.

shawwn commented 11 months ago

Ah, found it: homebrew defaults to /opt/homebrew for arm64, and /usr/local for intel. https://docs.brew.sh/Installation

So yes, this PR makes complete sense.

It does raise the question of "why doesn't our check_darwin_paths.m4 support macports, which installs to /opt/local?", but that's a separate issue.

For what it’s worth, I haven’t heard of anyone using macports, and I’ve worked with a lot of macOS devs. Especially now that intel macs are being phased out.

Oh, I should probably mention… before fixing this properly, I was trying to figure out how to pass extra flags to cc and ld so that I could inject my /opt/homebrew OpenSSL include and lib dirs into the build process. I finally got cc with ./configure CCFLAGS=“-I /opt/homebrew…” (I think?) and eventually figured out the corresponding ld flag, but it was obscure.

If you happen to know the proper way to pass -I and -L flags, I’d like to submit a PR explaining that in the build docs. That way if a macports user pops into existence in 2025 after being catapulted through time by a game of Jumanji, they’ll be able to point tarsnap at their /opt/local OpenSSL lib. (Or if someone builds OpenSSL from source and wants to link tarsnap to their local copy for some reason. Though I guess the system OpenSSL might accidentally take precedence in that case.)

gperciva commented 11 months ago

For what it’s worth, I haven’t heard of anyone using macports, and I’ve worked with a lot of macOS devs.

Good to know! That's something that we can't easily pick up from skimming the wikipedia page. :)

As for ./configure flags, at first glance I'd suggest that our instructions for scrypt should just be copied over: https://github.com/Tarsnap/scrypt/blob/master/BUILDING#L42

I suspect that I intended to copy those lines to tarsnap/BUILDING after the next scrypt release had been out for a few months, in case anybody found a problem with it. But that was 7 years ago, so I'm not certain what I was thinking.

gperciva commented 11 months ago

As part of your fixes, could you please rebase this PR on top of current master (in order to get #585, which should allow github actions to run).

gperciva commented 10 months ago

Thanks for the fix! This was merged as part of #594.