[PackageLoading] Move PackageBuilder to FileSystem #521
Conversation
aciidgh
force-pushed
the
walk-filesystem
branch
2 times, most recently
from
b549366 to
1f5eddb
Compare
| if !fileSystem.isFile(path) { return false } | ||
| guard let ext = path.suffix else { return false } | ||
| // FIXME: This isn't efficient. | ||
| return validExtensions.contains(String(ext.utf8.dropFirst())) |
There was a problem hiding this comment.
There should be a method on path to get this (the extension, defined to be the suffix without the .).
/cc @abertelrud any objections to that name?
aciidgh
force-pushed
the
walk-filesystem
branch
2 times, most recently
from
142c29b to
fb6b836
Compare
|
@ddunbar Updated and rebased this |
| public init(manifest: Manifest, path: AbsolutePath, fileSystem: FileSystem = localFileSystem) { | ||
| self.manifest = manifest | ||
| self.packagePath = path | ||
| self.fileSystem = fileSystem |
There was a problem hiding this comment.
One thing I wanted to do as part of this was to rewrite the logic to use a FileSystem rooted at packagePath (using RerootedFileSystem, when dealing with a local FS)... do you think that makes sense, and is it best as a separate commit?
There was a problem hiding this comment.
Probably will be better as separate. Also, do you mean PackageBuilder reroots it or the client/caller?
There was a problem hiding this comment.
Eventually I think the PackageBuilder should assume it is given a rerooted one, but that might need to be staged in.
The use case I am working towards is that we can run PackageBuilder directly against a git repository that doesn't even exist in the file system.
There was a problem hiding this comment.
(for example, I am trying to get it so that lower-level things are agnostic to the actual paths of things, so we can guarantee it is safe to share them)
There was a problem hiding this comment.
That sounds cool! I tried it a bit current system is tied a lot to AbsolutePath, Rerooting will need some refactoring. Will add to TODO
|
LGTM, I wonder if we can continue to simplify things so |
|
I tried getting rid of |
2 participants
No description provided.