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

Fixed missing system modules in app wrapper after upgrade to Python 3.6 #474

Merged
merged 1 commit into from Jan 31, 2017

Conversation

Projects
None yet
2 participants
@lenlo
Contributor

lenlo commented Jan 29, 2017

After upgrading to Python 3.6 [using brew on macOS 10.12], moneyGuru kept crashing on startup. The error was:

File "build/moneyGuru.app/Contents/Resources/py/encodings/init.py", line 31, in
ModuleNotFoundError: No module named 'codecs'

The problem was that none of the system modules had been copied into the app wrapper because the path to them -- as supplied by get_path('stdlib') in sysconfig.py -- included a symlink that made the path not match exactly what hscommon/build.py's collect_stdlib_dependencies expected.

Simple fix: Normalize the path using os.path.realpath before comparing with it.

Fixed missing system modules in app wrapper after upgrade to Python 3.6
After upgrading to Python 3.6 [using brew], moneyGuru kept crashing on
startup. The error was:

> File "build/moneyGuru.app/Contents/Resources/py/encodings/__init__.py",
  line 31, in <module>
> ModuleNotFoundError: No module named 'codecs'

The problem was that none of the system modules had been copied into the app
wrapper becuse the path to them -- as supplied by get_path('stdlib') in
sysconfig.py -- included a symlink that made the path not match exactly what
hscommon/build.py's collect_stdlib_dependencies expected.

Simple fix: Normalize the path using os.path.realpath before comparing with
it.

@hsoft hsoft self-requested a review Jan 29, 2017

@hsoft

Thanks! I could indeed verify the problem with Homebrew's python and I also verified that the fix was appropriate. There would just be that misleading comment to fix and it's good to merge.

@@ -415,11 +415,12 @@ def copy_embeddable_python_dylib(dst):
def collect_stdlib_dependencies(script, dest_folder, extra_deps=None):
sysprefix = sys.prefix # could be a virtualenv
real_lib_prefix = sysconfig.get_config_var('LIBDEST')
# python 3.6's resources gets installed under a symlink, at least under brew on macOS

This comment has been minimized.

@hsoft

hsoft Jan 29, 2017

Owner

I had never used homebrew's python before and I'm pretty sure that the problem is Homebrew-specific, not py36-specific. That would make this comment misleading. Could we change it?

@hsoft

hsoft Jan 29, 2017

Owner

I had never used homebrew's python before and I'm pretty sure that the problem is Homebrew-specific, not py36-specific. That would make this comment misleading. Could we change it?

This comment has been minimized.

@lenlo

lenlo Jan 31, 2017

Contributor

Well, I actually think the comment is pretty darn accurate as it stands: This is a problem with py36, at least as it is installed by Homebrew on macOS. It wasn't a problem with Homebrew's py35 and may or may not be a problem with py36 (or any other py for that matter) in other environments, I don't know. If you're sure that this only pertains to Homebrew's py36 and really feel that the comment needs to be changed, then by all means feel free to do so. I'm all for accuracy and correctness, but personally, I think this might be spending a bit too much time on barely meaningful nitpicks. Just IMHO. Peace.

@lenlo

lenlo Jan 31, 2017

Contributor

Well, I actually think the comment is pretty darn accurate as it stands: This is a problem with py36, at least as it is installed by Homebrew on macOS. It wasn't a problem with Homebrew's py35 and may or may not be a problem with py36 (or any other py for that matter) in other environments, I don't know. If you're sure that this only pertains to Homebrew's py36 and really feel that the comment needs to be changed, then by all means feel free to do so. I'm all for accuracy and correctness, but personally, I think this might be spending a bit too much time on barely meaningful nitpicks. Just IMHO. Peace.

This comment has been minimized.

@hsoft

hsoft Jan 31, 2017

Owner

I'm sorry to leave an impression of nitpicking. Please understand that I don't have the context you have. I've never tested Homebrew's py35, I don't use MacOS. This pull request gave little context (it wasn't clear what python you upgraded from), so I just wanted to make sure that the comment wasn't a mis-comment. With the additional context you've just provided, I'll gladly trust your word and merge as-is.

@hsoft

hsoft Jan 31, 2017

Owner

I'm sorry to leave an impression of nitpicking. Please understand that I don't have the context you have. I've never tested Homebrew's py35, I don't use MacOS. This pull request gave little context (it wasn't clear what python you upgraded from), so I just wanted to make sure that the comment wasn't a mis-comment. With the additional context you've just provided, I'll gladly trust your word and merge as-is.

This comment has been minimized.

@lenlo

lenlo Feb 1, 2017

Contributor

Thanks. No worries.

@lenlo

lenlo Feb 1, 2017

Contributor

Thanks. No worries.

@hsoft hsoft merged commit 07f6c73 into hsoft:master Jan 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment