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

bindepend: suppress missing library messages for macOS system libraries #5464

Merged
merged 1 commit into from Jan 19, 2021

Conversation

rokm
Copy link
Member

@rokm rokm commented Jan 11, 2021

Starting with macOS Big Sur, the system libraries are hidden. This results in a slew of error messages about missing libraries when
a shared library is scanned for imports. However, all these error messages are harmless, because we avoid collecting system libraries on macOS, nor do we need to further analyze them for dependencies (as they can depend only on other system libraries). Therefore, suppress the missing-library error messages for system libraries.

Closes #5107.

Copy link
Contributor

@BoboTiG BoboTiG left a comment

Choose a reason for hiding this comment

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

A NEWs fragment would be needed IMO.

# we do not collect system libraries on any macOS version
# anyway, so suppress the corresponding error messages.
if not in_system_path(lib):
logger.error('Can not find path %s (needed by %s)',
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
logger.error('Can not find path %s (needed by %s)',
logger.error('Cannot find path %s (needed by %s)',

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was thinking about changing the message. But then, it would need to be changed in other places for consistency, and that makes it out-of-scope for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Addressing such tiny issue when the code is changed seems reasonable. But yes, this is a very small thing to consider, it is OK for me if you change it or not :)

Starting with macOS Big Sur, the system libraries are hidden. This
results in a slew of error messages about missing libraries when
a shared library is scanned for imports. However, all these error
messages are harmless, because we avoid collecting system libraries
on macOS, nor do we need to further analyze them for dependencies
(as they can depend only on other system libraries). Therefore,
suppress the missing-library error messages for system libraries.
@rokm rokm force-pushed the macos-supress-syslib-errors branch from 413c0c0 to cb70096 Compare January 18, 2021 13:09
@rokm rokm marked this pull request as ready for review January 18, 2021 13:12
@bwoodsend
Copy link
Member

Out of curiosity, how does in_system_path() behave on Big Sur?

@rokm
Copy link
Member Author

rokm commented Jan 18, 2021

Out of curiosity, how does in_system_path() behave on Big Sur?

It should be version-agnostic, as it's just checking the path prefix.

@bwoodsend
Copy link
Member

Also, Github actions now allow you to set runs-on: macos-11.0. Our current CI is running macos-latest which ironically is actually 10.15.7. Would it be worth creating a second branch from this one, setting runs-on: macos-11.0 and letting CI run to find any other issues with Big Sur?

@rokm
Copy link
Member Author

rokm commented Jan 18, 2021

Also, Github actions now allow you to set runs-on: macos-11.0. Our current CI is running macos-latest which ironically is actually 10.15.7. Would it be worth creating a second branch from this one, setting runs-on: macos-11.0 and letting CI run to find any other issues with Big Sur?

I've triggered a run on a branch in my fork: https://github.com/rokm/pyinstaller/actions/runs/493923759

@bwoodsend
Copy link
Member

Hmm, in hindsight we probably should have loosened the max of 3 failures rule.

FAILED tests/unit/test_depend_utils.py::test_ctypes_util_find_library - asser...
FAILED tests/functional/test_apple_events.py::test_osx_custom_protocol_handler
FAILED tests/functional/test_apple_events.py::test_osx_event_forwarding - sub...
!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 3 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!
!!!!!!!!!!!! xdist.dsession.Interrupted: stopping after 3 failures !!!!!!!!!!!!!
= 3 failed, 326 passed, 28 skipped, 22 xfailed, 4 warnings in 292.95s (0:04:52) =

This is still a lot better than I'd hoped. I was expecting every second test to pop and die.

I'd guess that the 1st error is to be expected. libc will now be hidden.

def test_ctypes_util_find_library(monkeypatch):
    # for lind_library() we need a lib actually existing on the system
    if is_win:
        libname = "KERNEL32"
    else:
        libname = "c"
    code = "ctypes.util.find_library('%s')" % libname
    res = __scan_code_for_ctypes(code, monkeypatch)
    assert res

@rokm
Copy link
Member Author

rokm commented Jan 18, 2021

Well, the good news is that in the full run, only those three tests fail:

FAILED tests/unit/test_depend_utils.py::test_ctypes_util_find_library - asser...
FAILED tests/functional/test_apple_events.py::test_osx_custom_protocol_handler
FAILED tests/functional/test_apple_events.py::test_osx_event_forwarding - sub...
= 3 failed, 622 passed, 100 skipped, 46 xfailed, 10 xpassed, 4 warnings in 1051.30s (0:17:31) =

And yeah, test_ctypes_util_find_library is expected to fail (maybe we can add compat.is_macos11 and xfail it). Will check if I can reproduce the other two locally.

Copy link
Member

@Legorooj Legorooj left a comment

Choose a reason for hiding this comment

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

Nice 👍.

@Legorooj Legorooj merged commit e2823f4 into pyinstaller:develop Jan 19, 2021
@rokm rokm mentioned this pull request Jan 19, 2021
@rokm rokm deleted the macos-supress-syslib-errors branch January 28, 2021 17:54
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pyinstaller can't find dynamically linked libs for Mac OS X Big Sur Beta
4 participants