bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
387 stars 180 forks source link

Should `bazel_skylib_gazelle_plugin_setup()` live elsewhere? #435

Open cgrindel opened 1 year ago

cgrindel commented 1 year ago

The instructions mention to call the following:

load("@bazel_skylib_gazelle_plugin//:setup.bzl", "bazel_skylib_gazelle_plugin_setup")

bazel_skylib_gazelle_plugin_setup()

This caused an error for me as I already had those same initialization calls in my WORKSPACE. I wonder if we should amend the instructions to only call that if you do not already have a Gazelle extension installed. Alternatively, I wonder if the boilerplate that bazel_skylib_gazelle_plugin_setup() includes should actually live in the bazel-gazelle repository.

achew22 commented 1 year ago

I think clearly indicating in this repo that it should be omitted would probably be the correct path forward. That said, I think most of this goes away with bzlmod which should solve all the worlds problems ™️

sudoforge commented 1 year ago

sure, but --enable_bzlmod is False by default as of bazel 6.2.1, and even in the "common rules" category of rule libraries, isn't fully supported (e.g. rules_go and gazelle still warn that it is experimental). until those two statements can be inverted, i think leaving a note to avoid calling bazel_skylib_gazelle_plugin will save consumers of the plugin a good few minutes of confusion during initial setup, increasing in time and value proposition when considering less experienced users.

fmeum commented 1 year ago

rules_go and gazelle both have proper support for Bzlmod by now.

The warnings you see are emitted by old versions that are still fetched during version resolution but not used. Getting rid of them is surprisingly difficult, but https://github.com/bazelbuild/bazel/pull/18698 will realize this for Bazel 6.3.0.

sudoforge commented 1 year ago

The warnings you see are emitted by old versions that are still fetched during version resolution but not used

Good catch; further review confirms this. I only have bzlmod enabled in a secondary build+test pipeline; I'm not using it by default yet, so I hadn't looked deeper into this.