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

setup.py: fix import error reporting for cythonize import, see #8208 #8210

Conversation

ThomasWaldmann
Copy link
Member

Looks like borg's setup.py has hidden the real cause of a cythonize ImportError.

There are basically 2 cases:

  • either there is no Cython installed, then the import fails because the module can not be found, or
  • there is some issue within Cython and the import fails due to that.

It's important not to hide the real cause, especially if we run into case 2.

case 1 is kind of expected and frequent, case 2 is rare.

@ThomasWaldmann
Copy link
Member Author

The macOS failure is due to:

  ld: library 'crypto' not found

Maybe some OpenSSL change in the action runner?

@codecov-commenter
Copy link

codecov-commenter commented May 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.47%. Comparing base (b961792) to head (8dcd8da).
Report is 3 commits behind head on 1.4-maint.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           1.4-maint    #8210   +/-   ##
==========================================
  Coverage      82.47%   82.47%           
==========================================
  Files             38       38           
  Lines          11027    11027           
  Branches        2116     2116           
==========================================
  Hits            9094     9094           
  Misses          1365     1365           
  Partials         568      568           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…ckup#8208

Looks like borg's setup.py has hidden the real cause of a cythonize ImportError.

There are basically 2 cases:
- either there is no Cython installed, then the import fails because the module can not be found, or
- there is some issue within Cython and the import fails due to that.

It's important not to hide the real cause, especially if we run into case 2.

case 1 is kind of expected and frequent, case 2 is rare.
That "failed to map segment from shared object" error msg is not
very helpful. Add a hint that the filesystem needs to be +exec
(== not noexec mounted, like it might be the case for /tmp on
some systems).
@ThomasWaldmann ThomasWaldmann force-pushed the fix-cythonize-import-error-reporting-1.4 branch from e3f66ee to 8dcd8da Compare May 12, 2024 14:38
@ThomasWaldmann ThomasWaldmann merged commit 053b09d into borgbackup:1.4-maint May 12, 2024
10 checks passed
@ThomasWaldmann ThomasWaldmann deleted the fix-cythonize-import-error-reporting-1.4 branch May 12, 2024 16:41
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

2 participants