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

critical bug fix in installer #629

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

GB609
Copy link
Collaborator

@GB609 GB609 commented Dec 11, 2024

Description

My last pullrequest with the reworked wine installer contains a critical bug that prevents wine installations from working:

_exe_cmd might not consume all of the subprocess stdout because the loop exits as soon as the subprocess has a return code, but only 1 line per poll() check is consumed, so there might still be more lines in the buffer. These must also be consumed, especially when the output is required, as is the case for lang_install.

The output of innoextract --list-languages is very short and it returns quickly, so the first poll() might already return 0 and just 1 line will be consumed, which will be 'Inspecting "Game" ...', the actual lines listing languages come afterwards.

As lang_install is now used in extract_wine, there are situations where the return of lang_install will not be in the expected format '--language=langname', therefore the split operation fails with an error and the install aborts.

This happened because of my last-minute changes to lang_install and innoextract installation introduced when fixing review comments.

This PR fixes this in 2 places:

  1. Make _exe_cmd consume everything
  2. make sure there will always be a default return from lang_install

Checklist

  • CHANGELOG.md was updated (format: - Change made (thanks to github_username))

@sharkwouter
Copy link
Owner

Good catch, I forgot about it reading only one line after changing it to readline.

@sharkwouter sharkwouter merged commit ec474bf into sharkwouter:master Dec 11, 2024
2 checks passed
@GB609 GB609 deleted the bugfix/exe_cmd_readall branch December 29, 2024 16:12
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.

2 participants