Replace os.system calls with subprocess calls.

Author: jaraco

setup.cfg

@@ -76,6 +76,7 @@ testing =
 	tomli
 	# No Python 3.12 dependencies require importlib_metadata, but needed for type-checking since we import it directly
 	importlib_metadata
+	pytest-subprocess
 
 	# workaround for pypa/setuptools#4333
 	pyproject-hooks!=1.1

setuptools/package_index.py

@@ -1,6 +1,7 @@
 """PyPI and direct package downloading."""
 
 import sys
+import subprocess
 import os
 import re
 import io
@@ -865,15 +866,15 @@ def _download_vcs(self, url, spec_filename):
         url, rev = self._vcs_split_rev_from_url(url)
 
         self.info(f"Doing {vcs} clone from {url} to {filename}")
-        os.system(f"{vcs} clone --quiet {url} {filename}")
+        subprocess.check_call([vcs, 'clone', '--quiet', url, filename])
 
         co_commands = dict(
-            git=f"git -C {filename} checkout --quiet {rev}",
-            hg=f"hg --cwd {filename} up -C -r {rev} -q",
+            git=[vcs, '-C', filename, 'checkout', '--quiet', rev],
+            hg=[vcs, '--cwd', filename, 'up', '-C', '-r', rev, '-q'],
         )
         if rev is not None:
             self.info(f"Checking out {rev}")
-            os.system(co_commands[vcs])
+            subprocess.check_call(co_commands[vcs])
 
         return filename
 

setuptools/tests/test_packageindex.py

@@ -3,7 +3,6 @@
 import urllib.error
 import http.client
 from inspect import cleandoc
-from unittest import mock
 
 import pytest
 
@@ -171,41 +170,38 @@ def test_egg_fragment(self):
             assert dists[0].version == ''
             assert dists[1].version == vc
 
-    def test_download_git_with_rev(self, tmpdir):
+    def test_download_git_with_rev(self, tmpdir, fp):
         url = 'git+https://github.example/group/project@master#egg=foo'
         index = setuptools.package_index.PackageIndex()
 
-        with mock.patch("os.system") as os_system_mock:
-            result = index.download(url, str(tmpdir))
+        expected_dir = str(tmpdir / 'project@master')
+        fp.register([
+            'git',
+            'clone',
+            '--quiet',
+            'https://github.example/group/project',
+            expected_dir,
+        ])
+        fp.register(['git', '-C', expected_dir, 'checkout', '--quiet', 'master'])
 
-        os_system_mock.assert_called()
+        result = index.download(url, str(tmpdir))
 
-        expected_dir = str(tmpdir / 'project@master')
-        expected = (
-            'git clone --quiet ' 'https://github.example/group/project {expected_dir}'
-        ).format(**locals())
-        first_call_args = os_system_mock.call_args_list[0][0]
-        assert first_call_args == (expected,)
-
-        tmpl = 'git -C {expected_dir} checkout --quiet master'
-        expected = tmpl.format(**locals())
-        assert os_system_mock.call_args_list[1][0] == (expected,)
         assert result == expected_dir
+        assert len(fp.calls) == 2
 
-    def test_download_git_no_rev(self, tmpdir):
+    def test_download_git_no_rev(self, tmpdir, fp):
         url = 'git+https://github.example/group/project#egg=foo'
         index = setuptools.package_index.PackageIndex()
 
-        with mock.patch("os.system") as os_system_mock:
-            result = index.download(url, str(tmpdir))
-
-        os_system_mock.assert_called()
-
         expected_dir = str(tmpdir / 'project')
-        expected = (
-            'git clone --quiet ' 'https://github.example/group/project {expected_dir}'
-        ).format(**locals())
-        os_system_mock.assert_called_once_with(expected)
+        fp.register([
+            'git',
+            'clone',
+            '--quiet',
+            'https://github.example/group/project',
+            expected_dir,
+        ])
+        index.download(url, str(tmpdir))
 
     def test_download_svn(self, tmpdir):
         url = 'svn+https://svn.example/project#egg=foo'