Demo (not to be submitted): change to fully qualified paths.#10484
Demo (not to be submitted): change to fully qualified paths.#10484hzeller wants to merge 1 commit into
Conversation
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>
|
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. |
|
FYI @QuantamHD |
There was a problem hiding this comment.
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.
| '#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"', |
There was a problem hiding this comment.
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.
| '#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; |
There was a problem hiding this comment.
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
- Rely on clang-format for code formatting instead of applying manual style changes.
| utl::Logger* logger, | ||
| RouterConfiguration* router_cfg) | ||
| = 0; | ||
| RouterConfiguration* router_cfg) = 0; |
There was a problem hiding this comment.
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
- Rely on clang-format for code formatting instead of applying manual style changes.
| frLayerNum lNum, | ||
| int pinAccessIdx) | ||
| = 0; | ||
| int pinAccessIdx) = 0; |
There was a problem hiding this comment.
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
- 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; |
There was a problem hiding this comment.
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
- Rely on clang-format for code formatting instead of applying manual style changes.
|
I formatted it with clang-format 22.1.5 (the |
|
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? |
that it is unambiguous and make the project well-defined. Every bazel project is like that (the 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. |
Just a demo of an implementation of #10449
not to be submitted in one go.
Done with
This will compile on bazel, but will require CMake to be adapted to take
${OPENROAD_HOME}as include.