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

enable threadsafe and production #48

Closed
wants to merge 6 commits into from
Closed

Conversation

marqh
Copy link
Contributor

@marqh marqh commented Oct 14, 2016

we use hdf5 in threaded processing, which can lead to segfaults if hdf is built without the threadsafe flag and the production flag

@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.

@jakirkham
Copy link
Member

Does this result in an SONAME change?

@marqh
Copy link
Contributor Author

marqh commented Oct 14, 2016

Does this result in an SONAME change?

I do not know, I am afraid, will the CI testing give us a clue on that?

@marqh
Copy link
Contributor Author

marqh commented Oct 14, 2016

checking for thread safe support... configure: error: --enable-cxx and --enable-threadsafe flags are incompatible. Use --enable-unsupported to override this error.
Command failed: /bin/bash -x -e /feedstock_root/build_artefacts/work/hdf5-1.8.17/conda_build.sh

I'm not sure how to proceed here.
what does enable-cxx do for us?
are we content with enable-unsupported?

@jakirkham
Copy link
Member

We probably can figure it out from the logs. Just noting that if it does, we need to get everything organized for rebuilding with a new HDF5 (assuming this change is agreeable). That will be a fair bit of working considering how important HDF5 is to conda-forge.

If it does, I don't think this should be a priority as Travis CI is changing images on us in less than a month. ( conda-forge/conda-forge.github.io#249 ) Also we are still on conda-build 1 and it is causing us all sorts of grief. So we need to switch to conda-build 2 soon. These two IMHO are higher priority.

This isn't to say that we won't do this at some point. Just that this month already has too many big ticket items for us to provide any promise of getting this done too.

cc @pelson

@jakirkham
Copy link
Member

jakirkham commented Oct 14, 2016

That all being said, this warning is troubling to me. I rely on C++ support from HDF5.

checking for thread safe support... configure: error: --enable-cxx and --enable-threadsafe flags are incompatible. Use --enable-unsupported to override this error.

xref: https://circleci.com/gh/conda-forge/hdf5-feedstock/130

@marqh
Copy link
Contributor Author

marqh commented Oct 14, 2016

cc @bjlittle

@marqh
Copy link
Contributor Author

marqh commented Oct 14, 2016

@jakirkham

thank you for the feedback. This is a blocking issue for our use of conda-forge, so we'll need to make other arrangements whilst this gets discussed; all updates are appreciated

@jakirkham
Copy link
Member

Sorry to hear that. Unfortunately there is already a full docket of issues that we are trying to address and we need to prioritize them. I hope you understand.

@marqh
Copy link
Contributor Author

marqh commented Oct 14, 2016

i have checked a local build process for hdf5 and it explicitly sets

--disable-static \
    --enable-linux-lfs --with-zlib --with-ssl \
    --enable-threadsafe --with-pthread=yes \
    --enable-production --enable-cxx \
    --enable-unsupported

so we've got prior with enable-unsupported

@jakirkham
Copy link
Member

If you want to help with those other issues that are slowing this down, I think that will make it much easier to get back to this sooner. The Travis CI change is being discussed in issue ( conda-forge/conda-forge.github.io#249 ). Also much of the conda-build 2 switch is being discussed in issue ( conda-forge/conda-forge.github.io#171 ). In particular, we have encountered issues with cmake and conda-build 2. Much more detail on this latter point is in issue ( conda-forge/cmake-feedstock#21 ). I understand if you don't have time to sink into these issues. Just providing you info about them in case you are willing to lend a hand on these org wide problems.

@gillins
Copy link
Contributor

gillins commented Oct 15, 2016

I think the C++ part has no thread safe support hence the need to specify --enable-unsupported to ensure you know that only the C part of the library is thread safe.

@marqh
Copy link
Contributor Author

marqh commented Oct 17, 2016

i've pushed a build with --enable-unsupported to see if this enables all the builds to succeed

for discussion...

@marqh
Copy link
Contributor Author

marqh commented Oct 17, 2016

i've pushed a build with --enable-unsupported to see if this enables all the builds to succeed

hmm, so progress; i get a build, but the hdf tests fail, on CircleCI

@marqh
Copy link
Contributor Author

marqh commented Oct 18, 2016

updated to disable-static and with-ssl to see if this enables the testing to pass for Linux

@ryanvolz
Copy link

I was all set to make this very pull request when I just saw that one already existed. I've been tracking down a segfault running some of our HDF5-using code in a conda environment, and I finally figured out it was because we needed a threadsafe HDF5 build.

I made the change and built it locally, and it seems to be working fine. The resulting SONAMEs are the same. I didn't rebuild anything I'm using that uses HDF5 and it still works.

I will note that Debian/Ubuntu build HDF5 with --enable-threadsafe and --enable-unsupported, the latter presumably because it is necessary to be able to keep --enable-cxx and the high-level library.

@jakirkham jakirkham mentioned this pull request Oct 20, 2016
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.

None yet

5 participants