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

Make collapsing opt-in #33

Open
eed3si9n opened this issue Apr 14, 2023 · 2 comments
Open

Make collapsing opt-in #33

eed3si9n opened this issue Apr 14, 2023 · 2 comments

Comments

@eed3si9n
Copy link
Contributor

steps

  • Import bunch of code from other build tools with more chunky modules
  • Regenerate

problem

Currently it's too easy for the generated BUILD.bazel to turn into a big ball of:

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_library")

filegroup(
    name = "shared_files",
    srcs = glob(include = ["**/*.scala"]),
    visibility = ["//visibility:public"],
)

scala_library(
    name = "shared",
    srcs = [
        ":shared_files",
        "//foo/shared/ante:ante_files",
        "//foo/shared/auctor:auctor_files",
        "//foo/shared/auctorgen:auctorgen_files",
        "//foo/shared/bibendum:bibendum_files",
        "//foo/shared/data:data_files",
        "//foo/shared/iaculis:iaculis_files",
        "//foo/shared/labelgen:labelgen_files",
        "//foo/shared/lectus/adaptor:adaptor_files",
        "//foo/shared/lectus:lectus_files",
        "//foo/shared/model:model_files",
        "//foo/shared/params:params_files",
        "//foo/shared/phasellus/placerat/lectus:lectus_files",
        "//foo/shared/phasellus/placerat:placerat_files",
        "//foo/shared/phasellus:phasellus_files",
        "//foo/shared/utils:utils_files",
    ],
    deps = [
       // huge list of 3rdparty dependencies
    ]
    ....

While it might be desirable at first to make one-big scala_library etc, the build is not healthy and hard to maintain and brittle, especially during migration, because transitively many targets would end up depending on the huge list of 3rdparty dependencies.

expectation

We should pass in an allow-list of 1:1:1 rule violators (in this case all the subdirectories above), so at least we are aware when it happens, and we can code review any new introduction of bad targets.

@weichunstevelee
Copy link
Contributor

+1, this would be really helpful and help with build time. It's hard to instruct people to stick to 1:1:1 and also be aware of not having cricular deps between targets (which will then force the build generator to rollup and generate a big ball target). Would be nice to force in the tooling

@ianoc
Copy link
Collaborator

ianoc commented Apr 28, 2023

I think having a whitelist is probably fine.. but its worth considering how much its worth the cognitive overhead on your users when/how you wish to deploy this. Most of the main offenders in that build as of when i last saw it were long time historical things. There isn't a huge amount of new targets added all the time. So feel free to add the flag, but I'd be less sure its ~worth blocking users inline as they work.

adhoc post reports could be done with bazel query or from the build tool to report these types of collapse for post-hoc cleanup.

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

No branches or pull requests

3 participants