1023670 - Fix DeviceManager.mkDirs with a Windows path

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[(not currently active) Ted Mielczarek]

I ported Negatus to run on Windows, and I was testing out running Steeplechase on it but I hit an issue with DeviceManager.mkDirs with Windows paths. It splits the path on '/' and then assembles a path starting with a '/', which produces broken Windows paths. You wind up with an error like:

mozdevice.devicemanager.DMError: Automation Error: Error processing command 'mkdr /C:'; err='Could not create directory /C:'

I figured the safest way to make this work was to special-case this when the remote end self-reports as Windows.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: Fix DeviceManager.mkDirs with a Windows path

Comment 2

[William Lachance (:wlach)]

Comment on attachment 8438085 [details] [diff] [review]
Fix DeviceManager.mkDirs with a Windows path

Review of attachment 8438085 [details] [diff] [review]:
-----------------------------------------------------------------

I feel like a more broad-ranging solution might be required here eventually, but I don't know what it is and don't want to block your work on it.

Comment 3

[(not currently active) Ted Mielczarek]

I agree, and that's why I put the top-level "remoteIsWin" property in for a tiny bit of future-proofing, but I haven't hit any other problems with the little bit of devicemanager API that Steeplechase consumes, so I figured I wouldn't borrow trouble. Thanks!

Comment 5

[(not currently active) Ted Mielczarek]

I had to fix a few unit tests as well because of the extra 'info' command that gets run:
https://hg.mozilla.org/integration/mozilla-inbound/rev/da73c6745a10

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5)
> I had to fix a few unit tests as well because of the extra 'info' command
> that gets run:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/da73c6745a10

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/e60fc9769bfc under suspicion of breaking android tests like https://tbpl.mozilla.org/php/getParsedLog.php?id=41810816&tree=Mozilla-Inbound (but really all android tests: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=95a1b1128c23&jobname=android

Comment 7

[(not currently active) Ted Mielczarek]

I rebased and pushed this back to Try and it's pretty green on Android:
https://tbpl.mozilla.org/?tree=Try&rev=6592a6175c07

It must have been something else.

Comment 8

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2885b71be45

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c2885b71be45

Comment 10

[Syd Polk :sydpolk]

Attachment: Pull request to update steeplechase's requirements to take this fix.

Not sure if I need to put this pull request here or file a separate bug.

Comment 11

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8497822 [details] [review]
Pull request to update steeplechase's requirements to take this fix.

This actually won't work.  Mozdevice 0.40 was released in August and doesn't contain the fix you need.

What we need to do is bump the version of mozdevice to 0.41, release it to pypi, and then update steeplechase to use that.  I think we'll handle this in a separate bug, which I'll file shortly.

Comment 12

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8497822 [details] [review]
Pull request to update steeplechase's requirements to take this fix.

My bad, this is the right thing.  :)

Comment 13

[(not currently active) Ted Mielczarek]

Merged that PR. In the future, yeah, it's better to file new bugs for things even if they're related issues, as it tends to make bugs confusing (and things get lost in already-resolved bugs).