-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[webkitapipy] Rework input files to recognize textual SPI and reduce configuration #47136
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
EWS run on previous version of this PR (hash 059bec5) |
059bec5
to
58ad39f
Compare
EWS run on previous version of this PR (hash 58ad39f) |
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.
Not much other than two nits. Appreciate your help explaining these changes in the context of the project as a whole :)
(Reviewing this has convinced me to add type hints in my future work!!)
|
||
binaries = parser.add_argument_group('framework and library dependencies', | ||
description='''ld-style arguments to | ||
support finding and allowing use of |
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.
super nit: Could this be "support finding and using binaries.." or "support finding and allow use of..."? "Support allowing use of" is a bit odd and hard to parse
local build products.''') | ||
|
||
binaries.add_argument('-framework', metavar='FRAMEWORK', type=str, | ||
action='append', dest='frameworks', |
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.
nit: For readability, it would be nice to have all the parameters in the same order/line. -framework
and -l
orders are similar but getting to -F
and -L
broke my brain a little bit
58ad39f
to
7f65461
Compare
EWS run on current version of this PR (hash 7f65461) |
…configuration https://bugs.webkit.org/show_bug.cgi?id=294760 rdar://153918728 Reviewed by Brianna Fan. A grab bag of changes to audit-spi's command line: - Instead of being invoked with one "primary file" to audit and other files to allow IPI (arbitrary) usage of, support multiple input files, and check them in one invocation. This works towards being able to exhaustively check an allowlist to ensure that allowed SPI is cleaned up once it is no longer used. - To indicate libraries for allowed IPI use, accept ld-style `-framework` and `-lx` argument to load frameworks and libraries on top of the base SDKDB, for allowed IPI use. For example, WebKit is allowed to use any symbol or declaration from JavaScriptCore and WebCore, so it is audited with `-framework WebCore -framework JavaScriptCore`. - Some ObjC interfaces are headers-only. Namely, they are part of a framework but do not have any binary representation in that framework, because they are intended for clients to implement. For example, WebKit has many delegate protocols that it never adopts locally, but will call methods on. These selectors are flagged as unknown SPI. Fix by reordering when audit-spi runs, so that it runs *after* a target builds and its InstallAPI postprocessing tasks have run. Add a flag to OTHER_TAPI_FLAGS which instructs tapi to dump a partial SDKDB of the framework's headers. Teach audit-spi to find these partial SDKDBs and ingest them. For example: WebKit now produces a `WebKit.partial.sdkdb` containing records of its headers-only declarations. After WebKit, WebKitSwift, and _WebKit_SwiftUI finish building, audit-spi runs and checks all three. It finds the partial SDKDB and adds it to the internal database. * Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig: Emit a partial SDKDB during InstallAPI. * Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj: Update audit-spi invocation and move it to the aggregate target that builds after JSC has finished. * Source/WTF/Scripts/audit-spi-if-needed.sh: Update audit-spi invocation. * Source/WebCore/WebCore.xcodeproj/project.pbxproj: Update audit-spi invocation. * Source/WebGPU/WebGPU.xcodeproj/project.pbxproj: Update audit-spi invocation. * Source/WebKit/Configurations/WebKit.xcconfig: * Source/WebKit/WebKit.xcodeproj/project.pbxproj: Update audit-spi invocation, and move it to the aggregate target than runs after WebKit, WebKitSwift, and _WebKit_SwiftUI have finished building. * Source/WebKitLegacy/WebKitLegacy.xcodeproj/project.pbxproj: Update audit-spi invocation. * Tools/Scripts/libraries/webkitapipy/webkitapipy/program.py: (Options): Added. Since there are a bunch of changes to the CLI, add a helper object which provides type hints for all the arguments we recognize. This makes future changes to the parser participate in type checking. (TSVReporter): Print file names in output when more than there's more than one input file. (get_parser): Add new arguments. * Tools/Scripts/libraries/webkitapipy/webkitapipy/program_unittest.py: Added. Canonical link: https://commits.webkit.org/296823@main
7f65461
to
5f96054
Compare
Committed 296823@main (5f96054): https://commits.webkit.org/296823@main Reviewed commits have been landed. Closing PR #47136 and removing active labels. |
5f96054
7f65461
🛠 win🧪 wpe-wk2🧪 win-tests🧪 api-mac🧪 ios-wk2-wpt🧪 mac-wk1🛠 🧪 jsc🧪 api-ios🧪 mac-AS-debug-wk2🧪 gtk-wk2🧪 api-gtk🧪 mac-intel-wk2🛠 playstation🧪 jsc-armv7-tests🛠 watch-sim