Skip to content
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

makensis exit code not checked on Windows #453

Closed
FaustinCarter opened this issue Jun 9, 2021 · 0 comments · Fixed by #475
Closed

makensis exit code not checked on Windows #453

FaustinCarter opened this issue Jun 9, 2021 · 0 comments · Fixed by #475
Assignees
Labels
locked [bot] locked due to inactivity os::windows relevant to Windows

Comments

@FaustinCarter
Copy link
Contributor

In verbose mode the makensis return code is not properly checked. Any failure of makensis will be treated as a success by constructor. This is because subprocess.Popen.communicate (line 209) does note raise an error for non-zero return codes the same way that subprocess.check_call (line 217) does.

if verbose:
sub = Popen(args, stdout=PIPE, stderr=PIPE)
stdout, stderr = sub.communicate()
for msg, information in zip((stdout, stderr), ('stdout', 'stderr')):
# on Python3 we're getting bytes
if hasattr(msg, 'decode'):
msg = msg.decode()
print('makensis {}:'.format(information))
print(msg)
else:
check_call(args)
shutil.rmtree(tmp_dir)

Suggested fix is to explicitly check sub.returncode and raise an error.

@jaimergp jaimergp added the os::windows relevant to Windows label Mar 10, 2022
@jaimergp jaimergp self-assigned this Mar 10, 2022
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Sep 15, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked [bot] locked due to inactivity os::windows relevant to Windows
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants