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

Missing dependencies: compilers and C++ runtime #40

Closed
wants to merge 7 commits into from

Conversation

mbargull
Copy link
Member

The recipe lacks C/C++ compilers in its build requirements and most importantly, in its run requirements, a C++ runtime library it's being linked to:
https://circleci.com/gh/bioconda/bioconda-recipes/3805

[Feb 23 16:27:36] SERR import pandas as pd
[Feb 23 16:27:36] SERR File "/usr/local/lib/python2.7/site-packages/pandas/__init__.py", line 58, in <module>
[Feb 23 16:27:36] SERR from pandas.io.api import *
[Feb 23 16:27:36] SERR File "/usr/local/lib/python2.7/site-packages/pandas/io/api.py", line 19, in <module>
[Feb 23 16:27:36] SERR from pandas.io.packers import read_msgpack, to_msgpack
[Feb 23 16:27:36] SERR File "/usr/local/lib/python2.7/site-packages/pandas/io/packers.py", line 67, in <module>
[Feb 23 16:27:36] SERR from pandas.io.msgpack import Unpacker as _Unpacker, Packer as _Packer, ExtType
[Feb 23 16:27:36] SERR File "/usr/local/lib/python2.7/site-packages/pandas/io/msgpack/__init__.py", line 22, in <module>
[Feb 23 16:27:36] SERR from pandas.io.msgpack._packer import Packer  # noqa
[Feb 23 16:27:36] SERR ImportError: libstdc++.so.6: cannot open shared object file: No such file or directory
[Feb 23 16:27:36] ERRO Task processing failed: Unexpected exit code [1] of container [975df041aa6f step-9c439807b8], container preserved

From looking at other recipes, I assumed you always use the system's compilers, right?
Also I couldn't find references about when libcxx is used on conda-forge.

I deliberately didn't add the vc* features, because of conda-forge/conda-forge.github.io#537.
But feel free to do any modifications to this PR as you see fit!

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@mbargull
Copy link
Member Author

Should I rather remove libgcc (libcxx?) from the build requirements? Having it in the build requirements means libraries from the newer libgcc-ng 7.2 get linked. Could that pose a problem if someone installs this package alongside the older libgcc? If yes, we can remove libgcc from the build requirements to let it link to the older system lib. Or (preferably (for forward-compatibility)?) we can temporarily pin it to 7.2 in build and constrain it to >=7.2 in run requirements (i.e., emulate what we'd currently get with CB3).

@jakirkham
Copy link
Member

So this is not limited to pandas, but conda-forge binaries are built on the notion that they will run on a stock CentOS 6 machine (with a handful of X11 libraries thrown in). The consequence is we assume libstdc++ will be provided in some form. Admittedly this will change when we switch to Anaconda's compilers and we will begin distributing our own libstdc++ (meaning Anaconda's). Is there some use case you have where libstdc++ is not available from the system?

@mbargull
Copy link
Member Author

mbargull commented Mar 2, 2018

Yes, http://biocontainers.pro/. In Bioconda, nearly every Linux package is also packaged inside a Container with a minimal base system (https://github.com/galaxyproject/galaxy-lib/blob/17.9.10/galaxy/tools/deps/mulled/invfile.lua#L63). The build log in the OP, for instance, is from a pure Python package, which obviously doesn't need libgcc by itself, but is not runnable due to pandas' extension being linked to the missing libstdc++.so.6.

@mbargull
Copy link
Member Author

mbargull commented Mar 2, 2018

cc @conda-forge/bioconda-recipes

@johanneskoester
Copy link

We strive to minimize system dependencies as much as possible. So, if libgcc is available via conda, we would like to use it. This way, stuff also runs out of the box, e.g., in busybox containers.

@jakirkham
Copy link
Member

Right. Am remembering a similar discussion from before ( conda-forge/matplotlib-feedstock#71 ). Unfortunately adding libgcc selectively in the stack caused us a lot of problems. As one example, please take a look at issue ( conda-forge/matplotlib-feedstock#32 ). This took a while to get worked out and caused people a significant amount of pain. Also adding such an old libgcc as a requirement is likely to cause problems for users on newer systems. Would be reluctant to add an old libgcc to just one package (particularly this low in the stack) as these issues will reoccur.

That said, expect we could avoid this pain if we added libgcc to all packages. We are actually planning to do this as we are planning to switch to some new compilers Anaconda released. As these are significantly newer, better optimized, and actively used by Anaconda, expect this will be a better experience all around. They will also address concerns of users that have been clamoring for a newer GCC on Linux as this will be GCC 7.2.0.

Our plan to implement this is to move to conda-build 3 and then switch over to the newer compilers. On the conda-build 3 front, @isuruf and I worked this weekend on getting a conda-smithy rc out (currently 3.0.0.rc2), which works with conda-build 3 and contains a host of new features that should help with various issues package maintainers have run into (including better pinning management). Various other people have contributed significantly to this work (@msarahan in particular). If you have time to help us test out the new conda-smithy, that would really help us progress towards these common goals. Also feedback on things like the upgrade of core packages in conda-forge ( zlib as one example conda-forge/zlib-feedstock#18 ). Thoughts on what we pin and how tightly in our global pinning list would be very helpful.

It would also be good to understand on what sort of time scale you are hoping to solve this problem. We might be able to devise a workaround in the interim.

@johanneskoester
Copy link

Ok, I think for now we can live with adding libgcc to downstream packages if needed. I am really looking forward to CB3 on conda-forge and bioconda. This will also help us with various problems. The global pinning package is a very good idea, and I think we will use it for bioconda as well. We will start porting bioconda-utils to CB3 in the next days.

@jakirkham
Copy link
Member

So we made some improvements such that the {{ compiler("cxx") }} syntax can be used for this. As an example, please see PR ( conda-forge/singularity-feedstock#6 ).

cc @bgruening

@mbargull
Copy link
Member Author

mbargull commented May 6, 2018

@conda-forge-admin, please rerender

@mbargull
Copy link
Member Author

mbargull commented May 6, 2018

Thanks @jakirkham. Yes, with the changes from https://github.com/conda-forge/toolchain-feedstock/pull/34/files#diff-e178b687b10a71a3348107ae3154e44cR8 this should pick up toolchain_c_linux-64 and friends with their libgcc-ng >=4.9 (or other) run_exports.

@jakirkham
Copy link
Member

Thoughts @TomAugspurger?

@TomAugspurger
Copy link
Contributor

I'm not really sure, but the changes here seem sensible :)

@mbargull
Copy link
Member Author

I'm not really sure,

Understandable. The expected base system requirements for Bioconda are rather on the very far side regarding minimalism (only glibc plus some POSIX tools provided by BusyBox + Bash).

but the changes here seem sensible :)

Well and with those changes the original issue is essentially void since the {{ compiler(...) }} packages (including toolchain_*) will just add the C/C++ RTLs.

@jakirkham
Copy link
Member

FWIW feel free to ignore files outside of the recipe directory. Those changes are mainly retooling the feedstock to more conveniently work with conda-build 3 and some niceties (new badges).

The main change is using the compiler syntax, which as @mbargull alluded too pulls in libgcc on Linux. The behavior is basically unchanged on the other platforms.

The numpy behavior is the same as before (just looks different). It now relies on these settings to update the files in .ci_support listed in this feedstock.

@jorisvandenbossche
Copy link
Member

I think for us (or at least for me, as pandas maintainer who is not an expert in those details) the main question is: is this nowadays thé recommended way by conda-forge to deal with a numpy build dependency ?
Because I still see many other feedstocks using the approach that we are using currently. Is it the goal of conda-forge that they all get updated?

@jakirkham
Copy link
Member

Yes, the docs were updated recently to reflect that. FWIW the change with numpy is just a simpler way to spell out the same thing.

It is the plan to update everything to match, but it will take time.

ref: https://conda-forge.org/docs/meta.html#building-against-numpy

@jorisvandenbossche
Copy link
Member

Thanks for the clarification, that's fine then!

Just added some questions inline about specifying the minimum numpy version.

cxx_compiler:
- toolchain_cxx
numpy:
- '1.9'
Copy link
Member

Choose a reason for hiding this comment

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

should this be 1.11? (as for 3.6 we now build against 1.11)

cxx_compiler:
- vs2008
numpy:
- '1.11'
Copy link
Member

Choose a reason for hiding this comment

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

windows is now building against 1.11 instead of 1.9 before?

@dpryan79
Copy link

dpryan79 commented Jul 6, 2018

Is there anything remaining (other than fixing the merge conflicts) that needs to be done for this PR? I had a user run into the missing libstdc++ issue that this would fix today.

@bgruening
Copy link

I think rebuilding this with cb3 should fix it.

@bgruening
Copy link

@conda-forge-admin, please rerender

@jakirkham
Copy link
Member

@jorisvandenbossche, the numpy pinning change is intentional. All pinnings, including numpy, live in this file.

Sorry for the out-of-diff comment. On mobile.

@TomAugspurger
Copy link
Contributor

We'll defer to the conda experts here.

There's currently a CI failure:

conda.CondaError: Unable to create prefix directory '/home/conda/feedstock_root/build_artifacts/pandas_1530895546964/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac'.

@TomAugspurger
Copy link
Contributor

I think we'll want to merge in master though. We have had a release or two since this was opened.

@ocefpaf
Copy link
Member

ocefpaf commented Jul 26, 2018

@mbargull I believe that #48 address this, so I missed this one.

@ocefpaf ocefpaf closed this Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants