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

[DEBUG] Add test for C++ exception error #234

Closed
wants to merge 6 commits into from

Conversation

xylar
Copy link

@xylar xylar commented Oct 22, 2024

This is a test PR to see whether the test that @gillins added in #233 passes for 1.14.4

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@xylar xylar mentioned this pull request Oct 22, 2024
@xylar
Copy link
Author

xylar commented Oct 22, 2024

@gillins, osx-64 is passing here, too. So I don't think we have an explanation for why kealib is failing in conda-forge/kealib-feedstock#65

@gillins
Copy link
Contributor

gillins commented Oct 22, 2024

Sigh. Thanks @xylar. @petebunting any chance you'll have some time to look into this some more?

@conda-forge-admin
Copy link
Contributor

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/meta.yaml) and found it was in an excellent condition.

recipe/build.sh Outdated

# test for hdf5 C++ exceptions
if [[ "${CONDA_BUILD_CROSS_COMPILATION:-}" != "1" || "${CROSSCOMPILING_EMULATOR}" != "" ]]; then
$CXX ${CXXFLAGS} $RECIPE_DIR/testhdf5exc.cpp -o testhdf5exc -L$PREFIX/lib -lhdf5_cpp -lhdf5 -I$PREFIX/include
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$CXX ${CXXFLAGS} $RECIPE_DIR/testhdf5exc.cpp -o testhdf5exc -L$PREFIX/lib -lhdf5_cpp -lhdf5 -I$PREFIX/include
${CXX} ${CXXFLAGS} ${LDFLAGS} $RECIPE_DIR/testhdf5exc.cpp -o testhdf5exc -lhdf5_cpp -lhdf5 -I$PREFIX/include

You can also follow the style and add the test to the test section:
https://github.com/xylar/hdf5-feedstock/blob/add-exception-catch-test/recipe/run_test.sh#L23

Copy link
Author

Choose a reason for hiding this comment

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

Agreed, I will move it if we decide we want to keep the test.

Co-authored-by: Mark Harfouche <[email protected]>
@gillins
Copy link
Contributor

gillins commented Oct 23, 2024

Update: valgrind shows no errors running the kealib test on Linux x86_64 with hdf5-1.14, so nothing useful there....

@xylar
Copy link
Author

xylar commented Oct 23, 2024

@gillins, no surprise. The issue seems to be specific to clang. My understanding based on some googling is that it's related to mixing up the system vs. the conda-forge (or in my google searches the homebrew) version of a core c++ library (can't recall which). Anyway, seems both osx and clang specific.

@gillins
Copy link
Contributor

gillins commented Oct 23, 2024

Ah ok, so might be a problem for other packages too? Unfortunately I no longer have a Mac to run other tests...

@xylar
Copy link
Author

xylar commented Nov 2, 2024

I'm going to close this as I don't think it was helpful.

@xylar xylar closed this Nov 2, 2024
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.

4 participants