Linuxbrew / legacy-linuxbrew

:skull: This repository is defunct, because it has been split into https://github.com/Linuxbrew/brew and https://github.com/Linuxbrew/homebrew-core
http://linuxbrew.sh
Other
2.23k stars 296 forks source link

cmake: fix https support in linux by using system curl #1064

Closed skystrife closed 8 years ago

skystrife commented 8 years ago

It's right to have cmake depend on curl on Linux, but the if/else block that disables system curl on old OS X versions was actually firing for Linux as well. The symptom is that all ExternalProject_Add() calls that used a https url failed to work.

This includes a revision bump because the bottle needs to be rebuilt to link against the linuxbrew curl (and OpenSSL). I assume that's right?

sjackman commented 8 years ago
  --system-curl           use system-installed curl library
  --no-system-curl        use cmake-provided curl library (default)
sjackman commented 8 years ago

Can you please quote here or gist the exact error message that you're seeing? I haven't had any trouble with cmake myself downloading from https.

This patch should probably also need

depends_on "curl" unless OS.mac?
skystrife commented 8 years ago

I'm away from my machine right now, but it is the exact same error reported here: http://stackoverflow.com/questions/29816529/unsupported-protocol-while-downlod-tar-gz-package

CMake, when compiled with its own curl, does not enable OpenSSL by default. Using the "system" curl (brewed curl) fixes this.

This patch should probably also need

depends_on "curl" unless OS.mac?

This is already in the formula.

sjackman commented 8 years ago

I'm not able to reproduce your error. Please report the output of:

cat >CMakeLists.txt <<EOF
cmake_minimum_required(VERSION 3.5)
file(DOWNLOAD https://github.com/lh3/bwa/releases/download/v0.7.13/bwa-0.7.13.tar.bz2 ./bwa-0.7.13.tar.bz2)
EOF
cmake .
sha256sum bwa-0.7.13.tar.bz2

I get this result:

$ cmake .
-- Configuring done
-- Generating done
-- Build files have been written to: /home/sjackman/tmp/cmake
$ sha256sum bwa-0.7.13.tar.bz2 
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  bwa-0.7.13.tar.bz2
skystrife commented 8 years ago

The problem is not with file(DOWNLOAD ...), but with ExternalProject_Add(...) as specified in the commit and the starting post for this PR.

Here is a reproduction on my system:

cat >CMakeLists.txt <<EOF
cmake_minimum_required(VERSION 3.5)
include(ExternalProject)
ExternalProject_Add(bwa
SOURCE_DIR "."
DOWNLOAD_DIR "."
URL https://github.com/lh3/bwa/releases/download/v0.7.13/bwa-0.7.13.tar.bz2
URL_HASH "SHA256=e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
INSTALL_COMMAND "")
EOF
cmake .
make

Which gives me this:

Scanning dependencies of target bwa
[ 12%] Creating directories for 'bwa'
[ 25%] Performing download step (download, verify and extract) for 'bwa'
-- downloading...
     src='https://github.com/lh3/bwa/releases/download/v0.7.13/bwa-0.7.13.tar.bz2'
     dst='/home/meta/derp/bwa-0.7.13.tar.bz2'
     timeout='none'
CMake Error at derp/bwa-prefix/src/bwa-stamp/download-bwa.cmake:27 (message):
  error: downloading
  'https://github.com/lh3/bwa/releases/download/v0.7.13/bwa-0.7.13.tar.bz2'
  failed

    status_code: 1
    status_string: "Unsupported protocol"
    log: Protocol "https" not supported or disabled in libcurl

  Closing connection -1

make[2]: *** [bwa-prefix/src/bwa-stamp/bwa-download] Error 1
make[1]: *** [CMakeFiles/bwa.dir/all] Error 2
make: *** [all] Error 2

Now, why these are different is completely beyond me. But they are, and this problem doesn't occur with the modifications in this PR.

sjackman commented 8 years ago

Thanks for the test case. I can now reproduce your error, and the patch is good. I haven't had to add a revision for a Linuxbrew bottle yet. I'll think on that. I may just delete the old bottle and replace it.

skystrife commented 8 years ago

Cool!

RE: revision bump---In that case, people who have the old bottle (without https support in ExternalProject) won't be made aware of the update though, right?

sjackman commented 8 years ago

That's the downside to what I propose. Oddly, yours is the first bug report I've seen about this error.

heinemml commented 8 years ago

I just noticed that this is basically the same problem I tried to fix here: https://github.com/Linuxbrew/homebrew-core/pull/383

sjackman commented 8 years ago

Superseded by https://github.com/Linuxbrew/homebrew-core/pull/383