Add cross-platform LocalTarget with macOS and Linux support

[denini08]

This PR introduces the new LocalTarget class, which unifies local target handling across macOS and Linux. Fix #728.

Key changes include:

• Automatic platform detection (mac, linux)
• Native command usage on macOS (e.g., sysctl, screencapture) and Linux (e.g., /proc/cpuinfo, scrot)
• Fallback to busybox commands for unsupported platforms
• Updated tests and configuration to use LocalTarget instead of platform-specific classes
• Maintained backward compatibility (LocalMacTarget remains as alias)

Benefits:

• Single class works on both macOS and Linux without manual selection
• Platform-specific logic centralized for easier maintenance
• Simplifies test configuration and improves code clarity

Let me know your thoughts. Happy to help.

[marcbonnici]

Hi,

Thank you very much for the pull request. I had some initial high level design questions with regards to the new Target.

Thanks

[marcbonnici]

I noticed that LocalTarget is subclassing Target directly rather than LinuxTarget or LocalLinuxTarget and was curious for the rationale?

It seems that with the current approach we have a lot of code duplication from the existing targets and if we were to subclass, for example LocalLinxTarget, then we'd be able to reuse the existing Linux implementations when we're running on a Linux platform (or the Mac implementation is the same) and we could maintain a single source for common operations.

For example in the os_version method of LocalTarget we have a different implementation than what we have in LinuxTarget [1].

Similarly for system_id[2][3], host_id[4] etc. we now have have more than one implementations attempting to achieve the same functionality (some times with differing results) rather than just calling into the parent class and handling the mac specific scenario separately.

[1]

devlib/devlib/target.py

Line 1794 in 9c4f09b

def os_version(self):

[2]

devlib/devlib/target.py

Line 1806 in 9c4f09b

def system_id(self):

[3]

devlib/devlib/bin/scripts/shutils.in

Line 331 in 9c4f09b

get_linux_system_id() {

[4]

devlib/devlib/target.py

Line 192 in 9c4f09b

def hostid(self):

[denini08]

I made a commit now trying to resolve all your feedbacks

[denini08]

also, feel free to make commits if you find something that can be fixed (I don't have much knowledge of the code base so I might have made a mistake, I'm just trying to help)

[marcbonnici]

Additionally I see previously this was split out from a LocalMacTarget.
I'm curious if there is a specific benefit for merging these two Linux/Mac targets together?

It seems from a code readability aspect it would be cleaner to keep separate LocalMacTarget and LocalLinuxTarget's and let a user choose the appropriate target based on their chosen system?

[denini08]

I agree with you. keeping LocalMacTarget and LocalLinuxTarget separate would indeed make the code more readable and give users a clearer choice depending on their system.

[douglas-raillard-arm]

This sort of thing is a common failure mode for OO approaches:

   if self._is_macos():

AFAICT this is implementing that aspect:

Platform-specific logic centralized for easier maintenance

This is really not in the benefits category. If all target classes were merged into a single class like this one with a bunch of if self._is_foobar(), the code would be unmaintainable: each method would be several times larger than it is today and it would be hard to assess when adding a new target whether a spot was missed or not. On top of that, there likely would be some broken code sharing between target that would lead to problems, e.g. mac code falling back on Linux code because it looks the same, and Linux code being updated one day to fix something in the linux world, accidentally breaking the mac world.

This could be replaced by having a class that instantiate another class chosen dynamically and redirects all method attribute lookup to that internal one using e.g. __getattribute__.

Once this is done, a LocalMacTarget can be created, mirroring the existing LocalLinuxTarget.

This way we keep the fundamental design property of devlib that each target class is specialized in a single OS, with the exception of a "dispatch" target that just redirects to an actual implementation.

Note that this design is what we implemented in LISA, where there is a single user-facing Target class that instantiate one of the devlib classes under the hood. Similarly, devlib currently instantiate connection classes dynamically, so that we don't have a class for each OS x connection.

If there is any code that makes sense to actually share, it can be split out in separate private standalone functions called by both Linux and Mac targets.

EDIT: adding to that, the dispatching class would probably benefit from having a metaclass that implements __instancecheck__ in order to not break user code that uses isinstance(target, LocalLinuxTarget), e.g.:

class Meta(type):
    def __instancecheck__(self, instance):
        print(instance)
        return True

class MyClass(metaclass=Meta):
    pass


assert isinstance(42, MyClass)

[douglas-raillard-arm]

Additionally if we start supporting OSes that are not based on the Linux kernel, there needs to be a way to fence the code that makes this assumption:

1. Target and Connection classes should be alright as they are already either os-specific (Target) and os-agnostic (Connection)
2. Devlib modules will need to be able to report what OS they support. E.g. I don't expect Mac to have a hotplug subsystem that works the same as linux with sysfs.

The module problem is actually 2-fold:

1. Modules need to cleanly fail to initialize when running on an unsupported OS
2. A given module name will likely need to be dispatched to different classes based on the OS, assuming all those classes implement the same public API. The alternative is to simply hardcode the OS name in the module name (e.g. mac_hotplug, with hotplug being Linux-only), but that would not allow users writing generic code where possible for the end user and duplicate the "os decision" (both in the Target class and later in modules used). Such names should really only exist if the API is different.

[douglas-raillard-arm]

Some generic comments that would likely stay relevant regardless of future refactoring. It's more things to keep in mind while reworking the code than something to address on the current PR version.

[douglas-raillard-arm]

Would be best to just raise an exception here. Otherwise, an update of devlib that brings support for another OS will become a breaking change, as code running on that OS will go from working with "unknown" to working with "foobar". If an exception is raised, either the calling code genuinely has a fallback that is just a fallback, or the exception would bubble and the code would simply not work, until the day devlib supports that platform.

[douglas-raillard-arm]

super(LocalTarget, self) is just super() in this context. Arguments are only needed when:

1. The parser does not provide the interpreter enough context (e.g. in a nested function).
2. When trying to achieve something unusual (not following the normal MRO)
3. When trying to be compatible with Python 2 (official eol in 2020, devlib has dropped support for it a long time ago)

[douglas-raillard-arm]

A new class that allows passing the various components explicitly is needed here. This would be a base class of the current Cpuinfo, and the latter could simply parse the text format in its __init__. That base class would be used directly here, or we can make a subclass that does essentially nothing to keep the base private.

The only issue with that scheme is that isinstance(x, Cpuinfo) would now be inaccurate, but I don't think anyone relies on that as there is only a single Cpuinfo class currently.

[douglas-raillard-arm]

but with native commands

What is the advantage for that ? The main reason we use busybox everywhere in devlib is so that we know what behaviour to expect. While a mac target has to deal with only one OS rather than multiple for linux-based code, there may still be changes from one version to another as we have painfully learnt with android over the years.