Get Unix file stats with LStat (instead of Stat)

[mjrousos]

Stat was ignoring symbolic links (and getting stats for the target
instead) which caused checks for whether or not a path was a sym link
to fail on Unix.

Fixes dotnet/corefx#5015

[mjrousos]

This will change other properties (such as file size and attributes like 'hidden') to report information on the symlink instead of the target file, also.

That seems desirable, though, because it looks like that matches the Windows behavior for those properties (FileInfo.Size on a sym link returns 0, for example).

[weshaggard]

cc @JeremyKuhne

[weshaggard]

Sorry didn't mean to close

[mjrousos]

Looks like the Jenkins task failed while trying to clone the repro. Is there something we can do to reset it?

[stephentoub]

I'm not convinced this is something we want to change, at least not in this manner. The point of a symlink is to represent some other file. If you're given the path to a symlink and you open it, you're actually opening the file it points to. So with this change, for example, getting the length via FileInfo.Length and via File.Open(,,,,).Length will end up being different, right? And File.Exists (which also uses stat) could end up returning a different result?

[mjrousos]

FileInfo.Length and File.Open length will differ, but that's consistent with our Windows behavior.

This code:

        var info = new FileInfo(args[0]);
        Console.WriteLine($"Info length: {info.Length}");
        var file = File.Open(args[0], FileMode.Open);
        Console.WriteLine($"File.Open length: {file.Length}");
        Console.WriteLine($"Is symlink: {((info.Attributes & FileAttributes.ReparsePoint) == FileAttributes.ReparsePoint).ToString()}");

Produces this output with Windows CoreFX bits currently (when args[0] is a sym link):

Info length: 0
File.Open length: 8718
Is symlink: True

File.Exists also works as you predict on Windows (a link with its target moved/deleted still returns true).

I see what you're saying about it being confusing that FileInfo and FileStream give different information for a file, but it's useful to be able to see both the linked file's information as well as information for the link itself. A very concrete case of this mattering is in dotnet/corefx#5015 (currently, it's not possible to determine whether a file/directory is a reparse point on Unix because the attributes returned are for the target file/directory).

[stephentoub]

Ok. If this new behavior better matches what's on Windows, then I'm ok with it. Can you please add corresponding tests, though, both to a) lock in the behavior, and b) help demonstrate that the behavior matches between Windows and Unix?

[mjrousos]

Yes, I agree adding some test cases here makes sense. I'll do that and update the PR.

[stephentoub]

Thanks, @mjrousos.

[stephentoub]

@mjrousos, are you still working on this?

[mjrousos]

Yes, sorry for the slow progress. I should have a chance to add these tests at the end of the week.

[mjrousos]

@stephentoub, I've added some test cases to confirm that FileInfo attributes reflect reparse point attributes, as expected (and that behavior is consistent across platforms). But it looks like the tests fail on Windows because creating symbolic links requires admin rights. The tests passed on my machine because I ran them from an elevated command prompt.

Do you know of a way to make sure Jenkins runs tests elevated? If not, I'll just change the tests to only run in cases when they can successfully create sym links.

[stephentoub]

Do you know of a way to make sure Jenkins runs tests elevated?

We don't currently have a mechanism for that.

[mjrousos]

Ah, bummer. I looked around but I think we can't create symbolic links on Windows without elevated permissions. So, I'll update the test cases to only execute if they can successfully create the links. I should have an update shortly.

[mjrousos]

@stephentoub, after updating the tests to only run if symbolic links can be created, it looks like everything's passing. Please let me know if you have any other feedback.

[stephentoub]

Can you use https://github.com/mjrousos/corefx/blob/UnixGetAttributesFix/src/Common/tests/System/IO/TempFile.cs instead of adding this?

[mjrousos]

Yep; that looks like exactly what I want.

[stephentoub]

Thanks, @mjrousos. Overall looks good, though left some comments on the tests.

[mjrousos]

Thanks, @stephentoub. I've updated the tests based on your feedback.

[stephentoub]

GetTempFileName() returns a full path.

[mjrousos]

Excellent; I'll lose the Path.Combine.

[stephentoub]

A few remaining things to address, then LGTM. Can you also squash this? Thanks for the fix.

[mjrousos]

Sure thing. I addressed the last couple issues and squashed.

[stephentoub]

Thanks!