The-OpenROAD-Project / The-OpenROAD-Project/OpenROAD

bazel: Fix dependencies to not need to disable layering_check

Open
#10,478 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Stale
Dominant language
Verilog
Stars
3.1k
Forks
1k
Avg merge
2d 23h
Merged PRs (30d)
136

Description

There are about 12 targets currently, that disable the layering check. This is because they include headers that are not exported (i.e. in hdrs = [...]) by any library but are private headers in srcs = [...].
(Note, there might also be other libraries that don't have the layering check disabled, but still accidentally compile due to wide use of includes = [...]; but let's focus on the layering check disabled targets first).

This should be fixed. Ideal would be to break out the targets that do have some private headers that are useful for other internal targets (the majority of these exceptions are currently on tests) break out into a separate library that is private in the package and allows visibility to the other internal targets that need it (if it is in the same package, nothing to do; if it is a test in a separate sub-directory: add visibility pattern to there (something like visibility = ["//src/web/test:__pkg__"].

To get a list of the affected targets:

$(etc/get-bant-path.sh) print ... -g "-layering_check"

Some of these targets are somewhat 'entangled' unfortunately, so often it is not possible to 'simple' break out a small portion, as they include yet other things. So the result might be breaking up things into more, smaller libraries, which will the code health by a lot, so it is very worthwhile.

I've done a bunch of these cleanups already but due to the entanglement and limited project insight, I can't always tell what would be good dependency boundaries to split out libraries. Passing to @maliberty to triage.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Run $(etc/get-bant-path.sh) print ... -g "-layering_check" to list the affected targets, then inspect their Bazel BUILD definitions and header dependencies. Identify private headers needed by other internal targets and separate them into appropriately visible libraries. Done means the affected targets no longer need layering_check disabled and their dependencies remain valid.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
build-system
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.