Skip to content

Commit a948f7d

Browse files
authored
Improve subprocess text handling and type safety (#485)
- BREAKING: Switch run() to use text=True by default for subprocess commands - Now uses unicode instead of bytes for subprocess commands - Removes console_to_str() helper function and related encoding logic - Removes universal_newlines and text parameters from run() - Improve type safety: - Replace StrOrBytesPath with StrPath for better type consistency - Fix args type handling in CommandError to properly handle string vs sequence inputs - Add proper None checks for stdout/stderr handling - Update type hints in subprocess.py for text parameter - Code cleanup: - Simplify output handling logic - Remove redundant string conversions - Improve error message formatting Note: This is a breaking change. Users experiencing compatibility issues should file a ticket.
2 parents bc943d3 + d4015ff commit a948f7d

File tree

3 files changed

+39
-32
lines changed

3 files changed

+39
-32
lines changed

CHANGES

+8
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ $ pip install --user --upgrade --pre libvcs
1515

1616
<!-- Maintainers, insert changes / features for the next release here -->
1717

18+
### Breaking changes
19+
20+
#### `run()` now uses `text=True` (#485)
21+
22+
This means that unicode, not bytes, will be used for running `subprocess`
23+
commands in libvcs. If there are any compatibility issues with this, please file
24+
a ticket.
25+
1826
### Development
1927

2028
#### chore: Implement PEP 563 deferred annotation resolution (#483)

src/libvcs/_internal/run.py

+28-29
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,10 @@
1818
from collections.abc import Iterable, Mapping, MutableMapping, Sequence
1919

2020
from libvcs import exc
21-
from libvcs._internal.types import StrOrBytesPath
21+
from libvcs._internal.types import StrPath
2222

2323
logger = logging.getLogger(__name__)
2424

25-
console_encoding = sys.stdout.encoding
26-
27-
28-
def console_to_str(s: bytes) -> str:
29-
"""From pypa/pip project, pip.backwardwardcompat. License MIT."""
30-
try:
31-
return s.decode(console_encoding)
32-
except UnicodeDecodeError:
33-
return s.decode("utf_8")
34-
except AttributeError: # for tests, #13
35-
return str(s)
36-
3725

3826
if t.TYPE_CHECKING:
3927
_LoggerAdapter = logging.LoggerAdapter[logging.Logger]
@@ -99,34 +87,32 @@ def __call__(self, output: t.AnyStr, timestamp: datetime.datetime) -> None:
9987
_ENV: TypeAlias = Mapping[str, str]
10088
else:
10189
_ENV: TypeAlias = t.Union[
102-
Mapping[bytes, StrOrBytesPath],
103-
Mapping[str, StrOrBytesPath],
90+
Mapping[bytes, StrPath],
91+
Mapping[str, StrPath],
10492
]
10593

106-
_CMD = t.Union[StrOrBytesPath, Sequence[StrOrBytesPath]]
94+
_CMD = t.Union[StrPath, Sequence[StrPath]]
10795
_FILE: TypeAlias = t.Optional[t.Union[int, t.IO[t.Any]]]
10896

10997

11098
def run(
11199
args: _CMD,
112100
bufsize: int = -1,
113-
executable: StrOrBytesPath | None = None,
101+
executable: StrPath | None = None,
114102
stdin: _FILE | None = None,
115103
stdout: _FILE | None = None,
116104
stderr: _FILE | None = None,
117105
preexec_fn: t.Callable[[], t.Any] | None = None,
118106
close_fds: bool = True,
119107
shell: bool = False,
120-
cwd: StrOrBytesPath | None = None,
108+
cwd: StrPath | None = None,
121109
env: _ENV | None = None,
122-
universal_newlines: bool = False,
123110
startupinfo: t.Any | None = None,
124111
creationflags: int = 0,
125112
restore_signals: bool = True,
126113
start_new_session: bool = False,
127114
pass_fds: t.Any = (),
128115
*,
129-
text: bool | None = None,
130116
encoding: str | None = None,
131117
errors: str | None = None,
132118
user: str | int | None = None,
@@ -191,13 +177,12 @@ def progress_cb(output, timestamp):
191177
shell=shell,
192178
cwd=cwd,
193179
env=env,
194-
universal_newlines=universal_newlines,
195180
startupinfo=startupinfo,
196181
creationflags=creationflags,
197182
restore_signals=restore_signals,
198183
start_new_session=start_new_session,
199184
pass_fds=pass_fds,
200-
text=text,
185+
text=True,
201186
encoding=encoding,
202187
errors=errors,
203188
user=user,
@@ -206,7 +191,7 @@ def progress_cb(output, timestamp):
206191
umask=umask,
207192
)
208193

209-
all_output: list[str] = []
194+
all_output: str = ""
210195
code = None
211196
line = None
212197
if log_in_real_time and callback is None:
@@ -220,18 +205,32 @@ def progress_cb(output: t.AnyStr, timestamp: datetime.datetime) -> None:
220205
code = proc.poll()
221206

222207
if callback and callable(callback) and proc.stderr is not None:
223-
line = console_to_str(proc.stderr.read(128))
208+
line = str(proc.stderr.read(128))
224209
if line:
225210
callback(output=line, timestamp=datetime.datetime.now())
226211
if callback and callable(callback):
227212
callback(output="\r", timestamp=datetime.datetime.now())
228213

229-
lines = filter(None, (line.strip() for line in proc.stdout.readlines()))
230-
all_output = console_to_str(b"\n".join(lines))
214+
lines = (
215+
filter(None, (line.strip() for line in proc.stdout.readlines()))
216+
if proc.stdout is not None
217+
else []
218+
)
219+
all_output = "\n".join(lines)
231220
if code:
232-
stderr_lines = filter(None, (line.strip() for line in proc.stderr.readlines()))
233-
all_output = console_to_str(b"".join(stderr_lines))
221+
stderr_lines = (
222+
filter(None, (line.strip() for line in proc.stderr.readlines()))
223+
if proc.stderr is not None
224+
else []
225+
)
226+
all_output = "".join(stderr_lines)
234227
output = "".join(all_output)
235228
if code != 0 and check_returncode:
236-
raise exc.CommandError(output=output, returncode=code, cmd=args)
229+
raise exc.CommandError(
230+
output=output,
231+
returncode=code,
232+
cmd=[str(arg) for arg in args]
233+
if isinstance(args, (list, tuple))
234+
else str(args),
235+
)
237236
return output

src/libvcs/_internal/subprocess.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ def Popen(
258258
args: _CMD | None = ...,
259259
universal_newlines: t.Literal[False] = ...,
260260
*,
261-
text: t.Literal[None, False] = ...,
261+
text: t.Literal[False] | None = ...,
262262
encoding: None = ...,
263263
errors: None = ...,
264264
) -> subprocess.Popen[bytes]: ...
@@ -370,7 +370,7 @@ def check_output(
370370
input: str | bytes | None = ...,
371371
encoding: None = ...,
372372
errors: None = ...,
373-
text: t.Literal[None, False] = ...,
373+
text: t.Literal[False] | None = ...,
374374
**kwargs: t.Any,
375375
) -> bytes: ...
376376

@@ -484,7 +484,7 @@ def run(
484484
encoding: None = ...,
485485
errors: None = ...,
486486
input: bytes | None = ...,
487-
text: t.Literal[None, False] = ...,
487+
text: t.Literal[False] | None = ...,
488488
) -> subprocess.CompletedProcess[bytes]: ...
489489

490490
def run(

0 commit comments

Comments
 (0)