Fix layering check issues in rsz

[hzeller]

For that, we have to pull out the headers that are used in the tests and provide in the visible headers.

Note, this is only the first step, the next one has to be to put the headers and sources in small libraries with appropriate visibility to build up the libraries from there. I've added a comment to that effect.

Now that all headers are specified in the tests and can be accounted for from a library that provides them, this gave the build cleaner the opportunity to remove superfluous libraries in the dependencies that it previously conservatively held on to as it didn't know which library to remove.

Issues: #10478

[gemini-code-assist]

Code Review

This pull request refactors the Bazel build configuration for the rsz library and its tests. It moves several private header files from srcs to hdrs in src/rsz/BUILD to make them accessible to tests. This allows the removal of the -layering_check feature flag and several unused dependencies from test targets in src/rsz/test/BUILD. There are no review comments, so I have no feedback to provide.

[hzeller]

Behind the scenes: To find the headers that were not accounted for, I used the double verbosity-vv in that directory

etc/run-build-cleaner.sh src/rsz/... -vv

Giving a useful feedback, which essentially just is a TODO list

src/rsz/test/BUILD:484:14-31:[ src/rsz/test/cpp/TestResizer.cc include dependency check (//src/rsz/test:TestResizer) ]
src/rsz/test/cpp/TestResizer.cc:8:11-26: #include "MoveCommitter.hh" (unknown provider)
src/rsz/test/cpp/TestResizer.cc:8:11-26:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizer.cc:9:11-27: #include "OptimizerTypes.hh" (unknown provider)
src/rsz/test/cpp/TestResizer.cc:9:11-27:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizer.cc:10:11-34: #include "RepairTargetCollector.hh" (unknown provider)
src/rsz/test/cpp/TestResizer.cc:10:11-34:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizer.cc:12:11-31: #include "move/MoveGenerator.hh" (unknown provider)
src/rsz/test/cpp/TestResizer.cc:12:11-31:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/BUILD:495:10-18: //src/ant dependency looks superfluous in //src/rsz/test:TestResizer, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:497:10-30: //src/dbSta:dbNetwork dependency looks superfluous in //src/rsz/test:TestResizer, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:496:10-20: //src/dbSta dependency looks superfluous in //src/rsz/test:TestResizer, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:498:10-18: //src/dpl dependency looks superfluous in //src/rsz/test:TestResizer, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:499:10-18: //src/est dependency looks superfluous in //src/rsz/test:TestResizer, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:500:10-18: //src/grt dependency looks superfluous in //src/rsz/test:TestResizer, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:505:10-18: //src/stt dependency looks superfluous in //src/rsz/test:TestResizer, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:506:10-18: //src/tst dependency looks superfluous in //src/rsz/test:TestResizer, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:508:10-18: //src/utl dependency looks superfluous in //src/rsz/test:TestResizer, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:546:14-37:[ src/rsz/test/cpp/TestNestedJournal.cc include dependency check (//src/rsz/test:TestNestedJournal) ]
src/rsz/test/cpp/TestNestedJournal.cc:25:11-26: #include "MoveCandidate.hh" (unknown provider)
src/rsz/test/cpp/TestNestedJournal.cc:25:11-26:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestNestedJournal.cc:26:11-26: #include "MoveCommitter.hh" (unknown provider)
src/rsz/test/cpp/TestNestedJournal.cc:26:11-26:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestNestedJournal.cc:27:11-24: #include "MoveTracker.hh" (unknown provider)
src/rsz/test/cpp/TestNestedJournal.cc:27:11-24:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestNestedJournal.cc:28:11-27: #include "OptimizerTypes.hh" (unknown provider)
src/rsz/test/cpp/TestNestedJournal.cc:28:11-27:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/BUILD:561:10-30: //src/sta:opensta_lib dependency looks superfluous in //src/rsz/test:TestNestedJournal, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:563:10-18: //src/utl dependency looks superfluous in //src/rsz/test:TestNestedJournal, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:571:14-33:[ src/rsz/test/cpp/TestResizerMt.cc include dependency check (//src/rsz/test:TestResizerMt) ]
src/rsz/test/cpp/TestResizerMt.cc:21:11-27: #include "DelayEstimator.hh" (unknown provider)
src/rsz/test/cpp/TestResizerMt.cc:21:11-27:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizerMt.cc:22:11-31: #include "RepairSetupContext.hh" (unknown provider)
src/rsz/test/cpp/TestResizerMt.cc:22:11-31:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizerMt.cc:26:11-32: #include "SetupLegacyMtPolicy.hh" (unknown provider)
src/rsz/test/cpp/TestResizerMt.cc:26:11-32:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizerMt.cc:29:11-26: #include "MoveCommitter.hh" (unknown provider)
src/rsz/test/cpp/TestResizerMt.cc:29:11-26:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizerMt.cc:30:11-27: #include "OptimizerTypes.hh" (unknown provider)
src/rsz/test/cpp/TestResizerMt.cc:30:11-27:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizerMt.cc:31:11-30: #include "VtSwapMtCandidate.hh" (unknown provider)
src/rsz/test/cpp/TestResizerMt.cc:31:11-30:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizerMt.cc:50:11-26: #include "MoveGenerator.hh" (unknown provider)
src/rsz/test/cpp/TestResizerMt.cc:50:11-26:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/cpp/TestResizerMt.cc:54:11-27: #include "SetupMt1Policy.hh" (unknown provider)
src/rsz/test/cpp/TestResizerMt.cc:54:11-27:    ?      ^  -- Missing or from non-standard bazel-rule ?
src/rsz/test/BUILD:582:10-18: //src/ant dependency looks superfluous in //src/rsz/test:TestResizerMt, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:585:10-18: //src/dpl dependency looks superfluous in //src/rsz/test:TestResizerMt, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:586:10-18: //src/est dependency looks superfluous in //src/rsz/test:TestResizerMt, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:587:10-18: //src/grt dependency looks superfluous in //src/rsz/test:TestResizerMt, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:591:10-18: //src/stt dependency looks superfluous in //src/rsz/test:TestResizerMt, but there are also unaccounted sources. Won't remove.
src/rsz/test/BUILD:592:10-18: //src/tst dependency looks superfluous in //src/rsz/test:TestResizerMt, but there are also unaccounted sources. Won't remove.
Checked DWYU on 14 targets.

[ Summary of includes that were seen in sources but no known libraries providing them. ]

Debugging tip to narrow (Also note, some might be benign, e.g. behind #ifdefs)

        $ bant print -m ... -g <include>  # prints any rule mentioning header

DelayEstimator.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestResizerMt.cc:21:11-27: in //src/rsz/test:TestResizerMt
MoveCandidate.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestNestedJournal.cc:25:11-26: in //src/rsz/test:TestNestedJournal
MoveCommitter.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestNestedJournal.cc:26:11-26: in //src/rsz/test:TestNestedJournal
        src/rsz/test/cpp/TestResizer.cc:8:11-26: in //src/rsz/test:TestResizer
        src/rsz/test/cpp/TestResizerMt.cc:29:11-26: in //src/rsz/test:TestResizerMt
MoveGenerator.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestResizerMt.cc:50:11-26: in //src/rsz/test:TestResizerMt
MoveTracker.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestNestedJournal.cc:27:11-24: in //src/rsz/test:TestNestedJournal
OptimizerTypes.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestNestedJournal.cc:28:11-27: in //src/rsz/test:TestNestedJournal
        src/rsz/test/cpp/TestResizer.cc:9:11-27: in //src/rsz/test:TestResizer
        src/rsz/test/cpp/TestResizerMt.cc:30:11-27: in //src/rsz/test:TestResizerMt
RepairSetupContext.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestResizerMt.cc:22:11-31: in //src/rsz/test:TestResizerMt
RepairTargetCollector.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestResizer.cc:10:11-34: in //src/rsz/test:TestResizer
SetupLegacyMtPolicy.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestResizerMt.cc:26:11-32: in //src/rsz/test:TestResizerMt
SetupMt1Policy.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestResizerMt.cc:54:11-27: in //src/rsz/test:TestResizerMt
VtSwapMtCandidate.hh (file without path will not be fuzzy-matched)
        src/rsz/test/cpp/TestResizerMt.cc:31:11-30: in //src/rsz/test:TestResizerMt
move/MoveGenerator.hh
        src/rsz/test/cpp/TestResizer.cc:12:11-31: in //src/rsz/test:TestResizer

[hzeller]

Ping.

[maliberty]

We have already created private_hdrs in cts, dpl, drt, gpl, grt, etc. Is there a reason not to do that here?

[hzeller]

Making it private headers is the wrong approach, as we see in the other examples: it detaches the code from the header and just makes something that then accidentally compiles.

Instead, the header and corresponding code need to go together in a library. If the code is well-structured, this might just be the *.cc and the *.h, but since this was not in focus before, it often is more work. So this is why in this first step want to just break out the headers to be visible, and then tackle the proper breakup in libraries.

[maliberty]

It appears you are giving private headers global visibility just to allow the unit tests to see them.

[hzeller]

yes, as a first step. The actual fix will follow, actually making these proper separate libraries as they should be. Then we can control the visibility of the headers seen there for the libraries that need them.