Symbolic Links to directories are treated as files by file.GetFiles

[simon-wh]

Also means that they do not show up when using file.GetDirectories, but show up under file.GetFiles.

I'm creating my symbolic links through ln -s

[RomanHargrave]

This should be a simple fix. I'll look in to it.

[Ozymandias117]

That should just require changing list_directory.

Is it worth counting DT_DIR and DT_LNK (symlinks) as directories, DT_REG as files, and ignoring any named pipes, etc. that are in a directory? That seems strictly more correct.

[RomanHargrave]

@Ozymandias117 that what I thought. I just hadn't gotten around to looking at it.

[RomanHargrave]

@Ozymandias117 so the deal here is that if we count DT_LNK, we need to make sure it's a symlink to a directory. We also need to make sure to follow recursive symlinks. I'm sitting here looking at fs::list_directory and I think that if we split off "is this a directory" in to a separate tail-recursive function that we could easily do it. Unfortunately, readlink and POSIX realpath accept a path as a file and not a file system node, so the body of fs::list_directory will probably get modified to accommodate that...

[Ozymandias117]

Yeah, we need to figure out what type of symlink it is and add it in either case, if it's a file or a directory.

Are you saying opendir doesn't follow symlinks in the path? I'm not sure where we really need to care about recursive symlinks, otherwise? We only enumerate the directories and files given a directory, right?

[RomanHargrave]

@Ozymandias117 lapi::getdir makes its way back to the more general purpose fs::list_directory_contents which is used all over the place. It would not make sense for opendir and friends to produce directory entries that followed their symlinks. What's more frustrating is that readlink/realpath have a bad interface that requires pointless string manipulation instead of file system primitives from the C library.

[Ozymandias117]

My point was that it looks like lapi::getdir looks like it only ever cares about one directory at a time. If that's the case, and opendir successfully deals with symlinks, I don't think we have to do any recursion or deal with it? We just need to properly list a single directory given a path. There's nothing that looks through directories recursively in the BLT stuff, is there?

I can look more closely when I'm at a real computer tomorrow night, if I need to.

[RomanHargrave]

@Ozymandias117 I think you may be missing the point. The original issue here was that lap::getdir returns a list of directories in a directory. The API calls list_directory which then checks next->d_type == DT_DIR, which is false for symlinks. If we allow all symlinks through, and a stray file exists in the directory being listed, then, depending on the robustness of the calling script, the caller may fail later by attempting to treat a file symlink as a directory instead of a file.

[Ozymandias117]

My confusion was at "I think that if we split off 'is this a directory' in to a separate tail-recursive function that we could easily do it" I wasn't clear why we need to recurse over anything to tell if a symlink is a directory. I'll leave it to you, do what you think is best.

[RomanHargrave]

@Ozymandias117 ah, that was just me trying to avoid realpath, which is a tail-recursive path resolver. Nonetheless, that's what I've gone with.

[RomanHargrave]

@GreatBigBushyBeard please test the issue_28 branch and see if it fixes your problem.

[simon-wh]

The symbolic links with file.GetDirectories appear to be fixed now, just tested the file.GetFiles and it isn't showing the symbolic linked directories anymore either. Looking good.

[RomanHargrave]

Sounds good.