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.

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

Troubleshoot dependency breakage #130

Closed
wants to merge 5 commits into from
Closed

Conversation

ahungrynacho
Copy link
Contributor

@ahungrynacho ahungrynacho commented Jul 3, 2021

Replace rules_proto with protobuf because rules_proto broke everything after I upgraded it to support bazel buildfarm-1.9.0. Finally fixed the dependency issue.

INFO: 5 processes: 3 internal, 2 linux-sandbox.
FAILED: Build did NOT complete successfully
//astore:go_default_test                                        (cached) PASSED in 0.2s
//astore/server/astore:go_default_test                          (cached) PASSED in 0.0s
//lib/cache:go_default_test                                     (cached) PASSED in 1.4s
//lib/config:go_default_test                                    (cached) PASSED in 0.1s
//lib/config/directory:go_default_test                          (cached) PASSED in 0.1s
//lib/config/remote:go_default_test                             (cached) PASSED in 0.1s
//lib/karchive:go_default_test                                  (cached) PASSED in 0.2s
//lib/kflags:go_default_test                                    (cached) PASSED in 0.1s
//lib/kflags/kcobra:go_default_test                             (cached) PASSED in 0.2s
//lib/kflags/kconfig:go_default_test                            (cached) PASSED in 9.1s
//lib/khttp:go_default_test                                     (cached) PASSED in 0.7s
//lib/khttp/downloader:go_default_test                          (cached) PASSED in 0.1s
//lib/khttp/kcache:go_default_test                              (cached) PASSED in 0.8s
//lib/khttp/scheduler:go_default_test                           (cached) PASSED in 0.1s
//lib/khttp/workpool:go_default_test                            (cached) PASSED in 0.4s
//lib/knetwork/kdns:go_default_test                             (cached) PASSED in 3.1s
//lib/srand:go_default_test                                     (cached) PASSED in 0.1s
//lib/token:go_default_test                                     (cached) PASSED in 0.2s
//machinist:go_default_test                                     (cached) PASSED in 0.2s
//machinist/mnode:go_default_test                               (cached) PASSED in 0.1s
//proxy/httpp:go_default_test                                   (cached) PASSED in 0.1s
//proxy/nasshp:go_default_test                                  (cached) PASSED in 1.8s
//proxy/nss:nss_autouser_test                                   (cached) PASSED in 0.1s
//proxy/nss/confparse:confparse_test                            (cached) PASSED in 0.2s
//proxy/ptunnel:go_default_test                                 (cached) PASSED in 0.3s
//proxy/ptunnel/commands:go_default_test                        (cached) PASSED in 0.1s
//ui:ptunnel-test                                                     NO STATUS
//astore/server/auth:go_default_test                                     FAILED in 7.1s
  /home/bbhuynh/.cache/bazel/_bazel_bbhuynh/ccbc8eec90d71932e4b174477612b27d/execroot/enkit/bazel-out/k8-fastbuild/testlogs/astore/server/auth/go_default_test/test.log
//lib/kcerts:go_default_test                                             FAILED in 7.2s
  /home/bbhuynh/.cache/bazel/_bazel_bbhuynh/ccbc8eec90d71932e4b174477612b27d/execroot/enkit/bazel-out/k8-fastbuild/testlogs/lib/kcerts/go_default_test/test.log

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants