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.

Sign up for GitHub

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

Jump to bottom

build.run: Ensure batch script returns proper error code. #149

Merged
merged 1 commit into from Jul 14, 2019
Merged

build.run: Ensure batch script returns proper error code. #149

merged 1 commit into from Jul 14, 2019

Conversation

cr1901
Copy link
Contributor

@cr1901 cr1901 commented Jul 14, 2019

The Windows command line is a mess.

When trying to synthesize some nmigen code today, I ran into a nextpnr problem that caused the whole build to fail. The batch script correctly exited early, but CalledProcessError was never raised (meaning return code from check_call was 0). Since I had do_program=True, plat.build continued, and this cascaded into more errors:

William@WILLIAM-THINK C:\msys64\home\William\src\icebreaker-nmigen-examples\pdm_fade_gamma
> python3 gamma_pdm.py
ERROR: Top-level input 'I0' also driven by I0.O.
ERROR: Loading design failed.
0 warnings, 2 errors
Traceback (most recent call last):
  File "C:\msys64\home\william\src\nmigen\nmigen\build\run.py", line 120, in extract
    file.write(self.get(filename))
  File "C:\msys64\home\william\src\nmigen\nmigen\build\run.py", line 143, in get
    with open(os.path.join(self.__root, filename), "r" + mode) as f:
FileNotFoundError: [Errno 2] No such file or directory: 'C:\\msys64\\home\\William\\src\\icebreaker-nmigen-examples\\pdm_fade_gamma\\build\\top.bin'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "gamma_pdm.py", line 169, in <module>
    plat.build(Top(), do_program=True)
  File "C:\msys64\home\william\src\nmigen\nmigen\build\plat.py", line 55, in build
    self.toolchain_program(products, name, **(program_opts or {}))
  File "C:\msys64\home\william\src\nmigen-boards\nmigen_boards\icebreaker.py", line 80, in toolchain_program
    with products.extract("{}.bin".format(name)) as bitstream_filename:
  File "C:\msys64\mingw64\lib\python3.7\contextlib.py", line 112, in __enter__
    return next(self.gen)
  File "C:\msys64\home\william\src\nmigen\nmigen\build\run.py", line 131, in extract
    os.unlink(file.name)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\William\\AppData\\Local\\Temp\\nmigen_q17qki6z_top.bin'

After doing some research, I found prepending call to the batch file command fixes this error. This SO answer goes into detail about how rather screwed up error code returning is on Windows, and is based on someone's assembly-level (!) debugging of cmd.exe.

With this patch, and with nothing else changes, nmigen now raises CalledProcessError if the batch script fails:

William@WILLIAM-THINK C:\msys64\home\William\src\icebreaker-nmigen-examples\pdm_fade_gamma
> python3 gamma_pdm.py
ERROR: Top-level input 'I0' also driven by I0.O.
ERROR: Loading design failed.
0 warnings, 2 errors
Traceback (most recent call last):
  File "gamma_pdm.py", line 169, in <module>
    plat.build(Top(), do_program=True)
  File "C:\msys64\home\william\src\nmigen\nmigen\build\plat.py", line 51, in build
    products = plan.execute_local(build_dir)
  File "C:\msys64\home\william\src\nmigen\nmigen\build\run.py", line 74, in execute_local
    subprocess.check_call(["cmd", "/c", "call {}.bat".format(self.script)])
  File "C:\msys64\mingw64\lib\python3.7\subprocess.py", line 347, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['cmd', '/c', 'call build_top.bat']' returned non-zero exit status 4294967295.

@codecov
Copy link

codecov bot commented Jul 14, 2019 •
edited

Codecov Report

Merging #149 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #149   +/-   ##
=======================================
  Coverage   80.67%   80.67%           
=======================================
  Files          32       32           
  Lines        5217     5217           
  Branches     1129     1129           
=======================================
  Hits         4209     4209           
  Misses        873      873           
  Partials      135      135
Impacted Files Coverage Δ
nmigen/build/run.py 32.35% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee15538...50e01cb. Read the comment docs.

@whitequark whitequark merged commit b963449 into m-labs:master Jul 14, 2019
@nmigen-issue-migration nmigen-issue-migration mentioned this pull request Jan 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@cr1901 @whitequark