Remove sh usages (#122)

* Began removing sh usages.

* Drop antsibull-core 1.x.y support. Drop direct sh dependency.

* Fix return type. Remove unnecessary imports and ignores.

* Import CalledProcessError from antsibull_core.subprocess_util.

* Document that we no longer depend directly on sh.

* p.stdout is already a string.

Author: felixfontein

.github/workflows/antsibull-docs.yml

@@ -38,7 +38,6 @@ jobs:
         include:
           - options: '--use-current --use-html-blobs --no-breadcrumbs community.crypto community.docker'
             python: '3.9'
-            antsibull_core_ref: stable-1
 
     steps:
       - name: Check out antsibull-docs

changelogs/fragments/122-antsibull-core-sh.yml

@@ -0,0 +1,3 @@
+minor_changes:
+  - "Now depends antsibull-core 2.0.0 or newer; antsibull-core 1.x.y is no longer supported (https://github.com/ansible-community/antsibull-docs/pull/122)."
+  - "Ansibull-docs now no longer depends directly on ``sh`` (https://github.com/ansible-community/antsibull-docs/pull/122)."

pyproject.toml

@@ -25,17 +25,15 @@ classifiers = [
 requires-python = ">=3.9"
 dependencies = [
     "ansible-pygments",
-    "antsibull-core >= 1.2.0, < 3.0.0",
+    # TODO: bump to >= 2.0.0
+    "antsibull-core >= 2.0.0a1, < 3.0.0",
     "antsibull-docs-parser ~= 0.3.0",
     "asyncio-pool",
     "docutils",
     "jinja2 >= 3.0",
     "packaging",
     "rstcheck >= 3.0.0, < 7.0.0",
     "sphinx",
-    # sh v2 has breaking changes.
-    # https://github.com/ansible-community/antsibull-core/issues/34
-    "sh >= 1.0.0, < 2.0.0",
     # pydantic v2 is a major rewrite
     "pydantic >= 1.0.0, < 2.0.0",
     "semantic_version",
@@ -124,10 +122,6 @@ source = [
 [tool.mypy]
 mypy_path = "stubs/"
 
-[[tool.mypy.overrides]]
-module = "sh"
-ignore_missing_imports = true
-
 [[tool.mypy.overrides]]
 module = "semantic_version"
 ignore_missing_imports = true

src/antsibull_docs/cli/doc_commands/plugin.py

@@ -12,9 +12,9 @@
 import traceback
 import typing as t
 
-import sh
 from antsibull_core.compat import asyncio_run
 from antsibull_core.logging import log
+from antsibull_core.subprocess_util import CalledProcessError
 from antsibull_core.vendored.json_utils import _filter_non_json_lines
 from antsibull_core.venv import FakeVenvRunner
 
@@ -47,26 +47,22 @@ def generate_plugin_docs(plugin_type: str, plugin_name: str,
     venv_ansible_doc = venv.get_command('ansible-doc')
     venv_ansible_doc = venv_ansible_doc.bake('-vvv')
     try:
-        ansible_doc_results = venv_ansible_doc('-t', plugin_type, '--json', plugin_name)
-    except sh.ErrorReturnCode as exc:
+        ansible_doc_results = venv.log_run(
+            ['ansible-doc', '-vvv', '-t', plugin_type, '--json', plugin_name])
+    except CalledProcessError as exc:
         err_msg = []
         formatted_exception = traceback.format_exception(None, exc, exc.__traceback__)
         err_msg.append(f'Exception while parsing documentation for {plugin_type} plugin:'
                        f' {plugin_name}.  Will not document this plugin.')
         err_msg.append(f'Exception:\n{"".join(formatted_exception)}')
 
-        stdout = exc.stdout.decode("utf-8", errors="surrogateescape")
-        stderr = exc.stderr.decode("utf-8", errors="surrogateescape")
-
-        err_msg.append(f'Full process stdout:\n{stdout}')
-        err_msg.append(f'Full process stderr:\n{stderr}')
+        err_msg.append(f'Full process stdout:\n{exc.stdout}')
+        err_msg.append(f'Full process stderr:\n{exc.stderr}')
 
         sys.stderr.write('\n'.join(err_msg))
         return 1
 
-    stdout = ansible_doc_results.stdout.decode("utf-8", errors="surrogateescape")
-
-    plugin_data = json.loads(_filter_non_json_lines(stdout)[0])
+    plugin_data = json.loads(_filter_non_json_lines(ansible_doc_results.stdout)[0])
     try:
         plugin_info = plugin_data[plugin_name]
     except KeyError:

src/antsibull_docs/docs_parsing/ansible_doc.py

@@ -10,8 +10,8 @@
 import re
 import typing as t
 
-import sh
 from antsibull_core.logging import log
+from antsibull_core.subprocess_util import CalledProcessError
 from antsibull_core.vendored.json_utils import _filter_non_json_lines
 from packaging.version import Version as PypiVer
 
@@ -64,38 +64,37 @@ def parse_ansible_galaxy_collection_list(json_output: t.Mapping[str, t.Any],
     return result
 
 
-def _call_ansible_version(
+async def _call_ansible_version(
     venv: t.Union['VenvRunner', 'FakeVenvRunner'],
-    env: t.Dict[str, str],
+    env: t.Optional[t.Dict[str, str]],
 ) -> str:
-    venv_ansible = venv.get_command('ansible')
-    ansible_version_cmd = venv_ansible('--version', _env=env)
-    return ansible_version_cmd.stdout.decode('utf-8', errors='surrogateescape')
+    p = await venv.async_log_run(['ansible', '--version'], env=env)
+    return p.stdout
 
 
-def _call_ansible_galaxy_collection_list(
+async def _call_ansible_galaxy_collection_list(
     venv: t.Union['VenvRunner', 'FakeVenvRunner'],
     env: t.Dict[str, str],
 ) -> t.Mapping[str, t.Any]:
-    venv_ansible_galaxy = venv.get_command('ansible-galaxy')
-    ansible_collection_list_cmd = venv_ansible_galaxy(
-        'collection', 'list', '--format', 'json', _env=env)
-    stdout = ansible_collection_list_cmd.stdout.decode('utf-8', errors='surrogateescape')
-    return json.loads(_filter_non_json_lines(stdout)[0])
+    p = await venv.async_log_run(
+        ['ansible-galaxy', 'collection', 'list', '--format', 'json'],
+        env=env,
+    )
+    return json.loads(_filter_non_json_lines(p.stdout)[0])
 
 
-def get_collection_metadata(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
-                            env: t.Dict[str, str],
-                            collection_names: t.Optional[t.List[str]] = None,
-                            ) -> t.Dict[str, AnsibleCollectionMetadata]:
+async def get_collection_metadata(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
+                                  env: t.Dict[str, str],
+                                  collection_names: t.Optional[t.List[str]] = None,
+                                  ) -> t.Dict[str, AnsibleCollectionMetadata]:
     collection_metadata = {}
 
     # Obtain ansible.builtin version and path
-    raw_result = _call_ansible_version(venv, env)
+    raw_result = await _call_ansible_version(venv, env)
     collection_metadata['ansible.builtin'] = _extract_ansible_builtin_metadata(raw_result)
 
     # Obtain collection versions
-    json_result = _call_ansible_galaxy_collection_list(venv, env)
+    json_result = await _call_ansible_galaxy_collection_list(venv, env)
     collection_list = parse_ansible_galaxy_collection_list(json_result, collection_names)
     for namespace, name, path, version in collection_list:
         collection_name = f'{namespace}.{name}'
@@ -105,28 +104,26 @@ def get_collection_metadata(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
     return collection_metadata
 
 
-def get_ansible_core_version(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
-                             env: t.Optional[t.Dict[str, str]] = None,
-                             ) -> PypiVer:
-    try:
-        venv_python = venv.get_command('python')
-        ansible_version_cmd = venv_python(
-            '-c', 'import ansible.release; print(ansible.release.__version__)', _env=env)
-        output = ansible_version_cmd.stdout.decode('utf-8', errors='surrogateescape').strip()
+async def get_ansible_core_version(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
+                                   env: t.Optional[t.Dict[str, str]] = None,
+                                   ) -> PypiVer:
+    p = await venv.async_log_run(
+        ['python', '-c', 'import ansible.release; print(ansible.release.__version__)'],
+        env=env,
+        check=False,
+    )
+    output = p.stdout.strip()
+    if p.returncode == 0 and output:
         return PypiVer(output)
-    except sh.ErrorReturnCode:
-        pass
 
     try:
         # Fallback: use `ansible --version`
-        venv_ansible = venv.get_command('ansible')
-        ansible_version_cmd = venv_ansible('--version', _env=env)
-        raw_result = ansible_version_cmd.stdout.decode('utf-8', errors='surrogateescape')
+        raw_result = await _call_ansible_version(venv, env)
         metadata = _extract_ansible_builtin_metadata(raw_result)
         if metadata.version is None:
             raise ValueError('Cannot retrieve ansible-core version from `ansible --version`')
         return PypiVer(metadata.version)
-    except sh.ErrorReturnCode as exc:
+    except CalledProcessError as exc:
         raise ValueError(
             f'Cannot retrieve ansible-core version from `ansible --version`: {exc}'
         ) from exc

src/antsibull_docs/docs_parsing/ansible_doc_core_213.py

@@ -23,18 +23,16 @@
 mlog = log.fields(mod=__name__)
 
 
-def _call_ansible_doc(
+async def _call_ansible_doc(
     venv: t.Union['VenvRunner', 'FakeVenvRunner'],
     env: t.Dict[str, str],
     *parameters: str,
 ) -> t.Mapping[str, t.Any]:
-    # Setup an sh.Command to run ansible-doc from the venv with only the collections we
-    # found as providers of extra plugins.
-    venv_ansible_doc = venv.get_command('ansible-doc')
-    venv_ansible_doc = venv_ansible_doc.bake('-vvv', _env=env)
-    ansible_doc_call = venv_ansible_doc('--metadata-dump', '--no-fail-on-errors', *parameters)
-    stdout = ansible_doc_call.stdout.decode('utf-8', errors='surrogateescape')
-    return json.loads(_filter_non_json_lines(stdout)[0])
+    p = await venv.async_log_run(
+        ['ansible-doc', '-vvv', '--metadata-dump', '--no-fail-on-errors', *parameters],
+        env=env,
+    )
+    return json.loads(_filter_non_json_lines(p.stdout)[0])
 
 
 async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
@@ -69,9 +67,9 @@ async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
     flog.debug('Retrieving and loading plugin documentation')
     if collection_names and len(collection_names) == 1:
         # ansible-doc only allows *one* filter
-        ansible_doc_output = _call_ansible_doc(venv, env, collection_names[0])
+        ansible_doc_output = await _call_ansible_doc(venv, env, collection_names[0])
     else:
-        ansible_doc_output = _call_ansible_doc(venv, env)
+        ansible_doc_output = await _call_ansible_doc(venv, env)
 
     flog.debug('Processing plugin documentation')
     plugin_map: t.MutableMapping[str, t.MutableMapping[str, t.Any]] = {}
@@ -120,7 +118,7 @@ async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
             plugin_type_data[fqcn] = plugin_data
 
     flog.debug('Retrieving collection metadata')
-    collection_metadata = get_collection_metadata(venv, env, collection_names)
+    collection_metadata = await get_collection_metadata(venv, env, collection_names)
 
     flog.debug('Leave')
     return (plugin_map, collection_metadata)

src/antsibull_docs/docs_parsing/parsing.py

@@ -55,7 +55,7 @@ async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
 
     doc_parsing_backend = app_ctx.doc_parsing_backend
     if doc_parsing_backend == 'auto':
-        version = get_ansible_core_version(venv)
+        version = await get_ansible_core_version(venv)
         flog.debug(f'Ansible-core version: {version}')
         if version < PypiVer('2.13.0.dev0'):
             raise RuntimeError(f'Unsupported ansible-core version {version}. Need 2.13.0 or later.')

src/antsibull_docs/lint_plugin_docs.py

@@ -12,8 +12,8 @@
 import typing as t
 from collections.abc import Sequence
 
-import sh
 from antsibull_core.compat import asyncio_run
+from antsibull_core.subprocess_util import log_run
 from antsibull_core.vendored.json_utils import _filter_non_json_lines
 from antsibull_core.venv import FakeVenvRunner
 
@@ -74,9 +74,8 @@ def __exit__(self, type_, value, traceback_):
 class CollectionFinder:
     def __init__(self):
         self.collections = {}
-        stdout = sh.Command('ansible-galaxy')('collection', 'list', '--format', 'json').stdout
-        raw_output = stdout.decode('utf-8', errors='surrogateescape')
-        data = json.loads(_filter_non_json_lines(raw_output)[0])
+        p = log_run(['ansible-galaxy', 'collection', 'list', '--format', 'json'])
+        data = json.loads(_filter_non_json_lines(p.stdout)[0])
         for namespace, name, path, _ in reversed(parse_ansible_galaxy_collection_list(data)):
             self.collections[f'{namespace}.{name}'] = path
 

tests/functional/ansible_doc_caching.py

@@ -16,7 +16,7 @@
 
 @contextmanager
 def ansible_doc_cache():
-    def call_ansible_doc(
+    async def call_ansible_doc(
         venv: t.Union['VenvRunner', 'FakeVenvRunner'],
         env: t.Dict[str, str],
         *parameters: str,
@@ -45,19 +45,22 @@ def call_ansible_doc(
                         doc[key] = os.path.join(root, doc[key])
         return data
 
-    def call_ansible_version(
+    async def call_ansible_version(
         venv: t.Union['VenvRunner', 'FakeVenvRunner'],
-        env: t.Dict[str, str],
+        env: t.Optional[t.Dict[str, str]],
     ) -> str:
         filename = os.path.join(os.path.dirname(__file__), 'ansible-version.output')
         with open(filename, 'rt', encoding='utf-8') as f:
             content = f.read()
 
-        root = env['ANSIBLE_COLLECTIONS_PATH']
-        return content.replace('<<<<<COLLECTIONS>>>>>', root).replace('<<<<<HOME>>>>>', env['HOME']).replace('<<<<<ANSIBLE>>>>>', os.path.dirname(ansible.__file__))
+        root = env['ANSIBLE_COLLECTIONS_PATH'] if env and 'ANSIBLE_COLLECTIONS_PATH' in env else '/collections'
+        content = content.replace('<<<<<COLLECTIONS>>>>>', root)
+        content = content.replace('<<<<<HOME>>>>>', (env or os.environ)['HOME'])
+        content = content.replace('<<<<<ANSIBLE>>>>>', os.path.dirname(ansible.__file__))
+        return content
 
 
-    def call_ansible_galaxy_collection_list(
+    async def call_ansible_galaxy_collection_list(
         venv: t.Union['VenvRunner', 'FakeVenvRunner'],
         env: t.Dict[str, str],
     ) -> t.Mapping[str, t.Any]: