Skip to content

Disable stripping for appimage#625

Closed
iamzili wants to merge 4 commits into
beeware:masterfrom
iamzili:disable_stripping
Closed

Disable stripping for appimage#625
iamzili wants to merge 4 commits into
beeware:masterfrom
iamzili:disable_stripping

Conversation

@iamzili

@iamzili iamzili commented Oct 12, 2021

Copy link
Copy Markdown
Contributor

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

freakboy3742 commented Nov 3, 2021

Copy link
Copy Markdown
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

iamzili commented Nov 3, 2021

Copy link
Copy Markdown
Contributor Author

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

freakboy3742 commented Feb 28, 2022

Copy link
Copy Markdown
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

codecov Bot commented Feb 28, 2022

Copy link
Copy Markdown

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

freakboy3742 commented Apr 4, 2022

Copy link
Copy Markdown
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.

@freakboy3742 freakboy3742 mentioned this pull request Apr 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@iamzili @freakboy3742