-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Build] allow to generate symbols for a subset of binaries #37120
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
Conversation
c3b5d73
to
dc76660
Compare
b040369
to
f2b31c3
Compare
@swift-ci please smoke test |
@swift-ci Please Build Toolchain |
@swift-ci please smoke test macOS |
Linux Toolchain (Ubuntu 16.04) Install command |
@swift-ci please smoke test macOS |
@swift-ci Please Build Toolchain macOS |
macOS Toolchain Install command |
@swift-ci please smoke test macOS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far... can you make the changes/look into my questions. Then I want to do another quick go over/read the tests in a deeper way. Ping me to do another go around and assuming no further issues and the tests from first principles test everything we need, this will LGTM.
validation-test/BuildSystem/extractsymbols-no-file-to-process.test
Outdated
Show resolved
Hide resolved
validation-test/BuildSystem/extractsymbols-darwin-symroot-path-filters.test
Outdated
Show resolved
Hide resolved
validation-test/BuildSystem/extractsymbols-default-behaviour.test
Outdated
Show resolved
Hide resolved
validation-test/BuildSystem/extractsymbols-darwin-symroot-path-filters.test
Outdated
Show resolved
Hide resolved
f2b31c3
to
d3fb1c7
Compare
@swift-ci please smoke test |
@swift-ci Please Build Toolchain |
Linux Toolchain (Ubuntu 16.04) Install command |
macOS Toolchain Install command |
A quick check of the log shows that python files and compatibility libraries are copied to the symroot
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Can you address the comments about adding more context to the tests and then feel free to merge without coming back for further review!
Thank you for your work here!
validation-test/BuildSystem/extractsymbols-no-file-to-process.test
Outdated
Show resolved
Hide resolved
validation-test/BuildSystem/extractsymbols-darwin-symroot-path-filters.test
Show resolved
Hide resolved
...before adding the logic to filter paths In particular: * print the list of files that `cpio` copies, so we can test explicitly which files end up in the symroot (and also see those when asking for a toolchain in PR testing) * use `find` instead of `grep` to filter files we want symbols generated for -- this is to avoid the script failing when there are no symbol to process, especially in lit tests * remove an unnecessary check for `swift-api-digester` -- this is now a symlink to `swift-frontend` and we only process regular files. Supports rdar://76865276
This would be needed to reduce overall build times in scenarios when generating symbols for all binaries is too expensive and/or not needed. Addresses rdar://76865276
d3fb1c7
to
1e5f790
Compare
@swift-ci please smoke test |
@swift-ci Please Build Toolchain |
Linux Toolchain (Ubuntu 16.04) Install command |
macOS Toolchain Install command |
This was introduced to avoid indentation changes when making this logic amenable to testing in swiftlang#37120 .
…wiftlang#37428) This was introduced to avoid indentation changes when making this logic amenable to testing in swiftlang#37120 .
This would be needed to reduce overall build times in scenarios when
generating symbols for all binaries is too expensive and/or not needed.
At the same time, introduce tests around the logic that handles symbols.
Addresses rdar://76865276