[Bazel6] Avoid copying in .h directories into Foo.framework/Headers
#624
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jerrymarino
approved these changes
Dec 5, 2022
Fix the follow mismatch between the declared output and the symlink’d input: ``` $ bazelisk test --local_test_jobs=1 --apple_platform_type=ios --deleted_packages='' -- //tests/ios/... … ERROR: /Users/matt.robinson/Developer/rules_ios/rules_ios/tests/ios/frameworks/intentdefinition/BUILD.bazel:3:16: in apple_framework_packaging rule //tests/ios/frameworks/intentdefinition:ObjCIntentConsumer: Traceback (most recent call last): File "/Users/matt.robinson/Developer/rules_ios/rules_ios/rules/framework.bzl", line 854, column 43, in _apple_framework_packaging_impl framework_files = _get_framework_files(ctx, deps) File "/Users/matt.robinson/Developer/rules_ios/rules_ios/rules/framework.bzl", line 383, column 38, in _get_framework_files header_out = _framework_packaging(ctx, "header", header_in, header_out, framework_manifest) File "/Users/matt.robinson/Developer/rules_ios/rules_ios/rules/framework.bzl", line 185, column 45, in _framework_packaging _framework_packaging_symlink_headers(ctx, inputs, outputs) File "/Users/matt.robinson/Developer/rules_ios/rules_ios/rules/framework.bzl", line 150, column 28, in _framework_packaging_symlink_headers ctx.actions.symlink(output = output, target_file = input) Error in symlink: symlink() with "target_file" directory param requires that "output" be declared as a directory (did you mean to use declare_directory() instead of declare_file()?) ERROR: /Users/matt.robinson/Developer/rules_ios/rules_ios/tests/ios/frameworks/intentdefinition/BUILD.bazel:3:16: Analysis of target '//tests/ios/frameworks/intentdefinition:ObjCIntentConsumer' failed ```
This aims to fix the issue on incremental builds where the intent headers are seemingly not cleaned up: ``` ERROR: /Users/matt.robinson/Developer/rules_ios/rules_ios/tests/ios/frameworks/intentdefinition/BUILD.bazel:3:16: Creating symlink bazel-out/applebin_ios-ios_sim_arm64-dbg-ST-8e9d4392bc2b/bin/tests/ios/frameworks/intentdefinition/ObjCIntentConsumer/ObjCIntentConsumer.framework/Headers/ObjCIntentConsumer_Intents.intentdefinition_gen.hdrs.h failed: failed to create symbolic link 'tests/ios/frameworks/intentdefinition/ObjCIntentConsumer/ObjCIntentConsumer.framework/Headers/ObjCIntentConsumer_Intents.intentdefinition_gen.hdrs.h' to 'tests/ios/frameworks/intentdefinition/ObjCIntentConsumer_Intents.intentdefinition_gen.hdrs.h' due to I/O error: /private/var/tmp/_bazel_matt.robinson/0e64031a9efbb95e4728094ee9457143/execroot/build_bazel_rules_ios/bazel-out/applebin_ios-ios_sim_arm64-dbg-ST-8e9d4392bc2b/bin/tests/ios/frameworks/intentdefinition/ObjCIntentConsumer/ObjCIntentConsumer.framework/Headers/ObjCIntentConsumer_Intents.intentdefinition_gen.hdrs.h (File exists) ``` It makes little sense to add `.h` “files” to `Foo.framework/Headers` that are actually directories.
mattrobmattrob
force-pushed
the
mr/bazel6/fix.symlink.actions.in._framework_packaging
branch
from
5e29d51 to
4a19aa9
Compare
mattrobmattrob
force-pushed
the
mr/bazel6/fix.symlink.actions.in._framework_packaging
branch
from
056af63 to
6ba1284
Compare
mattrobmattrob
changed the title
_framework_packaging.h directories into Foo.framework/Headers
|
Re-requesting your review, @jerrymarino, since the change is now quite different from the initial implementation. |
jerrymarino
approved these changes
Dec 6, 2022
| @@ -323,7 +323,7 @@ def _get_framework_files(ctx, deps): | |||
| for provider in [CcInfo, apple_common.Objc]: | |||
| if provider in dep: | |||
| for hdr in _get_direct_public_headers(provider, dep): | |||
| if hdr.path.endswith((".h", ".hh", ".hpp")): | |||
| if not hdr.is_directory and hdr.path.endswith((".h", ".hh", ".hpp")): | |||
There was a problem hiding this comment.
This LGTM - I think the upstream code would break if it was a dir anyhow
mattrobmattrob
deleted the
mr/bazel6/fix.symlink.actions.in._framework_packaging
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix the following mismatch between the declared output and the
symlink’d input:If we
declare_directory/declare_filecorrectly for the symlink'd outputs then we still run into an issue where there's a conflict on what's seemingly an incremental build:It makes little sense to add
.h“files” toFoo.framework/Headersthat are actually directories so stop adding them to the resulting frameworksHeadersdirectory unless they're files.