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

utils/win32: trap invalid --icon arguments and terminate with a message #3126

Merged
merged 2 commits into from Mar 1, 2018

Conversation

Projects
None yet
2 participants
@tallforasmurf
Contributor

tallforasmurf commented Dec 15, 2017

An invalid file path argument to --icon is detected in one of two places. If it ends in .ico an attempt to open the file is in the __init__() method of the IconFile class. Here trap any IOError subclass, which would include nonexistent file, unreadable file, etc, and raise SystemExit with a simple "could not open" message.

Other file extensions are opened using win32api.LoadLibraryEx which expects to get an EXE (or a DLL?). If it fails, win32api raises its unique exception which includes the windows error string and error code. These are formatted into a message string for SystemExit.

raise SystemExit(
"Unable to load icon file {}\n {} (Error code {})".format(
srcpath, W32E.strerror, W32E.winerror)
)

This comment has been minimized.

@ghost

ghost Dec 16, 2017

This line is still indented too much.

Nat Picker
@tallforasmurf

This comment has been minimized.

Contributor

tallforasmurf commented Dec 17, 2017

Based on advice in this blog post I did the following:

  • corrected spacing and saved file

    git add -A
    git commit --amend
    git push -f origin HEAD

Clicking the "View changes" button above gives the scary message "We went looking everywhere, but couldn’t find those commits." However clicking the link for 7163b17 shows the correct stuff.

@ghost

This comment has been minimized.

ghost commented Dec 17, 2017

@tallforasmurf Honestly, I wouldn't worry about it, but your workflow is still not correct. The HEAD branch should NOT be named "develop." You should have checked out a different branch with a descriptive name for this feature.

However, IMO it's not a big deal and not worth it to try again (and possibly lose work).

FIXUP Lint-error
	F841 @ 92:27 - local variable 'OSexception' is assigned to but never used

@htgoebel htgoebel merged commit 1f32ef7 into pyinstaller:develop Mar 1, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@htgoebel

This comment has been minimized.

Member

htgoebel commented Mar 1, 2018

Thanks :-)

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