Skip to content

[CMake] Create tool header collector target only when building tools #70877

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 12, 2024
Merged

[CMake] Create tool header collector target only when building tools #70877

merged 2 commits into from
Jan 12, 2024

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Jan 12, 2024 •
edited by tshortli
Loading

HeaderDependencies.cpp cannot be compiled with SWIFT_INCLUDE_TOOLS=OFF because it depends on Config.h which is only generated while building tools. Exclude it from the build to fix all target.

Follow-up to b39dba3

Hopefully this fixes https://ci.swift.org/job/oss-swift-test-stdlib-with-toolchain/7124

In file included from /Users/ec2-user/jenkins/workspace/oss-swift-test-stdlib-with-toolchain/build/stdlib_RA_standalone/swift-macosx-x86_64/include/HeaderDependencies.cpp:6:
In file included from /Users/ec2-user/jenkins/workspace/oss-swift-test-stdlib-with-toolchain/swift/include/swift/AST/ASTBridging.h:26:
In file included from /Users/ec2-user/jenkins/workspace/oss-swift-test-stdlib-with-toolchain/swift/include/swift/AST/DiagnosticEngine.h:21:
In file included from /Users/ec2-user/jenkins/workspace/oss-swift-test-stdlib-with-toolchain/swift/include/swift/AST/ActorIsolation.h:19:
In file included from /Users/ec2-user/jenkins/workspace/oss-swift-test-stdlib-with-toolchain/swift/include/swift/AST/Type.h:23:
In file included from /Users/ec2-user/jenkins/workspace/oss-swift-test-stdlib-with-toolchain/swift/include/swift/AST/LayoutConstraint.h:22:
In file included from /Users/ec2-user/jenkins/workspace/oss-swift-test-stdlib-with-toolchain/swift/include/swift/AST/PrintOptions.h:17:
/Users/ec2-user/jenkins/workspace/oss-swift-test-stdlib-with-toolchain/swift/include/swift/AST/AttrKind.h:22:10: fatal error: 'swift/Config.h' file not found
#include "swift/Config.h"
         ^~~~~~~~~~~~~~~~
1 error generated.

Resolves rdar://120907760

@kateinoigakukun
Copy link
Member Author

@swift-ci Please test stdlib with toolchain

@kateinoigakukun
Copy link
Member Author

Hmm, it seems like we need to update Jenkins pipeline configuration of swift-PR-stdlib-with-toolchain-osx to include cmake in PATH as other pipelines do. CC @shahmishal

+ /Users/ec2-user/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/swift/utils/build-script --preset=stdlib_RA_standalone,build,test toolchain_path=/Users/ec2-user/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/latest_toolchain/usr/bin/
[/Users/ec2-user/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/swift/utils/build-script] NOTE: using preset "stdlib_RA_standalone,build,test", which expands to 

/Users/ec2-user/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/swift/utils/build-script --skip-build-llvm --skip-build-cmark --skip-build-benchmarks --skip-test-cmark --build-swift-tools=0 --build-swift-libexec=0 --skip-early-swift-driver --skip-early-swiftsyntax --native-swift-tools-path=/Users/ec2-user/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/latest_toolchain/usr/bin/ --native-llvm-tools-path=/Users/ec2-user/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/latest_toolchain/usr/bin/ --native-clang-tools-path=/Users/ec2-user/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/latest_toolchain/usr/bin/ --build-ninja '--lit-args=--filter='"'"':: (stdlib/|Concurrency/)'"'"' -v' --only-executable-test --build-subdir=stdlib_RA_standalone --release --assertions --verbose-build --test --validation-test

[/Users/ec2-user/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/swift/utils/build-script] NOTE: Using toolchain default
+ /usr/libexec/PlistBuddy -c 'Print :SupportedTargets:macosx:Archs' /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/SDKSettings.plist
[/Users/ec2-user/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/swift/utils/build-script] ERROR: can't find CMake (please install CMake)

https://ci.swift.org/job/swift-PR-stdlib-with-toolchain-osx/8/console

@rintaro
Copy link
Member

rintaro commented Jan 12, 2024 •
edited
Loading

Huh, sorry I didn't notice. Maybe we should just move this to lib/CMakeLists.txt

@kateinoigakukun
[CMake] Create tool header collector target only when building tools
3feb5ae 
`HeaderDependencies.cpp` cannot be compiled with
`SWIFT_INCLUDE_TOOLS=OFF` because it depends on `Config.h` which is only
generated while building tools. Exclude it from the build to fix `all`
target.

Follow-up to b39dba3
@kateinoigakukun kateinoigakukun force-pushed the pr-881a2ba91d1f6a7bc84606dea6aa0d069c0793ac branch from 4ff5e8f to 3feb5ae Compare January 12, 2024 17:14
@kateinoigakukun
Copy link
Member Author

Okay, putting it in lib/CMakeLists.tx sounds reasonable to me 👍

@kateinoigakukun
Copy link
Member Author

@swift-ci Please smoke test

rintaro
rintaro reviewed Jan 12, 2024
View reviewed changes
@kateinoigakukun
[CMake] Cleanup TableGen dependency handling
960086c 
Clang and LLVM always provide those tablegen targets, so we don't need
to check their existence. Also adding `intrinsic_gen` as a dependency.
See swiftlang#12147
@kateinoigakukun kateinoigakukun force-pushed the pr-881a2ba91d1f6a7bc84606dea6aa0d069c0793ac branch from 3a64c63 to 960086c Compare January 12, 2024 17:55
@kateinoigakukun
Copy link
Member Author

@swift-ci Please smoke test

@tshortli tshortli enabled auto-merge January 12, 2024 18:13
@tshortli
Copy link
Contributor

Thanks for the fix! I'm enabling auto-merge since this fixes some CI failures and I'd like it to get in quickly.

@tshortli tshortli merged commit 0bbcae4 into swiftlang:main Jan 12, 2024
@kateinoigakukun kateinoigakukun deleted the pr-881a2ba91d1f6a7bc84606dea6aa0d069c0793ac branch January 17, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rintaro rintaro rintaro approved these changes

@tshortli tshortli tshortli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kateinoigakukun @rintaro @tshortli