Remove use of AbsolutePath.appending(RelativePath(_:))
#4237
Conversation
This replaces the use of `AbsolutePath.appending(RelativePath(_:))` in the code base in favour of `AbsolutPath(_:relativeTo:)`. This form allows us to compute the absolute path prior to the normalization of the path which ensures that we can validate the path properly. This is particular important to enable the use of relative paths on Windows which cannot normalize at the parent path without a reference as there may be substitutions which need to be applied.
compnerd
requested review from
abertelrud,
neonichu,
tomerd and
elsh
as code owners
|
@swift-ci please smoke test |
|
cc @tomerd Without this patch, C targets with complex header requirements do not build as their header search path settings may not be able to look in directories outside the target's in the package. |
|
(Or any equivalent fix that prevents the stripping of '../'s from a header search path C setting.) |
When constructing a path relative to a root, compose the final path before canonicalization as the canonicalization may require lookups in the base location. We additionally pay a small penalty to convert the slashes to native separators. This is purely for aesthetic purposes, it ensures that the path is computed initially rather than delayed until use. This in conjunction with swiftlang/swift-package-manager#4237 enables the use of header search paths in SPM which are based in other targets.
|
thanks for this fix @compnerd. could we construct a few test cases to make sure this works correctly? eg Fixture based ones that test it end-to-end? @abertelrud and @neonichu wdyt? |
|
I think that the current tests cover this. This is effectively a change that has no impact on anything, simply goes through a different codepath to spell the same thing. On Windows, this path can then be changed to gain the proper behaviour (see swiftlang/swift-tools-support-core#298). |
|
@swift-ci please smoke test |
Add a test case for the handling of cross-target header lookups which would fail on Windows.
|
@swift-ci please smoke test |
|
thanks for adding a test. this seems fine to me. would like to get @abertelrud and @neonichu thoughts |
|
@swift-ci smoke test |
Sorry for the delay in responding here. Yes, this looks like the right fix. IIRC |
When constructing a path relative to a root, compose the final path before canonicalization as the canonicalization may require lookups in the base location. We additionally pay a small penalty to convert the slashes to native separators. This is purely for aesthetic purposes, it ensures that the path is computed initially rather than delayed until use. This in conjunction with swiftlang/swift-package-manager#4237 enables the use of header search paths in SPM which are based in other targets.
|
We should probably also remove/deprecate |
|
I think that is a good idea, that will be in TSC, I can try to get to that soon. |
This deprecates the `AbsolutePath.appending(_:)` overload for constructing an `AbsolutePath` by appending a `RelativePath`. Users should prefer `AbsolutePath(_:relativeTo:)`. Addresses feedback from swiftlang/swift-package-manager#4237
This deprecates the `AbsolutePath.appending(_:)` overload for constructing an `AbsolutePath` by appending a `RelativePath`. Users should prefer `AbsolutePath(_:relativeTo:)`. Addresses feedback from swiftlang/swift-package-manager#4237
When constructing a path relative to a root, compose the final path before canonicalization as the canonicalization may require lookups in the base location. We additionally pay a small penalty to convert the slashes to native separators. This is purely for aesthetic purposes, it ensures that the path is computed initially rather than delayed until use. This in conjunction with swiftlang/swift-package-manager#4237 enables the use of header search paths in SPM which are based in other targets.
This replaces the use of
AbsolutePath.appending(RelativePath(_:))inthe code base in favour of
AbsolutPath(_:relativeTo:). This formallows us to compute the absolute path prior to the normalization of the
path which ensures that we can validate the path properly. This is
particular important to enable the use of relative paths on Windows
which cannot normalize at the parent path without a reference as there
may be substitutions which need to be applied.
[One line description of your change]
Motivation:
[Explain here the context, and why you're making that change. What is the problem you're trying to solve.]
Modifications:
[Describe the modifications you've done.]
Result:
[After your change, what will change.]