cgat-developers / cgat-core

Core functionality of the CGAT code
MIT License
31 stars 13 forks source link

SSL connect error #44

Closed bio-la closed 6 years ago

bio-la commented 6 years ago

Hi, just got this error, i'm not sure if it's suggesting I should use another version of curl, or any other dependency needs updating:

curl: (35) SSL connect error

##########################################################

An error occurred in:

The script will abort now. User input was:

bash install-CGAT-tools.sh --devel

Debugging: CFLAGS: CPATH: C_INCLUDE_PATH: /usr/include:/usr/X11R6/include:/usr/local/include CPLUS_INCLUDE_PATH: /usr/include:/usr/X11R6/include:/usr/local/include LIBRARY_PATH: /lib:/usr/lib:/usr/X11R6/lib:/usr/local/lib LD_LIBRARY_PATH: /lib:/usr/lib:/usr/X11R6/lib:/usr/local/lib CGAT_HOME: /home/fcurion/cgat-install CONDA_INSTALL_DIR: /home/fcurion/cgat-install/conda-install CONDA_INSTALL_TYPE: core-devel.yml CONDA_INSTALL_ENV: cgat-c PYTHONPATH: INSTALL_BRANCH: master RELEASE: CODE_DOWNLOAD_TYPE: 0

sebastian-luna-valero commented 6 years ago

Hi,

Could you please copy and paste the output of:

curl -v -LOk https://github.com/cgat-developers/cgat-core/archive/master.zip

Do you get master.zip after running that command?

Best regards, Sebastian

bio-la commented 6 years ago

Hi, I do get the master.zip after running

curl -v -LOk https://github.com/cgat-developers/cgat-core/archive/master.zip

here's the output - sorry it's very long

sebastian-luna-valero commented 6 years ago

That's strange. Exit status initially was 35, which according to: https://ec.haxx.se/usingcurl-returns.html

means:

A TLS/SSL connect error. The SSL handshake failed. The SSL handshake can fail due to numerous different reasons so the error message may offer some additional clues. Maybe the parties couldn't agree to a SSL/TLS version, an agreeable cipher suite or similar

You should have reproduced that with the manual curl command.

I suggest trying the installation again. Please do:

# get rid of previous installation folder
cd /home/fcurion
rm -rf  cgat-install

# try again
bash install-CGAT-tools.sh --devel

Please let me know how it goes.

Best regards, Sebastian

snsansom commented 6 years ago

The reason for this is the call to the following function:

# clean up environment
# deliberately use brute force
cleanup_env() {
   set +e
   source deactivate >& /dev/null || true
   source deactivate >& /dev/null || true
   unset -f conda || true
   unset PYTHONPATH || true
   module purge >& /dev/null || true
   mymodule purge >& /dev/null || true
   set -e
}

If the user is on a system on which curl is installed as a module, this will unload it. On our systems, this drops us back to an ancient centos6 version of curl and breaks the script.

Ensuring that we're not in a conda environment seems reasonable-ish, but purging the user modules looks very inappropriate and unnecessary to me!

sebastian-luna-valero commented 6 years ago

We are routinely using a centos6 system for continuous integration and we are not experiencing this issue with curl (version 7.19 as we speak).

The reason for unloading the user modules before going ahead with the installation is that you might have Python and R packages loaded, which could conflict with the Python and R packages going to be installed. Purging user modules should ensure a clean installation.

When I first had a look at this issue, I thought it was sporadic. Does it happen in your end consistently?

snsansom commented 6 years ago

You do not need to, and should not touch user modules in such an installation script. This is the same as trying to remove system software! - on many systems it would also break e.g. the compilation tool chain.

The only check needed would be whether you are in a conda env at which point the script can raise a warning and exit.

bio-la commented 6 years ago

sorry @sebastian-luna-valero I should have replied to this thread before. bash install-CGAT-tools.sh --devel still gave me errors (@snsansom spotted the issue). Eventually, I just bypassed the problem by downloading the master.zip with curl -v -LOk https://github.com/cgat-developers/cgat-core/archive/master.zip, and used the unzipped content like this: cgat-core-master/install-CGAT-tools.sh --devel --location cgat-core-test (my folder) --branch master --git-ssh

thanks for the support!

snsansom commented 6 years ago

(also worth noting that this function does not ensure that Python/R are not on the path as almost all systems will have a least python installed via a package manger)

snsansom commented 6 years ago

Unless there are conda docs which state that this:

   unset PYTHONPATH || true
   module purge >& /dev/null || true
   mymodule purge >& /dev/null || true

is necessary (or recommended practise), can we please not do it?

snsansom commented 6 years ago

Yes issue is consistent.

I checked the conda docs and can’t see that such pre-cleaning of environment variables is needed. Taking care of such things is central to Conda’s purpose as I understand it?

sebastian-luna-valero commented 6 years ago

Many thanks for your input and sorry for the hassle.

I will open a pull request to get rid of offending code.

snsansom commented 6 years ago

Hi Sebastian,

Many thanks for taking care of this.

I think it will help to avoid what are potentially difficult-to-diagnose installation problems in future.

I would suggest that we also do the same for the cgat-flow and cgat-apps repos.

Best wishes, Steve

sebastian-luna-valero commented 6 years ago

Sorry this is taking longer than expected to merge. There is currently an issue with sqlite on OS X.

While I wait for feedback in https://github.com/conda/conda/issues/7553 I would like to do one more test.

@snsansom would you be able to start a clean bash terminal on your CentOS 6 environment without user modules? Specifically, comment out the module load statements in your .bashrc. Then run the installer and let me know if you still get the curl issue.

sebastian-luna-valero commented 6 years ago

FYI: If you do:

curl -O https://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh
export PYTHONPATH=example
bash Miniconda3-latest-Linux-x86_64.sh -b -p test

The installer complains with:

WARNING:
    You currently have a PYTHONPATH environment variable set. This may cause
    unexpected behavior when running the Python interpreter in Miniconda3.
    For best results, please verify that your PYTHONPATH only points to
    directories of packages that are compatible with the Python interpreter
    in Miniconda3: /tmp/tmp.SJsuMTkjVA/test

That's why I unset PYTHONPATH in the installer as well.

sebastian-luna-valero commented 6 years ago

Update: @snsansom I have managed to enforce sqlite on OS X so the pending issue seems to be solved. However, I would like to get your feedback from my last comments before merging.

sebastian-luna-valero commented 6 years ago

I am planning to merge these changes in the next hour. Finally, I have commented out the module part so user modules aren't touched as requested. However, I have kept the unset PYTHONPATH since it looks like a good idea. Please feel free to provide feedback before or after the merge. Happy to discuss further.