Improved the resolution of dependencies in darwin MachO files.

[cainesi]

This improves the resolution of dependences between dynamic libraries on OSX, by keeping track of the rpaths set in each file in the chain of dependencies and searching through the rpaths to find the correct dependent files.

(This fixes a problem I was having with cx_Freeze not finding certain dependencies in PyQt5.)

[marcelotduarte]

Hi, I'm new maintainer of cx-freeze. Can you resolve the conflicts to continue the review?

[cainesi]

Hi Marcelo, Congratulations on taking over as maintainer. I’m a bit tied-up at the moment, but will put this on my todo list. Best, Ian

…

On Jul 9, 2020, at 9:19 AM, Marcelo Duarte ***@***.***> wrote: Hi, I'm new maintainer of cx-freeze. Can you resolve the conflicts to continue the review? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#590 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE5F6UIBBFKLPEAEW6G6QXLR2W7VNANCNFSM4KWAZBLQ>.

[cainesi]

FYI -- resolved the conflicts.

[marcelotduarte]

@cainesi
I do a small test in a Github Action and get an exception.
#sample.py
import sys

print(sys.argv)
print(sys.executable)
print(sys.path)

#traceback
File "/Users/runner/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/distutils/dist.py", line 966, in run_commands
self.run_command(cmd)
File "/Users/runner/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/distutils/dist.py", line 985, in run_command
cmd_obj.run()
File "/Users/runner/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/cx_Freeze-6.2-py3.8-macosx-10.14-x86_64.egg/cx_Freeze/dist.py", line 219, in run
freezer.Freeze()
File "/Users/runner/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/cx_Freeze-6.2-py3.8-macosx-10.14-x86_64.egg/cx_Freeze/freezer.py", line 654, in Freeze
self._WriteModules(fileName, self.finder)
File "/Users/runner/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/cx_Freeze-6.2-py3.8-macosx-10.14-x86_64.egg/cx_Freeze/freezer.py", line 632, in _WriteModules
self._CopyFile(module.file, target, copyDependentFiles=True,
File "/Users/runner/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/cx_Freeze-6.2-py3.8-macosx-10.14-x86_64.egg/cx_Freeze/freezer.py", line 193, in _CopyFile
self._CopyFile(dependent_file, target, copyDependentFiles,
File "/Users/runner/hostedtoolcache/Python/3.8.3/x64/lib/python3.8/site-packages/cx_Freeze-6.2-py3.8-macosx-10.14-x86_64.egg/cx_Freeze/freezer.py", line 150, in _CopyFile
raise Exception("Attempting to copy two files to "{}" ("{}" and "{}")".format(
Exception: Attempting to copy two files to "dist/libcrypto.1.0.0.dylib" ("/usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib" and "/usr/local/Cellar/openssl/1.0.2t/lib/libcrypto.1.0.0.dylib")

[cainesi]

[Sorry, didn't mean to close -- was a mis-click!]

Hi Marcelo -- would you mind sending me a copy of the script, so I can see what is actually being done that triggers this error.

My guess is that you have two packages that (i) are both being included in the zip file, and (ii) each separately includes a MachO file with the same name. If a package is being included in the zip file, then any binaries it references get copied into a specific directory. So if we are trying to copy two libraries with the same name, there will be problems.

(I think this was always an issue, but before my changes it was just sort of silently ignored-- maybe on the assumption that the two files would likely be equivalent anyway.)

I suppose in this case (as a stop-gap) we could print a warning message telling the user to remove one of the packages from the zip and try again? A better but more complicated change would be to rename one of the libraries, and then trace through to make sure all the binary linkage data was updated correctly.

@cainesi
I do a small test in a Github Action and get an exception.
#sample.py
import sys

[...]

Why do you close?

[marcelotduarte]

Why do you close?

[cainesi]

Sorry, closed by mistake.

[marcelotduarte]

Is a very simple test. My GA used to test is not good.
But before your patch, it builds, doesn't run. After your patch, it doesn't build.

#samples.py

import sys

print(sys.argv)
print(sys.executable)
print(sys.path)

#setup.py

from cx_Freeze import setup, Executable

buildOptions = dict(packages = [], excludes = [], build_exe = "dist",
                   zip_include_packages=["*"], zip_exclude_packages=[])

base = 'Console'

executables = [
    Executable('samples.py', base=base)
]

setup(name='samples',
      version = '0.1',
      description = '',
      options = dict(build_exe = buildOptions),
      executables = executables)

python setup.py build

[marcelotduarte]

suppose in this case (as a stop-gap) we could print a warning message telling the user to remove one of the packages from the zip and try again?

In my test it not use crypto, but two crypto libraries are found. Python itself uses a crypto library, but I think that it uses only one.
We'll have to discover the correct.

[cainesi]

suppose in this case (as a stop-gap) we could print a warning message telling the user to remove one of the packages from the zip and try again?

In my test it not use crypto, but two crypto libraries are found. Python itself uses a crypto library, but I think that it uses only one.
We'll have to discover the correct.

Thanks Marcelo,

The two conflicting files that cx-freeze is trying to include are "/usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib" and "/usr/local/Cellar/openssl/1.0.2t/lib/libcrypto.1.0.0.dylib". It seems like those are probably both installed by Homebrew. I'm not very familiar with Homebrew, but I just googled it and it looks like the files in /usr/local/opt/ are just sym-links of the files in /usr/local/Cellar/ (and the opt directory there so that libraries can have version-independent paths).

So I guess what is happening is that cx-freeze, when it is tracing through all the binary dependencies, gets to the libcrypto.1.0.0.dylib library in two different ways resulting in the two different paths above. It thinks those are actually different files, and fails out.

If you look on my fork of the project, I have another branch that prints out a detailed report of why each file is being included (e.g., what it is a dependency of) ("trace_includes" branch). When I run that, it looks like libcrypto is a dependency of three files:

1. _ssl.cpython-37m-darwin.so, which is included in the _ssl module, which looks like it is always included by default,
2. _hashlib.cpython-37m-darwin.so which is a dependency of the _hashlib module, which is also included by default, and
3. libssl, which is also a dependency of the previous two files.

But I'm using macports, and on my machine those three ways of getting to libcrypto all result in the same path...

[marcelotduarte]

@cainesi I'll release a version in a few days, but I'll leave you PR for the next, because of a need to test it in some way. I don't have a mac, then I'm trying to test in GA but I have errors not related to your patch. Then, I'll dedicate myself to release this version and the next to issues and improvements on the mac side.

I also want to ask you to put the samples in a separate PR.

"looks like the files in /usr/local/opt/ are just sym-links of the files in /usr/local/Cellar/ "
I think that you can use os.path.realpath to get/compare these paths.

[cainesi]

"looks like the files in /usr/local/opt/ are just sym-links of the files in /usr/local/Cellar/ "
I think that you can use os.path.realpath to get/compare these paths.

I've removed the new samples and implemented the os.path.realpath fix in the latest commit. Let me know if if fixes your issue.

Incidentally-- would you prefer to have the pull request squashed down into a single commit?

[cainesi]

This PR doesn't work correctly with the latest master because of the following new code that was added to
Freezer._FreezeExecutable():

        # Ensure the copy of default python libraries
        dependent_files = self._GetDependentFiles(exe.base) or []
        dependent_files += self._GetDependentFiles(sys.executable) or []
        dependent_files = list(set(dependent_files))

The problem is that the logic of the PR assumes that _GetDependentFiles would only be called from _CopyFile (where a DarwinFile object would already have been created for the file for which dependencies are being determined). I'm not sure that the code above is necessary--wouldn't the same dependencies be found when we did the _CopyFile later in the function?

[marcelotduarte]

I'm not sure that the code above is necessary

Initially I have implemented only on Windows (#640), and fixed some issues, including the use of pyside2/shiboken2.
In Linux, I have an issue too. And in macOS, some users reported errors that lead me to make this (#706) for all platforms (#701 (comment)).

I isn't always necessary, only in some circumstances. It can be detected and copied in some systems, in others no, then I force the copy.

[cainesi]

Okay, I think I can revise the PR to work around that then.

I am a bit curious how the person in #701 could have ended up with the wrong Python version on OSX. The Console base file links to Python, so that should have brought it in? I wonder if maybe the person had built the extension with a different version of Python, resulting in a bad link in that binary?

[marcelotduarte]

cx-freeze needs to find the shared library to copy to build folder. In some systems, Python is static linked and distribute the dynamic lib (See this: conda-forge/python-feedstock#222)

It is possible, in that case, as it has an official python 382 with shared lib and pyenv python 384 static linked (without distributing a shared lib).
Luckily I was looking for information to convince myself to apply my code and it helped me a lot by testing.

[cainesi]

The PR should now accommodate the additional _GetDependencies calls in _FreezeExecutable.

I left this as separate commits for now to ease review. But would you mind if I squashed it into a single commit before this gets accepted?

[marcelotduarte]

Then I'll take a look.

But would you mind if I squashed it into a single commit before this gets accepted?
There is no need because I have been using 'squash and merge' so there it is a single commit.

ok

[marcelotduarte]

Hi @anthony-tuininga, How are you?
When you have some time, help me by doing a review. I've already checked out a few things, but it's a big patch, and it would be nice if you could help.

[marcelotduarte]

@cainesi I will merge your patches and publish a new version as soon as I can.
If you have any revisions to make, please let me know.

[klensy]

Some type annotations not compatible with supported python 3.5 version

[cainesi]

Thanks for the heads-up. I have no more revisions on this pull request. (I've been focused on the Windows side more, recently.)

Cannot comment on the python3.5 issue--have not been using that for a while. The type annotations are obviously not critical (though I like them!) if you want some removed to maintain compatibility.

[marcelotduarte]

@klensy @cainesi The next release can be 3.6 compatible, as 3.5 EOL.

[klensy]

@cainesi I'll fix annotations later, when (if) @marcelotduarte will merge my PR