Less problems with dirEntries.

[epi]

0ee90d8 solves a problem with code such as dirEntries("somedir", SpanMode.whatever).filter!(de => de.isFile)(), when broken symlinks are encountered. A broken symlink is simply neither a file nor a directory (but it is still a symlink), and no exception is thrown.

0ac4055 solves an issue with recursive iteration - when a directory was encountered to which the current user had insufficient rights to list its contents, an exception was thrown. Now such a directory is simply omitted from the iteration.

Both changes are Posix-only. They don't break any contracts ensured by previously existing unit tests, although the semantics of bool exists(in char[]) and possibly some other functions may need to be revised and defined more strictly for symlinks in the near future.
bool isDir(in char[]) and bool isFile(in char[]) still assume that the symlinked object exists, and throw if it doesn't.

[alexrp]

Breaks on FreeBSD.

[epi]

What version is it? Passes for me on FreeBSD 8.3-RELEASE amd64.

[alexrp]

We only support FreeBSD 9.0 (which is what the auto tester is running).

[braddr]

Incorrect. The auto-testers are running 8.1 and 8.2 (one is 32 and one is 64 bit -- I'd have to login to each to remember which is which).

[alexrp]

I seem to recall someone saying on a druntime pull request that they were running 9.0... I guess that's not the case then. But we have been taking some patches that are specific to FreeBSD 9.0, so how possible would it be to get the auto tester machines up to date with regards to this?

[epi]

std.file with my additions passes also on FreeBSD 9.0-RELEASE amd64.
Some other modules fail during tests, though:

Testing generated/freebsd/debug/64/unittest/std/datetime
core.exception.AssertError@std/datetime.d(28078): _assertPred!"==" failed: [PPT] is not == [PDT].


Testing generated/freebsd/debug/64/unittest/std/traits
std/traits.d(2448): Error: constructor std.traits.__unittestL2426_60.S5.this field s must be initialized in constructor, because it is nested struct


Testing generated/freebsd/debug/64/unittest/std/parallelism
totalCPUs = 1

(this one doesn't finish in 5 minutes, I assume a deadlock or sth alike)

[jmdavis]

PPT? Seriously? I don't think that that's been valid since 1945 ( http://www.timeanddate.com/worldclock/timezone.html?n=137&syear=1925 ). Not unless there's some timezone other than "America/Los_Angeles" that's been set erroneously. It sounds like a bug in FreeBSD to be honest, though I'd obviously have to investigate in detail to know for sure. It's just grabbing tzname[1] for the local time zone after setting the TZ environment variable to /usr/share/zoneinfo/America/Los_Angeles. So, unless the timezone files aren't in /usr/share/zoneinfo on FreeBSD anymore, if that's broken, I don't see how it could be anything but a FreeBSD bug, especially since it's returning a value that was valid nearly 70 years ago.

Does your local timezone have the acronym PPT? If it does, then it's probably a case of setting the TZ environment variable failing (either due to the time zone files being somewhere other than /usr/share/zoneinfo or because of a bug in the C code setting the timezone). If it doesn't, then the C code which sets tzname[1] is probably buggy. My guess would be the latter, because that was America/LosAngele's acronym during DST in 1945.

[epi]

It's a fresh FreeBSD setup (I installed it today just to run tests on phobos), and the local timezone is WET.
After setting the time zone to Pacific Time, date shows PST. After setting the date to some sunny July afternoon (this year), date shows PDT.

[jmdavis]

Okay. When you say that you set the time zone, I assume that you mean that you set in on the box as opposed to messing around with the TZ environment variable? std.datetime is specifically messing with the TZ environment variable, and it's quite conceivable that that's not working right. And if you're local time zone is WET, then I expect that it's a case of it incorrectly setting the time zone acronym when TZ is set to "/usr/share/zoneinfo/America/Los_Angeles" rather than the time zone not being set when TZ is set.

Regardless, I believe that the behavior is indicative of a bug in FreeBSD. All std.datetime is doing in that test is setting the TZ environment variable to "/usr/share/zoneinfo/America/Los_Angeles" and then verifying that tzname[1] is PDT like it's supposed to be. And that's all glibc stuff.

[epi]

Yup, I used sysinstall to set the timezone.
Indeed, that must be a bug in FreeBSD.
Just a few more checks:

[root@bsd90-64 ~]# date
Fri Nov  9 10:28:48 WET 2012
[root@bsd90-64 ~]# env | grep TZ
[root@bsd90-64 ~]# export TZ=America/Los_Angeles
[root@bsd90-64 ~]# date
Fri Nov  9 02:29:50 PST 2012
[root@bsd90-64 ~]# date 201207091400
Mon Jul  9 14:00:00 PDT 2012
Jul  9 14:00:00 bsd90-64 date: date set by root

#include "stdio.h"
#include "stdlib.h"
#include "time.h"
int main()
{
    setenv("TZ", "America/Los_Angeles", 1);
    tzset();
    printf("`%s' `%s'\n", tzname[0], tzname[1]);
    unsetenv("TZ");
    return 0;
}

Result:

`PST' `PPT'

[epi]

braddr: Could you please check which version exactly is installed on the FreeBSD_32 auto tester so that I can try to examine the problem?

[braddr]

$ uname -a
FreeBSD ip-10-204-81-235 8.3-RC2 FreeBSD 8.3-RC2 #0: Wed Mar 28
18:50:04 UTC 2012
root@ip-10-17-32-247:/usr/obj/i386/usr/src/sys/XENHVM i386

[andralex]

_ensureStatDone was quite a bit more descriptive. Why change it?

[jmdavis]

No. These changes are unacceptable. isDir, isFile, etc. must return correct data. It's reasonable for them to throw an exception if they can't, but it's not reasonable for them to return anything if they can't. We can look at adding a function to DirEntry which returns whether the file exists or is accessible. We can look at adding a function for checking whether it's a broken symlink. We can look at adding a wrapper for dirEntries which will (under some set of circumstances - exactly what would have to be discussed) catch and ignore FileException. But it is fundamentally wrong to have a function which cannot do what you ask it to do to return a value which is not specifically an error condition. That is precisely the sort of situation where exception should be used. I grant you that when iterating with foreach like this, an exception may be irritating to deal with, but this solution is very wrong.

The simplest solution would simply be to add exists to DirEntry, though technically I suppose that exists checks whether a file is stat-able rather than whether it exists, which isn't quite the same thing. But as that's what the free function exists already does, it would be completely consistent to add such a property to DirEntry. Then it would be incredibly straightforward to use filter to avoid broken symlinks if that's what you want to do.

I'll throw together an exists function for DirEntry and create a new pull request with that.

[jmdavis]

Looking into the details further, I'd say that your changes to dirEntries to skip files which you don't have permission for are probably okay, but the changes to DirEntry are definitely unacceptable. It may make sense to add an exists function to it, but I have to think this through a bit more before I'm sure of what the best approach is. The fact that broken symlinks return false for that is kind of ugly in my opinion, but that follows the behavior of the underlying C function.