win32: support hidden file notion

[GoogleCodeExporter]

Currently ls calls out to fs.get_basename_is_ignored, which we only have an
implementation for Unix.  For windows, we need to use the native APIs probably.

Original issue reported on code.google.com by cgwalt...@gmail.com on 26 Jan 2008 at 4:11

[GoogleCodeExporter]

"ls" on root gives this exception:

FileStatError: [Error 13] The process cannot access the file because it is
being
 used by another process: u'c:/hiberfil.sys'

Original comment by cgwalt...@gmail.com on 26 Jan 2008 at 4:12

[GoogleCodeExporter]

Copying this from issue 126:

Windows has a concept of hidden files, I think.  Probably the best solution 
here is
to skip them unless you specify -a, just like how we hide files starting with 
'.' on
Unix.
I took a look at MSDN, here is the .NET API:
http://msdn2.microsoft.com/en-us/library/system.io.file.getattributes(VS.71).asp
x

But we need to figure out how to get the file attributes from Python.  Hm...ah 
hah! 
Google to the rescue:
http://aspn.activestate.com/ASPN/Cookbook/Python/Recipe/303343

So we need to figure out how to best use that.  That should fix the 'ls' error. 
 It
looks like it's throwing if we just call os.stat() on it, so we have to do 
something
before that.

Maybe the method get_file_sync() needs to have an optional flag like
stat_hidden=True, and "ls" will pass False for that if the option '-a' was not
passed.  Then inside the Win32File implementation, we override _do_get_stat, and
check using win32api.GetFileAttributes(self.path, 
win32con.FILE_ATTRIBUTE_HIDDEN)
whether the file is hidden or not.  If it is - then we throw an exception such 
as
FileHiddenError, and "ls" knows to ignore it.

Now as for the first character being stripped...that bug is in
hotwire_ui/renderers/file.py, function _render_objtext.  That function is 
assuming
that the filesystem root is one character long.  I'm not totally sure how to 
fix this
one cleanly.  Maybe we could do something ugly like checking with is_windows() 
and
looking to see if the path is of the form [A-Z]:/  But that's ugly...

Original comment by cgwalt...@gmail.com on 26 Jan 2008 at 4:12

[GoogleCodeExporter]

Actually, this is not caused by the "hidden" attribute, as os.stat can success 
on
other hidden files and win32api.GetFileAttributes() also fails on 
"c:/hiberfil.sys".

Original comment by Zeng.Shi...@gmail.com on 26 Jan 2008 at 4:44

[GoogleCodeExporter]

Ah, ok.  Well, thinking about this more, I was wrong - actually on Unix the 
"stat"
function can never fail if the file exists.  So the permissions thing doesn't 
apply,
and it should be OK to apply your patch.  

Committed r887
    M   hotwire/builtins/ls.py
r887 = 9c348196d7bb70f05954123228f5748cb14e1170 (git-svn)

Thanks very much for the patch!  Now though it would still be nice to also use 
the
Win32 HIDDEN notion for "ls".  I think what we should do is rename drop the
fs.get_basename_is_ignored in favor of a file.is_hidden property.  Looking at 
the
Java API, this is what they have:
http://java.sun.com/j2se/1.4.2/docs/api/java/io/File.html

Original comment by cgwalt...@gmail.com on 26 Jan 2008 at 3:14

[GoogleCodeExporter]

Index: C:/tools/hotwire-shell-read-only/hotwire/builtins/ls.py

===================================================================

--- C:/tools/hotwire-shell-read-only/hotwire/builtins/ls.py (revision 886)

+++ C:/tools/hotwire-shell-read-only/hotwire/builtins/ls.py (revision 887)

@@ -41,13 +41,22 @@

     def __ls_dir(self, dir, show_all):
         fs = Filesystem.getInstance()
         for x in iterd_sorted(dir):
-            if show_all:
-                yield fs.get_file_sync(x)
-            else:
-                bn = os.path.basename(x)
-                if not (fs.get_basename_is_ignored(bn)):
+            try:
+                if show_all:
                     yield fs.get_file_sync(x)

+                else:
+                    bn = os.path.basename(x)
+                    if not (fs.get_basename_is_ignored(bn)):
+                        yield fs.get_file_sync(x)
+            except:
+                # An exception here should ordinarily only happen on Windows;
+                # if we know the path exists because it was returned by
+                # listdir(), on Unix the stat() call cannot fail.  
+                # See 
http://code.google.com/p/hotwire-shell/issues/detail?id=126
+                _logger.debug("Failed to stat %r", x, exc_info=True)
+                pass
+
I don't think "pass" is needed here. ;)

Original comment by Zeng.Shi...@gmail.com on 26 Jan 2008 at 5:55

[GoogleCodeExporter]

True =)  But I kept it in there as a sort of style thing, meaning "I explicitly 
chose
not to do anything here".  The other reason is to try to keep the ideal that you
could remove all _logger.debug() statements from the program and it would still 
work.
 If the "pass" wasn't there then it wouldn't compile.

Original comment by cgwalt...@gmail.com on 26 Jan 2008 at 9:17

[GoogleCodeExporter]

here's a patch to implement '-a' option to 'ls' builtin command. It moves 
__ls_dir
from LsBuiltin to BaseFilesystem, and then it is overwritten by Win32Filesystem.
Perhaps some codes need cleanups, it is now just bases on some random hacks. :)

I have not tested it on Linux system yet. Hope this doesn't break anything.

Original comment by Zeng.Shi...@gmail.com on 27 Jan 2008 at 5:48

Attachments:

• hidden.patch

[GoogleCodeExporter]

Ok, this looks like a good start.  I took this patch and reworked it a bit.  
The main
idea is to add a new "hidden" attribute to the File object, obsoleting the old
basename_is_ignored thing.  

I simplified the fs_unix.py ls_dir implementation because we can't get an 
exception
(well, unless a file disappears after the listing but before we call stat(), 
but I'm
not worried about that yet).

It passes the test suite here, can you tell me if this patch works on Windows?

Original comment by cgwalt...@gmail.com on 27 Jan 2008 at 7:35

Attachments:

• hotwire-hidden-rework-1.patch

[GoogleCodeExporter]

This work also made me realize we need to share common code between fsearch.py 
and
walk.py.

Original comment by cgwalt...@gmail.com on 27 Jan 2008 at 7:36

[GoogleCodeExporter]

Yes, the patch in comment #8 works fine on my Windows XP. Thanks!

Original comment by Zeng.Shi...@gmail.com on 27 Jan 2008 at 7:05

[GoogleCodeExporter]

Cool!  Ok, committed:

Committed r888
    M   hotwire/builtins/walk.py
    M   hotwire/builtins/fsearch.py
    M   hotwire/builtins/ls.py
    M   hotwire/command.py
    M   hotwire/sysdep/fs.py
    M   hotwire/sysdep/fs_impl/fs_win32.py
    M   hotwire/sysdep/fs_impl/fs_unix.py
r888 = 57a15fee04451bdbe1e8e83887a4c55348510477 (git-svn)

Original comment by cgwalt...@gmail.com on 27 Jan 2008 at 7:17

• Changed state: Fixed

[GoogleCodeExporter]

From issue 146:

Type:

  cd c:/
  ls - l

Dies with following traceback:

17:37:49 [3980] hotwire.Completion ERROR failed to get completions
Traceback (most recent call last):
  File "C:\compile\hotwire\hotwire\completion.py", line 211, in __do_async_compl
ete
    result = self.__get_completions(completer, text, cwd)
  File "C:\compile\hotwire\hotwire\completion.py", line 205, in __get_completion
s
    return CompletionResults(list(completer.completions(text, cwd)))
  File "C:\compile\hotwire\hotwire\builtins\cd.py", line 36, in completions
    for completion in super(CdCompleter, self).completions(text, cwd, **kwargs):

  File "C:\compile\hotwire\hotwire\completion.py", line 101, in completions
    yield _mkfile_completion(text, fpath)
  File "C:\compile\hotwire\hotwire\completion.py", line 80, in _mkfile_completio
n
    fobj = fileobj or fs.get_file_sync(fpath)
  File "C:\compile\hotwire\hotwire\sysdep\fs.py", line 67, in get_file_sync
    f.get_stat_sync()
  File "C:\compile\hotwire\hotwire\sysdep\fs.py", line 287, in get_stat_sync
    self._do_get_hidden()
  File "C:\compile\hotwire\hotwire\sysdep\fs_impl\fs_win32.py", line 85, in _do_
get_hidden
    self._hidden = win32api.GetFileAttributes(self.path) & win32con.FILE_ATTRIBU
TE_HIDDEN
error: (32, 'GetFileAttributes', 'The process cannot access the file because it
is being used by another process.')

Original comment by cgwalt...@gmail.com on 8 Feb 2008 at 6:29

• Changed state: Started

[GoogleCodeExporter]

I finally got around this one by these three patches
please take a look

Original comment by Zeng.Shi...@gmail.com on 20 Feb 2008 at 11:54

Attachments:

• 0001-correct-stat-references.patch
• 0002-Use-_stat-from-msvcrt-to-implement-a-fallback-when-o.patch
• 0003-use-the-Attributes-got-from-FindFiles-to-determine-i.patch

[GoogleCodeExporter]

Thanks a lot for these patches!  

Taking a look...for 0002, can we cache this part:

+                from ctypes import CDLL, Structure, c_char_p, c_uint, c_ushort,
c_short, c_long, byref
+                from ctypes.util import find_library
+                libc = CDLL(find_library("msvcrt"))
...

in the class constructor? I am thinking it might be slow to call find_library 
each
time we want to stat a file. The Stat32 class definition could probably just go 
at
the top of the module.

On 0003, I'm a bit confused - the only reference to FindFiles I can find on 
MSDN is
this one: http://msdn2.microsoft.com/en-us/library/bb774079(VS.85).aspx
But that sounds like it's about showing a dialog box?

Original comment by cgwalt...@gmail.com on 21 Feb 2008 at 2:50

[GoogleCodeExporter]

For 0002, I think you are right. I just wanted it to work, never thought about 
the
efficiency. ;)

For 0003, FindFiles is from win32api, the document says:

win32api.FindFiles
list = FindFiles(fileSpec)

Retrieves a list of matching filenames. An interface to the API
FindFirstFile/FindNextFile/Find close functions.

Parameters

fileSpec : string

A string that specifies a valid directory or path and filename, which can 
contain
wildcard characters (* and ?).

Win32 API References

Search for FindFirstFile at msdn, google or google groups.

Search for FindNextFile at msdn, google or google groups.

Search for FindClose at msdn, google or google groups.

Return Value
Returns a sequence of WIN32_FIND_DATA tuples

Original comment by Zeng.Shi...@gmail.com on 21 Feb 2008 at 3:18

[GoogleCodeExporter]

Here are the reworked patches.
Sorry that I'm still learning git, and I don't know how to produce a patch 
combining
two commits. 0002 and 0005 should be combined into one to avoid noise.

Original comment by Zeng.Shi...@gmail.com on 21 Feb 2008 at 4:06

Attachments:

• 0001-correct-stat-references.patch
• 0002-Use-_stat-from-msvcrt-to-implement-a-fallback-when-o.patch
• 0003-use-the-Attributes-got-from-FindFiles-to-determine-i.patch
• 0004-Add-a-library-msvcrt-loaded-by-ctypes.-This-could-be.patch
• 0005-rework-the-_do_get_stat.patch

[GoogleCodeExporter]

Ohh hmm, it sounds like using FindFiles correctly would require a lot of code
changes, because we don't actually want to search for globs in that part of the 
code.
Right now we process globs like * and ? inside Hotwire using the Python 
libraries. 

So if there was a file that had a '*' in it, we would pass that * down to 
FindFiles,
which would return multiple matches.

Hmm.  Going back to the root cause of this problem, on Windows, os.listdir() is
returning files that we cannot call stat() or any other access functions on, 
but on
Unix these functions never fail.   You've now changed the stat() to be more 
like Unix
- the problem we're hitting is the GetFileAttributes.  What if we just changed 
the
patch 0003 to look like this:

try:
  self._hidden = win32api.GetFileAttributes(self.path) & win32con.FILE_ATTRIBUTE_HIDDEN)
except:
  self._hidden = None

Original comment by cgwalt...@gmail.com on 21 Feb 2008 at 4:51

[GoogleCodeExporter]

But how can a File constructed from a path with glob?
IMHO, globs should be processed before being passed to a File constructor. So
self.path must not contain a glob like "*" or "?"

Original comment by Zeng.Shi...@gmail.com on 21 Feb 2008 at 5:27

[GoogleCodeExporter]

The * character is processed before calling the File constructor if it was not
quoted.  But it is legal on Unix at least for file names to contain the * 
character.
 For example in Hotwire:

touch foo\*bar
ls foo\*bar      # finds the exact file
ls foo*bar       # processes * as a glob

But, do you think that if we changed the GetFileAttributes call to never fail 
either,
along with the rest of your patches the overall problem here would be solved?

Original comment by cgwalt...@gmail.com on 21 Feb 2008 at 1:32

[GoogleCodeExporter]

OK, that does make sense in that case.

Yes, I think it is solved by letting GetFileAttributes not fail.

Original comment by Zeng.Shi...@gmail.com on 21 Feb 2008 at 3:53

[GoogleCodeExporter]

Your patches seem good - I actually went ahead and gave you commit access.  You
should be able to check out the code now as a developer - see the Source tab at 
the
top for how to do that.  Remember to check

http://code.google.com/p/hotwire-shell/wiki/Contributing

for some guidelines.  

(If you still want me to commit them I can though)

Original comment by cgwalt...@gmail.com on 21 Feb 2008 at 4:03

[GoogleCodeExporter]

All right, I'll try to commit myself some time later.

Thanks for giving me the commit permission.

Original comment by Zeng.Shi...@gmail.com on 21 Feb 2008 at 4:29

[GoogleCodeExporter]

Committed to trunk. Closing.

Original comment by Zeng.Shi...@gmail.com on 22 Feb 2008 at 5:48

• Changed state: Fixed
• Added labels: OpSys-Windows