Allow relative paths in output file map #63
Conversation
| @@ -46,7 +46,7 @@ public struct Driver { | |||
| case subcommandPassedToDriver | |||
| case relativeFrontendPath(String) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Do you mind breaking these whitespace changes out into a separate PR? We can probably get those merged faster and they make this diff a little harder to read.
| @@ -233,10 +233,10 @@ public struct Driver { | |||
| } | |||
|
|
|||
| // Compute the working directory. | |||
| let cwd = localFileSystem.currentWorkingDirectory | |||
There was a problem hiding this comment.
I'm not sure we necessarily want to set the cwd here if we're not overriding it. It doesn't look like the C++ driver applies the cwd to the inputs in that case, it just uses relative paths.
There was a problem hiding this comment.
The thing that bothers me here, that you can override working directory with relative path, and if you override it with . like this -working-directory=. and not override it at all, code will take completely different paths.
Do you think that instead of making all arguments absolute we need to teach code to correctly handle relative paths?
There was a problem hiding this comment.
Removed this change for now, it works only when -working-directory supplied. To work relative to cwd, further changes needed to teach code to work with relative paths.
| return .absolute(.init(base, relPath)) | ||
| } | ||
|
|
||
| fileprivate static let singleDep = VirtualPath.relative(.init("")) |
There was a problem hiding this comment.
This is only used once, so I think you can just inline it into the if statement above with a comment explaining what it means.
| (try VirtualPath(path: input), try entry.paths.mapValues(VirtualPath.init(path:))) | ||
| }) | ||
| }, uniquingKeysWith: { $1 }) |
There was a problem hiding this comment.
This behavior looks good to me, and I think it's consistent with the C++ driver
zienag
force-pushed
the
rel_paths_in_outputmap
branch
from
a911c1d to
224d9e6
Compare
zienag
force-pushed
the
rel_paths_in_outputmap
branch
from
224d9e6 to
bc1d31b
Compare
|
@owenv thanks for reviewing! |
|
@swift-ci please test |
This allows having relative paths on output file map json file.
Also, I've made working directory to default to cwd if no
-working-directoryarg is supplied.Also, there is small bug, if there is two identical inputs in output file map, driver would silently crash, as it uses
Dictionary(uniqueKeysWithValues:)initializer, now it will choose last record (behaviour that usually seen in clang, for example), but is it better to have warning or error? There is some difficulty thought, as closure that handles duplicates doesn't have key as argument, so we need to find duplicates beforehand.Also, it looks that there is a lot of dangling whitespaces, is it ok to remove them? May be e need some codestyle tool (swift-format?) that will make codestyle automated?
This is my first contribution to swift project, so any help would be very much appreciated :)