Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: error in invoke_tool when running vivado.bat on Windows #1559

Merged
merged 1 commit into from
Feb 1, 2025

Conversation

wipeseals
Copy link
Contributor

@wipeseals wipeseals commented Feb 1, 2025

Amaranth version

0.5.3

minimal program that demonstrates

  • OS: Windows 11
  • PATH includes: C:\xilinx\Vivado\2024.2\bin, C:\xilinx\Vivado\2024.2\xsct-trim\bin

run ArtyA7_35Platform().build(Blinky(), do_build=True, do_program=True)

'"E:\repos\bonsai\build/setupEnv.bat"' は、内部コマンドまたは外部コマンド、
操作可能なプログラムまたはバッチ ファイルとして認識されていません。
'"/setupEnv.bat"' は、内部コマンドまたは外部コマンド、
操作可能なプログラムまたはバッチ ファイルとして認識されていません。
'"/loader.bat"' は、内部コマンドまたは外部コマンド、
操作可能なプログラムまたはバッチ ファイルとして認識されていません。
Traceback (most recent call last):
  File "E:\repos\bonsai\bonsai\cli.py", line 63, in <module>
    main()
  File "E:\repos\bonsai\bonsai\cli.py", line 57, in main
    test_arty_a7_35_platform()
  File "E:\repos\bonsai\bonsai\cli.py", line 30, in test_arty_a7_35_platform
    ArtyA7_35Platform().build(Blinky(), do_build=True, do_program=True)
  File "E:\repos\bonsai\.venv\Lib\site-packages\amaranth\build\plat.py", line 103, in build
    products = plan.execute_local(build_dir)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "E:\repos\bonsai\.venv\Lib\site-packages\amaranth\build\run.py", line 115, in execute_local
    subprocess.check_call(["cmd", "/c", f"call {self.script}.bat"],
  File "C:\Users\user\AppData\Local\Programs\Python\Python312\Lib\subprocess.py", line 413, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['cmd', '/c', 'call build_top.bat']' returned non-zero exit status 1.

What you expected to happen, and what actually happened

The problem seems to be caused by vivado.bat on Windows.
Here is the result of my experiment by modifying the build_{name}.bat generated by platform.py.

@rem Automatically generated by Amaranth 0.5.3.
@echo off
SetLocal EnableDelayedExpansion
if defined AMARANTH_ENV_VIVADO call “%AMARANTH_ENV_VIVADO%”
if [!VIVADO!] equ [“”] set VIVADO=
if [!VIVADO!] equ [“”] set VIVADO=vivado

-“%VIVADO%” -mode batch -log top.log -source top.tcl || exit /b
+%VIVADO% -mode batch -log top.log -source top.tcl || exit /b

I am not familiar with batch files, but there seems to be a behavior difference between calling vivado and “vivado”.

In the invoked vivado.bat, pushd “%~dp0” failed to move to the vivado.bat directory and _RDI_BINROOT became invalid and vivado did not run as expected.

However, the fix to remove the double quotes did not work properly in cases where the full path to vivado.bat was manually specified in VIVADO and there was a blank space, so the fix is call %VIVADO%.

@whitequark
Copy link
Member

invoke_tool is used to run non-batch-files too. Does it work if the tool is an .exe?

@wipeseals
Copy link
Contributor Author

Thanks for reading! I have confirmed that the .exe works as follows.

[OSS CAD Suite] C:\Users\user>set YOSYS_PATH="C:\bin\oss-cad-suite\bin\yosys.exe”

[OSS CAD Suite] C:\Users\user>%YOSYS_PATH% -V
Yosys 0.49+3 (git sha1 3d35f367c, x86_64-w64-mingw32-g++ 13.2.1 -O3)

[OSS CAD Suite] C:\Users\user>%YOSYS_PATH%” -V
Yosys 0.49+3 (git sha1 3d35f367c, x86_64-w64-mingw32-g++ 13.2.1 -O3)

[OSS CAD Suite] C:\Users\user>call %YOSYS_PATH% -V
Yosys 0.49+3 (git sha1 3d35f367c, x86_64-w64-mingw32-g++ 13.2.1 -O3)

Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

Looks good. Could you please change the commit message to start with a module name, like this:

build.plat: fix error in invoke_tool when running vivado.bat on Windows

After that I'll merge it.

@whitequark
Copy link
Member

Thanks!

@whitequark whitequark added this pull request to the merge queue Feb 1, 2025
Merged via the queue into amaranth-lang:main with commit 4a242ff Feb 1, 2025
18 of 19 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants