Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: python #84

Closed
wants to merge 3 commits into from
Closed

WIP: python #84

wants to merge 3 commits into from

Conversation

pelson
Copy link
Member

@pelson pelson commented Mar 10, 2016

No description provided.

@pelson
Copy link
Member Author

pelson commented Mar 10, 2016

Should be an interesting one. I expect conda-build-all to work nicely in this repo, but suspect that we will trip up when it comes to conda-building the recipe in the feedstock (as a special environment var needs setting CONDA_PY). It is a good opportunity for testing conda-forge at least.

@pelson
Copy link
Member Author

pelson commented Mar 10, 2016

Looks like we should have the unxz package on the docker image to decompress xy files.

@pelson
Copy link
Member Author

pelson commented Mar 10, 2016

 readline: 6.2-2 defaults setuptools: 20.2.2-py35_0 defaults sqlite: 3.9.2-0 defaults wheel: 0.29.0-py35_0 defaults zlib: 1.2.8-0 defaults ===== testing package: python-3.5.1-0 ===== import: 'sys' import: 'os' import: 'sqlite3' import: 'readline' Traceback (most recent call last): File "/opt/conda/conda-bld/test-tmp_dir/run_test.py", line 34, in <module> import readline ImportError: No module named 'readline' TESTS FAILED: python-3.5.1-0 ./scripts/run_docker_build.sh returned exit code 1

I've been getting a similar failure locally. Looks like readline isn't building correctly.

@pelson
Copy link
Member Author

pelson commented Mar 10, 2016

building 'readline' extension
gcc -pthread -fPIC -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -Werror=declaration-after-statement -I./Include -I/opt/conda/envs/_build/include -I. -IInclude -I/usr/local/include -I/opt/conda/conda-bld/work/Python-3.5.1/Include -I/opt/conda/conda-bld/work/Python-3.5.1 -c /opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c -o build/temp.linux-x86_64-3.5/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.o
In file included from /opt/conda/envs/_build/include/readline/readline.h:36,
                 from /opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c:32:
/opt/conda/envs/_build/include/readline/rltypedefs.h:34: warning: function declaration isn't a prototype
/opt/conda/envs/_build/include/readline/rltypedefs.h:35: warning: function declaration isn't a prototype
/opt/conda/envs/_build/include/readline/rltypedefs.h:36: warning: function declaration isn't a prototype
/opt/conda/envs/_build/include/readline/rltypedefs.h:37: warning: function declaration isn't a prototype
In file included from /opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c:32:
/opt/conda/envs/_build/include/readline/readline.h:381: warning: function declaration isn't a prototype
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c: In function 'flex_complete':
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c:1003: warning: passing argument 1 of 'completion_matches' discards qualifiers from pointer target type
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c:40: note: expected 'char *' but argument is of type 'const char *'
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c: In function 'setup_readline':
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c:1059: warning: will never be executed
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c: In function 'on_completion':
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c:960: warning: will never be executed
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c: In function 'on_hook':
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c:842: warning: will never be executed
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c: In function 'call_readline':
/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.c:1262: warning: will never be executed
gcc -pthread -shared build/temp.linux-x86_64-3.5/opt/conda/conda-bld/work/Python-3.5.1/Modules/readline.o -L/usr/lib/termcap -L. -L/opt/conda/envs/_build/lib -L/usr/local/lib -lreadline -lpython3.5m -o build/lib.linux-x86_64-3.5/readline.cpython-35m-x86_64-linux-gnu.so
*** WARNING: renaming "readline" since importing it failed: /opt/conda/envs/_build/lib/libreadline.so.6: undefined symbol: PC

Python build finished successfully!
The necessary bits to build these optional modules were not found:
_curses               _curses_panel         _dbm               
_gdbm                 _lzma                 _tkinter           
To find the necessary bits, look in setup.py in detect_modules() for the module's name.


Following modules built successfully but were removed because they could not be imported:
readline 

@jjhelmus
Copy link
Contributor

Looks like we should have the unxz package on the docker image to decompress xy files.

Python.org also supplies a .tgz file which might be a better choice. The manylinux1 Docker image does not include xz (and unxz) but it is available from yum.

@jjhelmus
Copy link
Contributor

