Skip to content

[cmake] Move a bunch of test binaries from install component 'tools' -> 'testsuite-tools'. #40077

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

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Nov 6, 2021

All of these are tools that are only meant to be used when testing swift. Thus
it doesn't make sense to include them in the catch all 'tools' install component
that distributions use to build all the tools.

I verified that all of the mac/linux presets in tree that have tools also has
testsuite-tools so this should be NFC. On Windows, I needed to add
testsuite-tools to build-windows.bat so should be NFC there as well.

@gottesmm gottesmm requested a review from compnerd November 6, 2021 03:41
@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 6, 2021

@swift-ci test

@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 6, 2021

@swift-ci build toolchain

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This definitely should help improve the CI space usage on Windows.

@swift-ci
Copy link
Contributor

swift-ci commented Nov 6, 2021

Linux Toolchain (Ubuntu 16.04)
Download Toolchain
Git Sha - df5f9f5d20844561e433f5b2adb29a4f0d49eb1a

Install command
tar zxf swift-PR-40077-727-ubuntu16.04.tar.gz
More info

@swift-ci
Copy link
Contributor

swift-ci commented Nov 6, 2021

macOS Toolchain
Download Toolchain
Git Sha - df5f9f5d20844561e433f5b2adb29a4f0d49eb1a

Install command
tar -zxf swift-PR-40077-1211-osx.tar.gz --directory ~/

@compnerd
Copy link
Member

compnerd commented Nov 7, 2021

Some measurements - this reduced the build artifacts from 3.9G to 3.5G.

@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 8, 2021

@compnerd just a FYI, I am checking with some people to make sure my guesstimates on what we can remove here is correct.

@compnerd
Copy link
Member

compnerd commented Nov 8, 2021

@gottesmm - sounds good; I would be more clear though - it isn't being removed, only being given additional control when building with the LLVM distribution mechanism. If you use a standalone Swift build, this will not have any impact.

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for swift-ide-test, swift-syntax-parser-test and swift-syntax-test.

…-> 'testsuite-tools'.

All of these are tools that are only meant to be used when testing swift. Thus
it doesn't make sense to include them in the catch all 'tools' install component
that distributions use to build all the tools.

I verified that all of the mac/linux presets in tree that have tools also has
testsuite-tools so this should be NFC. On Windows, I needed to add
testsuite-tools to build-windows.bat so should be NFC there as well.
@gottesmm gottesmm force-pushed the pr-2ea314186d00544a71b7886ab6ef6743b4b560ab branch from df5f9f5 to 06cb5c0 Compare November 9, 2021 20:52
@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 9, 2021

I removed swift-ide-test since I think some people depend on it. I think I am going to include it in a different category though.

@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 9, 2021

@swift-ci smoke test

@gottesmm gottesmm merged commit b02dc65 into swiftlang:main Nov 10, 2021
@gottesmm gottesmm deleted the pr-2ea314186d00544a71b7886ab6ef6743b4b560ab branch November 10, 2021 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants