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

Export bzl files with bzl_library to be able to use stardoc #1092

Closed
wants to merge 4 commits into from
Closed

Export bzl files with bzl_library to be able to use stardoc #1092

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d51d2da
Export bzl files with bzl_library to be able to use stardoc
ngeor Aug 12, 2021
523d49e
trying to add gazelle/bzl language
ngeor Aug 18, 2021
c9ef4cd
Move skylib definition into WORKSPACE
ngeor Aug 19, 2021
658bd6d
manually removed some private deps added by gazelle/skylib plugin
ngeor Aug 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
1 change: 1 addition & 0 deletions BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ gazelle_binary(
"//language/proto",
"//language/go",
"//internal/language/test_filegroup",
"@bazel_skylib//gazelle/bzl"
],
)

Expand Down
2 changes: 1 addition & 1 deletion deps.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def gazelle_dependencies(
_tools_git_repository,
name = "bazel_skylib",
remote = "https://github.com/bazelbuild/bazel-skylib",
commit = "3fea8cb680f4a53a129f7ebace1a5a4d1e035914", # 0.5.0 as of 2018-11-01
tag = "1.0.3"
Copy link
Member

Choose a reason for hiding this comment

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

I would guess the 1.0.3 tag (from > a year ago) doesn't contain the Gazelle plugin at all. I think you need to grab a more recent commit like df3c9e2735f02a7fe8cd80db4db00fec8e13d25f

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, it still fails with that commit, but also I see the Gazelle plugin existed even in the 1.0.3 tag, i.e. https://github.com/bazelbuild/bazel-skylib/blob/1.0.3/gazelle/bzl/BUILD

I'm a bit new to Bazel so maybe I'm doing something completely wrong.

Is there some possibility that this is causing a circular dependency? You are depending on skylib, they are depending on gazelle, maybe this is Bazel's way of telling me that this isn't going to work. After all, as you're already depending on skylib, why didn't you add support for bzl language inside gazelle to begin with? Maybe someone tried and failed because of this?

Thanks for your help and patience btw :)

Copy link
Member

Choose a reason for hiding this comment

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

Is there some possibility that this is causing a circular dependency? You are depending on skylib, they are depending on gazelle, maybe this is Bazel's way of telling me that this isn't going to work. After all, as you're already depending on skylib, why didn't you add support for bzl language inside gazelle to begin with? Maybe someone tried and failed because of this?

It's possible but I don't think so (see below). When Bazel has circular deps it prints out a VERY specific error that says circular dependency detected and then it renders a graph that shows you the loop that you need to resolve.

Thanks for your help and patience btw :)

Truly, you're the one contributing here, you deserve all the praise. Thanks for helping out!

I think the issue is that the version of skylib in this PR doesn't have gazelle. I validated this by cloning your repo and querying for all the targets that live in @bazel_skylib//... (basically everything in there).

$ bazel query @bazel_skylib//...
@bazel_skylib//toolchains/unittest:cmd_toolchain
@bazel_skylib//toolchains/unittest:cmd
@bazel_skylib//toolchains/unittest:bash_toolchain
@bazel_skylib//toolchains/unittest:toolchain_type
@bazel_skylib//toolchains/unittest:bash
@bazel_skylib//rules/private:maprule_util
@bazel_skylib//rules:write_file
@bazel_skylib//rules/private:write_file_private
@bazel_skylib//rules:select_file
@bazel_skylib//rules:run_binary
@bazel_skylib//rules:native_binary
@bazel_skylib//rules:diff_test
@bazel_skylib//rules:copy_file
@bazel_skylib//rules/private:copy_file_private
@bazel_skylib//rules:common_settings
@bazel_skylib//rules:build_test
@bazel_skylib//rules:analysis_test
@bazel_skylib//lib:old_sets
@bazel_skylib//:workspace
@bazel_skylib//:version
@bazel_skylib//:test_deps
@bazel_skylib//toolchains/unittest:test_deps
@bazel_skylib//rules:test_deps
@bazel_skylib//lib:test_deps
@bazel_skylib//:lib
@bazel_skylib//lib:versions
@bazel_skylib//lib:unittest
@bazel_skylib//lib:types
@bazel_skylib//lib:structs
@bazel_skylib//lib:shell
@bazel_skylib//lib:sets
@bazel_skylib//lib:selects
@bazel_skylib//lib:paths
@bazel_skylib//lib:partial
@bazel_skylib//lib:new_sets
@bazel_skylib//lib:dicts
@bazel_skylib//lib:collections
@bazel_skylib//:distribution
@bazel_skylib//toolchains/unittest:distribution
@bazel_skylib//rules/private:distribution
@bazel_skylib//rules:distribution
@bazel_skylib//lib:distribution
@bazel_skylib//:bzl_library
@bazel_skylib//:bins
@bazel_skylib//rules:bins

Looking that makes me very suspicious because it looks just like the directory listing for a long time ago. With a little bit more digging I was able to confirm the directory listing is identical to what you get when you grab an old skylib.

Turns out that WORKSPACEs suck and have all kinds of footguns. For example, if you re-register an external workspace that differs from the one that's already been defined it silently ignores your request. I believe that's what is happening here. I can't find the transitive dependency that's wrong, but when I add to the head of the WORKSPACE file right after the workspace(name = "bazel_gazelle") line:

load(
    "@bazel_tools//tools/build_defs/repo:git.bzl",
    "git_repository",
)

git_repository(
    name = "bazel_skylib",
    commit = "df3c9e2735f02a7fe8cd80db4db00fec8e13d25f",  # `master` as of 2021-08-19
    remote = "https://github.com/bazelbuild/bazel-skylib",
)

Everything works immediately. Would you be willing to manually make that change to WORKSPACE and run Gazelle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow, I would expect Bazel to complain about this... thanks for figuring it out!

So I guess now the question is should I leave the skylib plugin permanently in gazelle or revert it?

)

_maybe(
Expand Down