Skip to content

[5.9] Improve handling of tool build flags #6709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Jul 14, 2023

Cherry-pick of #6475.

Refactors the various compilerArguments and linkerArgument functions slightly to ensure flags from a user's sdk and cli properly propagate to the expected subcommand invocations.

Adds a fairly comprehensive test to cover the toolset and cli flag passing end result.


Fixes a bug where clang targets didn't respect the isCXX override.

Fixes a bug where cxxCompiler extraCLIFlags where not passed to clang when building cxx targets. Future work: #6491. cxxCompiler extraCLIFlags should also be passed to swiftc when compiling swift modules to ensure the clang importer has the same interpretation of the module as the initial compile.

Fixes a bug where linker extraCLIFlags were not passed to swiftc prefixed by -Xlinker. The previous workaround to pass these flags in swiftCompiler extraCLIFlags is no longer necessary.

Fixes a bug where a user could pass -whole-module-optimization or -wmo to swiftc when linking causing the driver to either attempt to compile the object files when using the legacy driver: rdar://27578292 or crash when using the new driver: rdar://108057797.

Fixes a bug where cCompiler extraCLIFlags were not passed to swiftc prefixed by -Xcc. The previous workaround to pass these flags in swiftCompiler extraCLIFlags is no longer necessary.

Fixes a bug where extraCLIFlags from an SDKs toolset.json files would be repeated multiple times in child process invocations. The repeated flags could cause issues with some downstream tools, such as repeated -segaddr flags to ld64 for the same segment.

Fixes a bug where extra toolchain flags would be added to an xcode build parameters file without escaping.

Fixes a bug where extra toolchain linker flags would not be added to an xcode build parameters file.

(cherry picked from commit 151343e)

# Conflicts:
#	Sources/Build/BuildPlan.swift
#	Tests/BuildTests/MockBuildTestHelper.swift

Refactors the various compilerArguments and linkerArgument functions
slightly to ensure flags from a user's sdk and cli properly propagate to
the expected subcommand invocations.

Adds a fairly comprehensive test to cover the toolset and cli flag
passing end result.

---

Fixes a bug where clang targets didn't respect the `isCXX` override.

Fixes a bug where cxxCompiler extraCLIFlags where not passed to clang
when building cxx targets. Future work: #6491. cxxCompiler extraCLIFlags
should also be passed to `swiftc` when compiling swift modules to ensure
the clang importer has the same interpretation of the module as the
initial compile.

Fixes a bug where linker extraCLIFlags were not passed to `swiftc`
prefixed by `-Xlinker`. The previous workaround to pass these flags in
swiftCompiler extraCLIFlags is no longer necessary.

Fixes a bug where a user could pass `-whole-module-optimization` or
`-wmo` to swiftc when linking causing the driver to either attempt to
compile the object files when using the legacy driver: rdar://27578292
or crash when using the new driver: rdar://108057797.

Fixes a bug where cCompiler extraCLIFlags were not passed to `swiftc`
prefixed by `-Xcc`. The previous workaround to pass these flags in
swiftCompiler extraCLIFlags is no longer necessary.

Fixes a bug where extraCLIFlags from an SDKs toolset.json files would
be repeated multiple times in child process invocations. The repeated
flags could cause issues with some downstream tools, such as repeated
`-segaddr` flags to ld64 for the same segment.

Fixes a bug where extra toolchain flags would be added to an xcode build
parameters file without escaping.

Fixes a bug where extra toolchain linker flags would not be added to an
xcode build parameters file.

(cherry picked from commit 151343e)

# Conflicts:
#	Sources/Build/BuildPlan.swift
#	Tests/BuildTests/MockBuildTestHelper.swift
@MaxDesiatov MaxDesiatov added swift 5.9 This PR targets the 5.9 branch cross-compilation labels Jul 14, 2023
@MaxDesiatov MaxDesiatov requested a review from rauhul July 14, 2023 15:34
@MaxDesiatov MaxDesiatov requested a review from abertelrud as a code owner July 14, 2023 15:34
@MaxDesiatov MaxDesiatov self-assigned this Jul 14, 2023
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@tomerd
Copy link
Contributor

tomerd commented Jul 14, 2023

👍 once CI is green

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test windows

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test windows

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) July 18, 2023 19:16
@MaxDesiatov MaxDesiatov disabled auto-merge July 18, 2023 19:16
@MaxDesiatov MaxDesiatov enabled auto-merge (squash) July 18, 2023 19:16
@MaxDesiatov MaxDesiatov disabled auto-merge July 18, 2023 19:17
@MaxDesiatov MaxDesiatov enabled auto-merge (squash) July 18, 2023 20:12
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test macos

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test windows

MaxDesiatov added a commit to swiftlang/sourcekit-lsp that referenced this pull request Jul 19, 2023
Cherry-pick of #742 required to unblock swiftlang/swift-package-manager#6709.

Updates tests to allow compile arguments with multiple `-F` options such as `-F <foo> -Xcc -F -Xcc <foo>`. This change is required to support SwiftPM changes which forward C compiler flags to the Swift compiler in more cases.

Co-authored-by: Rauhul Varma <rauhul@users.noreply.github.com>
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test macos

@MaxDesiatov MaxDesiatov merged commit cd3f8f3 into release/5.9 Jul 19, 2023
@MaxDesiatov MaxDesiatov deleted the maxd/5.9-build-flags branch July 19, 2023 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-compilation swift 5.9 This PR targets the 5.9 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants