geefr
commented
4 years ago -
Overwriting winetricks: Sure, ideally we don't want to affect anything else, so running in a temp dir may be better (Did I not have a mkstemp/pushd/popd in that script?)
-
Deleting existing wine prefix: Contentious, would break my personal setup for one, and I expect many folks to just run one wine prefix. I'll say no to this one, but in any troubleshooting steps 'try nuking the wine prefix' should be a step
I did experiment with forcing a specific wine prefix, but it ended up more hassle than it's worth. Anyone that's used wine already for mod assistant, or just general .net stuff already has a setup and probably doesn't want to wait 20 minutes for another install (Ideally we'd have a .net core or similar version of IPA.exe, can't imagine it /needs/ to run under the microsoft .net framework for the initial patching step).
For the script being interactive - Okay in principle but there's an issue here. It's fine to have an interactive script if the user is running the script, but if they're using QBeat it should 'just do it'. At least the original aim was to hide everything behind a UI/wizard, so any user interaction at the script level would need to feed all the way back up and give some nice clicky buttons.
So overall it looks like you've got some ideas, and generally I'm happy to accept them as long as they don't break existing functionality/concepts. Rework/functional additions are welcome but consider testing overhead - Got at least 5 distros being used all with their own quirks, various levels of experience from the users ;)
ominitay
This PR modifies the wine setup script's behaviour in an attempt to reduce the number of non-bugs getting into issues. The current commit amends wget's behaviour in the wine setup script to overwrite a pre-existing winetricks script. This means that if a user has ran this script and downloaded a bad version of winetricks which does not work, they do not have to delete it. The issues caused by not doing so have already been shown in issue #50, where wget did not overwrite the broken winetricks script, causing significant confusion.
Ideally, we would be deleting a pre-existing Wine prefix instead, however that would impact users running only one prefix for multiple applications. What are your thoughts of these ideas for further modifications before this PR is marked ready for review: There should be a check for a pre-existing Wine prefix which warns the user both before and after the script runs with a message something like "WARN: A pre-existing Wine prefix was detected. Please delete it and/or use an independent prefix before reporting an issue." Also, the script should delete the Wine prefix it made if installation or verification fails, or it should at the very least instruct the user to delete it and start again if it doesn't work.
Any comments would be appreciated, and I will commit further to this branch before marking ready for review if my further ideas are deemed sensible.