Skip to content
New issue

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

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

Already on GitHub? # to your account

fix osx ci :) #538

Merged
merged 5 commits into from
Mar 27, 2023
Merged

fix osx ci :) #538

merged 5 commits into from
Mar 27, 2023

Conversation

k-dominik
Copy link
Collaborator

@k-dominik k-dominik commented Mar 7, 2023

okay - finally it seems to work. I'll summarize the changes here:

  • test segfaulted on OSX when linking the vigranumpy stuff against python (see here
  • failing tests because cmake picked up some framework version of libpng (despite stating otherwise, this seems to be a cmake bug) - resolved by changing the order of package discovery via CMAKE_FIND_FRAMEWORK (and CMAKE_FIND_APPBUNDLE).
  • also the ctest wouldn't properly run the shellscripts on osx (added shebang to test scripts)

fixes #535, closes #523, closes #490

I would also suggest to close #469 - don't think nightly builds are needed now that vigra gets some love.

My comments further down were mostly recording thoughts during debugging, and don't need to be read now anymore :)

@k-dominik k-dominik changed the title temporarily disable win/lnx ci fix osx ci :) Mar 7, 2023
@k-dominik
Copy link
Collaborator Author

k-dominik commented Mar 10, 2023

update:

When I left this, I saw some problems with openexr and vigra picking up different versions of imath, with the suspicion that openexr doesn't correctly specify the version of imath. Then I got sick. In the meantime I saw some activity... and with the latest openexr we are down to 4 test failures... Investigating...

Failing Tests
The following tests FAILED:
	 25 - test_impex (Failed)
	 30 - test_multiarray (Failed)
	 45 - test_registration (Failed)
	 58 - vigranumpytest (BAD_COMMAND)

update: funny enough the only overlap with test failures on my silicon mac is vigranumpy....

@k-dominik
Copy link
Collaborator Author

k, the osx tests run through now on my intel macbook air.

@k-dominik k-dominik force-pushed the fix-osx-ci branch 2 times, most recently from c0c2195 to 842e4cc Compare March 21, 2023 14:01
@k-dominik
Copy link
Collaborator Author

Hm.. okay that's strange. Locally the tests pass, but in CI, there seems to be an error related to libpng.

The failing tests issue

Application built with libpng-1.4.12 but running with 1.6.39

e.g. here

I tried finding how in the world it picks it up...

My local env is exactly the same, it also picks up the same lib, but on ci it claims version 1.4.12

Truncated cmake output
# before cmake
otool -L /usr/local/miniconda/envs/vigra/lib/libpng.dylib
/usr/local/miniconda/envs/vigra/lib/libpng.dylib:
	@rpath/libpng16.16.dylib (compatibility version 56.0.0, current version 56.0.0)
	@rpath/libz.1.dylib (compatibility version 1.0.0, current version 1.2.13)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1197.1.1)

# during cmake
...
Found PNG: /usr/local/miniconda/envs/vigra/lib/libpng.dylib (found version "1.4.12") 
...
Using PNG  libraries: /usr/local/miniconda/envs/vigra/lib/libpng.dylib;/usr/local/miniconda/envs/vigra/lib/libz.dylib
...


@k-dominik k-dominik force-pushed the fix-osx-ci branch 5 times, most recently from 3b7c726 to 8be9f24 Compare March 22, 2023 19:19
@k-dominik k-dominik marked this pull request as ready for review March 22, 2023 19:20
k-dominik and others added 2 commits March 22, 2023 20:27
otherwise they won't run on osx
The interpreter is statically linked to `libpython` on macOS. Likely
causes symbol clashes, which causes the segmentation faults we have
seen.

See also https://github.com/conda-forge/vigra-feedstock/blob/34e1048c818424995997b59817bbf909f43b221b/recipe/patches/ignore_libpython_macos.diff

Co-authored-by: John Kirkham <jakirkham@users.noreply.github.com>
@hmaarrfk
Copy link
Collaborator

The OSX Stuff seems pretty Conda-forge specific. Maybe we should report to boost at conda-forge? or ask them for advice.

@k-dominik
Copy link
Collaborator Author

k-dominik commented Mar 23, 2023

how about pinging @jakirkham first - in the end that's the originator of the change.

For reference, we're discussing the following code snippet:

    IF(APPLE)
        SET(VIGRANUMPY_LIBRARIES ${Boost_PYTHON_LIBRARY})
    ELSE()
        SET(VIGRANUMPY_LIBRARIES ${Python_LIBRARIES} ${Boost_PYTHON_LIBRARY})
    ENDIF()

linking agains both Python_LIBRARIES and Boost_PYTHON_LIBRARY results in a segfault. Is this a boost.Python bug, or something inherent to using conda to build?

if I have some time I'll try to construct a "clean" example, not using conda and see if that issue persists (before that it would be hard to open an issue with boost/conda-forge/cmake).

@hmaarrfk
Copy link
Collaborator

I think this is due to some static linking in Python at conda-forge. I remember having run into this somewhere else.

I'm sorry I "moved" that patch. it makes finding the original author hard. I think it was jakirkham.

@k-dominik
Copy link
Collaborator Author

k - looks good. Tests still pass :)

@hmaarrfk hmaarrfk merged commit 63cd2b2 into ukoethe:master Mar 27, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OSX Pipelines broken and skipped
2 participants