Skip to content
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.

Sign up for GitHub

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

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

Merged
merged 19 commits into from
Mar 31, 2021
Merged

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

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
6427656
Use Bazel 4.0.0
dgcoffman Feb 23, 2021
bd4fb41
Allow PINCache to compile
dgcoffman Feb 23, 2021
c5dbf58
Use latest rules_apple
dgcoffman Feb 23, 2021
c4f6cc5
Use latest rules_apple everywhere
dgcoffman Feb 23, 2021
bd0b3e0
Import objc_library
dgcoffman Feb 23, 2021
4df6f47
Revert superfluous Examples WORKSPACE change
dgcoffman Feb 23, 2021
763f038
--spawn_strategy=standalone is deprecated
dgcoffman Feb 23, 2021
8b9a425
Spawn local everywhere
dgcoffman Feb 23, 2021
f3253ee
use_top_level_targets_for_symlinks fixed bazel-bin symlink
dgcoffman Feb 26, 2021
fad7587
Set bazelversion 4.0.0 in every .bazelversion
dgcoffman Feb 26, 2021
fcc7b50
Fix Error: 'ObjcProvider' value has no field or method 'header'
dgcoffman Feb 26, 2021
d53e8c0
Missed a couple references to old rules_apple
dgcoffman Feb 26, 2021
7cc97f1
Change gen_includes to use a CcInfo
dgcoffman Mar 8, 2021
bca80ef
Cleanup
dgcoffman Mar 8, 2021
a051d4d
Use CcInfo for module maps
dgcoffman Mar 8, 2021
90ce6dd
make new goldmaster
dgcoffman Mar 8, 2021
93efb03
Simplify gen_includes return
dgcoffman Mar 11, 2021
2a32559
Modify make_headermap to includes headers from the CompilationContext
dgcoffman Mar 11, 2021
47f2d72
Make the React example actually build an iOS app
dgcoffman Mar 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# A few settings for XCHammer's .bazelrc
# Debugging: Use the remapping feature of rules_swift.
build \
--spawn_strategy=standalone \
--spawn_strategy=local \
--strategy=SwiftCompile=worker \
--swiftcopt=-Xwrapped-swift=-debug-prefix-pwd-is-dot \
--experimental_strict_action_env=true

test \
--spawn_strategy=standalone \
--spawn_strategy=local \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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


2 changes: 1 addition & 1 deletion .bazelversion
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.4.1
4.0.0
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ external
Pods/
PodsHost/
*.xcworkspace
PodToBUILD.zip
Copy link
Contributor Author

@dgcoffman dgcoffman Mar 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

9 changes: 5 additions & 4 deletions BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buildifier now warns

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


objc_library(
name = "ObjcSupport",
Expand All @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix 👍

swift_library(
name = "PodToBUILD",
srcs = glob(["Sources/PodToBUILD/*.swift"]),
Expand Down Expand Up @@ -53,7 +54,7 @@ swift_library(

alias(name = "update_pods", actual = "//bin:update_pods")

# This tests RepoToolsCore and Skylark logic
# This tests RepoToolsCore and Starlark logic
swift_library(
name = "PodToBUILDTestsLib",
srcs = glob(["Tests/PodToBUILDTests/*.swift"]),
Expand All @@ -68,13 +69,13 @@ macos_unit_test(
)

swift_library(
name = "BuildTestsLib",
name = "BuildTestsLib",
srcs = glob(["Tests/BuildTests/*.swift"]),
deps = [":RepoToolsCore", "@podtobuild-SwiftCheck//:SwiftCheck"],
data = glob(["Examples/**/*.podspec.json"])
)

# This tests RepoToolsCore and Skylark logic
# This tests RepoToolsCore and Starlark logic
macos_unit_test(
name = "BuildTests",
deps = [":BuildTestsLib"],
Expand Down
34 changes: 26 additions & 8 deletions BazelExtensions/extensions.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"imput" typo

# Therefore, bin/RepoTools must be updated when this changes.

# Acknowledgements
Expand Down Expand Up @@ -157,7 +157,7 @@ def _module_map_impl(ctx):
relative_path = "".join(["../" for i in range(len(module_map.dirname.split("/")))])

system_tag = " [system] "
content = "module " + module_name + (system_tag if ctx.attr.is_system else "" ) + " {\n"
content = "module " + module_name + (system_tag if ctx.attr.is_system else "" ) + " {\n"
umbrella_header_file = None
if ctx.attr.umbrella_hdr:
# Note: this umbrella header is created internally.
Expand Down Expand Up @@ -190,24 +190,31 @@ module {module_name}.Swift {{
# If the name is `module.modulemap` we propagate this as an include. If a
# module map is added to `objc_library` as a dep, bazel will add these
# automatically and add a _single_ include to this module map. Ideally there
# would be an API to invoke clang with -fmodule-map=
# would be an API to invoke clang with -fmodule-map=
providers = []
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]),
Copy link
Contributor Author

@dgcoffman dgcoffman Mar 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ObjcProvider no longer takes include. bazelbuild/bazel#10674

header=depset(provider_hdr)
)

compilation_context = cc_common.create_compilation_context(
includes=depset([ctx.outputs.module_map.dirname]))

providers.append(CcInfo(compilation_context=compilation_context))
else:
# This is an explicit module map. Currently, we use these for swift only
provider_hdr = [module_map] + ([umbrella_header_file] if umbrella_header_file else [])
objc_provider = apple_common.new_objc_provider(
header=depset(provider_hdr + [module_map])
)

providers.append(objc_provider)

return struct(
files=depset([module_map]),
providers=[objc_provider],
providers=providers,
objc=objc_provider,
headers=depset([module_map]),
)
Expand Down Expand Up @@ -257,12 +264,22 @@ def gen_module_map(name,
def _gen_includes_impl(ctx):
includes = []
includes.extend(ctx.attr.include)

for target in ctx.attr.include_files:
for f in target.files.to_list():
includes.append(f.path)

return apple_common.new_objc_provider(
include=depset(includes))
compilation_context = cc_common.create_compilation_context(
includes=depset(includes))

providers = []
dgcoffman marked this conversation as resolved.
Show resolved Hide resolved

providers.append(CcInfo(compilation_context=compilation_context))

# objc_library deps requires an ObjcProvider
providers.append(apple_common.new_objc_provider())

return providers

_gen_includes = rule(
implementation=_gen_includes_impl,
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, looking into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

@jerrymarino jerrymarino Mar 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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


for hdr in hdrs:
if hdr.path.endswith(".hmap"):
# Add headermaps
Expand Down
1 change: 1 addition & 0 deletions Examples/.PodsHost/BUILD
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Examples/ArcSplitting/.bazelversion
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.4.1
4.0.0
2 changes: 2 additions & 0 deletions Examples/ArcSplitting/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
load("@rules_cc//cc:defs.bzl", "objc_library")

objc_library(name="all", deps=["//Vendor/ArcSplitting:ArcSplitting"])
25 changes: 9 additions & 16 deletions Examples/ArcSplitting/WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
name = "rules_pods",
urls = ["https://note-this-is-overridden"],
)

git_repository(
http_archive(
name = "build_bazel_rules_apple",
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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrading to the latest release of rules_apple.

)

load(
"@build_bazel_rules_apple//apple:repositories.bzl",
"apple_rules_dependencies",
)

git_repository(
name = "build_bazel_rules_swift",
remote = "https://github.com/bazelbuild/rules_swift.git",
commit = "d07d880dcf939e0ad98df4dd723f8516bf8a2867",
apple_rules_dependencies()

load(
"@build_bazel_apple_support//lib:repositories.bzl",
"apple_support_dependencies",
)

apple_rules_dependencies()
apple_support_dependencies()

load(
"@build_bazel_rules_swift//swift:repositories.bzl",
Expand All @@ -32,16 +32,9 @@ load(

swift_rules_dependencies()

load(
"@build_bazel_apple_support//lib:repositories.bzl",
"apple_support_dependencies",
)

apple_support_dependencies()
load(
"@com_google_protobuf//:protobuf_deps.bzl",
"protobuf_deps",
)

protobuf_deps()

4 changes: 2 additions & 2 deletions Examples/BasiciOS/.bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# A few settings for XCHammer's .bazelrc
# Debugging: Use the remapping feature of rules_swift.
build \
--spawn_strategy=standalone \
--spawn_strategy=local \
--strategy=SwiftCompile=worker \
--swiftcopt=-Xwrapped-swift=-debug-prefix-pwd-is-dot \
--experimental_strict_action_env=true

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

2 changes: 1 addition & 1 deletion Examples/BasiciOS/.bazelversion
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.4.1
4.0.0
1 change: 1 addition & 0 deletions Examples/BasiciOS/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
load("@build_bazel_rules_apple//apple:ios.bzl", "ios_application")
load("@rules_cc//cc:defs.bzl", "objc_library")

# This is needed for implicit entitlement rules created for
# files.
Expand Down
30 changes: 13 additions & 17 deletions Examples/BasiciOS/WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,31 @@
load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
name = "rules_pods",
urls = ["https://note-this-is-overridden.com"],
urls = ["https://note-this-is-overridden"],
)
load("@rules_pods//BazelExtensions:workspace.bzl", "new_pod_repository")

load("@rules_pods//BazelExtensions:workspace.bzl", "new_pod_repository")

git_repository(
http_archive(
name = "build_bazel_rules_apple",
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",
)

load(
"@build_bazel_rules_apple//apple:repositories.bzl",
"apple_rules_dependencies",
)

git_repository(
name = "build_bazel_rules_swift",
remote = "https://github.com/bazelbuild/rules_swift.git",
commit = "d07d880dcf939e0ad98df4dd723f8516bf8a2867",
apple_rules_dependencies()

load(
"@build_bazel_apple_support//lib:repositories.bzl",
"apple_support_dependencies",
)

apple_rules_dependencies()
apple_support_dependencies()

load(
"@build_bazel_rules_swift//swift:repositories.bzl",
Expand All @@ -33,16 +34,11 @@ load(

swift_rules_dependencies()

load(
"@build_bazel_apple_support//lib:repositories.bzl",
"apple_support_dependencies",
)

apple_support_dependencies()
load(
"@com_google_protobuf//:protobuf_deps.bzl",
"protobuf_deps",
)

protobuf_deps()

new_pod_repository(
Expand Down
2 changes: 1 addition & 1 deletion Examples/ChildPodspec/.bazelversion
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.4.1
4.0.0
2 changes: 2 additions & 0 deletions Examples/ChildPodspec/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
load("@rules_cc//cc:defs.bzl", "objc_library")

objc_library(name="all", deps=["//Vendor/Parent:Parent"])
24 changes: 9 additions & 15 deletions Examples/ChildPodspec/WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
name = "rules_pods",
urls = ["https://note-this-is-overridden"],
)

git_repository(
http_archive(
name = "build_bazel_rules_apple",
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",
)

load(
"@build_bazel_rules_apple//apple:repositories.bzl",
"apple_rules_dependencies",
)

git_repository(
name = "build_bazel_rules_swift",
remote = "https://github.com/bazelbuild/rules_swift.git",
commit = "d07d880dcf939e0ad98df4dd723f8516bf8a2867",
apple_rules_dependencies()

load(
"@build_bazel_apple_support//lib:repositories.bzl",
"apple_support_dependencies",
)

apple_rules_dependencies()
apple_support_dependencies()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this automatically pull in rules_swift and rules_cc now - nice


load(
"@build_bazel_rules_swift//swift:repositories.bzl",
Expand All @@ -32,12 +32,6 @@ load(

swift_rules_dependencies()

load(
"@build_bazel_apple_support//lib:repositories.bzl",
"apple_support_dependencies",
)

apple_support_dependencies()
load(
"@com_google_protobuf//:protobuf_deps.bzl",
"protobuf_deps",
Expand Down
8 changes: 4 additions & 4 deletions Examples/Example.Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ BAZEL=../../tools/bazel
REPOSITORY_OVERRIDE=--override_repository=rules_pods=$(RULES_PODS_DIR)
BAZEL_OPTS=$(REPOSITORY_OVERRIDE) \
--disk_cache=$(HOME)/Library/Caches/Bazel \
--spawn_strategy=standalone \
--spawn_strategy=local \
--apple_platform_type=ios

all: bootstrap pod_test fetch build
Expand All @@ -25,10 +25,10 @@ pod_test:

# Build everything in this workspace
.PHONY: build
build: info
build: info
$(BAZEL) build :* $(BAZEL_OPTS)

test: info
test: info
$(BAZEL) test :* $(BAZEL_OPTS)

# Fetch vendored pods if there's a Pods.WORKSPACE. In normal operation it isn't
Expand All @@ -42,7 +42,7 @@ vendorize:
# however, `rules_pods` is overriden
ditto $(RULES_PODS_DIR)/BazelExtensions Vendor/rules_pods/BazelExtensions

fetch: info
fetch: info
[[ ! -f Pods.WORKSPACE ]] || $(MAKE) vendorize
$(BAZEL) fetch :* $(REPOSITORY_OVERRIDE)

Expand Down
2 changes: 1 addition & 1 deletion Examples/FBSDK/.bazelversion
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.4.1
4.0.0
Loading