-
Notifications
You must be signed in to change notification settings - Fork 271
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
Remove analysis failures for use_tree_artifacts_outputs flag #2545
base: master
Are you sure you want to change the base?
Remove analysis failures for use_tree_artifacts_outputs flag #2545
Conversation
25900ce
to
f55fc83
Compare
f55fc83
to
e458baf
Compare
We started using `apple_*_xcframework_import` rules to bring in XCFrameworks and we are getting failures during a query. We don't actually use the version macOS framework in our build, it's just shipped along side the iOS framework. Because of the `fail` we need to remove usage of the `use_tree_artifacts_outputs` flag, which causes a big regression to our disk usage: - bin/Code/Apps/CashApp/CashApp_archive-root/Payload/Cash.app (732 MB) - bin/Code/Apps/CashApp/CashApp-intermediates/unprocessed_archive.zip (732 MB) - bin/Code/Apps/CashApp/CashApp.ipa (740 MB) With `use_tree_artifacts_outputs` flag enabled we only have: - bin/Code/Apps/CashApp/CashApp_archive-root/Payload/Cash.app (732 MB) We should see about better supporting this flag in the future, consider making it the default, or improve the disk usage without it.
e458baf
to
c00823a
Compare
If you exclude them from the glob you would probably also sidestep this |
Unfortunately the targets themselves are generated from |
If we have other approaches here I'm happy to take any we prefer over this, just trying to find a way to not block usage of the flag in cases where using the unsupported stuff isn't actually possible |
This is the error a user would see if they do try to use a versioned framework via File "/private/var/tmp/_bazel_lpadron/d8d9324fa7fd2d11d5178d8cf2834c55/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/tools/bundletool/bundletool_experimental.runfiles/_main/tools/bundletool/bundletool_experimental.py", line 147, in run
self._sign_bundle(output_path, code_signing_commands)
File "/private/var/tmp/_bazel_lpadron/d8d9324fa7fd2d11d5178d8cf2834c55/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/tools/bundletool/bundletool_experimental.runfiles/_main/tools/bundletool/bundletool_experimental.py", line 307, in _sign_bundle
raise CodeSignError(exit_code)
CodeSignError: Code signing failed with exit code 256
ERROR:
bazel-out/darwin_arm64-fastbuild-macos-arm64-min11.0-applebin_macos-ST-516b26a33bca/bin/examples/macos/HelloWorldSwift/HelloWorldSwift.app/: replacing existing signature
bazel-out/darwin_arm64-fastbuild-macos-arm64-min11.0-applebin_macos-ST-516b26a33bca/bin/examples/macos/HelloWorldSwift/HelloWorldSwift.app/: code object is not signed at all
In subcomponent: /private/var/tmp/_bazel_lpadron/d8d9324fa7fd2d11d5178d8cf2834c55/execroot/_main/bazel-out/darwin_arm64-fastbuild-macos-arm64-min11.0-applebin_macos-ST-516b26a33bca/bin/examples/macos/HelloWorldSwift/HelloWorldSwift.app/Contents/Frameworks/Lottie.framework
|
I can also work on fixing that if you have tips, it looks like the Bazel issue the TODO is referencing has somewhat been resolved so potentially this is possible now |
That does look fixable. Ideally we would fix that before landing this change. If it's still broken only for RBE, I don't think we should block this change. |
We started using
apple_*_xcframework_import
rules to bring in XCFrameworks and we are getting failures during queries because of theuse_tree_artifacts_outputs
flag. We don't actually use the versioned macOS frameworks in our build, it's just shipped alongside the iOS framework and we have no control over this.Because of the
fail
we needed to remove usage of theuse_tree_artifacts_outputs
flag, which causes a big regression in disk usage:With
use_tree_artifacts_outputs
flag enabled we only have:We should see about better supporting this flag in the future, consider making it the default, or improve the disk usage without it.
In the meantime letting this fail later in the build allows folks who wouldn't actually hit the unsupported case to continue using the flag