http://d.puremagic.com/issues/show_bug.cgi?id=2838

           Summary: std.file.rmdirRecurse fails
           Product: D
           Version: 2.027
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: graham.stjack@internode.on.net


The problem seems to be caused by incorrect behaviour of druntime's core.sys.posix.readdir, which fails to populate the dirent's d_type field - it is always set to 0.

This causes directories to be identified as files and thus not recursed into.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=2838





------- Comment #1 from graham.stjack@internode.on.net  2009-04-15 19:33 -------
Created an attachment (id=323)
 --> (http://d.puremagic.com/issues/attachment.cgi?id=323&action=view)
Simple test case

The unittest code in std.file will demonstrate the problem, but for what it is worth, here is my test code. I tracked the problem down as far as printing out the values passed to std.file.DirEntry.init, and fd.d_type was always zero when invoked by my test code. I haven't included my hacked version of file.d.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=2838


unknown@simplemachines.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |unknown@simplemachines.org




------- Comment #2 from unknown@simplemachines.org  2009-04-15 21:33 -------
Unable to reproduce on Windows or Linux.  Everything deletes fine.

I should note that readdir() is a libc function, afaik.  It's not a D-specific function.  If it's not properly filling the structure it's a system problem.

Also note, readdir() is not re-entrant.  There's a readdir_r() for that, which
phobos is not using.  If you are using rmdirRecurse() in a threaded app with
other directory reading happening, that may be the cause of your problems.

-[Unknown]


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=2838





------- Comment #3 from graham.stjack@internode.on.net  2009-04-16 20:05 -------
I have looked into it some more, and it looks like setting of d_type is only supported on some filesystems. I am using reiserfs, which I guess is a bit unusual, and it could be that.

Yes, I just tried it with a vfat filesystem on a flash-stick, and d_type was populated ok.

Can I suggest that, since the posix standard makes population of d_type (and
others) optional, we do something to handle the case where it is set to 0
(DT_UNKNOWN)?

Maybe the easiest thing to do is to modify std.file.DirEntry.init to be something like:

   void init(string path, dirent *fd)
   {
       immutable len = std.c.string.strlen(fd.d_name.ptr);
       name = std.path.join(path, fd.d_name[0 .. len].idup);
       d_type = fd.d_type;
       didstat = false;

       if (d_type == DT_UNKNOWN) {
           ensureStatDone;
       }
   }

or maybe defer it until isdir or isdile are called.

ensureStatDone would need to be modified to be something like:

   void ensureStatDone()
   {
       if (didstat) return;
       enforce(core.sys.posix.sys.stat.stat(toStringz(name), &statbuf) == 0,
               "Failed to stat file `"~name~"'");
       _size = cast(ulong)statbuf.st_size;
       _creationTime = cast(d_time)statbuf.st_ctime * std.date.TicksPerSecond;
       _lastAccessTime = cast(d_time)statbuf.st_atime *
std.date.TicksPerSecond;
       _lastWriteTime = cast(d_time)statbuf.st_mtime * std.date.TicksPerSecond;

       if (d_type == DT_UNKNOWN) {
           if (S_ISDIR(statbuf.st_mode)) {
               d_type = DT_DIR;
           }
           else if (S_ISREG(statbuf.st_mode)) {
               d_type = DT_REG;
           }
       }

       didstat = true;
   }

And of course std.file needs to be changed to use readdir_r too. If this approach is acceptable to you, I could have a go at submitting a patch if you like...


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=2838





------- Comment #4 from unknown@simplemachines.org  2009-04-16 21:03 -------
Ah, yes, that makes sense.

Please feel free to attach a patch.  I'm not the one who'd accept it, but I think your approach looks sound.  But, beware of convention - curlies, call syntax, etc. - it's best to stay common within a file.

-[Unknown]


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=2838





------- Comment #5 from graham.stjack@internode.on.net  2009-04-16 22:05 -------
Created an attachment (id=325)
 --> (http://d.puremagic.com/issues/attachment.cgi?id=325&action=view)
Proposed patch for std.file to handle case of linux filesystems that don't set
d_type

Submitted a proposed patch to set DirEntry.d_type in the linux version when the filesystem doesn't provide d_type is direntry, such as reiserfs.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=2838


Andrei Alexandrescu <andrei@metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |andrei@metalanguage.com
         AssignedTo|nobody@puremagic.com        |andrei@metalanguage.com


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=2838


Andrei Alexandrescu <andrei@metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


--- Comment #6 from Andrei Alexandrescu <andrei@metalanguage.com> 2010-09-26 15:09:53 PDT ---
http://www.dsource.org/projects/phobos/changeset/2061

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=2838


Jonathan M Davis <jmdavisProg@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg@gmx.com


--- Comment #7 from Jonathan M Davis <jmdavisProg@gmx.com> 2010-09-26 20:41:38 PDT ---
I expect that this fix fixes bug# 1635 (which you (Andrei) recently closed as
worksforme) and bug #4929. In my patch for bug# 3848 (which also fixes this
problem), I added this test to the unittest for dirEntries so that this
situation would be caught:

foreach (DirEntry e; dirEntries("/usr/share/zoneinfo", SpanMode.depth))
{
    assert(e.isfile || e.isdir, e.fullname);
}


I don't know whether or not you think that it would be worth adding such a test, but it it seems to be a test case which reliably catches the problem.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------