Support virtualenv/venv without separate cache #190

tkf · 2018-08-22T05:37:35Z

(To be rebased after #188)

With this patch, we should be able to have multiple Python virtual environments without duplicating Julia's precompilation cache, provided that those Python executables are linked against same libpython.

Since PyCall.jl only cares about the identity of libpython when it is already initialized #182 (comment), i.e., it does not call Py_SetPythonHome etc., I think we only need to compare the path to libpython.

@stevengj Is it correct? It seems that it works in my laptop.

tkf · 2018-08-22T23:28:41Z

julia/core.py

+                    cache = joinpath(path, "PyCall.ji")
+                    backup = joinpath(path, "PyCall.ji.backup")
+                    if isfile(cache)
+                        mv(cache, backup; remove_destination=true)


This is a bit orthogonal to this PR but I figured fixing #175 is required for passing the test. Merging this fix should also resolve #109 and #169.

In .travis.yml we have:

pyjulia/.travis.yml

Lines 62 to 64 in 138d47d

- if [ "$CROSS_VERSION" = "1" ]; then

$PYTHON -m tox -e py,py27 -- -s;

fi

Previously, use_separate_cache was True for both py and py27 virtual environments. Thus, no global cache ~/.julia/lib/v0.6/PyCall.ji existed while running the above test. However, with this patch, use_separate_cache is False in py because the code I added sees that PYTHON configured for PyCall uses the same libpython used by py virtual environment prepared by tox. So, after running tox -e py, there is a file ~/.julia/lib/v0.6/PyCall.ji. Now then running tox -e py27 hits the bug #175 because julia 0.6 does not recompile PyCall when it finds ~/.julia/lib/v0.6/PyCall.ji in Base.LOAD_CACHE_PATH[2].

tkf · 2018-08-22T23:40:25Z

julia/core.py

+        return libpython_from_ldd_output(subprocess.check_output(
+            ["ldd", path],
+            universal_newlines=True))
+# TODO: somebody has to write it for in Windows and macOS:


Note: it only works in Linux at the moment

tkf · 2018-08-24T02:50:35Z

julia/core.py

+            ["ldd", path],
+            universal_newlines=True))
+    return None
+# TODO: somebody has to write it for Windows and macOS:


Note: it only works in Linux at the moment

Why do we need this at all? Oh, I see, to compare the path of libpython.

tkf · 2018-08-28T04:55:25Z

I merged #188 and rebased this PR onto master. There are test failures in macOS. But they are likely to be due to a bug in the latest pytest pytest-dev/pytest#3888 (Those failures are coming from doctests. Functions tested there are pure Python "pure" functions. So it doesn't make sense to have different results in different OSes.)

