Skip to content

Comments

BUGFIX: copy dylibs#504

Open
Tokarak wants to merge 2 commits intoronaldoussoren:mainfrom
Tokarak:patch-3
Open

BUGFIX: copy dylibs#504
Tokarak wants to merge 2 commits intoronaldoussoren:mainfrom
Tokarak:patch-3

Conversation

@Tokarak
Copy link

@Tokarak Tokarak commented Aug 30, 2023

A rough patch copying all dylibs available in the runtime into the app. This should close most issues that #502 was intended to fix.

Tested in standalone mode, Anaconda python installation, Apple Silicon.

Reverts #315 (it never helped and only affected x86 architecture; #502 was supposed to port the fix to other architectures, and it definitely did not resolve the bug).
Closes: #494 #482 #286 #472

The behaviour exhibited only fits the default standalone mode; not alias mode and probably not semi-standalone mode. It's hell to test a project where you aren't sure whether the problem is in the modified code, versioning, or an original bug, and I haven't even touched the other modes yet, so I'm hoping a maintainer will adapt this code to how it should be.

I’m not sure all edge cases are covered, especially semi_standalone

Signed-off-by: Tokarak <63452145+Tokarak@users.noreply.github.com>
This reverts ronaldoussoren#315, which never worked and only applied to x86 architecture.

Signed-off-by: Tokarak <63452145+Tokarak@users.noreply.github.com>
dylib_dir = os.path.dirname(runtime)
for fn in os.listdir(dylib_dir):
if fn.endswith(".dylib"):
mm.mm.run_file(os.path.join(dylib_dir, fn))
Copy link
Author

Choose a reason for hiding this comment

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

Found a bug! Some dylibs are nested in further directories, and so aren't copied. The may also have to be copied as-specified?

Therefore, it may be better to copy the whole environment — it seems like best practice too, keeping a minimal VENV.

What HASN'T been answered, is why did this work for so long, and then suddenly stop working, and for (maybe) apple silicon only? I hope a maintainer who knows the code picks this up. I spent enough time, so I'm done here.

Copy link

Choose a reason for hiding this comment

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

From what I can gather, it seems that py2app fails to copy the dylibs stored inside .dylibs directories (notice dot in the beginning - i.e. hidden dirs).

An example:

Contents/Resources/lib/python3.11/sklearn/__check_build/_check_build.cpython-311-darwin.so is trying to load libomp.dylib from @loader_path/../.dylibs/libomp.dylib. This means that the directory should be a folder .dylibs with libomp.dylib from the original virtual environment directory is not being copied.

After copying and re-signing these manually, the bundled app works, but it's a nightmare when you're dealing with tens or hundreds of missing dylibs...

Copy link

Choose a reason for hiding this comment

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

Sorry, false alarm, please ignore my last message. It seems that this happens only over github actions due to the fact that the runners are ignoring hidden files by default when uploading artifacts.

"name": "main-x86_64",
"target": "10.5",
"cflags": "-g -arch x86_64 -Wl,-rpath,@executable_path/../Frameworks",
"cflags": "-g -arch x86_64",
Copy link
Author

Choose a reason for hiding this comment

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

I'm trying to work out why the issues are popping up only for apple silicon... As far as I know, this is the only difference. If it has no change at runtime, maybe it has an effect at build-time? This could be a quick lead which could be easy to test experimentally.

Copy link
Author

Choose a reason for hiding this comment

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

I sent a probe in #507 and #508. I personally got a negative result on arm64 (#508), so it's likely that #315 truly did do nothing... and this massive bug has existed for more than 3 years?

Copy link

Choose a reason for hiding this comment

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

I sent a probe in #507 and #508. I personally got a negative result on arm64 (#508), so it's likely that #315 truly did do nothing... and this massive bug has existed for more than 3 years?

yep,Even now, still can't pack torchaudio. It's unbelievable

@glyph
Copy link
Contributor

glyph commented Sep 8, 2024

Is this the same bug as python-pillow/Pillow#8073 ?

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.

Missing libraries

4 participants