From 5ded24fa3fb4c9ec5d7f0f85c4c31ed3fe25d084 Mon Sep 17 00:00:00 2001 From: Sahil Lenka Date: Sat, 3 Oct 2026 04:49:44 +0530 Subject: [PATCH] fix(mkconcore): exit with status 1 on generation errors mkconcore.py used quit() on every error path, which exits with status 0. concore build only checks the return code, so errors like an unsupported file extension or duplicate node labels were reported as a successful build, STUDY.json got written into a half generated study, and the actual error message was never printed. Use sys.exit(1) on the error paths so build sees the failure. The quit() at the end of docker generation is a normal exit and is left as is. --- mkconcore.py | 28 ++++++++++++++-------------- tests/test_cli.py | 27 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/mkconcore.py b/mkconcore.py index 987eadf..30980a3 100644 --- a/mkconcore.py +++ b/mkconcore.py @@ -259,7 +259,7 @@ def _resolve_concore_path(): if not os.path.isdir(sourcedir): logging.error(f"{sourcedir} does not exist") - quit() + sys.exit(1) if len(sys.argv) == 4: # Use only the output directory name in generated prefixes. @@ -269,7 +269,7 @@ def _resolve_concore_path(): concoretype = sys.argv[4] if not (concoretype in ["posix","windows","docker","macos","ubuntu"]): logging.error(" type must be posix (macos or ubuntu), windows, or docker") - quit() + sys.exit(1) ubuntu = False #6/24/21 if concoretype == "ubuntu": concoretype = "posix" @@ -280,7 +280,7 @@ def _resolve_concore_path(): if os.path.exists(outdir): logging.error(f"{outdir} already exists") logging.error(f"if intended, Remove/Rename {outdir} first") - quit() + sys.exit(1) os.makedirs(outdir) os.chdir(outdir) @@ -366,7 +366,7 @@ def cleanup_script_files(): duplicates = {label for label in label_values if label_values.count(label) > 1} if duplicates: logging.error(f"Duplicate node labels found: {sorted(duplicates)}") - quit() + sys.exit(1) for edge in edges_text: try: @@ -623,7 +623,7 @@ def cleanup_script_files(): fsource = open(CONCOREPATH+"/concore.py") except (FileNotFoundError, IOError): print(CONCOREPATH+" is not correct path to concore (missing python files)") - quit() + sys.exit(1) with open(outdir+"/src/concore.py","w") as fcopy: fcopy.write(fsource.read()) fsource.close() @@ -633,14 +633,14 @@ def cleanup_script_files(): fcopy.write(fbase.read()) except (FileNotFoundError, IOError): print(CONCOREPATH+" is not correct path to concore (missing concore_base.py)") - quit() + sys.exit(1) if 'jl' in required_langs and concoretype=="docker": try: fsource = open(CONCOREPATH+"/concoredocker.jl") except (FileNotFoundError, IOError): print(CONCOREPATH+" is not correct path to concore (missing concoredocker.jl)") - quit() + sys.exit(1) with open(outdir+"/src/concore.jl","w") as fcopy: fcopy.write(fsource.read()) fsource.close() @@ -653,7 +653,7 @@ def cleanup_script_files(): fsource = open(CONCOREPATH+"/concore.hpp") except (FileNotFoundError, IOError): print(CONCOREPATH+" is not correct path to concore (missing C++ files)") - quit() + sys.exit(1) with open(outdir+"/src/concore.hpp","w") as fcopy: fcopy.write(fsource.read()) fsource.close() @@ -666,7 +666,7 @@ def cleanup_script_files(): fsource = open(CONCOREPATH+"/concore.v") except (FileNotFoundError, IOError): print(CONCOREPATH+" is not correct path to concore (missing Verilog files)") - quit() + sys.exit(1) with open(outdir+"/src/concore.v","w") as fcopy: fcopy.write(fsource.read()) fsource.close() @@ -678,7 +678,7 @@ def cleanup_script_files(): fcore = open(CONCOREPATH+"/ConcoreJavaRuntimeCore.java") except (FileNotFoundError, IOError): print(CONCOREPATH+" is not correct path to concore (missing Java files)") - quit() + sys.exit(1) with open(outdir+"/src/"+java_runtime,"w") as fcopy: fcopy.write(fsource.read()) fsource.close() @@ -729,7 +729,7 @@ def cleanup_script_files(): os.chmod(outdir+"/src/mkcompile",stat.S_IRWXU) except Exception as e: print(CONCOREPATH+" is not correct path to concore (missing MATLAB files):", e) - quit() + sys.exit(1) # --- Generate iport and oport mappings --- logging.info("Generating iport/oport mappings...") @@ -828,7 +828,7 @@ def cleanup_script_files(): source_content = fsource.read() except: logging.error(f"{CONCOREPATH} is not correct path to concore") - quit() + sys.exit(1) dockerfile_parent = os.path.dirname(dockerfile_path) if dockerfile_parent: os.makedirs(dockerfile_parent, exist_ok=True) @@ -1100,7 +1100,7 @@ def cleanup_script_files(): if len(sourcecode)!=0: if sourcecode.find(".")==-1: logging.error("cannot pull container "+sourcecode+" with control core type "+concoretype) #3/28/21 - quit() + sys.exit(1) dockername,langext = sourcecode.rsplit(".", 1) fbuild.write('mkdir '+containername+"\n") source_subdir = os.path.dirname(sourcecode).replace("\\", "/") @@ -1219,7 +1219,7 @@ def cleanup_script_files(): dockername,langext = sourcecode.rsplit(".", 1) if not (langext in ["py","m","sh","cpp","v","java"]): # 6/22/21 logging.error(f"Extension .{langext} is unsupported") - quit() + sys.exit(1) if concoretype=="windows": # manual double quoting for Windows + Input validation above prevents breakout q_container = f'"{containername}"' diff --git a/tests/test_cli.py b/tests/test_cli.py index d746040..a4c1cd1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -738,6 +738,33 @@ def test_build_command_existing_output(self): ) self.assertIn("already exists", result.output.lower()) + def test_build_command_fails_on_unsupported_extension(self): + with self.runner.isolated_filesystem(temp_dir=self.temp_dir): + result = self.runner.invoke(cli, ["init", "test-project"]) + self.assertEqual(result.exit_code, 0) + + Path("test-project/src/script.py").rename("test-project/src/script.rb") + workflow_path = Path("test-project/workflow.graphml") + content = workflow_path.read_text() + workflow_path.write_text(content.replace("N1:script.py", "N1:script.rb")) + + result = self.runner.invoke( + cli, + [ + "build", + "test-project/workflow.graphml", + "--source", + "test-project/src", + "--output", + "out", + "--type", + "posix", + ], + ) + self.assertNotEqual(result.exit_code, 0) + self.assertIn("Extension .rb is unsupported", result.output) + self.assertFalse(Path("out/STUDY.json").exists()) + def test_inspect_command_basic(self): with self.runner.isolated_filesystem(temp_dir=self.temp_dir): result = self.runner.invoke(cli, ["init", "test-project"])