Skip to content

Demo (not to be submitted): change to fully qualified paths.#10484

Draft
hzeller wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
hzeller:feature-20260521-full-path
Draft

Demo (not to be submitted): change to fully qualified paths.#10484
hzeller wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
hzeller:feature-20260521-full-path

Conversation

@hzeller
Copy link
Copy Markdown
Collaborator

@hzeller hzeller commented May 21, 2026

Just a demo of an implementation of #10449
not to be submitted in one go.

Done with

. <(~/bin/bant srcs-canonical -u -s | awk '{ printf("sed \047s|#include \"%s\"|#include \"%s\"|\047 -i $(rg -l \047#include.*\"%s\"\047)\n", $1, $2, $1); }')

This will compile on bazel, but will require CMake to be adapted to take ${OPENROAD_HOME} as include.

Just a demo of an implementation of The-OpenROAD-Project#10449
not to be submitted in one go.

Done with

```bash
. <(~/bin/bant srcs-canonical -u -s | awk '{ printf("sed \047s|#include \"%s\"|#include \"%s\"|\047 -i $(rg -l \047#include.*\"%s\"\047)\n", $1, $2, $1); }')
```

This will compile on bazel, but will require CMake to
be adapted to take `${OPENROAD_HOME}` as include.

Signed-off-by: Henner Zeller <h.zeller@acm.org>
@hzeller
Copy link
Copy Markdown
Collaborator Author

hzeller commented May 21, 2026

Just as draft, just to show how it would look like.

Compilation will work only in bazel with that, cmake first needs to be told to properly include from project root.

@hzeller
Copy link
Copy Markdown
Collaborator Author

hzeller commented May 21, 2026

FYI @QuantamHD

Copy link
Copy Markdown
Contributor

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

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 implements a comprehensive refactoring of include paths across the codebase, transitioning to fully qualified paths to improve consistency and build reliability. The review feedback identifies a missed path update in the docs/getPatch.py script and flags several instances of unrelated manual formatting changes that introduce unnecessary diff noise. The reviewer recommends reverting these manual styling adjustments and adhering to automated tools like clang-format for all code formatting to maintain project standards.

Comment thread docs/getPatch.py
'#include "utl/MakeLogger.h"',
'#include "utl/MakeLogger.h"\n#include "tool/MakeTool.hh"',
'#include "src/utl/include/utl/MakeLogger.h"',
'#include "src/utl/include/utl/MakeLogger.h"\n#include "tool/MakeTool.hh"',
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.

high

The added include #include "tool/MakeTool.hh" should be updated to use a fully qualified path, consistent with the rest of the refactoring (e.g., src/tool/include/tool/MakeTool.hh). If the include paths are restricted to the root directory, this relative path will likely cause build failures.

Suggested change
'#include "src/utl/include/utl/MakeLogger.h"\n#include "tool/MakeTool.hh"',
'#include "src/utl/include/utl/MakeLogger.h"\n#include "src/tool/include/tool/MakeTool.hh"',

GridX xh,
GridY yh)
= 0;
GridY yh) = 0;
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.

medium

This formatting change is unrelated to the main changes. Manual style changes should be avoided; please rely on clang-format for code formatting instead to maintain consistency and minimize diff noise.

                         GridY yh)\n      = 0;
References
  1. Rely on clang-format for code formatting instead of applying manual style changes.

utl::Logger* logger,
RouterConfiguration* router_cfg)
= 0;
RouterConfiguration* router_cfg) = 0;
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.

medium

This formatting change is unrelated to the main changes. Manual style changes should be avoided; please rely on clang-format for code formatting instead to maintain consistency and minimize diff noise.

                     RouterConfiguration* router_cfg)\n      = 0;
References
  1. Rely on clang-format for code formatting instead of applying manual style changes.

frLayerNum lNum,
int pinAccessIdx)
= 0;
int pinAccessIdx) = 0;
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.

medium

This formatting change is unrelated to the main changes. Manual style changes should be avoided; please rely on clang-format for code formatting instead to maintain consistency and minimize diff noise.

                                        int pinAccessIdx)\n      = 0;
References
  1. Rely on clang-format for code formatting instead of applying manual style changes.

virtual void searchNode(const FlexGridGraph* grid_graph,
const FlexWavefrontGrid& grid)
= 0;
const FlexWavefrontGrid& grid) = 0;
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.

medium

This formatting change is unrelated to the main changes. Manual style changes should be avoided; please rely on clang-format for code formatting instead to maintain consistency and minimize diff noise.

                          const FlexWavefrontGrid& grid)\n      = 0;
References
  1. Rely on clang-format for code formatting instead of applying manual style changes.

@hzeller
Copy link
Copy Markdown
Collaborator Author

hzeller commented May 21, 2026

I formatted it with clang-format 22.1.5 (the = 0 things look better). I suspect I should use 18.1.8 to avoid the change noise (but, doesn't matter in this change anyway, as it is only a demo)

@maliberty
Copy link
Copy Markdown
Member

This does have the downside of locking you into a particular directory structure (any changes will require another large update of includes). For example if we moved sta to third-party or we renamed src to openroad (similar to Verible).

What is the gain from this approach?

@hzeller
Copy link
Copy Markdown
Collaborator Author

hzeller commented May 21, 2026

What is the gain from this approach?

that it is unambiguous and make the project well-defined. Every bazel project is like that (the includes = [] stuff is just a hack around until it is possible to make things well-defined).

the fully qualified names make it possible to even change the filenames and move them around as every reference is fully qualified, so it is exactly clear what to rename.

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.

2 participants