ARROW-848: [Python] Another pass on conda dev guide #562

wesm · 2017-04-18T15:28:24Z

Per feedback in bb8514c

…all everything in a single conda environment Change-Id: I4a655e39df1c3ffe08864e6345f390a2dfd04213

xhochy · 2017-04-18T15:47:58Z

python/DEVELOPMENT.md

+```
+export ARROW_BUILD_TYPE=release
+
+export BOOST_ROOT=$CONDA_PREFIX


We definitely should invest into getting all those variables into a single one. Better would be even that we could detect if we're in a conda env and try to prefer the libraries installed there.

We could e.g. just try and search first for all dependencies in $CMAKE_INSTALL_PREFIX

What if we had an $ARROW_BUILD_TOOLCHAIN that automatically set all of the *_HOME variables?

Yes, that is in scope I would hope for :)

see ARROW-849 and PARQUET-957

Change-Id: Ib38249c4a85ed414ef4a9c669c3d5aaa488339fc

xhochy · 2017-04-18T16:46:01Z

I can reproduce @mrocklin problem on a fresh Ubuntu 17.04 installation: https://gist.github.com/xhochy/7bb0e4d47cbe25b6d79a3d6b1db7a9ea

wesm · 2017-04-18T16:52:08Z

Using these latest instructions?

xhochy · 2017-04-18T16:54:36Z

Yes, made a JIRA about this as I probably don't have time today to fix it: https://issues.apache.org/jira/browse/ARROW-850

wesm · 2017-04-18T16:55:15Z

If you build with ARROW_BOOST_USE_SHARED=on do things work in this environment? I can set up a VM later to try it out myself

xhochy · 2017-04-18T16:59:23Z

This leads to another problem (C++ ABI mismatch): https://gist.github.com/xhochy/01d0a8cbbea3f38cbb85dbb70a5ec2a5

wesm · 2017-04-18T17:02:25Z

Right, you can't use gcc >= 5 with the conda-forge stack (unless you disable the gcc5 ABI), so we'll need to include instructions to either install gcc 4.9 on newer Linuxes or add compiler flags.

xhochy · 2017-04-18T17:02:48Z

cmake -DARROW_BOOST_USE_SHARED=on CMAKE_CXX_FLAGS="-D_GLIBCXX_USE_CXX11_ABI=0" did the trick.

xhochy · 2017-04-18T17:03:45Z

I was using gcc-5 before but on an older Ubuntu version that by default was using the old ABI. The new Ubuntu release switched the default ABI.

wesm · 2017-04-18T17:04:33Z

excellent intel, thanks @xhochy!

xhochy · 2017-04-18T17:13:28Z

In my setup, this still needs export LD_LIBRARY_PATH=$CONDA_PREFIX/lib

xhochy · 2017-04-18T17:22:07Z

JIRA for the CXX ABI: https://issues.apache.org/jira/browse/ARROW-851

wesm · 2017-04-18T17:29:57Z

@xhochy at what point do you need the LD_LIBRARY_PATH?

xhochy · 2017-04-18T17:35:07Z

For running the tests in pyarrow.

wesm · 2017-04-18T17:37:52Z

Very odd. And the conda environment is activated?

xhochy · 2017-04-18T17:40:18Z

Yes

Change-Id: I964df5aab950061326119fb4672da721afff3382

wesm · 2017-04-19T01:04:55Z

@xhochy I figured out the problem -- after the libarrow_python refactor, for conda-based setups, we do not need to modify the Cython module RPATH. I think you still need to do this for manylinux1 builds. I found that removing

    if(APPLE)
        set(module_install_rpath "@loader_path")
    else()
        set(module_install_rpath "\$ORIGIN")
    endif()
    list(LENGTH directories i)
    while(${i} GREATER 0)
        set(module_install_rpath "${module_install_rpath}/..")
        math(EXPR i "${i} - 1" )
    endwhile(${i} GREATER 0)

    set_target_properties(${module_name} PROPERTIES
      INSTALL_RPATH ${module_install_rpath})

solved the problem. I'm opening a new JIRA about this

See discussion in #562. Modifying RPATH is no longer needed when libarrow/libarrow_python are installed someplace else in the loader path. Author: Wes McKinney <wes.mckinney@twosigma.com> Closes #564 from wesm/ARROW-853 and squashes the following commits: 262f43a [Wes McKinney] Only set RPATH when bundling the shared libraries

cpcloud · 2017-04-19T19:19:02Z

python/DEVELOPMENT.md

-conda create -y -q -p $CPP_TOOLCHAIN \
-      flatbuffers rapidjson boost-cpp thrift-cpp snappy zlib brotli jemalloc
+```shell
+export CC=gcc-4.9


Should this be export CC=$(which gcc-4.9)? (and same for CXX)

either work, but I'll change to the more explicit variety.

The output of $(which FOO) doesn't produce an executable command for me when FOO is found multiple times in the path. I'll leave this as is for now

$ which gcc gcc is /home/wesm/.conda/envs/$NAME/bin/gcc gcc is /usr/bin/gcc

Sounds good.

cpcloud · 2017-04-19T19:20:40Z

python/DEVELOPMENT.md

-      -DCMAKE_INSTALL_PREFIX=$CPP_TOOLCHAIN \
+      -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX \
      -DARROW_PYTHON=on \
      -DARROW_BUILD_TESTS=OFF \


Tiny nit: Maybe make the case consistent here.

cpcloud · 2017-04-19T19:21:49Z

python/DEVELOPMENT.md

+export ARROW_BUILD_TYPE=release
+
+export BOOST_ROOT=$CONDA_PREFIX
+export BOOST_LIBRARYDIR=$CONDA_PREFIX/lib


Will this need to be lib64 when the system is 64 bit?

conda-forge hardcodes lib https://github.com/conda-forge/boost-cpp-feedstock/blob/master/recipe/build.sh#L14

wesm · 2017-04-19T19:48:18Z

+1. We can improve after adding the *_TOOLCHAIN variables

See discussion in apache#562. Modifying RPATH is no longer needed when libarrow/libarrow_python are installed someplace else in the loader path. Author: Wes McKinney <wes.mckinney@twosigma.com> Closes apache#564 from wesm/ARROW-853 and squashes the following commits: 262f43a [Wes McKinney] Only set RPATH when bundling the shared libraries

Per feedback in apache@bb8514c Author: Wes McKinney <wes.mckinney@twosigma.com> Closes apache#562 from wesm/conda-quickstart-iterate and squashes the following commits: 881a44d [Wes McKinney] Add system requirements notes about gcc 4.9, use boost shared libs 8c95705 [Wes McKinney] Install cmake in conda env 8c2885e [Wes McKinney] Another pass on conda dev guide, do not require LD_LIBRARY_PATH. Install everything in a single conda environment

…apache#562) Bumps com.gradle:develocity-maven-extension from 1.23 to 1.23.1. Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Another pass on conda dev guide, do not require LD_LIBRARY_PATH. Inst…

8c2885e

…all everything in a single conda environment Change-Id: I4a655e39df1c3ffe08864e6345f390a2dfd04213

xhochy reviewed Apr 18, 2017

View reviewed changes

Install cmake in conda env

8c95705

Change-Id: Ib38249c4a85ed414ef4a9c669c3d5aaa488339fc

Add system requirements notes about gcc 4.9, use boost shared libs

881a44d

Change-Id: I964df5aab950061326119fb4672da721afff3382

wesm mentioned this pull request Apr 19, 2017

ARROW-853: [Python] Only set RPATH when bundling the shared libraries #564

Closed

cpcloud reviewed Apr 19, 2017

View reviewed changes

asfgit closed this in 391242a Apr 19, 2017

wesm deleted the conda-quickstart-iterate branch April 19, 2017 19:49

asfimport mentioned this pull request Apr 19, 2017

[Python] It is no longer necessary to modify the RPATH of the Cython extensions on many environments #16460

Closed

ARROW-848: [Python] Another pass on conda dev guide #562

ARROW-848: [Python] Another pass on conda dev guide #562

Uh oh!

Conversation

wesm commented Apr 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wesm Apr 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xhochy commented Apr 18, 2017

Uh oh!

wesm commented Apr 18, 2017

Uh oh!

xhochy commented Apr 18, 2017

Uh oh!

wesm commented Apr 18, 2017

Uh oh!

xhochy commented Apr 18, 2017

Uh oh!

wesm commented Apr 18, 2017

Uh oh!

xhochy commented Apr 18, 2017

Uh oh!

xhochy commented Apr 18, 2017

Uh oh!

wesm commented Apr 18, 2017

Uh oh!

xhochy commented Apr 18, 2017

Uh oh!

xhochy commented Apr 18, 2017

Uh oh!

wesm commented Apr 18, 2017

Uh oh!

xhochy commented Apr 18, 2017

Uh oh!

wesm commented Apr 18, 2017

Uh oh!

xhochy commented Apr 18, 2017

Uh oh!

wesm commented Apr 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wesm commented Apr 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wesm commented Apr 18, 2017 •

edited

Loading

wesm Apr 18, 2017 •

edited

Loading