Add comments and clear up some of the code in preparation for SE-0021…

[abertelrud]

… (test naming convention) changes.

[ddunbar]

Seems ok to me...

[aciidgh]

Actually Daniel suggested that we don't keep this in filesystem/utility because clients almost always endup needing the entry name. If the method provides full path client will extract the basename from path which is a waste. So I put this method until we can refactor the PackageBuilder code to not use AbsolutePath at all. I should have added that FIXME here, my bad.

[abertelrud]

Why would PackageBuilder switch off of AbsolutePath? Would it just be using strings again?

Note that I'm not for a moment saying we should remove the ability to get the just the names of the entries: that's a fundamental building block of directory access. But instead of having every client construct their own AbsolutePaths, there should also be a way to iterate over subpaths.

[aciidgh]

No no, it will use relative paths to the sources instead of absolute ones. So you can pass in a rerooted filesystem and get modules out of it.

[aciidgh]

thats a typo above... *AbsolutePath -> absolute path

[aciidgh]

Here is the discussion about rerooted fs: #521 (comment)

[abertelrud]

Thanks for the link to #521. But it seems orthogonal to me, because part of the point of FileSystem is that the client code (PackageBuilder in this case) shouldn't need to know or care whether it's given a file system that maps directly to the underlying file system, to a git repo or cpio file or whatever, or to a RerootedFileSystemView on top of one of those other kinds of file system.

In all cases the API deals in terms of just paths (currently called AbsolutePath but eventually intending to be renamed to Path). And it doesn't change the fact that sometimes you want to walk a file system hierarchy. Other times you just want to get the names of the entries in a single directory.

[abertelrud]

A good way to think of it is as a stack of file systems. A RerootedFileSystemView just means that you can pretend that everything exists at the top level of the file system, but other than that it isn't any different from any other file system.

[aciidgh]

We use absolute paths here because Sources takes absolute paths, it can be refactored to contain only relative paths and only probably the module would be aware of its base path. After that point PackageLoader should be able to work with a rerooted fs. You're right this method is orthogonal to all this but once that is in, this function should not be needed anymore (ofcourse until we need this kind of facility somewhere else).