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

Conditionally Set KLAYOUT_VERSION Based on KLAYOUT_CMD #2219

Merged

Conversation

ivysochyn
Copy link
Contributor

Conditionally Set KLAYOUT_VERSION Based on KLAYOUT_CMD

This PR updates the flow/Makefile to conditionally set the KLAYOUT_VERSION variable only if KLAYOUT_CMD is set. This change ensures that the KLAYOUT_VERSION assignment is performed safely and only when KLAYOUT_CMD is defined, preventing potential errors when KLAYOUT_CMD is not set. These errors usually result in noisy logs, but they don't crash the build.

An example error typically observed during the synth stage:

bash: - : invalid option
Usage:  bash [GNU long option] [option] ...
    bash [GNU long option] [option] script-file ...
GNU long options:
    --debug
    --debugger
    --dump-po-strings
    --dump-strings
    --help
    --init-file
    --login
    --noediting
    --noprofile
    --norc
    --posix
    --pretty-print
    --rcfile
    --restricted
    --verbose
    --version
Shell options:
    -ilrsD or -c command or -O shopt_option     (invocation only)
    -abefhkmnptuvxBCHP or -o option

This PR modifies the following:

  • flow/Makefile - Updates the assignment of KLAYOUT_VERSION to use the $(if ...) function to check if KLAYOUT_CMD is set before executing the shell command. Also, fixes typos.

Updated the Makefile to use the $(if ...) function to conditionally set
the KLAYOUT_VERSION variable only if KLAYOUT_CMD is set.

- Previous method directly assigned KLAYOUT_VERSION using a shell command.
- If KLAYOUT_CMD is not set, the shell command would fail and cause an
  error printed to the console.

Fixes typos.

Signed-off-by: Illia Vysochyn <ivysochyn@antmicro.com>
@oharboe oharboe requested a review from maliberty August 6, 2024 08:54
@oharboe
Copy link
Collaborator

oharboe commented Aug 6, 2024

Tested. Works now.

Before:

$ make KLAYOUT_CMD= print-KLAYOUT_VERSION
bash: - : invalid option
Usage:	bash [GNU long option] [option] ...
	bash [GNU long option] [option] script-file ...
GNU long options:
	--debug
	--debugger
	--dump-po-strings
	--dump-strings
	--help
	--init-file
	--login
	--noediting
	--noprofile
	--norc
	--posix
	--pretty-print
	--rcfile
	--restricted
	--verbose
	--version
Shell options:
	-ilrsD or -c command or -O shopt_option		(invocation only)
	-abefhkmnptuvxBCEHPT or -o option
KLAYOUT_VERSION = 
$

After:

$ make KLAYOUT_CMD= print-KLAYOUT_VERSION
KLAYOUT_VERSION = 
$

@maliberty maliberty merged commit 0d9324b into The-OpenROAD-Project:master Aug 6, 2024
6 checks passed
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.

3 participants