Compute relative paths to support sources outside a spec's directory #524
Conversation
There was a problem hiding this comment.
Fantastic, thank you for this @elliottwilliams! Nice work 👏
As you say it would be good to have this in PathKit itself, so I would encourage you to open a PR there as well. That doesn't stop us from adding this here for now though.
Could you also please add a changelog entry
This uncovered a seeming inconsisteny with how folder reference paths were specified vs all other paths.
| @@ -474,7 +474,7 @@ class SourceGenerator { | |||
| var sourcePath = path | |||
| switch type { | |||
| case .folder: | |||
| let folderPath = Path(targetSource.path) | |||
| let folderPath = project.basePath + Path(targetSource.path) | |||
There was a problem hiding this comment.
I had to change this line to fix a test that broke when I swapped out byRemovingBase(path:) with relativePath(from:).
It seems like folder target sources were being created without the base directory as part of the path. In the tests, I saw that a folder "Sources/A" would passed to getFileReference as Path("Sources/A"), but a file "Sources/a.swift" would be passed as Path("TestDirectory/Sources/a.swift").
|
Thanks! It was fun to write :) Changelog updated. |
| /// - `../a/b` simplifies to `../a/b` | ||
| /// - `a/../../c` simplifies to `../c` | ||
| public func simplifyingParentDirectoryReferences() -> Path { | ||
| return normalize().components.reduce(Path(), +) |
There was a problem hiding this comment.
Does normalize just do the same thing? What does adding the components together again do?
There was a problem hiding this comment.
normalize calls NSString.standardizingPath, which almost does the same thing but doesn't remove parent directory references in relative paths:
For absolute paths only, resolving references to the parent directory (that is, the component “..”) to the real parent directory if possible using resolvingSymlinksInPath. For relative paths, references to the parent directory are left in place.
PathKit's + operator backs up a level if you try to add .. to a path, which means it can handle this normalization edge case for us. In other words:
Path("foo/../bar").normalize() == Path("foo/../bar")
Path("foo") + ".." + "bar" == Path("bar")
Closes #318
Previously, XcodeGen would determine the relative path to a source by removing the project's
basePathfrom the source's path, but this doesn't work when the source's path is outside the base path. For example, given a base path/a/band a path to a source../c, XcodeGen would unsuccessfully try to remove "/a/b" from "/a/c" and wind up creating a "relative" file reference to "/a/c".This change adds a method to
Paththat computes the relative path between two paths, inspired by methods likePathname#relative_path_fromin Ruby, and uses it to determine the paths for top-level references in the project.I'm trying to decide whether I should really be contributing this to PathKit, since this solves a generalizable path problem, but since it fixes a specific bug in XcodeGen I wanted to put it up here. I'm happy to open a PR on PathKit in addition to this one, so that you don't get stuck maintaining a pathing algorithm 🙂