820989 - mozdevice: should be able to ls a file path

Status: RESOLVED WONTFIX

(Testing :: Mozbase, defect)

Description

[Mark Côté [:mcote]]

If you use the ls command on the terminal and provide a path to a file (as opposed to a directory), you get "##AGENT-WARNING## <path> is not a directory".  I find this particularly weird with dmcli, which attempts to emulate shell commands, since from bash it's totally acceptable to ls a file path.  It's a good way to determine if a file exists.

Comment 1

[Mihnea Dobrescu-Balaur (:mihneadb)]

I think the problem is the fact that in fact 'ls' is mapped onto the listFiles method, which is thought as it would only act on directories - actually, in dmSUT it actually cd's in there and then calls ls.

What do you think about adding a fileExists method in dmcli?

Comment 2

[Mark Côté [:mcote]]

Actually there is a fileExists() method already.  I just thought it would be nice to support "ls" on a file specifically from dmcli.

Comment 3

[William Lachance (:wlach)]

I agree. No harm in adding a bit of logic into dmcli to do this. No one said it had to map exactly the devicemanager interface. :)

Comment 4

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: make dmcli ls work on files

Ok.

Comment 5

[Mark Côté [:mcote]]

Comment on attachment 744887 [details] [diff] [review]
make dmcli ls work on files

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

Did you fully test this?  On my system, I now get an exception if the argument is a directory or does not exist.

It turns out that this is actually due to a bug in fileExists(), which I have filed as bug 868505 (see there for the full exception traceback).  However I can't approve this patch without that bug being solved, as it would cause a regression in behaviour by having "ls" not work on directories anymore.  Feel free to ask for a rereview once that bug is fixed (which you can also do, if you like) and you have fully tested this patch.

Comment 6

[Mark Côté [:mcote]]

Also, if the result of fileExists() is False, the function can just return an empty list right away.  No point in the subsequent call to listFiles() in this case.

Comment 7

[Mihnea Dobrescu-Balaur (:mihneadb)]

(In reply to Mark Côté ( :mcote ) from comment #6)
> Also, if the result of fileExists() is False, the function can just return
> an empty list right away.  No point in the subsequent call to listFiles() in
> this case.

Sorry, not sure what you mean. Are you talking about listfiles in dmcli? Because the patch does that (line 280).

Comment 8

[William Lachance (:wlach)]

Comment on attachment 744887 [details] [diff] [review]
make dmcli ls work on files

Unflagging myself for review here, looks like mcote has this one.