-
Notifications
You must be signed in to change notification settings - Fork 85
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
Set clang flags in lldb, propagate linker options #119
Changes from all commits
ae3c3ab
5f59b38
e018f67
0335fff
63479e8
847aa85
9bf6f0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -407,7 +407,7 @@ def _xcodeproj_impl(ctx): | |
"BAZEL_BUILD_EXEC": "$BAZEL_STUBS_DIR/build-wrapper", | ||
"BAZEL_OUTPUT_PROCESSOR": "$BAZEL_STUBS_DIR/output-processor.rb", | ||
"BAZEL_PATH": ctx.attr.bazel_path, | ||
"BAZEL_RULES_IOS_OPTIONS": "--@build_bazel_rules_ios//rules:local_debug_options_enabled --features=-swift.cacheable_swiftmodules --features=-swift.use_global_module_cache", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any particular reason these features are removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of this change, debugging works in local development, even when cacheable swiftmodules are enabled. Therefore, we don't need to explicitly turn off the cacheable swiftmodules feature ... the developer can decide what to do with the feature in their bazelrc if they wish. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. awesome!! |
||
"BAZEL_RULES_IOS_OPTIONS": "--@build_bazel_rules_ios//rules:local_debug_options_enabled", | ||
"BAZEL_WORKSPACE_ROOT": "$SRCROOT/%s" % script_dot_dots, | ||
"BAZEL_STUBS_DIR": "$PROJECT_FILE_PATH/bazelstubs", | ||
"BAZEL_INSTALLERS_DIR": "$PROJECT_FILE_PATH/bazelinstallers", | ||
|
@@ -456,7 +456,7 @@ def _xcodeproj_impl(ctx): | |
if target_info.product_type != existing_type: | ||
fail("""\ | ||
Failed to generate xcodeproj for "{}" due to conflicting targets: | ||
Target "{}" is already defined with type "{}". | ||
Target "{}" is already defined with type "{}". | ||
A same-name target with label "{}" of type "{}" wants to override. | ||
Double check your rule declaration for naming or add `xcodeproj-ignore-as-target` as a tag to choose which target to ignore. | ||
""".format(ctx.label, target_name, existing_type, target_info.bazel_build_target_name, target_info.product_type)) | ||
|
@@ -510,6 +510,9 @@ Double check your rule declaration for naming or add `xcodeproj-ignore-as-target | |
target_settings["SWIFT_ACTIVE_COMPILATION_CONDITIONS"] = " ".join( | ||
["\"%s\"" % d for d in defines_without_equal_sign], | ||
) | ||
target_settings["BAZEL_LLDB_SWIFT_EXTRA_CLANG_FLAGS"] = " ".join( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the key for getting swift to work again right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two issues: (1) LLDB settings need to be right or else LLDB will dump a bunch of errors and (2) the linker command for the application/test needs to include the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thx for this! I should have caught them on the description section. Also did you manually ensure that breakpoints work in test projects in this repo? If not that's fine I can do another round of tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I can explain how I went about finding these problems. I first added the line
As far as the actual implementation of this change, Sam noticed that framework.bzl was not propagating linker opts for transitive deps, which would explain why these linker options were not present.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will double check the breakpoints in the test project, but note that debugging in the test project only works when sandboxing is disabled (which requires changing local bazelrc.) In order to test this change, I updated build.sh to check for the presence of AST files in the symbol table for the binary. I think that should be sufficient. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the detailed explanation! Learnt that we can do logging for lldb just like we can do it for SourceKIT (so in future get the logs is the priority so we can not shooting in the dark) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amberdixon Can point to me where are the changes on build.sh for presence of AST files? 🙏 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
saw the updated checks, that's awesome! |
||
["-D%s" % d for d in target_info.cc_defines.to_list()], | ||
) | ||
|
||
if target_info.product_type == "application": | ||
target_settings["INFOPLIST_FILE"] = "$BAZEL_STUBS_DIR/Info-stub.plist" | ||
|
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.
Here is the reference for objcprovider: https://docs.bazel.build/versions/master/skylark/lib/ObjcProvider.html
I searched for the string "link" and "final bundle" on this page to see what fields have to get propagated.