Skip to content

DependenciesScanner: report command-line arguments for building pcms explicitly #31922

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 4 commits into from
May 23, 2020

Conversation

nkcsgexi
Copy link
Contributor

@nkcsgexi nkcsgexi commented May 20, 2020

Clang PCM dependencies are currently implicitly built by the compiler. To allow build-system level to build these PCMs explicitly, we need to report the command line arguments from the compiler to the build system via a JSON file emitted from the dependencies scanner.

This PR combines and emits two sets of Clang compiler arguments: (1) arguments Swift used to construct ClangImporter instance, and (2) arguments collected from running Clang's fast dependency scanner. A high level build system can explicitly schedule PCM building by feeding these arguments to a Swift front-end invocation with an action kind -emit-pcm.

For testing purposes, a Swift script is also added to build modules explicitly by using these arguments from the JSON file.

rdar://62612967
rdar://62613017

@nkcsgexi nkcsgexi requested a review from Bigcheese May 20, 2020 19:38
@nkcsgexi nkcsgexi force-pushed the report-pcm-command branch from 5e14814 to 7f5e1d7 Compare May 20, 2020 19:49
Copy link
Contributor

@Bigcheese Bigcheese left a comment

Choose a reason for hiding this comment

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

This also needs to handle the other stuff that getFullCommandLine does for -fmodule-map-file= and -fmodule-file=.

@Bigcheese
Copy link
Contributor

This also needs to handle the other stuff that getFullCommandLine does for -fmodule-map-file= and -fmodule-file=.

Actually, I guess this would be handled at a higher level.

@nkcsgexi nkcsgexi force-pushed the report-pcm-command branch from 7f5e1d7 to adc4660 Compare May 20, 2020 21:36
@nkcsgexi
Copy link
Contributor Author

Yeah, I think -fmodule-map-file= and -fmodule-file= are directly handled by Swift in emitPrecompiledModule function.

@nkcsgexi nkcsgexi force-pushed the report-pcm-command branch 6 times, most recently from a784a97 to 8938095 Compare May 21, 2020 22:16
@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

@nkcsgexi
Copy link
Contributor Author

@swift-ci Please smoke test OS X platform

@nkcsgexi
Copy link
Contributor Author

@swift-ci Please smoke test OS X platform

@nkcsgexi
Copy link
Contributor Author

@swift-ci Please smoke test OS X platform

@nkcsgexi nkcsgexi force-pushed the report-pcm-command branch from 8938095 to 5e47c58 Compare May 22, 2020 00:22
@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

@nkcsgexi nkcsgexi force-pushed the report-pcm-command branch from 5e47c58 to 854d86f Compare May 22, 2020 01:31
@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

@nkcsgexi nkcsgexi force-pushed the report-pcm-command branch from 854d86f to b49d9d8 Compare May 22, 2020 05:10
@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

@nkcsgexi nkcsgexi force-pushed the report-pcm-command branch from b49d9d8 to 4bf3a46 Compare May 22, 2020 05:31
@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

@nkcsgexi
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 7f5e1d78156bf8172b0dcb4994db68363172f014

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 7f5e1d78156bf8172b0dcb4994db68363172f014

@nkcsgexi nkcsgexi changed the title [WIP]DependenciesScanner: report command-line arguments for building pcm explicitly DependenciesScanner: report command-line arguments for building pcms explicitly May 22, 2020
@nkcsgexi nkcsgexi force-pushed the report-pcm-command branch from 4bf3a46 to cb3f278 Compare May 22, 2020 14:50
@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

@nkcsgexi nkcsgexi requested review from DougGregor and artemcm May 22, 2020 16:26
@nkcsgexi
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 4bf3a464debf6dd99eadd5b7acdfa2463597695c

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 4bf3a464debf6dd99eadd5b7acdfa2463597695c

@DougGregor
Copy link
Member

rdar://62612967
rdar://62613017

Please replace this with a full description of which this pull request does. The radar numbers are good for automation, and can be at the end, but the pull request description should be understandable to anyone who cannot follow that link.

@nkcsgexi
Copy link
Contributor Author

rdar://62612967
rdar://62613017

Please replace this with a full description of which this pull request does. The radar numbers are good for automation, and can be at the end, but the pull request description should be understandable to anyone who cannot follow that link.

Good point. I've updated the PR description.

…t instead from Swift

Running shell commands using Swift in simulator tests is hard. We change the mechanism so that
BuildModulesFromGraph.swift prints command line arguments and a simple python script
picks these arguments and actually runs the command.
@nkcsgexi nkcsgexi force-pushed the report-pcm-command branch from f001a00 to 2202398 Compare May 22, 2020 19:30
@nkcsgexi
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - cb3f278

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - cb3f278

@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

@nkcsgexi nkcsgexi merged commit e2b554d into swiftlang:master May 23, 2020
@nkcsgexi nkcsgexi deleted the report-pcm-command branch May 23, 2020 03:12
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.

4 participants