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

Disable stripping for appimage #625

Closed
wants to merge 4 commits into from

Conversation

iamzili
Copy link
Contributor

@iamzili iamzili commented Oct 12, 2021

We discovered that stripping some shared object can cause issues, e.g numpy.

This PR relates to #583

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@freakboy3742
Copy link
Member

@zili55 What was your testing process for this PR? I tried adding an import numpy (plus adding numpy to the requires list), and I'm still getting errors about being unable to find libquadmath and libfortran dependencies. Is there something else needed to demonstrate this working?

@iamzili
Copy link
Contributor Author

iamzili commented Nov 3, 2021

Strange, if you build my briefcase branch (zili55:disable_stripping), and install that briefcase version instead the one from pypi then import numpy should work. I think you're testing the wrong Briefcase version, did you installed my briefcase version correctly?

if yes then do the followings:

briefcase new
cd <app_name>

Add import numpy to app.py and I think numpy should be added to requires in pyproject.toml.
Then execute the following commands:

briefcase create
briefcase run -u

Build should succeed.

@freakboy3742
Copy link
Member

(Apologies for the delayed response)

Confirming: I'm definitely using the dev version; passing in -vvv to briefcase build linux shows the commands being executed; when it gets to the AppImage build, it logs:

[togatest] Entering Docker context...
>>> docker run --tty --volume /Users/rkm/projects/beeware/briefcase/local/togatest/linux:/app:z --volume /Users/rkm/.briefcase:/home/brutus/.briefcase:z --env VERSION=0.0.1 --env NO_STRIP=1 briefcase/com.example.togatest:py3.9 /home/brutus/.briefcase/tools/linuxdeploy-x86_64.AppImage --appimage-extract-and-run --appdir=/app/appimage/togatest/togatest.AppDir -d /app/appimage/togatest/togatest.AppDir/com.example.togatest.desktop -o appimage --deploy-deps-only /app/appimage/togatest/togatest.AppDir/usr/app_packages/cairo --deploy-deps-only /app/appimage/togatest/togatest.AppDir/usr/app_packages/gi --deploy-deps-only /app/appimage/togatest/togatest.AppDir/usr/app_packages/numpy/core --deploy-deps-only /app/appimage/togatest/togatest.AppDir/usr/app_packages/numpy/fft --deploy-deps-only /app/appimage/togatest/togatest.AppDir/usr/app_packages/numpy/linalg --deploy-deps-only /app/appimage/togatest/togatest.AppDir/usr/app_packages/numpy/random --deploy-deps-only /app/appimage/togatest/togatest.AppDir/usr/app_packages/numpy.libs --deploy-deps-only /app/appimage/togatest/togatest.AppDir/usr/lib/python3.9/lib-dynload
linuxdeploy version 1-alpha (git commit ID 9fb2ac3), <local dev build> built on 2021-01-03 14:26:39 UTC

which clearly includes NO_STRIP=1.

@codecov
Copy link

codecov bot commented Feb 28, 2022

Codecov Report

Merging #625 (67f4599) into master (860fc39) will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted Files Coverage Δ
src/briefcase/platforms/linux/appimage.py 96.22% <ø> (ø)
src/briefcase/commands/new.py 93.98% <0.00%> (ø)
src/briefcase/integrations/linuxdeploy.py 100.00% <0.00%> (ø)
src/briefcase/commands/create.py 97.09% <0.00%> (+0.04%) ⬆️
src/briefcase/integrations/xcode.py 93.75% <0.00%> (+0.41%) ⬆️
src/briefcase/exceptions.py 92.10% <0.00%> (+0.43%) ⬆️

@freakboy3742
Copy link
Member

Closing due to lack of activity; it's unclear if NO_STRIP is needed, but a focussed approach on improving Linux binary packaging is clearly needed.

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.

2 participants