Devicemanager makes assumptions it shouldn't when operating on remote paths

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
6 years ago
8 months ago

People

(Reporter: ahal, Assigned: gbrown)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 years ago
Some examples:
https://github.com/mozilla/mozbase/blob/master/mozdevice/mozdevice/devicemanagerADB.py#L173
https://github.com/mozilla/mozbase/blob/master/mozdevice/mozdevice/devicemanagerADB.py#L194
https://github.com/mozilla/mozbase/blob/master/mozdevice/mozdevice/devicemanagerADB.py#L221

Ideally we'd have some kind of _getRemotePath function that takes in one or more parameters and returns a properly formatted remote path (joining the paths if there are more than 1). Consumers should never need to worry about whether or not to include a trailing slash. Consumers should also be able to pass in os.path style paths, even on windows.

Note: dmSUT also suffers from this problem, but at least there we usually check if the path has a trailing slash or not before manipulating it.
(Reporter)

Updated

6 years ago
OS: Windows 2000 → All
Hmm, I have to admit I'm a bit confused by this bug. Could you give some specific examples of DeviceManagerADB not behaving as you expect?

I think we can pretty safely assume that any device running an ADB server is posix-compliant and thus uses '/' for paths. I believe :mcote at one point advocated using the posixpath module here, instead of manually inserting /'s everywhere (which I agree is ugly).

Comment 2

6 years ago
Yeah the only problem I see here is that we assume that / works as a path separator on the client machine, and even that only applies to the first example.  It looks like we're always adding slashes where they are required, and duplicating / in paths doesn't have any negative effects, on POSIX-based systems at least.  Granted it's *ugly*, and as wlach said I think we should use posixpath, but from the above examples I don't see any problems except the one (and even then, I wouldn't be surprised if that actually works on Windows machines these days).

When designing Negatus, to keep the interface as close as possible across devices I specifically called out that the SUTAgent on any platform should handle only POSIX-style paths, doing any translation it needs to internally. https://wiki.mozilla.org/Auto-tools/Projects/SUTAgent#Interface
(Reporter)

Comment 3

6 years ago
Yep, I had seen a bug which I thought was a problem in dirExists, but on closer look it's caused by something else.

I just checked and adb doesn't handle windows style paths, so I still think we should use posixpath. Plus it would be nice to clean up the code a bit.

Fwiw, I didn't intend this to be something that was high priority.
(Assignee)

Updated

8 months ago
Assignee: nobody → gbrown
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.