Merge branch 'master' into find-executable-fix

CHANGELOG.md

@@ -1,5 +1,5 @@
 **1.3.2**
-- Completely reworked windows wine installation. This should solve a lot of problems with failing game installs (thanks to GB609)
+- Completely reworked windows wine installation. This should solve a lot of problems with failing game installs. Innoextract (if installed) is only used to detect and configure the installation language. (thanks to GB609)
 - Variables and arguments in game settings can now contain blanks when quoted shell-style (thanks to GB609)
 - Minigalaxy will now create working Desktop Shortcuts for wine games (thanks to GB609)
 - Make games Unreal Gold able to launch

minigalaxy/constants.py

@@ -23,6 +23,31 @@
     ["ro", _("Romanian")],
 ]
 
+# match locale ids to special language names used by some installers
+# mapping supports 1:n so we can add more than one per language if needed later
+GAME_LANGUAGE_IDS = {
+    "br": ["brazilian"],
+    "cn": ["chinese"],
+    "da": ["danish"],
+    "nl": ["dutch"],
+    "en": ["english"],
+    "fi": ["finnish"],
+    "fr": ["french"],
+    "de": ["german"],
+    "hu": ["hungarian"],
+    "it": ["italian"],
+    "jp": ["japanese"],
+    "ko": ["korean"],
+    "no": ["norwegian"],
+    "pl": ["polish"],
+    "pt": ["portuguese"],
+    "ru": ["russian"],
+    "es": ["spanish"],
+    "sv": ["swedish"],
+    "tr": ["turkish"],
+    "ro": ["romanian"]
+}
+
 SUPPORTED_LOCALES = [
     ["", _("System default")],
     ["pt_BR", _("Brazilian Portuguese")],

minigalaxy/installer.py

@@ -9,6 +9,7 @@
 import re
 
 from minigalaxy.config import Config
+from minigalaxy.constants import GAME_LANGUAGE_IDS
 from minigalaxy.game import Game
 from minigalaxy.logger import logger
 from minigalaxy.translation import _
@@ -52,8 +53,7 @@ def install_game(  # noqa: C901
         language: str,
         install_dir: str,
         keep_installers: bool,
-        create_desktop_file: bool,
-        use_innoextract: bool = True,  # not set externally as of yet
+        create_desktop_file: bool
 ):
     error_message = ""
     tmp_dir = ""
@@ -147,45 +147,15 @@ def extract_linux(installer, temp_dir):
 
 
 def extract_windows(game: Game, installer: str, language: str):
-    if shutil.which("innoextract"):
-        game_lang = lang_install(installer, language)
-        game_lang = game_lang.split('=')[1]  # lang_install returns '--language=localeCode'
-    else:
-        game_lang = 'en-US'
-
+    languageLog = os.path.join(game.install_dir, 'minigalaxy_setup_languages.log')
+    if not os.path.exists(game.install_dir):
+        os.makedirs(game.install_dir)
+    game_lang = match_game_lang_to_installer(installer, language, languageLog)
     logger.info(f'use {game_lang} for installer')
 
     return extract_by_wine(game, installer, game_lang), False
 
 
-def extract_by_innoextract(installer: str, temp_dir: str, language: str, use_innoextract: bool):
-    err_msg = ""
-    if use_innoextract:
-        lang = lang_install(installer, language)
-        cmd = ["innoextract", installer, "-d", temp_dir, "--gog", lang]
-        stdout, stderr, exitcode = _exe_cmd(cmd)
-        if exitcode not in [0]:
-            err_msg = _("Innoextract extraction failed.")
-        else:
-            # In the case the game is installed in "temp_dir/app" like Zeus + Poseidon (Acropolis)
-            inno_app_dir = os.path.join(temp_dir, "app")
-            if os.path.isdir(inno_app_dir):
-                _mv(inno_app_dir, temp_dir)
-            # In the case the game is installed in "temp_dir/game" like Dragon Age™: Origins - Ultimate Edition
-            inno_game_dir = os.path.join(temp_dir, "game")
-            if os.path.isdir(inno_game_dir):
-                _mv(inno_game_dir, temp_dir)
-            innoextract_unneeded_dirs = ["__redist", "tmp", "commonappdata", "app", "DirectXpackage", "dotNet35"]
-            innoextract_unneeded_dirs += ["MSVC2005", "MSVC2005_x64", "support", "__unpacker", "userdocs", "game"]
-            for unneeded_dir in innoextract_unneeded_dirs:
-                unneeded_dir_full_path = os.path.join(temp_dir, unneeded_dir)
-                if os.path.isdir(unneeded_dir_full_path):
-                    shutil.rmtree(unneeded_dir_full_path)
-    else:
-        err_msg = _("Innoextract not installed.")
-    return err_msg
-
-
 def extract_by_wine(game, installer, game_lang, config=Config()):
     # Set the prefix for Windows games
     prefix_dir = os.path.join(game.install_dir, "prefix")
@@ -212,10 +182,10 @@ def extract_by_wine(game, installer, game_lang, config=Config()):
         # this avoids issues with varying path and spaces
         "/DIR=c:\\game",
         # capture information for debugging during install
+        f"/LANG={game_lang}",
         "/LOG=c:\\install.log",
     ]
     installer_args_full = [
-        f"/LANG={config.lang}",
         "/SAVEINF=c:\\setup.inf",
         # installers can run very long, give at least a bit of visual feedback
         # by using /SILENT instead of /VERYSILENT
@@ -414,28 +384,24 @@ def _exe_cmd(cmd, print_output=False):
     os.set_blocking(process.stdout.fileno(), False)
     os.set_blocking(process.stderr.fileno(), False)
     while not done:
-        if (return_code := process.poll()) is not None:
-            done = True
-        if data := process.stdout.readline():
-            std_out += data
-            if print_output:
-                print(data, end='')
-        if data := process.stderr.readline():
-            std_err += data
+        if std_line := process.stdout.readline():
+            std_out += std_line
             if print_output:
-                print(data, end='')
-        time.sleep(0.01)
+                print(std_line, end='')
 
-    # there might some lines left in the buffer, get them all
-    data = process.stdout.readlines()
-    std_out += ''.join(data)
-    if print_output:
-        print(data, end='')
+        if err_line := process.stderr.readline():
+            std_err += err_line
+            if print_output:
+                print(err_line, end='')
 
-    data = process.stderr.readlines()
-    std_err += ''.join(data)
-    if print_output:
-        print(data, end='')
+        # continue the loop until there is
+        # 1. a return code and
+        # 2. nothing more to consume
+        # this makes sure everything was read
+        time.sleep(0.02)
+        return_code = process.poll()
+        line_read = len(std_line) + len(err_line)
+        done = return_code is not None and line_read == 0
 
     process.stdout.close()
     process.stderr.close()
@@ -459,9 +425,26 @@ def _mv(source_dir, target_dir):
 # Some installers allow to choose game's language before installation (Divinity Original Sin or XCom EE / XCom 2)
 # "--list-languages" option returns "en-US", "fr-FR" etc... for these games.
 # Others installers return "French : Français" but disallow to choose game's language before installation
-def lang_install(installer: str, language: str):
-    arg = "--language=en-US"
+# When outputLogFile is given, the output of --list-languages is also saved in this file to have a bit of
+# additional debug information in GH tickets in case the wrong language is picked during installation
+def match_game_lang_to_installer(installer: str, language: str, outputLogFile=None):
+    if not shutil.which('innoextract'):
+        return 'en-US'
+
     stdout, stderr, ret_code = _exe_cmd(["innoextract", installer, "--list-languages"])
+    if ret_code not in [0]:
+        logger.error(stderr)
+        return "en-US"
+
+    lang_keys = GAME_LANGUAGE_IDS.get(language, [])
+    # match lines like ' - french : French'
+    # gets the first lowercase word which is the key
+    lang_name_regex = re.compile('(\\w+)\\s*:\\s*.*')
+
+    if outputLogFile is not None:
+        logger.info('write setup language data: ', outputLogFile)
+        with open(outputLogFile, "w") as text_file:
+            text_file.write(stdout)
 
     for line in stdout.split('\n'):
         if not line.startswith(' -'):
@@ -470,7 +453,11 @@ def lang_install(installer: str, language: str):
         lang = line[3:]
         if "-" in lang:  # lang must be like "en-US" only.
             if language == lang[0:2]:
-                arg = "--language={}".format(lang)
-                break
+                return lang
+
+        elif match := lang_name_regex.match(lang):
+            lang_id = match.group(1)
+            if lang_id in lang_keys:
+                return lang_id
 
-    return arg
+    return "en-US"

tests/test_installer.py

@@ -86,27 +86,6 @@ def test2_extract_installer(self, mock_subprocess, mock_listdir, mock_is_file):
         obs, use_temp = installer.extract_installer(game, installer_path, temp_dir, "en")
         self.assertEqual(exp, obs)
 
-    # TODO: Delete - innoextract not used for installation anymore
-    # test is only made not to fail, but it is pointless, as wine is called
-    # internally anyway
-    @mock.patch('minigalaxy.installer.try_wine_command')
-    @mock.patch('os.path.exists')
-    @mock.patch('subprocess.Popen')
-    @mock.patch('shutil.which')
-    def test3_extract_installer(self, mock_which, mock_subprocess, mock_exists, mock_cmd):
-        """[scenario: innoextract, unpack success]"""
-        mock_which.return_value = True
-        mock_subprocess().poll.return_value = 0
-        mock_subprocess().stdout.readline.return_value = " - en-US"
-        mock_exists.return_value = True
-        mock_cmd.side_effect = [True, True]
-        game = Game("Absolute Drift", install_dir="/home/makson/GOG Games/Absolute Drift", platform="windows")
-        installer_path = "/home/makson/.cache/minigalaxy/download/Absolute Drift/setup_absolute_drift_1.0f_(64bit)_(47863).exe"
-        temp_dir = "/home/makson/.cache/minigalaxy/extract/1136126792"
-        exp = ""
-        obs, use_temp = installer.extract_installer(game, installer_path, temp_dir, "en")
-        self.assertEqual(exp, obs)
-
     @mock.patch('os.path.exists')
     @mock.patch('os.listdir')
     @mock.patch('subprocess.Popen')
@@ -121,49 +100,53 @@ def test_extract_linux(self, mock_subprocess, mock_listdir, mock_is_file):
         obs, temp_used = installer.extract_linux(installer_path, temp_dir)
         self.assertEqual(exp, obs)
 
-    @mock.patch('minigalaxy.installer.try_wine_command')
     @mock.patch('os.path.exists')
-    @mock.patch('subprocess.Popen')
-    def test_extract_windows(self, mock_subprocess, mock_exists, mock_cmd):
-        """[scenario: innoextract, unpack success]"""
-        mock_subprocess().poll.return_value = 0
-        mock_subprocess().stdout.readlines.return_value = ["stdout", "stderr"]
+    @mock.patch('minigalaxy.installer.extract_by_wine')
+    @mock.patch('shutil.which')
+    def test1_get_lang_with_innoextract(self, mock_which, mock_wine_extract, mock_exists):
+        """[scenario: no innoextract - default en-US used]"""
+        mock_which.return_value = False
         mock_exists.return_value = True
-        mock_cmd.side_effect = [True, True]
-        game = Game("Absolute Drift", install_dir="/home/makson/GOG Games/Absolute Drift", platform="windows")
         installer_path = "/home/makson/.cache/minigalaxy/download/Absolute Drift/setup_absolute_drift_1.0f_(64bit)_(47863).exe"
-        exp = ""
-        obs, uses_tmp = installer.extract_windows(game, installer_path, "en")
-        self.assertEqual(exp, obs)
+        game = Game("Absolute Drift", install_dir="/home/makson/GOG Games/Absolute Drift", platform="windows")
+        exp = "en-US"
+        # check that lang passed to the wine installer is set up correctly
+        mock_wine_extract.side_effect = lambda game, installer, lang: self.assertEqual(exp, lang)
+        installer.extract_windows(game, installer_path, "en")
 
-    @mock.patch('subprocess.Popen')
-    def test1_extract_by_innoextract(self, mock_subprocess):
-        """[scenario: success]"""
-        mock_subprocess().poll.return_value = 0
-        mock_subprocess().stdout.readlines.return_value = ["stdout", "stderr"]
+    @mock.patch('shutil.which')
+    @mock.patch('minigalaxy.installer._exe_cmd')
+    def test2_get_lang_with_innoextract(self, mock_exe, mock_which):
+        """[scenario: innoextract --list-languages returns locale ids]"""
+        lines = [" - fr-FR\n", " - jp-JP\n", " - en-US\n", " - ru-RU"]
+        mock_exe.return_value = ''.join(lines), '', 0
+        mock_which.return_value = '/bin/innoextract'
         installer_path = "/home/makson/.cache/minigalaxy/download/Absolute Drift/setup_absolute_drift_1.0f_(64bit)_(47863).exe"
-        temp_dir = "/home/makson/.cache/minigalaxy/extract/1136126792"
-        exp = ""
-        obs = installer.extract_by_innoextract(installer_path, temp_dir, "en", use_innoextract=True)
+        exp = "jp-JP"
+        obs = installer.match_game_lang_to_installer(installer_path, "jp")
         self.assertEqual(exp, obs)
 
-    def test2_extract_by_innoextract(self):
-        """[scenario: not installed/disabled]"""
+    @mock.patch('shutil.which')
+    @mock.patch('minigalaxy.installer._exe_cmd')
+    def test3_get_lang_with_innoextract(self, mock_exe, mock_which):
+        """[scenario: innoextract --list-languages returns language names]"""
+        lines = [" - english: English\n", " - german: Deutsch\n", " - french: Français"]
+        mock_exe.return_value = ''.join(lines), '', 0
+        mock_which.return_value = '/bin/innoextract'
         installer_path = "/home/makson/.cache/minigalaxy/download/Absolute Drift/setup_absolute_drift_1.0f_(64bit)_(47863).exe"
-        temp_dir = "/home/makson/.cache/minigalaxy/extract/1136126792"
-        exp = "Innoextract not installed."
-        obs = installer.extract_by_innoextract(installer_path, temp_dir, "en", use_innoextract=False)
+        exp = "french"
+        obs = installer.match_game_lang_to_installer(installer_path, "fr")
         self.assertEqual(exp, obs)
 
-    @mock.patch('subprocess.Popen')
-    def test3_extract_by_innoextract(self, mock_subprocess):
-        """[scenario: unpack failed]"""
-        mock_subprocess().poll.return_value = 1
-        mock_subprocess().stdout.readlines.return_value = ["stdout", "stderr"]
+    @mock.patch('shutil.which')
+    @mock.patch('minigalaxy.installer._exe_cmd')
+    def test4_get_lang_with_innoextract(self, mock_exe, mock_which):
+        """[scenario: innoextract --list-languages can't be matched - default en-US is used]"""
+        mock_exe.return_value = '', '', 0
+        mock_which.return_value = '/bin/innoextract'
         installer_path = "/home/makson/.cache/minigalaxy/download/Absolute Drift/setup_absolute_drift_1.0f_(64bit)_(47863).exe"
-        temp_dir = "/home/makson/.cache/minigalaxy/extract/1136126792"
-        exp = "Innoextract extraction failed."
-        obs = installer.extract_by_innoextract(installer_path, temp_dir, "en", use_innoextract=True)
+        exp = "en-US"
+        obs = installer.match_game_lang_to_installer(installer_path, "en")
         self.assertEqual(exp, obs)
 
     @mock.patch('subprocess.Popen')