Fix issue when using exe wavtools on Linux #1198
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
June 26, 2024 10:35
The system for running exe wavtools is to create a temporary batch file, but OpenUtau tries to run this batch file directly, which does not work on Linux. This commit adds a check for the operating system and will create a shell script wrapper that runs the batch file through wine if the OS is Linux. A few other changes also had to be made for this to work, such as using different character encodings that did not include byte order marks, because they cause problems both when running shell scripts and when running batch files through wine. Because the current implementation uses the path style appropriate for the operating system, it was writing linux-style paths to the batch file, which was another cause of issues that this commit fixes. Finally, the wavtool and resampler paths both point to the wrapper created by the user during the install process (see link at bottom), but this is not the correct path to give the batch file because windows batch files cannot run ilnux shell scripts; therefore, this commit includes code to extract the path to the exe file from the sh wrapper. https://github.com/stakira/OpenUtau/wiki/Resamplers-and-Wavtools#linux
OpenUtau was trying to run a batch file directly when using an external .exe wavtool, which does not work on Linux. This commit includes several changes needed to make .exe wavtools work on Linux. Rather than running a batch file directly, a temporary .sh file is created which runs the batch file through wine. There are a few other changes that had to be made to accomodate this: - Byte order marks cause problems when executing shell scripts and when executing batch files through wine, so there is code to use character encodings that do not include them when writing to the files, but only on Linux. - The Path class always returns linux-style paths when running on Linux, (as it should), but when writing to a .bat file, the paths need to be Windows-style, so this commit includes code to convert the paths to the proper style. - The batch file needs to know the location of the .exe file for the resampler / wavtool, not the location of the .sh wrapper that the user created while installing it. OpenUtau only knows the location of this wrapper, so there is a method to parse the .sh file to resolve the path to the .exe file.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I encountered this error when trying to use an external .exe wavtool on Linux: (the first image shows the original error, and the second shows the error after I marked temp.bat and temp_helper.bat as executable)
The cause was that OpenUtau creates a temporary batch file to run the wavtool and attempts to run it directly, which does not work on Linux because batch files can only run on Windows or through Wine.
This patch fixes it by creating a temporary shell wrapper to run the batch file, but there were a few other changes I had to make to accommodate this, which is described in more detail in the commit messages (there are 5 commits when really there are 2 because I changed one of the commit messages to be worded more clearly). All of the new code is behind checks for OS.IsLinux(), so none of the new code affects things on Windows, but to make sure, I tested it in a Windows VM and external wavtools and resamplers worked fine.
I don't know if macOS faces the same issue, nor do I know if the same fix would work on macOS (because I have never used a mac or developed anything for a mac), so for now this patch affects only Linux.