bazel: fix install script default path for standalone builds

[alokkumardalei-wq]

What does this PR do?

Fixes #9775
The install script now uses a standalone-friendly default path instead of assuming orfs directory structure.
Change default DEST_DIR from orfs-specific path to workspace-local path. The previous default assumed OpenROAD was part of OpenROAD-flow-scripts.

Users can now:

• Install to default: bazel run :install
• Install to custom: bazel run :install -- /custom/path

[gemini-code-assist]

Code Review

This pull request correctly updates the default installation path in the bazel/install.sh script, making it suitable for standalone builds by default. The change is simple and effectively addresses the issue of the previous path being specific to the OpenROAD-flow-scripts environment. I have one suggestion to improve the usage comment for better clarity.

[gemini-code-assist]

The usage comment is slightly incorrect. Arguments for a script executed with bazel run should be passed after --. Please update the comment to reflect the correct usage: bazel run :install -- [DEST_DIR]. This will prevent user confusion.

Suggested change

	# Usage: bazel run :install [DEST_DIR]
	# Usage: bazel run :install -- [DEST_DIR]

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[alokkumardalei-wq]

Hello @maliberty and @gadfort I think all checks are passing now please have a look onto this when you have time and I would be happy to address further feedback.

Thank you.

[oharboe]

this breaks the cd flow && make DESIGN_CONFIG... use-case

[alokkumardalei-wq]

Hello @oharboe, thanks for the review!
You're right — I didn't consider that the default path ../install/OpenROAD/bin is what ORFS expects when users run cd flow && make DESIGN_CONFIG=.... Changing it to bazel-install would break that lookup.

