PackageModel: relative paths for cross-compilation destinations #5812
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
neonichu
reviewed
Oct 14, 2022
| if isRelative { | ||
| return directory.appending(RelativePath(string)) | ||
| } else { | ||
| return AbsolutePath(string) |
There was a problem hiding this comment.
This should use AbsolutePath(validating:)
MaxDesiatov
force-pushed
the
maxd/destination-relative-path
branch
from
103ed52 to
d678992
Compare
stevapple
reviewed
Oct 15, 2022
|
This PR is still a draft, there highly likely will be incompatible changes in future commits. |
MaxDesiatov
force-pushed
the
maxd/destination-relative-path
branch
from
d678992 to
b5ae29e
Compare
tomerd
reviewed
Oct 24, 2022
| // Check schema version. | ||
| guard version.version == 1 else { | ||
| if version.version == 1 { |
tomerd
reviewed
Oct 24, 2022
| return directory.appending(RelativePath(string)) | ||
| } else { | ||
| return try AbsolutePath(validating: string) | ||
| } |
There was a problem hiding this comment.
iirc the absolute path constructor already takes care of it? @neonichu ?
There was a problem hiding this comment.
Yah, init(validating:, relativeTo:) should do the same thing, I believe
MaxDesiatov
force-pushed
the
maxd/destination-relative-path
branch
2 times, most recently
from
b42b666 to
29f73b4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
MaxDesiatov
force-pushed
the
maxd/destination-relative-path
branch
from
c5ad2a6 to
ae196db
Compare
|
@swift-ci please smoke test |
MaxDesiatov
force-pushed
the
maxd/destination-relative-path
branch
from
ae196db to
07efc00
Compare
MaxDesiatov
force-pushed
the
maxd/destination-relative-path
branch
from
f9da45f to
8e3c204
Compare
|
@swift-ci please test |
neonichu
approved these changes
Nov 3, 2022
This introduces support for relative paths in v2 scheme of `destination.json` files. The path is then normalized relative to the location of `destination.json` files. As a result, it makes it easier for moving cross-compilation sysroots between different directories on the same filesystem, or even between different machines.
MaxDesiatov
force-pushed
the
maxd/destination-relative-path
branch
from
02c9ff2 to
b997076
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
|
@MaxDesiatov is this ready to be merged? |
|
After double-checking, it seems both PRs don't overlap on specific lines, although |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #5871.
This introduces support for relative paths in v2 scheme of
destination.jsonfiles. The path is then normalized relative to the location ofdestination.jsonfile.Motivation:
Currently,
destination.jsonfiles only support absolute paths, which makes it hard to move cross-compilation sysroots between directories and copying them to different machines.Modifications:
struct DestinationInfois renamed toDestinationInfoV1. Newstruct DestinationInfoV2is added to handle the new version that supports relative paths. This is also used as an opportunity to make the schema more consistent with manifests for artifact bundles, in which properties usecamelCaseinstead ofdash-casenaming. Also new in schema version 2 arelinkerFlagsand renaming ofextraCPPFlagstoextraCXXFlags, to follow up on #5827 and #5837.Added tests to verify that both schema versions are parsed correctly and that relative paths in V2 are correctly expanded to absolute paths.
Result:
New version of
destination.jsonfiles can be supplied as a--destinationoption passed toswift build.As a result, it's to move cross-compilation sysroots and their corresponding
destination.jsonfiles between different directories on the same filesystem, or even between different machines.