PackageModel: add struct BuildFlags, reduce reliance on TSC
#5837
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
MaxDesiatov
requested review from
abertelrud,
neonichu,
tomerd and
elsh
as code owners
This comment was marked as outdated.
This comment was marked as outdated.
MaxDesiatov
force-pushed
the
maxd/build-flags
branch
from
23b792f to
542f0ea
Compare
|
@swift-ci please smoke test |
tomerd
approved these changes
Oct 24, 2022
|
swiftlang/sourcekit-lsp#659 |
neonichu
approved these changes
Oct 25, 2022
MaxDesiatov
added a commit
to swiftlang/swift-tools-support-core
that referenced
this pull request
Oct 25, 2022
MaxDesiatov
added a commit
that referenced
this pull request
Nov 15, 2022
Currently, `destination.json` files only support absolute paths, which makes it hard to move cross-compilation sysroots between directories and copying them to different machines. This change support for relative paths in v2 scheme of `destination.json` files. The path is then normalized relative to the location of `destination.json` file. `struct DestinationInfo` is renamed to `DestinationInfoV1`. New `struct DestinationInfoV2` is 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 use `camelCase` instead of `dash-case` naming. Also new in schema version 2 are `linkerFlags` and renaming of `extraCPPFlags` to `extraCXXFlags`, to follow up on #5827 and #5837.
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.
Motivation:
We're relying on
BuildFlagsfrom TSC for passing flags in some places, but there are also cases where flags are passed as separate properties, such asextraCFlags,extraCXXFlagsetc.BuildFlagsis quite useful, as it allows grouping of these properties instead of passing them around separately. In most if not all cases they need to be passed all at once anyway. This apparently already let to issues, whereDestinationdidn't have its own linker flags property, also see #5793. We also had issues with inconsistent naming of these properties. Unifying everything underBuildFlagscould prevent such issues from happening.Modifications:
Adding
BuildFlagstoPackageModel, mostly a copy of the declaration in TSC, but with a cleaned up initializer arguments naming. Modified uses of build tools flags to use this new struct instead of passing arounds properties separately.Result:
Reduced reliance on TSC, unified use of
BuildFlagsacross the codebase.