Skip to content

cts: make private_hdrs self-contained instead of borrowing cts headers#10891

Closed
maliberty wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:cts-private-hdrs-self-contained
Closed

cts: make private_hdrs self-contained instead of borrowing cts headers#10891
maliberty wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:cts-private-hdrs-self-contained

Conversation

@maliberty

Copy link
Copy Markdown
Member

private_hdrs headers (HTreeBuilder.h, SinkClustering.h, TreeBuilder.h, LatencyBalancer.h) include CtsOptions.h and TechChar.h, which were declared in the cts target. Since cts already depends on private_hdrs, the dependency actually runs the other way at the header level.

Move CtsOptions.h and TechChar.h into private_hdrs and give it the deps those headers require (dbSta, dbNetwork, est, rsz, odb, utl). This keeps the acyclic direction cts -> private_hdrs; adding cts as a dep of private_hdrs would instead create a cycle.

Relates to #10889

@maliberty maliberty self-assigned this Jul 12, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the src/cts/BUILD file by moving src/CtsOptions.h and src/TechChar.h from the public headers (hdrs) list to the internal sources list. Additionally, it introduces several new dependencies to the cc_library target, including dbSta, dbNetwork, est, odb, rsz, and utl. There are no review comments, and I have no feedback to provide.

@maliberty

Copy link
Copy Markdown
Member Author

@hzeller FYI

@maliberty

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 547f8b811c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

private_hdrs headers (HTreeBuilder.h, SinkClustering.h, TreeBuilder.h,
LatencyBalancer.h) include CtsOptions.h and TechChar.h. Since cts already
depends on private_hdrs, the header dependency actually runs the other way,
and it only compiled because header-only targets are not built standalone
under layering_check.

Give private_hdrs ownership of CtsOptions.h and TechChar.h (they belong with
the cluster -- CtsOptions.h itself includes CtsObserver.h and Util.h) plus
the deps those headers need (dbSta, dbNetwork, est, rsz, odb, utl). This
makes private_hdrs self-contained; adding cts as a dep of private_hdrs would
instead create a cycle.

Keep the two headers listed in cts.hdrs as well so cts re-exports them to
consumers that depend on cts directly (e.g. the aggregated swig-py module),
which layering_check requires.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@openroad-ci openroad-ci force-pushed the cts-private-hdrs-self-contained branch from 547f8b8 to e06011e Compare July 12, 2026 15:48
@maliberty

Copy link
Copy Markdown
Member Author

needs more thought

@maliberty maliberty closed this Jul 12, 2026
@maliberty maliberty deleted the cts-private-hdrs-self-contained branch July 12, 2026 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant