Skip to content
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

Fix package access modifier in XCBuild support #7258

Merged
merged 37 commits into from
Feb 24, 2024

Conversation

MaxDesiatov
Copy link
Member

package access modifier was previously not supported in swift build --build-system xcode. This causes build issues when attempting to produce Universal binaries for SwiftPM, for example in this job https://ci.swift.org/job/swift-PR-source-compat-suite-debug-macos/1297/consoleFull.

The reason was that -package-name option was not added to OTHER_SWIFT_FLAGS in PIFBuilder.swift. Additionally, in llbuild support code we were shelling out to Swift Driver for every target to check whether -package-name is supported. Now with -package-name options calculation generalized across build systems, Swift Driver checks are done once per BuildParameters initialization, which reduces excessive shelling for llbuild.

Resolves rdar://120925895.

`package` access modifier was previously not supported in `swift build --build-system xcode`. This causes build issues when attempting to produce Universal binaries for SwiftPM, for example in this job https://ci.swift.org/job/swift-PR-source-compat-suite-debug-macos/1297/consoleFull.

The reason was that `-package-name` option was not added to `OTHER_SWIFT_FLAGS` in `PIFBuilder.swift`. Additionally, in llbuild support code we were shelling out to Swift Driver for every target to check whether `-package-name` is supported. Now with `-package-name` options calculation generalized across build systems, Swift Driver checks are done once per `BuildParameters` initialization, which reduces excessive shelling for llbuild.

Resolves rdar://120925895.
@MaxDesiatov MaxDesiatov added bug build system Changes to interactions with build systems labels Jan 16, 2024
@MaxDesiatov MaxDesiatov self-assigned this Jan 16, 2024
@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) January 16, 2024 16:01
@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

@neonichu
Copy link
Member

Additionally, in llbuild support code we were shelling out to Swift Driver for every target to check whether -package-name is supported.

I don't think that's true since #7057? There's a static cache of computed flags, isn't there?

@MaxDesiatov
Copy link
Member Author

I don't think that's true since #7057? There's a static cache of computed flags, isn't there?

Fair enough. But we'd have to migrate away from ThreadSafeBox anyway for Swift Concurrency. It feels better to add caching outside of model types. Here we don't have to hit the cache and ThreadSafeBox with locks at all, as the computation is in a stored property of BuildParameters.

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

MaxDesiatov added a commit that referenced this pull request Feb 22, 2024
XCBuildSupport is not available on non-Darwin platforms, especially Windows, where we have to have full CMake support at the moment. Maintaining XCBuildSupport in these unsupported configurations adds unnecessary overhead, especially in cases like #7258, where we have to add new dependencies only when `XCBuildSupport` is available. We should exclude from CMake builds to reduce this maintenance overhead.
MaxDesiatov added a commit that referenced this pull request Feb 23, 2024
XCBuildSupport is not available on non-Darwin platforms, especially
Windows, where we have to have full CMake support at the moment.
Maintaining XCBuildSupport in these unsupported configurations adds
unnecessary overhead, especially in cases like
#7258, where we have
to add new dependencies only when `XCBuildSupport` is available. We
should exclude from CMake builds to reduce this maintenance overhead.
@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

1 similar comment
@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

@MaxDesiatov MaxDesiatov merged commit 31bc808 into main Feb 24, 2024
5 checks passed
@MaxDesiatov MaxDesiatov deleted the maxd/fix-xcbuild-package-name branch February 24, 2024 01:17
bnbarham added a commit to bnbarham/swift-package-manager that referenced this pull request Feb 27, 2024
This was added to the `Package.swift` in apple#7258 but skipped in the CMake
because apple#7358 excluded XCBuildSupport as a CMake target. But that's
reverted here, so we need to add the dependency in.
MaxDesiatov pushed a commit that referenced this pull request Feb 27, 2024
…7371)

This reverts commit 5287062, which
causes macOS toolchain builds to fail with:
```
main/main.swift:345: Fatal error: SwiftPM was built without XCBuild support
```

Also adds the DriverSupport dependency to XCBuildSupport, which was
skipped in #7258 because #7358 was merged (but we're reverting that).
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
XCBuildSupport is not available on non-Darwin platforms, especially
Windows, where we have to have full CMake support at the moment.
Maintaining XCBuildSupport in these unsupported configurations adds
unnecessary overhead, especially in cases like
apple#7258, where we have
to add new dependencies only when `XCBuildSupport` is available. We
should exclude from CMake builds to reduce this maintenance overhead.
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
`package` access modifier was previously not supported in `swift build
--build-system xcode`. This causes build issues when attempting to
produce Universal binaries for SwiftPM, for example in this job
https://ci.swift.org/job/swift-PR-source-compat-suite-debug-macos/1297/consoleFull.

The reason was that `-package-name` option was not added to
`OTHER_SWIFT_FLAGS` in `PIFBuilder.swift`. Additionally, in llbuild
support code we were shelling out to Swift Driver for every target to
check whether `-package-name` is supported. Now with `-package-name`
options calculation generalized across build systems, Swift Driver
checks are done once per `BuildParameters` initialization, which reduces
excessive shelling for llbuild.

Resolves rdar://120925895.
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
…" (apple#7371)

This reverts commit 5287062, which
causes macOS toolchain builds to fail with:
```
main/main.swift:345: Fatal error: SwiftPM was built without XCBuild support
```

Also adds the DriverSupport dependency to XCBuildSupport, which was
skipped in apple#7258 because apple#7358 was merged (but we're reverting that).
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
XCBuildSupport is not available on non-Darwin platforms, especially
Windows, where we have to have full CMake support at the moment.
Maintaining XCBuildSupport in these unsupported configurations adds
unnecessary overhead, especially in cases like
apple#7258, where we have
to add new dependencies only when `XCBuildSupport` is available. We
should exclude from CMake builds to reduce this maintenance overhead.
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
`package` access modifier was previously not supported in `swift build
--build-system xcode`. This causes build issues when attempting to
produce Universal binaries for SwiftPM, for example in this job
https://ci.swift.org/job/swift-PR-source-compat-suite-debug-macos/1297/consoleFull.

The reason was that `-package-name` option was not added to
`OTHER_SWIFT_FLAGS` in `PIFBuilder.swift`. Additionally, in llbuild
support code we were shelling out to Swift Driver for every target to
check whether `-package-name` is supported. Now with `-package-name`
options calculation generalized across build systems, Swift Driver
checks are done once per `BuildParameters` initialization, which reduces
excessive shelling for llbuild.

Resolves rdar://120925895.
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
…" (apple#7371)

This reverts commit 5287062, which
causes macOS toolchain builds to fail with:
```
main/main.swift:345: Fatal error: SwiftPM was built without XCBuild support
```

Also adds the DriverSupport dependency to XCBuildSupport, which was
skipped in apple#7258 because apple#7358 was merged (but we're reverting that).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug build system Changes to interactions with build systems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants