Fixes #81: Missing layout-dir will now fail the pack command. (#82)

Also cleans up command handling in __main__ more generally.

.gitignore

@@ -1,5 +1,6 @@
 /build
 /dist
-/env
+/env*
 __pycache__/
 /.vscode
+/pymsbuild/__init__.py.ver

pymsbuild/__main__.py

@@ -5,6 +5,7 @@
 from os import getenv
 from pathlib import PurePath, Path
 from pymsbuild._build import BuildState
+from pymsbuild._init import run as run_init
 
 
 def _env(var, default=None):
@@ -30,8 +31,11 @@ def _envbool(var, default=None, is_true=True, is_false=False):
     return is_true
 
 
-def parse_args():
-    parser = argparse.ArgumentParser("pymsbuild",)
+def parse_args(commands):
+    parser = argparse.ArgumentParser(
+        "pymsbuild",
+        formatter_class=argparse.RawTextHelpFormatter,
+    )
     parser.add_argument(
         "--force", "-f", action="store_true", help="Force a full rebuild"
     )
@@ -82,21 +86,14 @@ def parse_args():
         nargs="*",
         help="Specify additional file(s) to package when using the 'pack' command."
     )
+    cmd_help_1 = ", ".join(f"'{k}'" for k, (f, doc) in commands.items() if doc)
+    cmd_help_2 = "\n".join(f"{k}: {doc}" for k, (f, doc) in commands.items() if doc)
     parser.add_argument(
         "command",
         type=str,
         default="build_in_place",
         nargs="?",
-        help="""one of 'init', 'generate', 'sdist', 'wheel', 'pack', 'distinfo', 'clean'
-
-init: Initialise a new _msbuild.py file.
-generate: Generate the build files without building.
-sdist: Build an sdist.
-wheel: Build a wheel.
-pack: Perform the second step of a two-step build.
-distinfo: Build just the wheel metadata.
-clean: Clean any builds.
-""",
+        help=f"one of {cmd_help_1}.\n\n{cmd_help_2}",
     )
 
     ns = parser.parse_args()
@@ -112,17 +109,24 @@ def parse_args():
     return ns
 
 
-ns = parse_args()
-if not getattr(ns, "command", None):
-    ns.command = "build_in_place"
+COMMANDS = {
+    "init": (run_init, "Initialise a new _msbuild.py file."),
+    "generate": (BuildState.generate, "Generate the build files without building."),
+    "sdist": (BuildState.build_sdist, "Build an sdist."),
+    "wheel": (BuildState.build_wheel, "Build a wheel."),
+    "pack": (BuildState.pack, "Perform the second step of a two-step build."),
+    "distinfo": (BuildState.prepare_wheel_distinfo, "Build just the wheel metadata"),
+    "clean": (BuildState.clean, "Clean any builds."),
+    "build_in_place": (BuildState.build_in_place, None),
+}
+
+
+ns = parse_args(COMMANDS)
+
 
 if ns.verbose:
     print("pymsbuild", pymsbuild.__version__, "running on", sys.version.partition("\n")[0])
 
-if "init" in ns.command:
-    from . import _init
-    _init.run(Path.cwd(), ns.config, ns.force)
-    sys.exit(0)
 
 bs = BuildState()
 bs.source_dir = Path.cwd() / (ns.source_dir or "")
@@ -139,21 +143,29 @@ def parse_args():
 if ns.debug:
     bs.configuration = "Debug"
 
-if ns.layout_dir:
-    COMMANDS = {
-        "sdist": "layout_sdist",
-        "wheel": "layout_wheel",
-        "distinfo": "prepare_wheel_distinfo",
-    }
-else:
-    COMMANDS = {
-        "sdist": "build_sdist",
-        "wheel": "build_wheel",
-        "distinfo": "prepare_wheel_distinfo",
-    }
-
-
-cmd = ns.command
-cmd = COMMANDS.get(cmd, cmd)
-f = getattr(bs, cmd)
-f()
+
+if _envbool("PYMSBUILD_SHOW_TRACEBACKS"):
+    f, doc = COMMANDS[ns.command]
+    f(bs)
+    sys.exit(0)
+
+
+try:
+    f, doc = COMMANDS[ns.command]
+except KeyError:
+    print(
+        "ERROR: Unrecognised command. See 'python -m pymsbuild --help' " +
+        "for the list of valid commands.",
+        file=sys.stderr,
+    )
+    sys.exit(1)
+
+try:
+    f(bs)
+except Exception as ex:
+    print("ERROR", ex, file=sys.stderr)
+    if getattr(ex, "winerror", 0):
+        sys.exit(ex.winerror)
+    if getattr(ex, "errno", 0):
+        sys.exit(ex.errno)
+    sys.exit(1)

pymsbuild/_build.py

@@ -83,6 +83,7 @@ def __init__(self, output_dir=None):
         self.output_dir = Path(output_dir) if output_dir else None
         self.build_dir = None
         self.temp_dir = None
+        self._perform_layout = None
         self.layout_dir = None
         self.layout_extra_files = []
         self.metadata_dir = None
@@ -108,6 +109,8 @@ def finalize_metadata(self, getenv=os.getenv, sdist=False, in_place=False):
 
         self.output_dir = self.source_dir / (self.output_dir or "dist")
         self.build_dir = self.source_dir / (self.build_dir or "build/bin")
+        if self._perform_layout is None:
+            self._perform_layout = bool(self.layout_dir)
         self.layout_dir = self.source_dir / (self.layout_dir or "build/layout")
         self.temp_dir = self.source_dir / (self.temp_dir or "build/temp")
         self.pkginfo = self.source_dir / (self.pkginfo or "PKG-INFO")
@@ -377,8 +380,11 @@ def layout_sdist(self, statefile=True):
 
     def build_sdist(self):
         self.finalize(sdist=True)
-        self.layout_sdist(statefile=False)
-        return self.pack_sdist()
+        if self._perform_layout:
+            self.layout_sdist(statefile=True)
+        else:
+            self.layout_sdist(statefile=False)
+            return self.pack_sdist()
 
     def pack_sdist(self, files=None):
         self.finalize(sdist=True)
@@ -444,8 +450,11 @@ def build_wheel(self, metadata_dir=None):
                 self.prepare_wheel_distinfo()
         else:
             self.prepare_wheel_distinfo()
-        self.layout_wheel()
-        return self.pack_wheel()
+        if self._perform_layout:
+            self.layout_wheel(statefile=True)
+        else:
+            self.layout_wheel(statefile=False)
+            return self.pack_wheel()
 
     def pack_wheel(self, files=None):
         self.finalize()
@@ -508,11 +517,7 @@ def _write_state(self, cmd):
     def pack(self):
         self.finalize()
         if not self.state_file or not self.state_file.is_file():
-            print(
-                "'--layout-dir' argument is required when invoking the 'pack' command",
-                file=sys.stderr
-            )
-            return
+            raise RuntimeError("'--layout-dir' argument is required when invoking the 'pack' command")
         cmd = None
         with self.state_file.open("r", encoding="utf-8-sig") as f:
             for i in f:

pymsbuild/_init.py

@@ -86,15 +86,17 @@ def _p(pattern):
         yield f'{_indent}source={offset!r},'
 
 
-def run(root, config_name="_msbuild.py", force=False):
-    if not config_name:
-        config_name = "_msbuild.py"
-    if (root / config_name).is_file():
+def run(build_state, config_name="_msbuild.py"):
+    root = build_state.source_dir
+    force = build_state.force
+
+    config_file = root / (build_state.config_file or config_name)
+
+    if config_file.is_file():
         if force:
-            (root / config_name).unlink()
+            config_file.unlink()
         else:
