sluongng / nogo-analyzer

A collection of common static analyzers (linters) that can be used with Bazel's rules_go
Apache License 2.0
31 stars 10 forks source link

bug: staticcheck dependency conflict #19

Closed heartless-clown closed 2 years ago

heartless-clown commented 2 years ago

Initially I proposed #14 as an attempt to solve this:

If your repo loads go_repository rules with outdated versions of staticcheck dependencies, your build fails with error like this one:

ERROR: /home/default/.cache/bazel/_bazel_default/ae7b499bfbe6014e30849362fa5591d9/external/co_honnef_go_tools/unused/BUILD.bazel:3:11: no such package '@org_golang_x_exp//typeparams': BUILD file not found in directory 'typeparams' of external repository @org_golang_x_exp. Add a BUILD file to a directory to mark it as a package. and referenced by '@co_honnef_go_tools//unused:unused'
ERROR: Analysis of target '//conflicting_deps_indirect:conflicting_deps_indirect' failed; build aborted: no such package '@org_golang_x_exp//typeparams': BUILD file not found in directory 'typeparams' of external repository @org_golang_x_exp. Add a BUILD file to a directory to mark it as a package.

Note that go_deps() rule that contains the outdated version of co_honnef_go_tools was placed after the staticcheck_deps().

Here is the example repo:

With the following commits:

  1. repo with originally proposed in #14 fix:
    • //:failure ignores nogo check and builds
    • //:conflicting_deps fails to build
    • //:conflicting_deps_indirect builds (but im guessing ignores nogo check too)
  2. with changes proposed by @sluongng at the end of #14:
    • //:failure fails to build
    • //:conflicting_deps fails to build
    • //:conflicting_deps_indirect fails to build

      guessing this is no different than loading staticcheck_deps after go_register_toolchains and other io_bazel_rules_go rules

  3. discovered workaround: let gazelle manage staticcheck deps:
    • //:failure fails to build as it should
    • //:conflicting_deps fails to build
    • //:conflicting_deps_indirect builds

Also here is the branch that sets up minimal test for dependency conflict:

sluongng commented 2 years ago

The way I have been avoiding conflicting deps when using go.work was to run go work sync to ensure that all the go.mod and go.sum files are in synced. 🤔 https://github.com/sluongng/nogo-analyzer/blob/master/tools/BUILD.bazel#L3-L13

So my workflow is usually something like:

code --> run gazelle --> run buildifier

if dep changes 
--> run go mod tidy in package 
--> run go work sync in workspace 
--> run gazelle update-repos
--> (optionally) run deps pruner (a custom tool i made to trim down unused go_repository)

This helps avoid all conflicting deps. Note how all these are quite manual today, I do plan to automate most of these, at least for this repository... but I am currently being blocked by a different effort in improving rules_go's bazel runfiles library first.


As regarding staticcheck version confusion, in a very early iteration of this repository (when I submitted this to rules_go and was adviced to create a separate repo instead), I did included this line:

Note that loading staticcheck_deps() is completely optional, advance users may want to manage their own staticcheck dependencies separately in your WORKSPACE file.

https://github.com/sluongng/rules_go/blob/0ee48782e07709b5b0df5c0634be05a250147d9a/go/analyzer/staticcheck/README.md

Perhaps we need to bring it back and give folks some example of simple setup? 🤔


Bazel's WORKSPACE load order could get very confusing which is why I remember this handy command:

> bazel query '//external:co_honnef_go_tools' --output=build

which helps validate which version is being used by the current workspace. I guess we can document this as well 🤔