Skip to content

linux: copy: Recognize any two SubprocessConnector machines as same-host - #131

Open
kvmajo wants to merge 1 commit into
Rahix:masterfrom
kvmajo:fix-copy-samehost
Open

linux: copy: Recognize any two SubprocessConnector machines as same-host#131
kvmajo wants to merge 1 commit into
Rahix:masterfrom
kvmajo:fix-copy-samehost

Conversation

@kvmajo

@kvmajo kvmajo commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #129.

linux.copy()'s same-host detection only matched when one machine's
class was a subclass of the other's:

if isinstance(p1.host, p2.host.__class__) or isinstance(p2.host, p1.host.__class__):

If a project defines two separate machine classes that both connect
via connector.SubprocessConnector (i.e. both actually run on
localhost) but don't inherit from each other, copy() falls through
every SSH/Paramiko-specific branch and raises NotImplementedError,
even though both machines run on the same filesystem and a plain cp
would work. Fixed by also recognizing any two SubprocessConnector
instances as being on the same host, regardless of their exact class.

A second bug found while testing

While testing this, a second, independent bug in the same code path
surfaced: even when the isinstance check above already matched (e.g.
literally the same class), the fast path still raised
WrongHostError for any two machine instances that aren't clones of
one another. This is because p2_w1 = linux.Path(p1.host, p2)
implicitly requires p2.host == p1.host, and Machine.__eq__ only
considers clones of the same original equal — not just
"same/compatible class". So the same-host fast path only ever worked
when p1.host and p2.host were already the exact same object to
begin with, making the isinstance check mostly dead code in practice.

Fixed by reinterpreting p2's raw path via p2._local_str() instead
of wrapping the Path object itself, since by the time we reach this
branch same-host compatibility has already been established by the
outer condition.

Testing

  • pre-commit's pinned black, flake8, and mypy all pass on the
    changed file.
  • Reproduced both bugs directly against unmodified master:
    • Two unrelated SubprocessConnector-based classes: NotImplementedError.
    • Two instances of the same class (not clones): WrongHostError.
      Both succeed on this branch.
  • selftest/ (excluding the subset that needs a real sshd, which
    this sandbox doesn't have) has identical pass/fail counts before and
    after this change: 5 failed / 104 passed on both master and this
    branch.

linux.copy()'s same-host detection only matched when one machine's
class was a subclass of the other's:

    if isinstance(p1.host, p2.host.__class__) or isinstance(p2.host, p1.host.__class__):

If a project defines two separate machine classes that both connect
via connector.SubprocessConnector (i.e. both actually run on
localhost) but don't inherit from each other, copy() falls through
every SSH/Paramiko-specific branch and raises NotImplementedError,
even though both machines run on the same filesystem and a plain cp
would work.  Fix this by also recognizing any two SubprocessConnector
instances as being on the same host, regardless of their exact class.

While testing this, a second, independent bug in the same code path
surfaced: even when the isinstance check above already matched (e.g.
literally the same class), the fast path still raised WrongHostError
for any two machine instances that aren't clones of one another.  This
is because `p2_w1 = linux.Path(p1.host, p2)` implicitly requires
`p2.host == p1.host`, and Machine.__eq__ only considers clones of the
same original equal - not just "same/compatible class".  So the
same-host fast path only ever worked when p1.host and p2.host were
already the exact same object to begin with, making the isinstance
check mostly dead code in practice.

Fixed by reinterpreting p2's raw path via p2._local_str() instead of
wrapping the Path object itself, since by the time we reach this
branch same-host compatibility has already been established by the
outer condition.

Fixes: Rahix#129
Signed-off-by: Martin Jocic <martin.jocic@kvaser.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

linux.copy() raises NotImplementedError between two independent SubprocessConnector machines on the same host

1 participant