OpenROAD-flow-scripts icon indicating copy to clipboard operation
OpenROAD-flow-scripts copied to clipboard

Allow to skip klayout related stages and error out early if not found in PATH

Open vvbandeira opened this issue 1 year ago • 3 comments

vvbandeira avatar Jan 23 '24 15:01 vvbandeira

Possible fix for #1494, based on initial work from #1499

vvbandeira avatar Jan 23 '24 15:01 vvbandeira

long time since I worked on this. I am unsure about all the usecases....

The two use cases I am trying to support with this PR are:

  1. A user wants to run the complete flow; it should fail fast if klayout is not present in the PATH instead of running routing and then failing.
  2. A user who does not want to use klayout (e.g., they only care about placement) should get the option to use the flow without installing klayout.

vvbandeira avatar Jan 23 '24 19:01 vvbandeira

I almost feel like this is getting overly complicated. The only point of getting klayout from the shell was originally to print out a clearer "KLayout isn't installed" message rather than the generic "Command klayout not found".

If you simply stick in klayout in the make target and avoid the shell completely, then you get the desired effect of it not failing until you try to run make finish, the only slight downside is that the message is the somewhat unhelpful command klayout not found

A user wants to run the complete flow; it should fail fast if klayout is not present

I actually disagree with this, I don't think there's any benefit to failing fast vs. failing slow. If it fails fast, the user has to go install KLayout, and no work is lost. If it fails slow, the user has to go install KLayout and no work is lost because it saved the state right before KLayout was called.

I don't really have guidance on what to do, I'm just offering perspective. I also don't know what problem

KLAYOUT_CMD := sh -c 'LD_LIBRARY_PATH=$(dir $(KLAYOUT_BIN_FROM_DIR)) $$0 "$$@"' $(KLAYOUT_BIN_FROM_DIR)

is trying to solve. That seems to be the core of the issue for #1494.

rovinski avatar Jan 24 '24 06:01 rovinski