Merge pull request #4670 from awsr/subprocess

Update subprocess usage

cli/nvidia-smi.py

@@ -13,9 +13,8 @@ def get_nvidia_smi(output='dict'):
     if smi is None:
         log.error("nvidia-smi not found")
         return None
-    result = subprocess.run(f'"{smi}" -q -x', shell=True, check=False, env=os.environ, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    xml = result.stdout.decode(encoding="utf8", errors="ignore")
-    d = xmltodict.parse(xml)
+    result = subprocess.run(f'"{smi}" -q -x', shell=True, check=False, env=os.environ, capture_output=True, text=True)
+    d = xmltodict.parse(result.stdout)
     if 'nvidia_smi_log' in d:
         d = d['nvidia_smi_log']
     if 'gpu' in d and 'supported_clocks' in d['gpu']:

installer.py

@@ -200,14 +200,32 @@ def uninstall(package, quiet = False):
     return res
 
 
-def run(cmd: str, arg: str):
-    result = subprocess.run(f'"{cmd}" {arg}', shell=True, check=False, env=os.environ, capture_output=True)
-    txt = result.stdout.decode(encoding="utf8", errors="ignore")
-    if len(result.stderr) > 0:
-        txt += ('\n' if len(txt) > 0 else '') + result.stderr.decode(encoding="utf8", errors="ignore")
-    txt = txt.strip()
-    debug(f'Exec {cmd}: {txt}')
-    return txt
+def run(cmd: str, *args: str, **kwargs):
+    """Run command and arguments with `subprocess.run`.
+
+    Default run options are `shell=True, check=False, env=os.environ`.
+
+    Args:
+        cmd (str): Main command to run.
+        *args (str): Additional command arguments.
+        **kwargs: `subprocess.run` option overrides.
+
+    Returns:
+        tuple[CompletedProcess[str], str]: Tuple with the results and the combined `stdout` and `stderr` values.
+    """
+    options = {
+        "check": False,
+        "env": os.environ,
+    }
+    options |= kwargs  # Override defaults with passed kwargs
+    result = subprocess.run(f'"{cmd}" {" ".join(args)}', **options, shell=True, capture_output=True, text=True)
+    result.stdout = result.stdout.strip()
+    result.stderr = result.stderr.strip()
+    txt = result.stdout
+    if result.stderr:
+        # Put newline between outputs only if stdout isn't empty
+        txt += "\n" + result.stderr if txt else result.stderr
+    return result, txt
 
 
 def cleanup_broken_packages():
@@ -246,18 +264,13 @@ def pip(arg: str, ignore: bool = False, quiet: bool = True, uv = True):
     all_args = f'{pip_log}{arg} {env_args}'.strip()
     if not quiet:
         log.debug(f'Running: {pipCmd}="{all_args}"')
-    result = subprocess.run(f'"{sys.executable}" -m {pipCmd} {all_args}', shell=True, check=False, env=os.environ, capture_output=True)
-    txt = result.stdout.decode(encoding="utf8", errors="ignore")
+    result, txt = run(sys.executable, "-m", pipCmd, all_args)
     if len(result.stderr) > 0:
         if uv and result.returncode != 0:
-            err = result.stderr.decode(encoding="utf8", errors="ignore")
             log.warning(f'Install: cmd="{pipCmd}" args="{all_args}" cannot use uv, fallback to pip')
-            debug(f'Install: uv pip error: {err}')
+            debug(f'Install: uv pip error: {result.stderr}')
             cleanup_broken_packages()
             return pip(originalArg, ignore, quiet, uv=False)
-        else:
-            txt += ('\n' if len(txt) > 0 else '') + result.stderr.decode(encoding="utf8", errors="ignore")
-    txt = txt.strip()
     debug(f'Install {pipCmd}: {txt}')
     if result.returncode != 0 and not ignore:
         errors.append(f'pip: {package}')
@@ -293,25 +306,21 @@ def git(arg: str, folder: str = None, ignore: bool = False, optional: bool = Fal
     git_cmd = os.environ.get('GIT', "git")
     if git_cmd != "git":
         git_cmd = os.path.abspath(git_cmd)
-    result = subprocess.run(f'"{git_cmd}" {arg}', check=False, shell=True, env=os.environ, capture_output=True, cwd=folder or '.')
-    stdout = result.stdout.decode(encoding="utf8", errors="ignore")
-    if len(result.stderr) > 0:
-        stdout += ('\n' if len(stdout) > 0 else '') + result.stderr.decode(encoding="utf8", errors="ignore")
-    stdout = stdout.strip()
+    result, txt = run(git_cmd, arg, cwd=folder or ".")
     if result.returncode != 0 and not ignore:
         if folder is None:
             folder = 'root'
-        if "couldn't find remote ref" in stdout: # not a git repo
+        if "couldn't find remote ref" in txt: # not a git repo
             log.error(f'Git: folder="{folder}" could not identify repository')
-        elif "no submodule mapping found" in stdout:
+        elif "no submodule mapping found" in txt:
             log.warning(f'Git: folder="{folder}" submodules changed')
-        elif 'or stash them' in stdout:
+        elif 'or stash them' in txt:
             log.error(f'Git: folder="{folder}" local changes detected')
         else:
-            log.error(f'Git: folder="{folder}" arg="{arg}" output={stdout}')
+            log.error(f'Git: folder="{folder}" arg="{arg}" output={txt}')
         errors.append(f'git: {folder}')
     ts('git', t_start)
-    return stdout
+    return txt
 
 
 # reattach as needed as head can get detached
@@ -925,13 +934,10 @@ def run_extension_installer(folder):
             if os.environ.get('PYTHONPATH', None) is not None:
                 seperator = ';' if sys.platform == 'win32' else ':'
                 env['PYTHONPATH'] += seperator + os.environ.get('PYTHONPATH', None)
-            result = subprocess.run(f'"{sys.executable}" "{path_installer}"', shell=True, env=env, check=False, capture_output=True, cwd=folder)
-            txt = result.stdout.decode(encoding="utf8", errors="ignore")
-            debug(f'Extension installer: file="{path_installer}" {txt}')
+            result, txt = run(sys.executable, path_installer, env=env, cwd=folder)
+            debug(f'Extension installer: file="{path_installer}" {result.stdout}')
             if result.returncode != 0:
                 errors.append(f'ext: {os.path.basename(folder)}')
-                if len(result.stderr) > 0:
-                    txt = txt + '\n' + result.stderr.decode(encoding="utf8", errors="ignore")
                 log.error(f'Extension installer error: {path_installer}')
                 log.debug(txt)
     except Exception as e:
@@ -1260,29 +1266,24 @@ def get_version(force=False):
         except Exception:
             pass
         try:
-            res = subprocess.run('git log --pretty=format:"%h %ad" -1 --date=short', capture_output=True, shell=True, check=True)
-            ver = res.stdout.decode(encoding = 'utf8', errors='ignore') if len(res.stdout) > 0 else '  '
+            ver = run('git', 'log --pretty=format:"%h %ad" -1 --date=short', check=True)[0].stdout or '  '
             commit, updated = ver.split(' ')
             version['commit'], version['updated'] = commit, updated
         except Exception as e:
             log.warning(f'Version: where=commit {e}')
         try:
-            res = subprocess.run('git remote get-url origin', capture_output=True, shell=True, check=True)
-            origin = res.stdout.decode(encoding = 'utf8', errors='ignore') if len(res.stdout) > 0 else ''
-            res = subprocess.run('git rev-parse --abbrev-ref HEAD', capture_output=True, shell=True, check=True)
-            branch_name = res.stdout.decode(encoding = 'utf8', errors='ignore') if len(res.stdout) > 0 else ''
-            version['url'] = origin.replace('\n', '').removesuffix('.git') + '/tree/' + branch_name.replace('\n', '')
-            version['branch'] = branch_name.replace('\n', '')
+            origin = run('git', 'remote get-url origin', check=True)[0].stdout
+            branch_name = run('git', 'rev-parse --abbrev-ref HEAD', check=True)[0].stdout
+            version['url'] = origin.removesuffix('.git') + '/tree/' + branch_name
+            version['branch'] = branch_name
             if version['branch'] == 'HEAD':
                 log.warning('Version: detached state detected')
         except Exception as e:
             log.warning(f'Version: where=branch {e}')
         try:
             if os.path.exists('extensions-builtin/sdnext-modernui'):
-                res = subprocess.run('git rev-parse --abbrev-ref HEAD', capture_output=True, shell=True, check=True, cwd='extensions-builtin/sdnext-modernui')
-                branch_ui = res.stdout.decode(encoding = 'utf8', errors='ignore') if len(res.stdout) > 0 else ''
-                branch_ui = 'dev' if 'dev' in branch_ui else 'main'
-                version['ui'] = branch_ui
+                branch_ui = run('git', 'rev-parse --abbrev-ref HEAD', check=True, cwd='extensions-builtin/sdnext-modernui')[0].stdout
+                version['ui'] = 'dev' if 'dev' in branch_ui else 'main'
             else:
                 version['ui'] = 'unavailable'
         except Exception as e:
@@ -1292,10 +1293,8 @@ def get_version(force=False):
             if os.environ.get('SD_KANVAS_DISABLE', None) is not None:
                 version['kanvas'] = 'disabled'
             elif os.path.exists('extensions-builtin/sdnext-kanvas'):
-                res = subprocess.run('git rev-parse --abbrev-ref HEAD', capture_output=True, shell=True, check=True, cwd='extensions-builtin/sdnext-kanvas')
-                branch_kanvas = res.stdout.decode(encoding = 'utf8', errors='ignore') if len(res.stdout) > 0 else ''
-                branch_kanvas = 'dev' if 'dev' in branch_kanvas else 'main'
-                version['kanvas'] = branch_kanvas
+                branch_kanvas = run('git', 'rev-parse --abbrev-ref HEAD', check=True, cwd='extensions-builtin/sdnext-kanvas')[0].stdout
+                version['kanvas'] = 'dev' if 'dev' in branch_kanvas else 'main'
             else:
                 version['kanvas'] = 'unavailable'
         except Exception as e:
@@ -1483,8 +1482,7 @@ def get_state():
         def _get_commit(item):
             ext, ext_dir = item
             try:
-                res = subprocess.run('git rev-parse HEAD', capture_output=True, shell=True, check=False, cwd=ext_dir)
-                return ext, res.stdout.decode(encoding='utf8', errors='ignore').strip()
+                return ext, run('git', 'rev-parse HEAD', cwd=ext_dir)[0].stdout
             except Exception:
                 return ext, ''
 

launch.py

@@ -97,13 +97,13 @@ def run(command, desc=None, errdesc=None, custom_env=None, live=False): # compat
         if result.returncode != 0:
             raise RuntimeError(f"""{errdesc or 'Error running command'} Command: {command} Error code: {result.returncode}""")
         return ''
-    result = subprocess.run(command, capture_output=True, check=False, shell=True, env=os.environ if custom_env is None else custom_env)
+    result = subprocess.run(command, capture_output=True, check=False, shell=True, env=os.environ if custom_env is None else custom_env, text=True)
     if result.returncode != 0:
         raise RuntimeError(f"""{errdesc or 'Error running command'}: {command} code: {result.returncode}
-{result.stdout.decode(encoding="utf8", errors="ignore") if len(result.stdout)>0 else ''}
-{result.stderr.decode(encoding="utf8", errors="ignore") if len(result.stderr)>0 else ''}
+{result.stdout}
+{result.stderr}
 """)
-    return result.stdout.decode(encoding="utf8", errors="ignore")
+    return result.stdout
 
 
 def check_run(command): # compatbility function

modules/devices.py

@@ -104,8 +104,8 @@ def get_gpu_info():
         elif torch.cuda.is_available() and torch.version.cuda:
             try:
                 import subprocess
-                result = subprocess.run('nvidia-smi --query-gpu=driver_version --format=csv,noheader', shell=True, check=False, env=os.environ, capture_output=True)
-                version = result.stdout.decode(encoding="utf8", errors="ignore").strip()
+                result = subprocess.run('nvidia-smi --query-gpu=driver_version --format=csv,noheader', shell=True, check=False, env=os.environ, capture_output=True, text=True)
+                version = result.stdout.strip()
                 return version
             except Exception:
                 return ''

modules/rocm.py

@@ -31,8 +31,8 @@ def dirname(path_: str, r: int = 1) -> str:
 
 
 def spawn(command: str | list[str], cwd: os.PathLike = '.') -> str:
-    process = subprocess.run(command, cwd=cwd, shell=True, check=False, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL)
-    return process.stdout.decode(encoding="utf8", errors="ignore")
+    process = subprocess.run(command, cwd=cwd, shell=True, check=False, capture_output=True, text=True)
+    return process.stdout
 
 
 def load_library_global(path_: str):