Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Build fix. #378

Merged
merged 2 commits into from
Aug 21, 2019
Merged

Build fix. #378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9b99b1
Build fix.
g-easy Aug 21, 2019
2c787c3
Remember to update pin_deps.
g-easy Aug 21, 2019
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
21 changes: 21 additions & 0 deletions WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ workspace(name = "io_opencensus_cpp")

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

# Build rules for C++ projects.
http_archive(
Copy link

Choose a reason for hiding this comment

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

For what it's worth, I would consider the fact that this dependency had to be added to your WORKSPACE a bug in the abseil-cpp repo. There doesn't appear to be anything stopping them from pulling this out to a deps() macro.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please correct me if I'm wrong but my understanding is that this will be required for all C++ projects after bazel 1.0(?) because rules like cc_library will move from native to skylark implementations.

Copy link

Choose a reason for hiding this comment

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

That may be the case for one hop. But suppose absl CPP requires rules_cc and your repo provides, say, a Golang tool. It's not intuitive to users of the Golang tool that they should have to pull in rules_cc despite never directly using a C++ rule.

This has already happened in the gRPC repo. We have to pull in a Golang-only tool used by Envoy even though we're only -- to my knowledge -- using some of their protos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto. opencensus-cpp pulls in e.g. grpc-java, because opencensus-proto depends on it.

As for rules_cc specifically, I think opencensus-cpp will end up using them directly after bazel 1.0.

name = "rules_cc",
strip_prefix = "rules_cc-master",
urls = ["https://github.com/bazelbuild/rules_cc/archive/master.zip"],
)

# We depend on Abseil.
http_archive(
name = "com_google_absl",
Expand All @@ -39,6 +46,20 @@ http_archive(
urls = ["https://github.com/google/benchmark/archive/master.zip"],
)

# Used by gRPC.
http_archive(
Copy link

Choose a reason for hiding this comment

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

You shouldn't need this particular http_archive invocation unless for some reason you really want master branch (which, btw, makes your build non-hermetic). You're already getting an instance from your call to grpc_deps(). It's just the load() and apple_rules_dependencies() invocations below that are necessary due to bazelbuild/bazel#1550.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! How does #379 look to you?

As for master, we are trying to follow Abseil's "live at head" philosophy. (...on our own master branch; whereas OpenCensus releases pin their deps)

name = "build_bazel_rules_apple",
strip_prefix = "rules_apple-master",
urls = ["https://github.com/bazelbuild/rules_apple/archive/master.zip"],
)

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

apple_rules_dependencies()

# Used by gRPC.
http_archive(
name = "build_bazel_apple_support",
Expand Down
2 changes: 2 additions & 0 deletions tools/pin_deps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@ def workspace_rule(self):

# grep -A1 http_archive\( WORKSPACE
PROJECTS = [
GitHubProject('rules_cc', 'bazelbuild', 'rules_cc'),
GitHubProject('com_google_absl', 'abseil', 'abseil-cpp'),
GitHubProject('com_google_googletest', 'google', 'googletest'),
GitHubProject('com_github_google_benchmark', 'google', 'benchmark'),
GitHubProject('build_bazel_apple_support', 'bazelbuild', 'apple_support'),
GitHubProject('build_bazel_rules_apple', 'bazelbuild', 'rules_apple'),
GitHubProject('com_github_grpc_grpc', 'grpc', 'grpc'),
GitHubProject('com_github_jupp0r_prometheus_cpp', 'jupp0r', 'prometheus-cpp'),
GitHubProject('com_github_curl', 'curl', 'curl'),
Expand Down