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

Jump to bottom

Troubleshoot dependency breakage #130

Closed
wants to merge 5 commits into from
Closed

Troubleshoot dependency breakage #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
891903c
Troubleshoot dependency breakage
ahungrynacho Jul 3, 2021
28ba92c
Merge branch 'master' into buildfarm-1.9.0
ahungrynacho Jul 9, 2021
c9a3e3f
Resolved dependency issues by primarily replacing rules_proto with pr…
ahungrynacho Jul 9, 2021
226ec50
remove uneccessary package
ahungrynacho Jul 10, 2021
5413a8c
Merge branch 'master' into buildfarm-1.9.0
ahungrynacho Jul 14, 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
27 changes: 20 additions & 7 deletions WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,35 @@ workspace(
managed_directories = {"@npm": ["ui/node_modules"]},
)

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

load("//bazel:deps.bzl", "enkit_deps")
enkit_deps()

load("//bazel:init.bzl", "enkit_init")
# packages must be loaded in this staggered manner to fix the dependency issue
# in order for protobuf and grpc to work hence the removal of enkit_init()
Copy link
Contributor

Choose a reason for hiding this comment

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

can't the steps here go into enkit_init()? Eg, you change the implementation of enkit_init() to just have the steps here in this order?

The underlying problem is that the enkit repository is used from within other repositories. The initializations below need to be done in other repositories as well.
The typical way to solve this problem is to have those initializations in a macro, that then any WORKSPACE can use.

So... if we get rid of enkit_init here, everyone else using this repository will need to cut and paste the lines. Which will make updates messy / hard to do, as now everyone will need to also update the copy of their rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't load and initialize the bazel packages in this exact order, you get this error

ERROR: /home/bbhuynh/.cache/bazel/_bazel_bbhuynh/ccbc8eec90d71932e4b174477612b27d/external/io_bazel_rules_nogo/BUILD.bazel:3:6: in alias rule @io_bazel_rules_nogo//:nogo: cycle in dependency graph:
    //kbuild/kapt:go_default_library
    //lib/khttp/protocol:go_default_library
    //lib/khttp/kclient:go_default_library
    @io_bazel_rules_go//:go_context_data
.-> @io_bazel_rules_nogo//:nogo
|   @io_bazel_rules_go//:default_nogo
|   @io_bazel_rules_go//go/tools/builders:nogo_srcs
|   @org_golang_x_tools//go/analysis/internal/facts:go_tool_library
|   @org_golang_x_tools//go/analysis:go_tool_library
|   @org_golang_x_tools//internal/analysisinternal:go_tool_library
|   @org_golang_x_tools//go/ast/astutil:go_tool_library
|   @org_golang_x_tools//internal/typeparams:typeparams
|   @io_bazel_rules_go//:go_context_data
`-- @io_bazel_rules_nogo//:nogo

which is being fixed by the bazel developers and will probably be in the next release according to the activity on
bazelbuild/bazel#11291
bazelbuild/bazel#13372
so let's just get this merged, then we can revisit this later once the bazel fix is in the next bazel release.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not arguing about changing the order, I'm arguing about moving this in the enkit_init() function in the .bzl file.
The order would be the same?

Copy link
Contributor

Choose a reason for hiding this comment

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

Moving it in enkit_init in the same order would also allow you to avoid a few changes in https://github.com/enfabrica/internal/pull/996, eg, would save you from duplicating the protobuf configuration.

load("@io_bazel_rules_go//go:deps.bzl", "go_download_sdk")
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
protobuf_deps()

enkit_init()
load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
go_register_toolchains(version = "1.15.14")
go_rules_dependencies()

# gazelle:repo bazel_gazelle
load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies")
gazelle_dependencies()

load("//bazel:go_repositories.bzl", "go_repositories")

go_repositories()

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

load("@rules_pkg//:deps.bzl", "rules_pkg_dependencies")
rules_pkg_dependencies()

load("@com_github_atlassian_bazel_tools//multirun:deps.bzl", "multirun_dependencies")
multirun_dependencies()

http_archive(
name = "com_github_bazelbuild_buildtools",
Expand Down Expand Up @@ -59,7 +73,6 @@ container_pull(
)

load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")

yarn_install(
# Name this npm so that Bazel Label references look like @npm//package
name = "npm",
Expand Down
18 changes: 9 additions & 9 deletions bazel/deps.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
def enkit_deps():
excludes = native.existing_rules().keys()


if "io_bazel_rules_go" not in excludes:
http_archive(
name = "io_bazel_rules_go",
Expand Down Expand Up @@ -38,17 +39,15 @@ def enkit_deps():
],
)

if "rules_proto" not in excludes:
if "com_google_protobuf" not in excludes:
http_archive(
name = "rules_proto",
sha256 = "aa1ee19226f707d44bee44c720915199c20c84a23318bb0597ed4e5c873ccbd5",
strip_prefix = "rules_proto-40298556293ae502c66579620a7ce867d5f57311",
urls = [
"https://mirror.bazel.build/github.com/bazelbuild/rules_proto/archive/40298556293ae502c66579620a7ce867d5f57311.tar.gz",
"https://github.com/bazelbuild/rules_proto/archive/40298556293ae502c66579620a7ce867d5f57311.tar.gz",
],
name = "com_google_protobuf",
sha256 = "c6003e1d2e7fefa78a3039f19f383b4f3a61e81be8c19356f85b6461998ad3db",
strip_prefix = "protobuf-3.17.3",
urls = [
"https://github.com/protocolbuffers/protobuf/archive/v3.17.3.tar.gz",
],
)

# rules_docker 0.14.4 is incompatible with rules_pkg 0.3.0 as of Oct/2020.
#
# When you update this dependency, please make sure rules_docker has been updated as well,
Expand All @@ -72,3 +71,4 @@ def enkit_deps():
"https://github.com/atlassian/bazel-tools/archive/5c3b9306e703c6669a6ce064dd6dde69f69cba35.zip",
],
)

Loading