@stevengj Can you have a look at it? Also please let me know if the idea of checking only the libpython path (my reasoning is in the issue description #190 (comment)) is the correct direction.

stevengj · 2018-08-28T13:15:00Z

It's annoying that finding libpython is so hard. See here for how PyCall does it.

The analogue of ldd on MacOS is otool -L, but that's only installed if the user installs the optional developer tools.

So, I think that we should fall back to searching the distutils.sysconfig.get_config_var('...') paths as PyCall does.

coveralls · 2018-08-29T01:02:14Z

Pull Request Test Coverage Report for Build 448

20 of 26 (76.92%) changed or added relevant lines in 1 file are covered.
6 unchanged lines in 1 file lost coverage.
Overall coverage increased (+3.7%) to 80.585%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
julia/core.py	20	26	76.92%

Files with Coverage Reduction	New Missed Lines	%
julia/core.py	6	87.46%

Totals
Change from base Build 447:	3.7%
Covered Lines:	303
Relevant Lines:	376

💛 - Coveralls

coveralls · 2018-08-29T01:02:14Z

Pull Request Test Coverage Report for Build 467

106 of 144 (73.61%) changed or added relevant lines in 2 files are covered.
4 unchanged lines in 1 file lost coverage.
Overall coverage increased (+1.6%) to 78.427%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
julia/core.py	15	25	60.0%
julia/find_libpython.py	91	119	76.47%

Files with Coverage Reduction	New Missed Lines	%
julia/core.py	4	84.87%

Totals
Change from base Build 447:	1.6%
Covered Lines:	389
Relevant Lines:	496

💛 - Coveralls

ihnorton · 2018-08-29T01:12:54Z

The analogue of ldd on MacOS is otool -L, but that's only installed if the user installs the optional developer tools.

FWIW, there is a pure-Julia library which can do this: https://github.com/staticfloat/ObjectFile.jl (see find_libraries).

fixes JuliaPy#175

tkf · 2018-08-29T05:35:51Z

@ihnorton Thanks. It looks like a good solution. Maybe PyCall.jl can use it in the build script and also expose it as a script tool. Then PyJulia can just call it.

But I've already ported find_libpython to a Python function :) I was actually about to suggest to use the script find_libpython.py I added from PyCall.jl as well since there is no point in repeating this and doing from Python is slightly easier. But if there is an out-of-the-box solution in Julia why don't we use it.

tkf · 2018-08-29T05:36:15Z

Actually, maybe we should just fetch dli_fbase as done in PyCall/src/startup.jl?
https://github.com/JuliaPy/PyCall.jl/blob/9224c606545d7659ab89d8389ac9f44b1f45d1f9/src/startup.jl#L64-L77
(Edit: implemented in 435ccf4)

tkf · 2018-08-29T11:12:36Z

appveyor.yml

-  # 32 julia latest Python-35
-  - JULIA_URL: "https://julialangnightlies-s3.julialang.org/bin/winnt/x86/julia-latest-win32.exe"
+  # 32 julia-1.0 Python-35
+  - JULIA_URL: "https://julialang-s3.julialang.org/bin/winnt/x86/1.0/julia-1.0-latest-win32.exe"


Testing with Julia nightly yields some bizarre failures (see below). The same code works with Julia 1.0. I suggest to test with stable release.

From Python 3.5:

ErrorException("ccall: could not find function PyString_AsStringAndSize in library C:\projects\pyjulia\.tox\py\Scripts\python35")
--- https://ci.appveyor.com/project/Keno/pyjulia/build/1.0.175/job/wwr318to7jr1b5h6#L433

From Python 2.7:

def test_call_julia_function_with_python_args(self): self.assertEqual(['A', 'B', 'C'], list(julia.map(julia.uppercase, > array.array('u', [u'a', u'b', u'c'])))) E RuntimeError: Julia exception: MethodError(uppercase, (PyObject u'a',), 0x000061ca)

--- https://ci.appveyor.com/project/Keno/pyjulia/build/1.0.175/job/wwr318to7jr1b5h6#L547

tkf · 2018-08-29T11:40:07Z

julia/core.py

+    if is_same_path(jlinfo.pyprogramname, sys.executable):
+        # In macOS and Windows, find_libpython does not work as good
+        # as in Linux.  We add this shortcut so that PyJulia can work
+        # in those environments.


I needed this shortcut to make it work since find_libpython.py and PyCall/deps/build.jl sometimes disagree:

For example, in macOS, PyCall/deps/build.jl finds:

L957 jlinfo.libpython = /usr/local/opt/python/Frameworks/Python.framework/Versions/3.7/lib/libpython3.7m

but find_libpython.py finds:

L958 py_libpython = /usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/Python

Another example in Windows: PyCall/deps/build.jl cannot find a fullpath:

L517 jlinfo.libpython = python27

but find_libpython.py "finds" it:

L518 py_libpython = C:\Windows\system32\python27.dl

(But it looks like the same path was found in win32 and win64. Ultimately, we need someone to write a function that uses dladdr equivalent in Windows. It should be possible by just extending: https://stackoverflow.com/a/16659821)

I think it's better to use the same script to discover libpython in PyCall.jl and PyJulia. We can then avoid a hack like this. Since find_libpython.py can call libdl.dladdr (though it works only if libpython is dynamically linked), I think it's better approach.

In the cases where they disagree, which is correct? Should deps/build.jl be fixed?

I don't have access to macOS so I'm not sure. But PyJulia printed

jlinfo.libpython = /usr/local/opt/python/Frameworks/Python.framework/Versions/3.7/lib/libpython3.7m py_libpython = /usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/Python jl_libpython = None

where jlinfo.libpython is libpython saved in deps/deps.jl and jl_libpython is the path I tried to resolved by

pyjulia/julia/find_libpython.py

Lines 176 to 182 in 6982ae8

if not os.path.isabs(path):

return None

if os.path.exists(path):

return os.path.realpath(path)

if os.path.exists(path + suffix):

return os.path.realpath(path + suffix)

return None

~~So jl_libpython = None means that libpython3.7m.dylib did not exist (I set suffix = ".dylib" in macOS). Maybe we should try libpython3.7m.so in macOS too?~~

In the case of Windows, deps/build.jl just gave up (jlinfo.libpython = python27) whereas ctypes.util.find_library finds C:\Windows\system32\python27.dl.

~~I'm trying it: 1d599be~~

os.name and sys.platform are more well-defined/documented and we don't need more detail than them.

tkf · 2018-08-29T23:09:58Z

Re: #190 (comment)

So with some more manual handling in macOS cdbefb1 now normalize_path can normalize what deps/build.jl finds to what find_libpython.py finds.

jlinfo.libpython = /usr/local/opt/python/Frameworks/Python.framework/Versions/3.7/lib/libpython3.7m
py_libpython = /usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/Python
jl_libpython = /usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/Python
use_separate_cache = False

--- https://travis-ci.org/JuliaPy/pyjulia/jobs/422307471#L954

tkf · 2018-08-29T23:16:42Z

julia/find_libpython.py

+if is_apple:
+    # sysconfig.get_config_var("SHLIB_SUFFIX") can be ".so" in macOS.
+    # Let's not use the value from sysconfig.
+    SHLIB_SUFFIX = ".dylib"


Here is the output of python -m sysconfig from Python 3.7 in macOS https://travis-ci.org/JuliaPy/pyjulia/jobs/421924267#L1439

tkf · 2018-08-29T23:29:17Z

So to directly answer #190 (comment) deps/build.jl and find_libpython.py agree in all cases where deps/build.jl finds the path.

That is to say, they both agree in Linux and macOS. In Windows and Python 2.7, deps/build.jl does not find libpython but find_libpython.py finds it via ctypes.util.find_library. Presumably, this is equivalent to what dlopen does. Python document says:

The purpose of the find_library() function is to locate a library in a way similar to what the compiler or runtime loader does (on platforms with several versions of a shared library the most recent should be loaded), while the ctypes library loaders act like when a program is run, and call the runtime loader directly.

--- https://docs.python.org/3/library/ctypes.html#finding-shared-libraries

stevengj · 2018-09-03T14:38:31Z

Sounds like deps/build.jl should be updated, then.

tkf · 2018-09-03T21:39:41Z

Why don't we use find_libpython.py in PyCall.jl as well? Since we can use dladdr/GetModuleFileName inside a Python process (if libpython is dynamically linked), I think this is much more robust solution.

tkf force-pushed the venv branch 2 times, most recently from 8a459a9 to 6bd383f Compare August 22, 2018 22:34

tkf commented Aug 22, 2018

View reviewed changes

tkf commented Aug 24, 2018

View reviewed changes

tkf mentioned this pull request Aug 24, 2018

Add IPython completion hook in julia.magic #193

Closed

tkf force-pushed the venv branch from 2b52b7c to 563dfd5 Compare August 28, 2018 03:38

tkf mentioned this pull request Aug 29, 2018

PyJulia does not work with Julia 1.0 & 0.7 with Python installed via Conda/Ubuntu/(what else?) #185

Open

tkf force-pushed the venv branch from 563dfd5 to 0389d41 Compare August 29, 2018 00:43

tkf added 2 commits August 28, 2018 18:22

Avoid the cryptic error about fake-julia/../lib/julia/sys.so

5290c8e

More robust pre-compilation cache handling for Julia 0.6

d49d819

fixes JuliaPy#175

tkf force-pushed the venv branch from 0389d41 to d29b149 Compare August 29, 2018 05:07

tkf added 3 commits August 28, 2018 22:08

Run doctest via pytest

a748dea

Don't install pytest 3.7.3 to avoid doctest failures in macOS

54d1116

Make PyJulia usable in virtual environments

1825d34

tkf force-pushed the venv branch from d29b149 to 1825d34 Compare August 29, 2018 05:08

Python < 3.7 does not support namedtuple(..., defaults=...)

1da6243

tkf force-pushed the venv branch from 996ba30 to 1da6243 Compare August 29, 2018 05:15

tkf added 3 commits August 28, 2018 22:46

Print debug message from is_compatible_exe

252cd55

More permissive test for finding_libpython

2ab64e7

Run find_libpython in tox

1f47404

tkf force-pushed the venv branch from de08a14 to 1f47404 Compare August 29, 2018 05:54

Use absolute_import in find_libpython.py

9adb074

Print jlinfo.libpython

6e91cf9

tkf force-pushed the venv branch from 0fd6d13 to d7b14b0 Compare August 29, 2018 07:50

tkf added 4 commits August 29, 2018 01:13

Find the linked libpython using dladdr

435ccf4

Manually set SHLIB_SUFFIX in non-Linux platforms

25631af

Use sysconfig.get_config_var("VERSION")

e98c265

Directly compare executable paths

f09ae42

tkf force-pushed the venv branch from d7b14b0 to f09ae42 Compare August 29, 2018 08:13

tkf added 2 commits August 29, 2018 02:03

Print PyCall/deps/build.log

96429c6

Test against 1.0 instead of nightly in AppVeyor

6982ae8

tkf force-pushed the venv branch from 9852109 to 6982ae8 Compare August 29, 2018 10:36

tkf commented Aug 29, 2018

View reviewed changes

Do not use platform.system()

fe0cd97

os.name and sys.platform are more well-defined/documented and we don't need more detail than them.

tkf force-pushed the venv branch from dc874ad to c896cb1 Compare August 29, 2018 22:28

More manual suffix handling for macOS

cdbefb1

tkf force-pushed the venv branch from c896cb1 to d72878f Compare August 29, 2018 22:49

tkf force-pushed the venv branch from d72878f to cdbefb1 Compare August 29, 2018 23:10

tkf commented Aug 29, 2018

View reviewed changes

This was referenced Aug 31, 2018

Use correct Python version in x64 AppVeyor JuliaPy/PyCall.jl#544

Merged

Julia 1.0 Error : julia.core.JuliaError: Exception 'UndefVarError' occurred while calling julia code: #196

Closed

tkf mentioned this pull request Sep 5, 2018

Improve juliainfo #197

Merged

stevengj merged commit 9d1833f into JuliaPy:master Sep 5, 2018

tkf mentioned this pull request Sep 6, 2018

Use find_libpython.py in deps/build.jl JuliaPy/PyCall.jl#556

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support virtualenv/venv without separate cache #190

Support virtualenv/venv without separate cache #190

tkf commented Aug 22, 2018

tkf Aug 22, 2018

tkf Aug 22, 2018

tkf Aug 24, 2018

stevengj Aug 28, 2018

tkf commented Aug 28, 2018

stevengj commented Aug 28, 2018

coveralls commented Aug 29, 2018

coveralls commented Aug 29, 2018 •

edited

Loading

ihnorton commented Aug 29, 2018

tkf commented Aug 29, 2018

tkf commented Aug 29, 2018 •

edited

Loading

tkf Aug 29, 2018

tkf Aug 29, 2018

stevengj Aug 29, 2018

tkf Aug 29, 2018 •

edited

Loading

tkf Aug 29, 2018 •

edited

Loading

tkf commented Aug 29, 2018

tkf Aug 29, 2018

tkf commented Aug 29, 2018

stevengj commented Sep 3, 2018

tkf commented Sep 3, 2018

	- if [ "$CROSS_VERSION" = "1" ]; then
	$PYTHON -m tox -e py,py27 -- -s;
	fi

	if not os.path.isabs(path):
	return None
	if os.path.exists(path):
	return os.path.realpath(path)
	if os.path.exists(path + suffix):
	return os.path.realpath(path + suffix)
	return None

Support virtualenv/venv without separate cache #190

Support virtualenv/venv without separate cache #190

Conversation

tkf commented Aug 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkf commented Aug 28, 2018

stevengj commented Aug 28, 2018

coveralls commented Aug 29, 2018

Pull Request Test Coverage Report for Build 448

💛 - Coveralls

coveralls commented Aug 29, 2018 • edited Loading

Pull Request Test Coverage Report for Build 467

💛 - Coveralls

ihnorton commented Aug 29, 2018

tkf commented Aug 29, 2018

tkf commented Aug 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkf Aug 29, 2018 • edited Loading

Choose a reason for hiding this comment

tkf Aug 29, 2018 • edited Loading

Choose a reason for hiding this comment

tkf commented Aug 29, 2018

Choose a reason for hiding this comment

tkf commented Aug 29, 2018

stevengj commented Sep 3, 2018

tkf commented Sep 3, 2018

coveralls commented Aug 29, 2018 •

edited

Loading

tkf commented Aug 29, 2018 •

edited

Loading

tkf Aug 29, 2018 •

edited

Loading

tkf Aug 29, 2018 •

edited

Loading