-            print(config_name, "already exists. Delete the file before using 'init'", file=sys.stderr)
-            return
+            raise RuntimeError(f"{config_file} already exists. Delete the file before using 'init'")
 
     substitutions = {}
     build_requires = [PYMSBUILD_REQUIRES_SPEC]
@@ -129,12 +131,12 @@ def run(root, config_name="_msbuild.py", force=False):
     substitutions["BUILD_REQUIRES"] = repr(build_requires)
 
     code = re.sub(r"\<(\w+)\>", lambda m: substitutions.get(m.group(1)), TEMPLATE)
-    with open(root / config_name, "w", encoding="utf-8") as f:
+    with open(config_file, "w", encoding="utf-8") as f:
         print(code, file=f, end="")
-    print("Wrote", root / config_name)
+    print("Wrote", root / config_file)
 
     toml = re.sub(r"\<(\w+)\>", lambda m: substitutions.get(m.group(1)), TOML_TEMPLATE)
-    pyproject = (root / config_name).parent / "pyproject.toml"
+    pyproject = config_file.parent / "pyproject.toml"
     if pyproject.is_file():
         if force:
             pyproject.unlink()

tests/test_build.py

@@ -29,6 +29,7 @@ def build_state(tmp_path, testdata):
     bs.output_dir = tmp_path / "out"
     bs.build_dir = tmp_path / "build"
     bs.layout_dir = tmp_path / "layout"
+    bs._perform_layout = False  # allows us to override layout_dir
     bs.temp_dir = tmp_path / "temp"
     bs.package = T.Package("package",
         T.PyFile(testdata / "empty.py", "__init__.py"),
@@ -59,6 +60,7 @@ def test_build(build_state, configuration):
     os.environ["BUILD_BUILDNUMBER"] = "1"
     bs = build_state
     bs.finalize(in_place=True)
+    assert not bs._perform_layout
     bs.generate()
     del os.environ["BUILD_BUILDNUMBER"]
     bs.target = "Build"
@@ -91,6 +93,7 @@ def test_build(build_state, configuration):
 def test_build_sdist(build_state, configuration):
     bs = build_state
     bs.finalize(sdist=True)
+    assert not bs._perform_layout
     bs.generate()
     bs.configuration = configuration
     bs.build_sdist()

tests/test_init.py

@@ -8,13 +8,17 @@
 sys.path.insert(0, os.path.dirname(os.path.dirname(__file__)))
 
 import pymsbuild
+from pymsbuild._build import BuildState
 from pymsbuild._init import run
 
 @pytest.fixture
 def init_project(inittestprojects, tmp_path):
     def do_init(name):
-        output = tmp_path / "_msbuild.py"
-        run(inittestprojects / name, output)
+        bs = BuildState()
+        bs.source_dir = inittestprojects / name
+        bs.config_file = output = (tmp_path / "_msbuild.py").absolute()
+        bs.force = True
+        run(bs)
         assert output.is_file()
         pyproj = output.parent / "pyproject.toml"
         assert pyproj.is_file()
@@ -39,14 +43,21 @@ def visit_Assign(self, n):
     def visit_METADATA(self, n):
         assert isinstance(n, ast.Dict)
         for k, v in zip(n.keys, n.values):
-            assert isinstance(k, ast.Str)
-            try:
-                expect = self.metadata[k.s]
-            except LookupError:
-                pass
+            assert isinstance(k, (ast.Constant, ast.Str))
+            if isinstance(k, ast.Str):
+                try:
+                    expect = self.metadata[k.s]
+                except LookupError:
+                    pass
+                else:
+                    assert expect == v.s
             else:
-                assert isinstance(v, ast.Str)
-                assert expect == v.s
+                try:
+                    expect = self.metadata[k.value]
+                except LookupError:
+                    pass
+                else:
+                    assert expect == v.value
 
     def visit_PACKAGE(self, n, path):
         assert isinstance(n, ast.Call)