Skip to content

Commit 8990ccb

Browse files
authored
sync: Return SyncResult from update_repo(), fix other bugs (#514)
update_repo() across Git, Hg, and SVN sync classes now returns a SyncResult instead of None. Callers can distinguish successful syncs from failures and inspect structured error details, instead of errors being silently swallowed or raising uncaught exceptions. New dataclasses in sync/base.py: - SyncResult: tracks ok status and a list of SyncError entries - SyncError: records step name, message, and original exception GitSync.update_repo() changes: - All 13 except-CommandError-return paths now record errors in SyncResult with labeled steps (obtain, set-remotes, fetch, rebase, checkout, stash-save, stash-pop, status, etc.) - Original early-return vs continue-on-error control flow is preserved per step - Best-effort cleanup uses contextlib.suppress(CommandError) for rebase abort, stash pop, and reset operations HgSync and SvnSync updated for API consistency, wrapping obtain and pull/update failures in SyncResult. Additional fixes: - cmd/git: Fix rev_list() _all parameter shadowed by builtin - sync/git: Disambiguate rev-list with refs/heads/ for local branch names that collide with filesystem paths - sync/git: Return early on invalid-upstream rebase instead of falling through to stash-pop - sync/git: Add __str__() to GitRemoteRefNotFound to avoid AttributeError from parent CommandError.__str__() - sync/git: Add explicit _message type annotation on GitRemoteRefNotFound - libvcs/__init__: Export SyncResult and SyncError from top-level package - tests/sync: Fix hg test isolation by using factory fixture instead of deleting session-scoped remote - ruff: Move A002 suppression to per-file-ignores in pyproject.toml Test coverage: - All 13 GitSync.update_repo() error steps now have dedicated tests covering obtain, set-remotes, symbolic-ref, rev-list, remote-name, remote-ref-not-found, fetch, status, stash-save, checkout, rebase (invalid upstream + conflict), stash-pop, and submodule-update - SyncResult multi-error accumulation test - Hg and SVN obtain/pull/update failure tests - Git rev_list _all parameter test See also: vcs-python/vcspull#512
2 parents 165747e + bca6778 commit 8990ccb

14 files changed

Lines changed: 1159 additions & 70 deletions

File tree

CHANGES

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,43 @@ $ uv add libvcs --prerelease allow
2020
_Notes on the upcoming release will go here._
2121
<!-- END PLACEHOLDER - ADD NEW CHANGELOG ENTRIES BELOW THIS LINE -->
2222

23+
### New features
24+
25+
#### sync: Return {class}`~libvcs.sync.base.SyncResult` from `update_repo()` (#514)
26+
27+
`update_repo()` across {class}`~libvcs.sync.git.GitSync`,
28+
{class}`~libvcs.sync.hg.HgSync`, and {class}`~libvcs.sync.svn.SvnSync`
29+
now returns a {class}`~libvcs.sync.base.SyncResult` instead of `None`.
30+
Callers can inspect `result.ok` and `result.errors` to distinguish
31+
successful syncs from failures.
32+
33+
- New dataclasses: {class}`~libvcs.sync.base.SyncResult` and
34+
{class}`~libvcs.sync.base.SyncError`
35+
- Git: 10+ silent `except CommandError: return` paths now record
36+
structured errors with labeled steps (`fetch`, `rebase`, `checkout`,
37+
`stash-save`, etc.)
38+
- Hg and SVN: Wrap `obtain` and `pull`/`update` failures for API
39+
consistency
40+
41+
Companion change: [vcspull#512](https://github.com/vcs-python/vcspull/pull/512)
42+
43+
### Bug Fixes
44+
45+
- cmd: Fix `Git.rev_list()` referencing builtin `all` instead of `_all`
46+
parameter (#514)
47+
48+
- sync: Disambiguate `rev-list` when a local branch name collides with a
49+
filesystem path by using fully-qualified `refs/heads/` refs (#514)
50+
51+
- sync: Return early with error on invalid-upstream rebase instead of
52+
falling through to stash-pop (#514)
53+
54+
### Tests
55+
56+
- sync: Fix `test_update_repo_pull_failure_returns_sync_result` (hg)
57+
destroying the session-scoped `hg_remote_repo` fixture, which broke
58+
downstream tests like `test_hg_url` (#514)
59+
2360
## libvcs 0.38.6 (2026-01-27)
2461

2562
### Bug Fixes

README.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,13 @@ repo = GitSync(
7070
)
7171

7272
# Clone (if not exists) or fetch & update (if exists)
73-
repo.update_repo()
73+
result = repo.update_repo()
7474

75-
print(f"Current revision: {repo.get_revision()}")
75+
if result.ok:
76+
print(f"Current revision: {repo.get_revision()}")
77+
else:
78+
for error in result.errors:
79+
print(f"Sync failed at {error.step}: {error.message}")
7680
```
7781

7882
### 2. Command Abstraction

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ convention = "numpy"
211211

212212
[tool.ruff.lint.per-file-ignores]
213213
"*/__init__.py" = ["F401"]
214+
"src/libvcs/_internal/subprocess.py" = ["A002"]
214215

215216
[tool.pytest.ini_options]
216217
addopts = [

src/libvcs/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import logging
66

77
from ._internal.run import CmdLoggingAdapter
8-
from .sync.base import BaseSync
8+
from .sync.base import BaseSync, SyncError, SyncResult
99
from .sync.git import GitSync
1010
from .sync.hg import HgSync
1111
from .sync.svn import SvnSync
@@ -16,6 +16,8 @@
1616
"GitSync",
1717
"HgSync",
1818
"SvnSync",
19+
"SyncError",
20+
"SyncResult",
1921
]
2022

2123
logger = logging.getLogger(__name__)

src/libvcs/_internal/subprocess.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
# ruff: NOQA: A002
21
r"""Invocable :mod:`subprocess` wrapper.
32
43
Defer running a subprocess, such as by handing to an executor.

src/libvcs/cmd/git.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2193,7 +2193,7 @@ def rev_list(
21932193

21942194
for flag, shell_flag in [
21952195
# Limiting output
2196-
(all, "--all"),
2196+
(_all, "--all"),
21972197
(author, "--author"),
21982198
(committer, "--committer"),
21992199
(grep, "--grep"),
@@ -2212,7 +2212,7 @@ def rev_list(
22122212
(first_parent, "--first-parent"),
22132213
(exclude_first_parent_only, "--exclude-first-parent-only"),
22142214
(_not, "--not"),
2215-
(all, "--all"),
2215+
(_all, "--all"),
22162216
(exclude, "--exclude"),
22172217
(reflog, "--reflog"),
22182218
(alternative_refs, "--alternative-refs"),

src/libvcs/sync/base.py

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import dataclasses
56
import logging
67
import pathlib
78
import typing as t
@@ -14,6 +15,82 @@
1415
logger = logging.getLogger(__name__)
1516

1617

18+
@dataclasses.dataclass
19+
class SyncError:
20+
"""An error encountered during a sync step.
21+
22+
Examples
23+
--------
24+
>>> error = SyncError(step="fetch", message="remote not found")
25+
>>> error.step
26+
'fetch'
27+
>>> error.message
28+
'remote not found'
29+
>>> error.exception is None
30+
True
31+
"""
32+
33+
step: str
34+
message: str
35+
exception: Exception | None = None
36+
37+
38+
@dataclasses.dataclass
39+
class SyncResult:
40+
"""Result of a repository synchronization.
41+
42+
Examples
43+
--------
44+
>>> result = SyncResult()
45+
>>> result.ok
46+
True
47+
>>> bool(result)
48+
True
49+
50+
>>> result = SyncResult()
51+
>>> result.add_error(step="fetch", message="remote not found")
52+
>>> result.ok
53+
False
54+
>>> bool(result)
55+
False
56+
>>> result.errors[0].step
57+
'fetch'
58+
"""
59+
60+
ok: bool = True
61+
errors: list[SyncError] = dataclasses.field(default_factory=list)
62+
63+
def __bool__(self) -> bool:
64+
"""Return True if the sync succeeded without errors.
65+
66+
Returns
67+
-------
68+
bool
69+
True if no errors were recorded, False otherwise.
70+
"""
71+
return self.ok
72+
73+
def add_error(
74+
self,
75+
step: str,
76+
message: str,
77+
exception: Exception | None = None,
78+
) -> None:
79+
"""Record an error and mark the result as failed.
80+
81+
Parameters
82+
----------
83+
step : str
84+
Name of the sync step that failed (e.g. ``"fetch"``, ``"checkout"``).
85+
message : str
86+
Human-readable description of the error.
87+
exception : Exception or None, optional
88+
The underlying exception, if available.
89+
"""
90+
self.ok = False
91+
self.errors.append(SyncError(step=step, message=message, exception=exception))
92+
93+
1794
class VCSLocation(t.NamedTuple):
1895
"""Generic VCS Location (URL and optional revision)."""
1996

@@ -200,8 +277,14 @@ def ensure_dir(self, *args: t.Any, **kwargs: t.Any) -> bool:
200277

201278
return True
202279

203-
def update_repo(self, *args: t.Any, **kwargs: t.Any) -> None:
204-
"""Pull latest changes to here from remote repository."""
280+
def update_repo(self, *args: t.Any, **kwargs: t.Any) -> SyncResult:
281+
"""Pull latest changes to here from remote repository.
282+
283+
Returns
284+
-------
285+
SyncResult
286+
Result of the sync operation, with any errors recorded.
287+
"""
205288
raise NotImplementedError
206289

207290
def obtain(self, *args: t.Any, **kwargs: t.Any) -> None:

0 commit comments

Comments
 (0)