There are some different between this recipe and the python-3.5 recipe available at ContinuumIO/anaconda-recipes. I am able to build the latter with only minor modification using the manylinux Docker image.

@jakirkham
Copy link
Member

Yeah, I have seen that readline issue before when building Python. Will need to check my notes to see what the solution was.

@@ -0,0 +1,56 @@
package:
name: python
version: "3.5.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do a series of these packages and name them with major minor version?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, could extract all of these version numbers and use jinja to make this simpler to maintain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'd like the feedstock to called <package-name>-feedstock. My aspiration it to use branches to manage different versions (as you would maintenance branches in the source repo).

Support is very loose at the moment in conda-forge, but the first repo to do this already exists (https://github.com/conda-forge/gdal-feedstock) and it is only a matter of time until I need to deal with it 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable. Is there some discussion of this happening somewhere? It would be nice to iron out how this works before we get a bunch of things like gcc, libgcc, etc.

@pelson
Copy link
Member Author

pelson commented Mar 18, 2016

The lack of effort on this PR (on my behalf), is a representation of my inbox over the last few days. I'm keen to move it forwards, but in truth, there is more value in me supporting of of the other super contributors. Essentially, I'm pausing this until I get some more time.

@jakirkham
Copy link
Member

The lack of effort on this PR (on my behalf), is a representation of my inbox over the last few days.

Sorry about my part in that. Just really excited about leveraging this infrastructure. 😄

I'm keen to move it forwards, but in truth, there is more value in me supporting of of the other super contributors.

Thanks for supporting us.

Essentially, I'm pausing this until I get some more time.

Sounds reasonable. I was just interested after seeing @jjhelmus work on this, as well. It would definitely be great to get a fully operational Python into our stack here.

@jjhelmus
Copy link
Contributor

There are recipes for Python 2.7, 3.4 and 3.5 at ContinuumIO/anaconda-recipes which seem promising and I've gotten the 3.5 recipe working on Linux with some minor modification. I am not sure what exactly the replace command in the build.sh is, maybe a sed alias of some kind.

One question that should be discussed is if we want to "brand" the Python like Continuum or this PR does. Personally, I do not think we should, we don't do to any other package so why is Python special.

@jakirkham
Copy link
Member

There are recipes for Python 2.7, 3.4 and 3.5 at ContinuumIO/anaconda-recipes which seem promising and I've gotten the 3.5 recipe working on Linux with some minor modification. I am not sure what exactly the replace command in the build.sh is, maybe a sed alias of some kind.

Definitely seems like the right place to start.

One question that should be discussed is if we want to "brand" the Python like Continuum or this PR does. Personally, I do not think we should, we don't do to any other package so why is Python special.

I think I agree with this.

Just to play devil's advocate though, one reason we might want to brand is to make it clear that they have are not using Continuum's brand of Python and they are not using the vanilla system Python.

@ocefpaf
Copy link
Member

ocefpaf commented Mar 18, 2016

one reason we might want to brand is to make it clear that they have are not using Continuum's brand of Python and they are not using the vanilla system Python.

Yep. That is the reason IMO.

@jakirkham
Copy link
Member

Whatever the decision is on branding, I think we should carry it forward with ipython and jupyter_console because the reasoning should still apply. We probably should get some feedback from the Jupyter community on this.

@jakirkham
Copy link
Member

Also, another good place to look for Python and things like this is pylodger.

@pelson
Copy link
Member Author

pelson commented Mar 20, 2016

one reason we might want to brand is to make it clear that they have are not using Continuum's brand of Python and they are not using the vanilla system Python.

Yep. That is the reason IMO.

I have 3 reasons (of different quality):

  • the branding is subtle and relatively unobtrusive and if you are used to conda, you're used to the branding and it is useful to know you aren't using the defaults python (the same argument could pretty much be said of any package though)
  • from a political view point - I'm keen for Continuum to see this as a recipe they could use canonically (by setting an environment variable in their own build system). If that is the case, the whole community would benefit from their input on the recipe.
  • any other company can lift the recipe and make use of it (same argument about the community benefit from keeping this python recipe the canonical one). For instance, whilst the repo is no longer public, I believe https://bitbucket.org/abilisoft/pkgs used to brand a Python recipe in a very similar way (though less generally, and through a patch file).

Whilst I'm completely against obtrusive / unconstructive branding, in this case I think it is helpful to the conda-forge community to have it.

@jakirkham
Copy link
Member

the branding is subtle and relatively unobtrusive...

Depends on the branding. 😄 Also, I don't think just because we can do it that we should.

...it is useful to know you aren't using the defaults python

That's true. Though it is never difficult to check (i.e. conda list python with channel listing enabled).

...the same argument could pretty much be said of any package though

This is also why if we do this it should be done with things like ipython and jupyter console.

I'm keen for Continuum to see this as a recipe they could use canonically...

Agreed. Though I think this is almost an argument against branding. Without the branding file, they can just drop in their own branding file. If they want to use this one, they need to change what they are doing to fit this (so changes in conda's activate/deactivate) or scrap our branding. In any event, there is a barrier to entry for them making use of this, which will make it less likely they will help maintain the branding file. Also, just guessing, but I don't know if they will like this ability to change it through the environment. In any event, we should get their feedback about this at some point, but we ultimately should make our own decision.

...the whole community would benefit from their input on the recipe.

👍

any other company can lift the recipe and make use of it...

Again I feel like this is more of an argument against branding then for it basically for the same reasons mentioned above.

@pelson pelson changed the title Added a python recipe. WIP: python Mar 23, 2016
@pelson
Copy link
Member Author

pelson commented Mar 23, 2016

This is a new one on me:

Fixing permissions
updating shebang: bin/2to3-3.5
updating shebang: bin/idle3.5
updating shebang: bin/pydoc3.5
Fixing linking of /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation in /Users/travis/miniconda/envs/_build/bin/python3.5
Fixing linking of /Users/travis/miniconda/envs/_build/lib/libpython3.5m.dylib in /Users/travis/miniconda/envs/_build/bin/python3.5
Fixing linking of /usr/lib/libSystem.B.dylib in /Users/travis/miniconda/envs/_build/bin/python3.5
install_name_tool -add_rpath @loader_path/../lib/ /Users/travis/miniconda/envs/_build/bin/python3.5
install_name_tool -delete_rpath /Users/travis/miniconda/envs/_build/lib /Users/travis/miniconda/envs/_build/bin/python3.5
Fixing linking of /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation in /Users/travis/miniconda/envs/_build/bin/python3.5m
Fixing linking of @rpath/libpython3.5m.dylib in /Users/travis/miniconda/envs/_build/bin/python3.5m
Fixing linking of /usr/lib/libSystem.B.dylib in /Users/travis/miniconda/envs/_build/bin/python3.5m
install_name_tool -add_rpath @loader_path/../lib/ /Users/travis/miniconda/envs/_build/bin/python3.5m
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool: object: /Users/travis/miniconda/envs/_build/bin/python3.5m malformed object (load command 18 cmdsize is zero)
Traceback (most recent call last):
  File "/Users/travis/miniconda/bin/conda-build-all", line 9, in <module>
    load_entry_point('conda-build-all==0.10.0', 'console_scripts', 'conda-build-all')()
  File "/Users/travis/miniconda/lib/python2.7/site-packages/conda_build_all/cli.py", line 85, in main
    b.main()
  File "/Users/travis/miniconda/lib/python2.7/site-packages/conda_build_all/builder.py", line 207, in main
    built_dist_location = self.build(meta)
  File "/Users/travis/miniconda/lib/python2.7/site-packages/conda_build_all/builder.py", line 164, in build
    return bldpkg_path(build.build(meta.meta))
  File "/Users/travis/miniconda/lib/python2.7/site-packages/conda_build_all/build.py", line 32, in build
    build_module.build(meta, post=None, **kwd)
  File "/Users/travis/miniconda/lib/python2.7/site-packages/conda_build/build.py", line 496, in build
    post_build(m, sorted(files2 - files1))
  File "/Users/travis/miniconda/lib/python2.7/site-packages/conda_build/post.py", line 322, in post_build
    mk_relative(m, f)
  File "/Users/travis/miniconda/lib/python2.7/site-packages/conda_build/post.py", line 291, in mk_relative
    mk_relative_osx(path)
  File "/Users/travis/miniconda/lib/python2.7/site-packages/conda_build/post.py", line 257, in mk_relative_osx
    macho.add_rpath(path, rpath, verbose = True)
  File "/Users/travis/miniconda/lib/python2.7/site-packages/conda_build/macho.py", line 197, in add_rpath
    % p.returncode)
RuntimeError: install_name_tool failed with exit status 1

@jakirkham
Copy link
Member

Is running autoconf an option here? That might be worth trying.

@jakirkham
Copy link
Member

I have seen this before. Took a moment to track down. Try setting the macro __ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORES to 0 on Mac. This may fix it. I ran into this issue with Boost and another dependency recently.

@jakirkham
Copy link
Member

Normally, I have seen this issue with CMake though and it needs another option.

@pelson
Copy link
Member Author

pelson commented Mar 23, 2016

Not the only one: openmm/openmm#1419

@msarahan
Copy link
Member

Ping @mingwandroid - he might know more about this.

On Wed, Mar 23, 2016, 04:09 Phil Elson notifications@github.com wrote:

Not the only one: openmm/openmm#1419
openmm/openmm#1419


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#84 (comment)

@jakirkham
Copy link
Member

This is the CMake issue's solution. Maybe that will get us going forward down the right path at least.

@mingwandroid
Copy link
Contributor

Seems the install_name_tool pre-installed on travis-ci is also broken with respect to -delete_rpath. This was something I added to conda-build to workaround SIP for OS X 10.11 which prevents DYLD_FALLBACK_LIBRARY_PATH from doing anything (DYLD_* in fact since Apple have classified them as exploit vectors).
My idea was to add -Wl,-rpath,$prefix/lib upfront to LDFLAGs instead, then as cleanup, to remove it finally. The cleanup isn't completely necessary, but I really don't like leaking build information into the final dylibs so I want to clean it out at the end, also, having weird build-time rpaths in dylibs in OS Xs own, more subtle form of DLL hell and takes us a step further away from reproducible builds.

To see the original issue, remove the following from conda-build/conda_build/environ.py:

        rpath = ' -Wl,-rpath,%(PREFIX)s/lib' % d # SIP workaround, DYLD_* no longer works.
        d['LDFLAGS'] = ldflags + rpath + ' -arch %(OSX_ARCH)s' % d

.. and then the following from conda-build/conda_build/macho.py:

        macho.delete_rpath(path, config.build_prefix + '/lib', verbose = True)

and then try to rebuild r-base on 10.11. You should find it fail to load conftest due to not being able to load gfortran,3.dylib.

I propose, for now, to add some hacks to detect the broken versions of install_name_tool and not do -delete_rpath when they are detected, then to initiate a parallel effort to rebuild the old binaries with fixes for this bug and to encourage travis-ci (and anyone else who is known to run them) to adopt the fixed ones (provided everything I've said here is correct and can be verified).

Does this sound reasonable?

@mingwandroid
Copy link
Contributor

I'd be interested to know what version you get from:

strings $(install_name_tool 2>&1 | cut -d' ' -f2) | grep cctools

.. they didn't make that easy, unless I missed something obvious :-)

@mingwandroid
Copy link
Contributor

Proposed reversion for now: conda/conda-build#844

@jakirkham
Copy link
Member

One thought here might be to actually switch to the latest version of Mac and XCode/clang here and actually just compile in compatibility mode with 10.7.

This might also solve the OpenMP problem by just having support in the system compiler. Though I don't know how this would be affected by the compatibility mode.

@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly conda-forge-admin automated user.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/python, recipes/readline) and found some lint 😢.

Here's what I've got:

recipes/readline
  • The top level meta keys are in an unexpected order. Expecting ['package', 'source', 'build', 'requirements', 'about', 'extra'].
  • The summary item is expected in the about section.
  • The recipe must have some tests.

@groutr
Copy link
Contributor

groutr commented Apr 26, 2016

@jjhelmus You're correct. the replace command in build script can be replaced with the appropriate call to sed. It is a python script someone wrote a long time ago that simply does text substitutions in files.

@pelson
Copy link
Member Author

pelson commented May 20, 2016

Superseded by #645. 🎉

@pelson pelson closed this May 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

8 participants