jvolkman / rules_pycross

Bazel + Python rules for cross-platform external dependencies
Apache License 2.0
60 stars 23 forks source link

generated loads are out of order for buildifier #116

Open chulkilee opened 2 months ago

chulkilee commented 2 months ago

The generated load is out of order for buildifier

Generated:

load("@@rules_pycross~//pycross:defs.bzl", "pycross_wheel_library", "pypi_file")
load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")

Formatted:

load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
load("@@rules_pycross~//pycross:defs.bzl", "pycross_wheel_library", "pypi_file")

One option is to add # buildifier: disable=out-of-order-load. However it seems like it tries to put that in order on purpose ("# External repo loads come before local loads." - but I'm wondering if it's even necessary to keep the order, as buildifier suggested to reorder it.

https://github.com/bazelbuild/buildtools/blob/main/WARNINGS.md#load-statements-should-be-ordered-by-their-labels

jvolkman commented 2 months ago

It was definitely correct at one point, but it looks like the addition of @@ with bzlmod support broke it.

If you're able, can you figure out if @@ always comes after @ according to buildifier, or if all of the lines are ordered by their repo names?

I.e., should it be

load("@@bar//:bar.bzl", "bar")
load("@foo//:foo.bzl", "foo")
load("@@qux//:qux.bzl", "qux")

or

load("@foo//:foo.bzl", "foo")
load("@@bar//:bar.bzl", "bar")
load("@@qux//:qux.bzl", "qux")
jvolkman commented 2 months ago

I just tested, and it's the first way. buildifier seems to order them by repo name, not by the label string.