Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vvbandeira
Copy link
Member

No description provided.

Signed-off-by: Vitor Bandeira <vvbandeira@precisioninno.com>
@vvbandeira
Copy link
Member Author

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

Copy link
Collaborator

@oharboe oharboe left a comment

Choose a reason for hiding this comment

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

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

@vvbandeira
Copy link
Member Author

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.

@rovinski
Copy link
Member

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.

@vvbandeira vvbandeira self-assigned this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants