Virtual hermetic frameworks #277
Conversation
jerrymarino
force-pushed
the
jmarino/starlark_virtual_frameworks_rebased
branch
from
e20d7d4
to
e9dc1b0
Compare
There was a problem hiding this comment.
This is awesome....
Here is my TODOs for this PR:
- Checkout how it impacts building of rules_ios projects before and after via studying the output in bazel-out
- DO the same for downstream project such as our own.
Also we need to consider at least some tests to safeguard this? At least some kind of scripts to verify the output of json files?
Lastly I would assign to a contributor outside Square to confirm it makes sense to them, unless we use the feature flag to guard everything.
rules/framework.bzl
Outdated
| import_vfsoverlays.append(dep[VFSOverlayInfo].vfs_info) | ||
|
|
||
| # Progated interface headers - this must encompass all of them | ||
| propagated_interface_headers = [] |
There was a problem hiding this comment.
this is an array of depset correct?
There was a problem hiding this comment.
Yep, you'll find where it's created downwind of this line https://github.com/bazel-ios/rules_ios/pull/277/files/2c7b4ba65c40c684f4f8e7efbed35c61e84ca582#diff-b5ab983bf150dfa86b7e361224e1c9b0b9856b0b57b5893dbc088c6ca6d8ba52R397
rules/framework.bzl
Outdated
| if virtualize_frameworks: | ||
| import_vfsoverlays = [] | ||
| for dep in ctx.attr.vfs: | ||
| if not VFSOverlayInfo in dep: |
There was a problem hiding this comment.
what's the situation where a dep of attr.vfs is not a VFSOverlayInfo?
There was a problem hiding this comment.
You'll find this in the main callsite of framework_packing - there's a few edges around imported frameworks and we pass this in ATM
| @@ -0,0 +1,4 @@ | |||
| feature_names = struct( | |||
| # Virtualize means that swift,clang read from llvm's in-memory file system | |||
| virtualize_frameworks = "apple.virtualize_frameworks", | |||
There was a problem hiding this comment.
If there any reason why we treat it as a feature? Could we simply delete the old way to simplify the code?
There was a problem hiding this comment.
Yeah it's easier to iterate on code that's behind a feautre. This and we don't want to disrupt the old code path and may roll this out as an a/b test. Probably longer term we'd want to remove the useage of frameworks altogether
There was a problem hiding this comment.
( that is, no remove the "frameworks" as a feature )
|
Thanks again for all the comments on the PR - making another pass to cleanup all of your suggestions 🙏 🥳 @gyfelton regarding the tests, we might be able to come up with some sort of test cases, my plan is to run the existing examples with the feature on - we might need to spin up more .github actions. The feature shouldn't impact any other projects if they don't enable the feature - which is another benefit of making it a feature! |
Add the ability to virtualize the framework structure. swift and clang compile as frameworks but the files don't move and we load them via VFS. The VFS is llvm's in-memory file system declared by JSON. VFS is already in use but this uses it in a very different way. This fixes many issues: 1. we don't hit O(n) deps includes - removing thousands of framework search paths 2. we don't have to "clean" frameworks. This feature added quite a bit of overhead and isn't necesary since it's declarative 3. imports are fully again. traditionally a build system uses -F on a build directory. This causes problems with reproducible builds. It cut clean build time in half in a big hybrid app from rules_ios baseline. More interestingly, hot top level compiler invocations are down nearly .5 orders of magnitude. Remaining tasks: - address xcodeproj integration - duplicate test suite to run with this on/off - address xcframework imports ( right now we're using -F ) Relates to: #211
jerrymarino
force-pushed
the
jmarino/starlark_virtual_frameworks_rebased
branch
from
07c2a6a
to
3396e48
Compare
jerrymarino
force-pushed
the
jmarino/starlark_virtual_frameworks_rebased
branch
from
7b1d30a
to
d929aa6
Compare
This required adding a way to build ios_application deps as a framework. If this feature is on, the apps need to have it set since they don't build as a framework Note that the line removed was always true so simplify it.
jerrymarino
force-pushed
the
jmarino/starlark_virtual_frameworks_rebased
branch
from
313ae5d
to
24b928a
Compare
Add the ability to virtualize the framework structure. swift and clang
compile as frameworks but the files don't move and we load them via VFS.
The VFS is llvm's in-memory file system declared by JSON. VFS is already
in use but this uses it in a very different way.
This fixes many issues:
search paths
of overhead and isn't necesary since it's declarative
build directory. This causes problems with reproducible builds.
It cut clean build time in half in a big hybrid app from rules_ios
baseline. More interestingly, hot top level compiler invocations are
down nearly .5 orders of magnitude.
Remaining tasks:
Relates to: #211