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

[kvmajo]

`linux.copy()` decides two paths are "on the same host" purely by
class inheritance:

https://github.com/Rahix/tbot/blob/master/tbot/machine/linux/copy.py#L118

if isinstance(p1.host, p2.host.__class__) or isinstance(p2.host, p1.host.__class__):
    # Both paths are on the same host
    ...

This only matches when one machine's class is literally a subclass of
the other's. 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()` doesn't
recognize them as the same host. It then falls through every
SSH/Paramiko-specific branch and hits:

raise NotImplementedError(f"Can't copy from {p1.host} to {p2.host}!")

even though both machines run commands on the same filesystem and a
plain `cp` would work.

Repro

class LabHost(connector.SubprocessConnector, linux.Bash):
    ...

class LocalHost(connector.SubprocessConnector, linux.Bash):
    ...

with tbot.ctx.request(tbot.role.LocalHost) as local, \
     tbot.ctx.request(tbot.role.LabHost) as lab:
    linux.copy(local.workdir / "foo", lab.workdir / "foo")
    # NotImplementedError: Can't copy from <LabHost ...> to <LocalHost ...>!

Suggested fix

Detect "same host" by checking that both machines are
`SubprocessConnector` instances (i.e. both run locally), rather than
requiring one machine's class to be a subclass of the other's:

if isinstance(p1.host, connector.SubprocessConnector) and isinstance(p2.host, connector.SubprocessConnector):
    ...

Currently working around this downstream by special-casing this
combination and calling `local.exec0("cp", ...)` directly instead of
`linux.copy()`/`utils.copy_to_dir()`.

[Rahix]

Hi,

I don't love this solution since it makes non-explicit assumptions about the hosts in question. Type-equality is a sure sign that the paths belong to the same host.

I'm not against going this route; in this particular case I do not see much that can go wrong.

But maybe you can share your motivation for this? Why can't those two machines be of the same type/inherit from each other?

[kvmajo]

The short version: LocalHost and LabHost are two different roles in the
same project, and nothing ties their class hierarchies together on purpose.

• tbot.role.LocalHost is satisfied by tbot's own built-in
  LocalLabHost(connector.SubprocessConnector, linux.Bash, linux.Lab). It's
  meant to be cheap, ad-hoc, and acquired all over the place (staging a
  downloaded artifact, holding an intermediate build product, etc.) without
  any assumptions about what else is going on.

• tbot.role.LabHost, in this project, is a project-defined class that mixes
  in stuff LocalHost has no business carrying: an exclusive lock so
  concurrent tbot runs don't stomp on shared hardware, and Wi-Fi/relay
  control for the DUT. Critically, LabHost is polymorphic across lab
  configs: in CI it's this local, SubprocessConnector-based stand-in (no
  real hardware attached), but in other configs the very same role is
  fulfilled by an SSH-connected class talking to a real Raspberry-Pi-based
  lab host. Test code just does cx.request(tbot.role.LabHost) and doesn't
  care which.

So making one inherit from the other isn't really available as an option:

• LabHost <- LocalHost would permanently commit every LabHost
  implementation to SubprocessConnector, breaking the SSH-connected
  variant.
• LocalHost <- LabHost would drag locking/Wi-Fi/relay concerns into every
  throwaway local machine acquired anywhere in the codebase, for a role that
  is explicitly documented as "very cheap, acquire as many as you want."

Concretely, this bit us in a linux.copy() call to stage a locally-built
image onto the (locally-running, in the CI config) lab host before flashing
it to a device. Today that's worked around with something like:

if isinstance(lab, LocalStandInLabHost):
    # linux.copy()'s same-host detection doesn't recognize `local` and
    # `lab` as the same host since neither's class is a subclass of the
    # other's, even though both are plain SubprocessConnector machines.
    # https://github.com/Rahix/tbot/issues/129
    target_dir = linux.Path(local, TARGET_DIR)
    for source in sources:
        local.exec0("cp", source.at_host(local), target_dir.at_host(local))
else:
    target_dir = linux.Path(lab, TARGET_DIR)
    utils.copy_to_dir(sources, target_dir, hashcmp=True)

i.e. a whole parallel code path that hand-rolls the copy and loses
copy_to_dir()'s hashcmp skip-if-unchanged optimization, purely because
copy() can't tell that two unrelated SubprocessConnector machines are
both just... localhost.

Here's that same shape as a minimal, runnable test, plus what actually
happens with and without the fix:

import tbot
from tbot.machine import connector, linux


class LocalHost(connector.SubprocessConnector, linux.Bash):
    name = "local"


class LabHost(connector.SubprocessConnector, linux.Bash):
    name = "lab"


def test_copy_between_unrelated_subprocess_machines():
    with LocalHost() as local, LabHost() as lab:
        src = local.workdir / "foo.txt"
        local.exec0("echo", "hello", linux.RedirStdout(src))

        dst_dir = lab.workdir / "staging"
        lab.exec0("mkdir", "-p", dst_dir)
        dst = dst_dir / "foo.txt"
        linux.copy(src, dst)

        assert lab.exec0("cat", dst) == "hello\n"
        print("PASS: copy() worked between LocalHost and LabHost")


test_copy_between_unrelated_subprocess_machines()

Without the fix (current master):

[local] echo " ${XDG_DATA_HOME}"
[local] echo " ${HOME}"
[local] mkdir -p /home/majoc/.local/share/tbot
[local] echo hello >/home/majoc/.local/share/tbot/foo.txt
[lab] echo " ${XDG_DATA_HOME}"
[lab] echo " ${HOME}"
[lab] mkdir -p /home/majoc/.local/share/tbot
[lab] mkdir -p /home/majoc/.local/share/tbot/staging
Traceback (most recent call last):
  ...
  File ".../tbot/machine/linux/copy.py", line 186, in copy
    raise NotImplementedError(f"Can't copy from {p1.host} to {p2.host}!")
NotImplementedError: Can't copy from <__main__.LocalHost object> to <__main__.LabHost object>!

With the fix from this PR:

[local] echo " ${XDG_DATA_HOME}"
[local] echo " ${HOME}"
[local] mkdir -p /home/majoc/.local/share/tbot
[local] echo hello >/home/majoc/.local/share/tbot/foo.txt
[lab] echo " ${XDG_DATA_HOME}"
[lab] echo " ${HOME}"
[lab] mkdir -p /home/majoc/.local/share/tbot
[lab] mkdir -p /home/majoc/.local/share/tbot/staging
[local] cp /home/majoc/.local/share/tbot/foo.txt /home/majoc/.local/share/tbot/staging/foo.txt
[lab] cat /home/majoc/.local/share/tbot/staging/foo.txt
PASS: copy() worked between LocalHost and LabHost

(Both machines really are on the same box, so local.workdir/lab.workdir
resolve to the same directory — that's the whole point.)

On the "non-explicit assumption" worry: I'd argue SubprocessConnector is
actually a stronger signal than class-equality here, not a weaker one —
there is exactly one localhost, so any two SubprocessConnector instances
are guaranteed to share a filesystem regardless of what else their classes
mix in. Class-equality only accidentally implies that because most
SubprocessConnector classes people write happen to be for the local
machine; the SubprocessConnector check says so directly.