Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Share the stderr with the user, when non-zero exit code #927

Merged
merged 7 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/setuptools_scm/_run_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from . import _log
from . import _types as _t
from ._types import Result

if TYPE_CHECKING:
BaseCompletedProcess = subprocess.CompletedProcess[str]
Expand Down Expand Up @@ -172,27 +173,38 @@ def _unsafe_quote_for_display(item: _t.PathT) -> str:

def has_command(
name: str, args: Sequence[str] = ["version"], warn: bool = True
) -> bool:
) -> Result:
message = ""
try:
p = run([name, *args], cwd=".", timeout=5)
except OSError as e:
log.warning("command %s missing: %s", name, e)
message = str(e)
res = False
except subprocess.TimeoutExpired as e:
log.warning("command %s timed out %s", name, e)
message = str(e)
res = False

else:
res = not p.returncode
if not res and warn:
warnings.warn("%r was not found" % name, category=RuntimeWarning)
return res

if not res and not message:
message = p.stderr

return Result(status=res, message=message)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets define the Result class in the same module as its only used for this particular function

it mgiht make sense to use the logging based output in any case (when has command initially was created, warnings was used as trace was only opt in by env var

now with the logging setup that chooses level, we can just log a error directly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are very fast to reply! Thank you. 😄

Just logging directly would indeed more straight forward. That will remove the need to return a complex type from has_command, so I can just skip the result wrapper entirely. I'll rework the PR.



class CommandNotFoundError(LookupError, FileNotFoundError):
pass


def require_command(name: str) -> None:
if not has_command(name, warn=False):
if not (result := has_command(name, warn=False)):
log.error(
f"There was a failure running require_command('{name}'). Below is the stderr from the attempt:"
)
log.error(result.message)
raise CommandNotFoundError(name)
12 changes: 12 additions & 0 deletions src/setuptools_scm/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import os
from typing import Callable
from typing import List
from typing import Optional
from typing import Sequence
from typing import Tuple
from typing import TYPE_CHECKING
Expand All @@ -20,3 +21,14 @@
VERSION_SCHEME: TypeAlias = Union[str, Callable[["version.ScmVersion"], str]]
VERSION_SCHEMES: TypeAlias = Union[List[str], Tuple[str, ...], VERSION_SCHEME]
SCMVERSION: TypeAlias = "version.ScmVersion"


class Result:
"""Just like a normal bool, but with a slot for a message"""

def __init__(self, status: bool, message: str | None = None):
self.status: bool = status
self.message: str = message or ""

def __bool__(self) -> bool:
return self.status
Loading