Allow scratchPath to be a relative path
#1824
Conversation
There was a problem hiding this comment.
Should swiftSDKsDirectory also be interpreted relative to the project root? And could you add that the paths are relative to the project root to Configuration File.md?
| explicitDirectory: options.swiftPMOrDefault.swiftSDKsDirectory.map { | ||
| try AbsolutePath(validating: $0, relativeTo: AbsolutePath(projectRoot)) | ||
| } |
There was a problem hiding this comment.
Does it make sense to have a SwiftSDK installed relative to your project? Don’t you typically install Swift SDKs globally?
There was a problem hiding this comment.
Yeah, it would be typically an absolute path. But I think it's not harmful to interpret it as relative to project root only when it's a relative path. Note that AbsolutePath(validating: relativeTo:) uses the first argument as is if it's absolute.
There was a problem hiding this comment.
The thing I’m wondering is whether interpreting the path relative to the project root is the right way to go. You could have a global SourceKit-LSP configuration file at ~/.sourcekit-lsp/config.json and it wouldn’t be unreasonable to expect that the Swift SDK path is interpreted relative to that configuration file. If we don’t allow relative paths, we don’t need to worry about any ambiguity here.
I think for the scratch directory it’s natural to assume that it’s relative to the project root because having a global scratch directory for all project makes very little sense, so making that path relative to the project root makes perfect sense, I think.
There was a problem hiding this comment.
Ahh, that makes sense. Thank you for your explanation, I reverted swiftSDKsDirectory change.
Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift
Outdated
Show resolved
Hide resolved
kateinoigakukun
force-pushed
the
katei/interpret-relative-scratch-dir
branch
from
8edd50c to
70618ca
Compare
kateinoigakukun
changed the title
scratchPath and swiftSDKsDirectory to be relative pathsscratchPath to be relative path
kateinoigakukun
changed the title
scratchPath to be relative pathscratchPath to be a relative path
|
@swift-ci Please test |
Interpret it as relative to the project root directory if it's a relative path.
kateinoigakukun
force-pushed
the
katei/interpret-relative-scratch-dir
branch
from
70618ca to
184fa12
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
swiftlang#1824 and swiftlang#1832 raced.
Interpret it as relative to the project root directory if it's a relative path.