Switch to Bazel 4, migrate ObjcProvider compile info to CcCompilationContext #174

dgcoffman · 2021-03-08T22:01:52Z

dgcoffman · 2021-03-08T22:03:19Z

.bazelrc

 --strategy=SwiftCompile=worker \
 --swiftcopt=-Xwrapped-swift=-debug-prefix-pwd-is-dot \
 --experimental_strict_action_env=true

 test \
- --spawn_strategy=standalone \
+ --spawn_strategy=local \


spawn_strategy standalone is deprecated. Docs say "Please use local instead."

https://docs.bazel.build/versions/master/user-manual.html#flag--spawn_strategy

dgcoffman · 2021-03-08T22:03:42Z

.gitignore

@@ -15,3 +15,4 @@ external
 Pods/
 PodsHost/
 *.xcworkspace
+PodToBUILD.zip


This doesn't seem to be checked in, but is produced by e.g. make build-test

dgcoffman · 2021-03-08T22:04:27Z

BUILD

@@ -1,5 +1,6 @@
 load("@build_bazel_rules_swift//swift:swift.bzl", "swift_library")
 load("@build_bazel_rules_apple//apple:macos.bzl", "macos_command_line_application", "macos_unit_test")
+load("@rules_cc//cc:defs.bzl", "objc_library")


Buildifier now warns

Function "objc_library" is not global anymore and needs to be loaded from "@rules_cc//cc:defs.bzl".buildifier(native-cc)

dgcoffman · 2021-03-08T22:05:20Z

BUILD

@@ -8,7 +9,7 @@ objc_library(
    includes = ["Sources/ObjcSupport/include"]
 )

-# PodToBUILD is a core library enabling Skylark code generation
+# PodToBUILD is a core library enabling Starlark code generation


Skylark was renamed to "Starlark" in August 2018 https://blog.bazel.build/2018/08/17/starlark.html

Nice fix 👍

dgcoffman · 2021-03-08T22:05:35Z

BazelExtensions/extensions.bzl

@@ -1,5 +1,5 @@
 # This file is part of PodSpecToBUILD
-# Warning: this file is not accounted for as an explict imput into the build.
+# Warning: this file is not accounted for as an explicit input into the build.


"imput" typo

dgcoffman · 2021-03-08T22:06:32Z

BazelExtensions/extensions.bzl

    if ctx.attr.module_map_name == "module.modulemap":
        provider_hdr = [module_map] + ([umbrella_header_file] if umbrella_header_file else [])
        objc_provider = apple_common.new_objc_provider(
            module_map=depset([module_map]),
-            include=depset([ctx.outputs.module_map.dirname]),


ObjcProvider no longer takes include. bazelbuild/bazel#10674

dgcoffman · 2021-03-08T22:07:21Z

Examples/ArcSplitting/WORKSPACE

-    remote = "https://github.com/bazelbuild/rules_apple.git",
-    commit = "1cdaf74e44c4c969d7ee739b3a0f11b993c49d2a",
+    sha256 = "a41a75c291c69676b9974458ceee09aea60cee0e1ee282e27cdc90b679dfd30f",
+    url = "https://github.com/bazelbuild/rules_apple/releases/download/0.21.2/rules_apple.0.21.2.tar.gz",


Upgrading to the latest release of rules_apple.

dgcoffman · 2021-03-08T22:08:59Z

Examples/PINRemoteImage/Pods.WORKSPACE

  generate_header_map = True
 )

 new_pod_repository(
  name = "PINCache",
-  url = "https://github.com/pinterest/PINCache/archive/d886490de6d297e38f80bb750ff2dec4822fb870.zip",
+  url = "https://github.com/pinterest/PINCache/archive/3.0.3.zip",


Fixing #173

The older version of PINCache does not compile in Xcode 12.

jerrymarino

@dgcoffman - thank you kindly for the nice PR and updating it to work with 4.0 🥳 Overall this LGTM, my only question was around the top level symlinks and transitive hmap merging

BazelExtensions/extensions.bzl

jerrymarino · 2021-03-09T03:32:13Z

Examples/ChildPodspec/WORKSPACE

 )