To fix issue #9775 without breaking the ORFS flow, I can update the script to auto-detect the context:
If ../flow/Makefile exists (i.e., we're inside an ORFS checkout), keep the current ORFS-compatible default (../install/OpenROAD/bin).
Otherwise (standalone OpenROAD repo), fall back to ./bazel-install.
This way both use-cases work out of the box. Would that approach work for you, or would you prefer a different solution?

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[alokkumardalei-wq]

Hello @oharboe I have implemented above solution for the issue you have described as no reply from your side for many days I thought I should go implement it and then ask to you if any other optimal approach I can go for .
Please have review when you have time in the commit db0da0b and I would be happy to address any other feedback.

Thank you!

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[oharboe]

I don't understand the use-case that this PR is solving.

[alokkumardalei-wq]

@oharboe The use-case this solves is for developers cloning OpenROAD directly as a standalone repository (outside of ORFS).

If a standalone user clones OpenROAD and types \bazel run :install, the current script blindly attempts to install the binaries to ../install/OpenROAD/bin. This creates an unwanted, messy installation folder entirely outside of the user's workspace tree.

This PR simply adds a clean standalone fallback: If the tool detects it is being executed inside an ORFS hierarchy (by checking for ../flow/Makefile), it preserves the exact legacy pipeline and installs to ../install/OpenROAD/bin(so 'cd flow && make' remains completely unbroken). If it is a standalone clone, it neatly installs to./bazel-install inside the workspace root.

[oharboe]

I see, yes, this is a suckerpunch. But will this fix the problem? Seems to me that we need to follow POLA

1. the user cloned only OpenROAD, totally reasonable
2. they ran install

What did they expect to happen?

[alokkumardalei-wq]

@oharboe You make a totally fair point about POLA!

If a brand new developer clones the standalone repo and runs \bazel run :installwithout passing an argument, they probably expect one of two things: either it drops into a standard user path like~/.local/bin, or it throws a clear usage error. They definitely don't expect it to silently create a random ../install/ folder entirely outside of their repository tree.

Since we have to keep that ../installfallback completely intact so we don't break the ORFSmakepipeline, does it make the most sense to **just throw a hard error for standalone users if they leave the path blank?** Something like:Error: Please specify an installation path (e.g. bazel run :install -- ~/.local/bin)?

Let me know if you agree with throwing a usage error, and I'll update the PR to enforce it!

[gadfort]

@alokkumardalei-wq I think fall back to .local/bin is reasonable.

[oharboe]

I think you're onto something. I'm wondering if we should always never install without a path, but that we can suggest the command to use if we think we're inside ORFS. Otherwise we need a robust check that OpenROAD is a submodule of ORFS...

[oharboe]

@alokkumardalei-wq I think fall back to .local/bin is reasonable.

I hate that idea. I don't want stale OpenROAD binaries in the path that I didn't specifically ask for.

[gadfort]

@oharboe I think I like the suggestion of requiring a path instead of assuming where users want them (it would require a small PR in ORFS if it's used there), but that would avoid any surprising behavior.

[oharboe]

Also, I think the use case of using ~/.local is perfectly reasnoable, perhaps we can print out some suggestions to clue in the user to pick a workflow that is right for them?

[alokkumardalei-wq]

@alokkumardalei-wq I think fall back to .local/bin is reasonable.

Hello @gadfort thanks for considering this but I think I should go with @oharboe suggestion of using ~/.local to show the suggestions directly to the users so that they can decide where to write their files rather automatically write stale binaries to ~/.local/bin.
I would update the PR if everyone agree with this.

Thank you!

[oharboe]

Thanks for working on this, @alokkumardalei-wq. The underlying issue (#9775) is real and worth fixing. I have concerns about the current implementation's readability and the direction it's heading relative to the discussion.

Background: why ../install/OpenROAD/bin exists

The current default path isn't arbitrary — it comes from the CMake-era build system. In ORFS, build_openroad.sh sets:

INSTALL_PATH="$(pwd)/tools/install"              # build_openroad.sh:21
OPENROAD_APP_ARGS+=" -D CMAKE_INSTALL_PREFIX=${INSTALL_PATH}/OpenROAD"  # :212

This creates the layout tools/install/OpenROAD/bin/openroad. The ORFS Make flow then picks up the binary from that exact path:

# flow/scripts/variables.mk:92
export OPENROAD_EXE ?= $(abspath $(FLOW_HOME)/../tools/install/OpenROAD/bin/openroad)

Since bazel/install.sh lives inside the OpenROAD submodule at tools/OpenROAD/, the relative path BUILD_WORKSPACE_DIRECTORY/../install/OpenROAD/bin resolves to tools/install/OpenROAD/bin — matching where CMake would have installed, so that cd flow && make just works.

The Bazel install script was written as a drop-in replacement for the CMake install path. That's important context that is completely invisible from reading the script itself.

The code doesn't communicate its intent

The core of this PR is a three-way branch:

if [ -n "$1" ]; then
    DEST_DIR="$1"
elif [ -f "$BUILD_WORKSPACE_DIRECTORY/../flow/Makefile" ]; then
    DEST_DIR="${BUILD_WORKSPACE_DIRECTORY}/../install/OpenROAD/bin"
else
    DEST_DIR="${BUILD_WORKSPACE_DIRECTORY}/bazel-install"
fi

To understand what this does, a reader has to:

1. Know that ../flow/Makefile is how you detect an ORFS checkout — there's nothing in the code that says so
2. Reverse-engineer that ../install/OpenROAD/bin matches the CMake CMAKE_INSTALL_PREFIX convention that ORFS's variables.mk depends on
3. Understand that bazel-install is the "standalone" fallback for when OpenROAD is cloned outside of ORFS

That's a lot of implicit knowledge baked into a file-existence check. A new contributor reading this script would have to trace through three repositories (build_openroad.sh → variables.mk → install.sh) to understand why these paths exist.

The heuristic is fragile

Checking for ../flow/Makefile ties this script to ORFS's internal directory layout. If ORFS restructures, renames, or someone has a stale/partial checkout, this silently picks the wrong path. Filesystem probing to detect "which project am I in" is a pattern that tends to accumulate more special cases over time.

The discussion has moved past this implementation

Reading the conversation, there seems to be emerging consensus on a cleaner approach: require an explicit path and print helpful suggestions when none is given. That approach:

• Follows POLA — no silent installation to a surprising location
• Eliminates the need for fragile heuristics
• Makes the script's behavior obvious from reading the code alone
• Puts the user in control

I'd suggest updating the PR to match that direction. Something like:

if [ -n "$1" ]; then
    DEST_DIR="$1"
else
    echo "Error: Please specify an installation path."
    echo ""
    echo "Examples:"
    echo "  bazel run :install -- ~/.local/bin    # Add to your PATH"
    echo "  bazel run :install -- ./build/install  # Project-local install"
    echo ""
    echo "If you are using ORFS (OpenROAD-flow-scripts), install to the path"
    echo "that flow/scripts/variables.mk expects:"
    echo ""
    echo "  bazel run :install -- \$(pwd)/../install/OpenROAD/bin"
    exit 1
fi

This is self-documenting: anyone reading the script immediately understands what it does and what choices the user has. The ORFS Makefile can pass the path explicitly (a small companion PR there), and standalone users get a clear prompt instead of a surprising default.

Document the CMake lineage in the script

Regardless of which approach is chosen, the CMake lineage and ORFS dependency should be documented right here in install.sh. This is the first file a developer will read when they wonder "why is it suggesting I install into ../install/OpenROAD/bin?" Today that answer is scattered across three files in two repositories. A block comment at the top of the script would save every future reader from having to do that archaeology:

# Install binary and runfiles from bazel build.
#
# ORFS context: When OpenROAD is checked out as a submodule of
# OpenROAD-flow-scripts at tools/OpenROAD/, the ORFS Make flow expects
# the binary at tools/install/OpenROAD/bin/openroad. This path
# originates from build_openroad.sh's CMAKE_INSTALL_PREFIX and is
# hardcoded in flow/scripts/variables.mk:
#
#   export OPENROAD_EXE ?= $(abspath $(FLOW_HOME)/../tools/install/OpenROAD/bin/openroad)
#
# The Bazel install target was created as a drop-in replacement for
# the CMake install step, so the suggested ORFS path below matches
# that convention.

This turns the implicit knowledge into explicit documentation, exactly where someone would look for it.

Minor: TARFILE is undefined

The original master version computes TARFILE via bazelisk info bazel-bin. The PR removes that line but still references $TARFILE in cp -f "$TARFILE" "$DEST_DIR". This would break direct execution outside of bazel run (which the new header comment says is supported).

[oharboe]

Need a bit more work...

[oharboe]

two more things:

• write bazelisk idiomatic _test to ensure that it works
• I think we should have a "-f" option to install it into the ORFS location. This communicates: I know what I'm doing and I have OR inside ORFS, install it so I an hack ORFS makefiles"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[alokkumardalei-wq]

@oharboe Thank you for the detailed feedback—you are completely right about avoiding fragile filesystem probes! I’ve updated the PR with exactly what you suggested:

Added the -f flag for ORFS and explicitly require paths otherwise.
Restored the TARFILE extraction logic.
Replaced the "archaeology" with a clear block comment detailing the CMAKE_INSTALL_PREFIX hook.
Added an idiomatic Bazel sh_test (install_test.sh) to prove the new argument parser safely handles empty paths and the -f flag.

bazelisk test //:install_test --test_output=all

Output:

alokkumardalei@Aloks-MacBook-Air OpenROAD % bazelisk test //:install_test --test_output=all

INFO: Invocation ID: 724606c7-db50-4338-a288-1881701e393f
INFO: Analyzed target //:install_test (0 packages loaded, 0 targets configured).
INFO: Found 1 test target...
Target //:install_test up-to-date:
  bazel-bin/install_test
INFO: Elapsed time: 0.328s, Critical Path: 0.15s
INFO: 1 process: 1 action cache hit, 1 internal.
INFO: Build completed successfully, 1 total action
PASSED: //:install_test (see /private/var/tmp/_bazel_alokkumardalei/f60303491871e2fbc066db2794eb7714/execroot/_main/bazel-out/darwin_arm64-opt/testlogs/install_test/test.log)
INFO: From Testing //:install_test
==================== Test output for //:install_test:
Running install.sh argument parser tests...
Mock TARFILE created at: /var/folders/g0/0tdm2_hj22l7kp855lz7xp9w0000gn/T/tmp.Nev0yspoIB/openroad.tar
--- Test 1: Explicit Destination ---
OpenROAD binary installed to /private/var/folders/g0/0tdm2_hj22l7kp855lz7xp9w0000gn/T/tmp.Nev0yspoIB/dest1
PASS: Test 1
--- Test 2: ORFS -f Flag ---
OpenROAD binary installed to /private/var/folders/g0/0tdm2_hj22l7kp855lz7xp9w0000gn/T/install/OpenROAD/bin
PASS: Test 2
--- Test 3: No Arguments Expected Failure ---
Error: Please specify an installation path.

Examples:
  bazel run :install -- ~/.local/bin    # Add to your PATH
  bazel run :install -- ./build/install  # Project-local install

If you are using ORFS (OpenROAD-flow-scripts), you can use the -f flag
to install to the path that flow/scripts/variables.mk expects:

  bazel run :install -- -f
PASS: Test 3 correctly aborted.
All tests passed successfully!
================================================================================
//:install_test                                                 (cached) PASSED in 0.1s

Executed 0 out of 1 test: 1 test passes.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[oharboe]

Author: @alokkumardalei-wq | Fixes: #9775 | State: Open, CHANGES_REQUESTED

Summary

The PR modifies bazel/install.sh to require an explicit install path (instead of
defaulting to the ORFS-relative ../install/OpenROAD/bin), adds a -f flag to
opt-in to the ORFS path, and adds a test. This addresses the original issue where
standalone OpenROAD users got a surprising install location.

Current state of the code vs. intent

What's good

1. Core logic matches the agreed direction. The discussion converged on "require
   an explicit path, print helpful suggestions" (oharboe + gadfort agreed). The
   latest commit implements exactly that with a clear error message.

2. The -f flag for ORFS was added per oharboe's request. It installs to
   ../install/OpenROAD/bin when the user explicitly asks for it.

3. The CMake lineage block comment was added verbatim from oharboe's suggestion,
   documenting the CMAKE_INSTALL_PREFIX / variables.mk connection.

4. TARFILE bug is fixed — the latest version uses
   TARFILE="${TARFILE:-$(cd "$BUILD_WORKSPACE_DIRECTORY" && bazelisk info bazel-bin)/openroad.tar}"
   which both restores the missing computation and allows override via env var.

5. A test exists (bazel/install_test.sh) covering three cases: explicit path,
   -f flag, and no-arg error.

Problems remaining

1. CI is failing: Buildifier lint

The Buildifier lint check is failing with:

bazel/BUILD:4: load: Loaded symbol "py_console_script_binary" is unused. Please remove it.

The PR removed the tclint/tclfmt py_console_script_binary targets from bazel/BUILD
but left the load() statement importing py_console_script_binary. This is a
one-line fix (remove the load), but it means CI is red right now.

2. Unrelated deletions in bazel/BUILD

The diff removes:

• exports_files entries for fix_lint.sh, tcl_lint_test.sh, tcl_fmt_test.sh, tcl_tidy.sh
• Both py_console_script_binary targets (tclint, tclfmt)

These are completely unrelated to the install script fix. On current master
these entries all exist and are used. Removing them will break other targets that
depend on them (e.g., tcl linting). This looks like the PR was based on an out-of-date
branch or these deletions were accidental. Either way, they need to be reverted.

3. Commit history is messy

Three commits, including a merge commit (Merge branch 'master' into fix/...). The
second commit message ("Fix missing parameter test using standard bash conventions")
doesn't match what the commit actually does (the major rewrite implementing -f, error
messages, tests, etc.). Should be squashed/rebased into a clean history before merge.

4. Test could be more robust

The test (install_test.sh) is reasonable but has minor issues:

• It creates a temp dir but never cleans it up (trap ... EXIT missing)
• The test runs ./bazel/install.sh with a relative path, which works in CI but
  is fragile — it should use $TEST_SRCDIR / runfiles for proper Bazel test hygiene
• No test that -f actually resolves to the expected absolute path (it checks
  directory existence but not the path value)

5. The -f flag semantics need clarification

-f is traditionally "force" in Unix tools. Using it to mean "install to ORFS
location" is surprising. A more descriptive flag like --orfs or even just
documenting a positional convention would be clearer. This is a minor point but
worth raising given the POLA theme of the PR.

6. $BUILD_WORKSPACE_DIRECTORY fallback to $PWD

The line BUILD_WORKSPACE_DIRECTORY="${BUILD_WORKSPACE_DIRECTORY:-$PWD}" enables
running outside bazel run, but the error message only shows bazel run :install
examples. If someone runs ./bazel/install.sh directly, the ORFS suggestion
(bazel run :install -- -f) won't work. Either remove the direct-execution support
or add a note about it.

Comparison with origin/master

On master, bazel/install.sh is 20 lines with a single unconditional default path.
The PR grows it to 42 lines. The bazel/BUILD changes on the PR are destructive
to master (removing lint tooling that exists and is used). The bazel/BUILD changes
must be reverted to just add install_test.sh to exports_files.

Recommended actions for the contributor

1. Fix the Buildifier lint failure — remove the unused py_console_script_binary load,
   OR better yet, revert all unrelated bazel/BUILD changes and only add the test file export.
2. Revert the exports_files and py_console_script_binary deletions — these are
   unrelated and will break tcl linting.
3. Squash commits into one clean commit with a descriptive message.
4. Consider renaming -f to something more descriptive (or document clearly in --help).
5. Add cleanup trap in the test script.

Recommended actions for reviewers

• Don't merge until the bazel/BUILD regressions are fixed — they'll break tcl linting.
• The core install.sh logic is sound and matches the discussed direction.
• A companion ORFS PR will be needed to pass the explicit path in the Makefile
  (as gadfort noted).

---

bazel-orfs linting: local vs. CI and advice

The bazel-orfs repo has a clean, simple linting setup worth noting as a contrast:

How it works

• CI (.github/workflows/lint.yml):
  runs bazelisk run //:fix_lint then checks git diff --exit-code. If there's a
  diff, the lint check fails. Single source of truth.

• Local: developers run the same bazelisk run //:fix_lint before committing.
  The fix_lint target handles buildifier (respecting .bazelignore), black, and
  lockfile regeneration.

• ci.bazelrc: CI imports .github/ci.bazelrc via user.bazelrc to handle
  module overrides. Currently this file is empty (heavy deps moved to sub-modules),
  but the mechanism exists for when it's needed.

Advice for avoiding linting hell

1. Run bazelisk run //:fix_lint locally before pushing. This is the exact
   same command CI runs. If it produces no diff locally, CI won't fail either
   (assuming the ci.bazelrc trick from CLAUDE.md is applied for lockfile changes).

2. For MODULE.bazel changes, follow the CLAUDE.md instructions:

   echo 'import %workspace%/.github/ci.bazelrc' >> user.bazelrc
   bazelisk mod tidy
   git checkout user.bazelrc

   This ensures the lockfile matches what CI generates with its overrides.

3. The pattern is: fix, not check. bazel-orfs doesn't have separate "check"
   and "fix" steps — it just fixes and diffs. This means you can never get into a
   state where the linter tells you something is wrong but you don't know how to
   fix it. You just run the fixer.

4. For OpenROAD PRs like bazel: fix install script default path for standalone builds #9796, the same principle applies: run buildifier on
   any changed .bzl/BUILD files before pushing. The PR's Buildifier lint failure
   is exactly the kind of thing caught by running the formatter locally first.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 21 days if no further activity occurs. Remove the Stale label or comment to keep it open.