-apple_rules_dependencies()
+apple_support_dependencies()


So this automatically pull in rules_swift and rules_cc now - nice

jerrymarino · 2021-03-09T03:34:47Z

Makefile

 build:
 	@tools/bazel build \
 		--disk_cache=$(HOME)/Library/Caches/Bazel \
-		--spawn_strategy=standalone \
+		--spawn_strategy=local \
+		--use_top_level_targets_for_symlinks \


@dgcoffman is this now required for the non header map build?

Sorry, I didn't understand the question. What's the "non header map build"?

I added --use_top_level_targets_for_symlinks here because something (I'm not sure what but I think the updated rules_apple?) causes an additional bindir to be created, which bazel-bin then symlinks to instead of the one which actually contains RepoTools and Compiler. use_top_level_targets_for_symlinks just makes bazel-bin point at the directory it did previously.

jerrymarino · 2021-03-09T03:38:06Z

BazelExtensions/extensions.bzl

@@ -313,7 +330,8 @@ def _make_headermap_impl(ctx):
        if not hasattr(hdr_provider, "objc"):
            continue

-        hdrs = hdr_provider.objc.header.to_list()
+        hdrs = hdr_provider.objc.direct_headers


Can you confirm if this has the same impact as it previously did? I believe here it merges transitive header maps in order to propagate the transitive includes, but would need to look more into that. If it still builds the top 50 swift cocoa pods and passes tests it should work.

Good call, looking into it.

from the migration description in bazelbuild/bazel#10674, it sounds like the right thing would be to change this to

hdrs = hdr_provider[CcInfo].compilation_context.headers.to_list())

but that actually fails, while direct_headers works fine!

I'm considering

hdrs = [] if CcInfo in hdr_provider: hdrs.extend(hdr_provider[CcInfo].compilation_context.headers.to_list()) if hasattr(hdr_provider, "objc"): hdrs.extend(hdr_provider.objc.direct_headers)

which should cover all bases, but since there's no case of which we're aware that fails with direct_headers alone, I'm not sure...

What do you think @jerrymarino?

@dgcoffman I'll have to pull this down and take a look at to give a clear answer or on it but - adding direct headers at the end seems right. There are likely some edge cases here which may or may not be exhibited in an integration test. If RN was running on the CI and this PR broke it that's probably something to look out for and vet the logic against. The CI is green, for what it's worth

dgcoffman · 2021-03-11T20:54:56Z

BazelExtensions/extensions.bzl


-        hdrs = hdr_provider.objc.direct_headers
+        if CcInfo in hdr_provider:
+            hdrs.extend(hdr_provider[CcInfo].compilation_context.headers.to_list())


Some targets do return the CcInfo provider with an embedded CompilationContext that should have the same headers that were previously attached to ObjcProvider -- but doing this alone fails with error

This makes sense, I think what you've done here seems more aligned with what we were doing before as opposed to only passing direct_headers. It may have been failing under darwin-sandbox if it didn't get all the headers.

farcaller · 2021-03-14T11:37:54Z

I'm trying to build the React Exmple app off this branch, and after running make && bazel build //PodsHost:ios-app I get random errors on headers missing, e.g.:

bazel build //PodsHost:ios-app
INFO: Analyzed target //PodsHost:ios-app (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
ERROR: /Users/farcaller/src/rn-bazel/PodToBUILD/Examples/React/Vendor/React/BUILD.bazel:2006:13: Compiling Vendor/React/React/CxxBridge/RCTCxxBridge.mm failed: (Exit 1): wrapped_clang failed: error executing command external/local_config_cc/wrapped_clang -arch x86_64 '-stdlib=libc++' '-std=gnu++11' '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign ... (remaining 81 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox wrapped_clang failed: error executing command external/local_config_cc/wrapped_clang -arch x86_64 '-stdlib=libc++' '-std=gnu++11' '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign ... (remaining 81 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
Vendor/React/React/CxxBridge/RCTCxxBridge.mm:19:9: fatal error: 'React/RCTDevSettings.h' file not found
#import <React/RCTDevSettings.h>
        ^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Target //PodsHost:ios-app failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 71.352s, Critical Path: 36.85s
INFO: 45 processes: 9 internal, 36 darwin-sandbox.
FAILED: Build did NOT complete successfully

INFO: Build options --apple_platform_type, --compilation_mode, --cpu, and 3 more have changed, discarding analysis cache.
INFO: Analyzed target //PodsHost:ios-app (34 packages loaded, 14031 targets configured).
INFO: Found 1 target...
ERROR: /Users/farcaller/src/rn-bazel/PodToBUILD/Examples/React/Vendor/React/BUILD.bazel:5344:13: Compiling Vendor/React/React/CoreModules/RCTWebSocketModule.mm failed: (Exit 1): wrapped_clang failed: error executing command external/local_config_cc/wrapped_clang -arch x86_64 '-stdlib=libc++' '-std=gnu++11' '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign ... (remaining 85 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox wrapped_clang failed: error executing command external/local_config_cc/wrapped_clang -arch x86_64 '-stdlib=libc++' '-std=gnu++11' '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign ... (remaining 85 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
Vendor/React/React/CoreModules/RCTWebSocketModule.mm:14:9: fatal error: 'React/RCTSRWebSocket.h' file not found
#import <React/RCTSRWebSocket.h>
        ^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Target //PodsHost:ios-app failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 182.230s, Critical Path: 40.33s
INFO: 265 processes: 136 internal, 129 darwin-sandbox.
FAILED: Build did NOT complete successfully

Looks like some include paths didn't propagate properly?

dgcoffman · 2021-03-15T19:02:26Z

@farcaller I think you need the specific BAZEL_OPTS from Examples/React/Makefile. Does cd Examples/React && make all work for you?

farcaller · 2021-03-19T16:55:35Z

Thanks for 47f2d72, makes a bit more sense now!

dgcoffman · 2021-03-19T17:01:45Z

Examples/React/BUILD

+    ],
+)
+
+ios_application(


@jerrymarino I expanded the React example to actually build the iOS app because there are a few gotchas in there that took me some time to figure out:

The 2016 version of Folly doesn't compile, and the newer v2020.01.13.00 requires an odd workaround

You have to use the React_hmap in order for your application code to import from <React/Whatever>

It's necessary to include certain SDK frameworks.

Thanks for updating @dgcoffman!

Agree, the docs could probably clarify how to use the hmap better, or even use the new flag provider to pass upwards, that’d be a great addition

loyaltyarm · 2021-05-25T04:57:43Z

Just curious if there is a plan to cut a release with this merged? Thanks in advance!

Beyond the initial done in bazel-xcode#174, we also need to include 'headers' in the generated CcCompilationContext and return a CcInfo provider with our headermap generation rule.

Beyond the initial done in #174, we also need to include 'headers' in the generated CcCompilationContext and return a CcInfo provider with our headermap generation rule.

dgcoffman added 16 commits February 23, 2021 11:26

Use Bazel 4.0.0

6427656

Allow PINCache to compile

bd4fb41

Use latest rules_apple

c5dbf58

Use latest rules_apple everywhere

c4f6cc5

Import objc_library

bd0b3e0

Revert superfluous Examples WORKSPACE change

4df6f47

--spawn_strategy=standalone is deprecated

763f038

Spawn local everywhere

8b9a425

use_top_level_targets_for_symlinks fixed bazel-bin symlink

f3253ee

Set bazelversion 4.0.0 in every .bazelversion

fad7587

Fix Error: 'ObjcProvider' value has no field or method 'header'

fcc7b50

Missed a couple references to old rules_apple

d53e8c0

Change gen_includes to use a CcInfo

7cc97f1

Cleanup

bca80ef

Use CcInfo for module maps

a051d4d

make new goldmaster

90ce6dd

dgcoffman commented Mar 8, 2021

View reviewed changes

jerrymarino approved these changes Mar 9, 2021

View reviewed changes

dgcoffman added 2 commits March 11, 2021 09:51

Simplify gen_includes return

93efb03

Modify make_headermap to includes headers from the CompilationContext

2a32559

dgcoffman commented Mar 11, 2021

View reviewed changes

Make the React example actually build an iOS app

47f2d72

dgcoffman commented Mar 19, 2021

View reviewed changes

jerrymarino merged commit db17580 into bazel-xcode:master Mar 31, 2021

This was referenced Sep 2, 2021

BazelExtensions - Incompatible with Bazel ObjcProvider to CcInfo migration #170

Closed

Headers are not found by objc_library on Bazel 4.0.0 #171

Closed

make build-test fails with Xcode 12 #173

Closed

This was linked to issues Sep 2, 2021

BazelExtensions - Incompatible with Bazel ObjcProvider to CcInfo migration #170

Closed

Headers are not found by objc_library on Bazel 4.0.0 #171

Closed

make build-test fails with Xcode 12 #173

Closed

jparise mentioned this pull request Sep 7, 2021

Improve CcInfo parity with ObjcProvider #184

Merged

jparise added a commit that referenced this pull request Sep 8, 2021

Improve CcInfo parity with ObjcProvider (#184)

5d03018

Beyond the initial done in #174, we also need to include 'headers' in the generated CcCompilationContext and return a CcInfo provider with our headermap generation rule.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to Bazel 4, migrate ObjcProvider compile info to CcCompilationContext #174

Switch to Bazel 4, migrate ObjcProvider compile info to CcCompilationContext #174

dgcoffman commented Mar 8, 2021

dgcoffman Mar 8, 2021

dgcoffman Mar 8, 2021 •

edited

Loading

dgcoffman Mar 8, 2021

dgcoffman Mar 8, 2021

jerrymarino Mar 9, 2021

dgcoffman Mar 8, 2021

dgcoffman Mar 8, 2021 •

edited

Loading

dgcoffman Mar 8, 2021

dgcoffman Mar 8, 2021

jerrymarino left a comment

jerrymarino Mar 9, 2021

jerrymarino Mar 9, 2021

dgcoffman Mar 11, 2021

jerrymarino Mar 9, 2021

dgcoffman Mar 11, 2021

dgcoffman Mar 11, 2021

jerrymarino Mar 17, 2021 •

edited

Loading

dgcoffman Mar 11, 2021

jerrymarino Mar 17, 2021 •

edited

Loading

farcaller commented Mar 14, 2021

dgcoffman commented Mar 15, 2021

farcaller commented Mar 19, 2021

dgcoffman Mar 19, 2021 •

edited

Loading

jerrymarino Mar 31, 2021

loyaltyarm commented May 25, 2021

Switch to Bazel 4, migrate ObjcProvider compile info to CcCompilationContext #174

Switch to Bazel 4, migrate ObjcProvider compile info to CcCompilationContext #174

Conversation

dgcoffman commented Mar 8, 2021

Choose a reason for hiding this comment

dgcoffman Mar 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgcoffman Mar 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerrymarino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerrymarino Mar 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerrymarino Mar 17, 2021 • edited Loading

Choose a reason for hiding this comment

farcaller commented Mar 14, 2021

dgcoffman commented Mar 15, 2021

farcaller commented Mar 19, 2021

dgcoffman Mar 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loyaltyarm commented May 25, 2021

dgcoffman Mar 8, 2021 •

edited

Loading

dgcoffman Mar 8, 2021 •

edited

Loading

jerrymarino Mar 17, 2021 •

edited

Loading

jerrymarino Mar 17, 2021 •

edited

Loading

dgcoffman Mar 19, 2021 •

edited

Loading