Repository: scikit-build/scikit-build

Issue:
Improve python setup.py build --help output
- [ ] Document the different build type option
- [ ] Document how to set CMake variables
- [ ] Document how to set the CMake build system generator

Discussion:
codecov-io: ## [Current coverage](https://codecov.io/gh/scikit-build/scikit-build/pull/118?src=pr) is 64.22% (diff: 54.00%)
> Merging [#118](https://codecov.io/gh/scikit-build/scikit-build/pull/118?src=pr) into [master](https://codecov.io/gh/scikit-build/scikit-build/branch/master?src=pr) will decrease coverage by **2.64%**

```diff
@@             master       #118   diff @@
==========================================
  Files            17         18     +1   
  Lines           486        506    +20   
  Methods           0          0          
  Messages          0          0          
  Branches         93         96     +3   
==========================================
  Hits            325        325          
- Misses          124        141    +17   
- Partials         37         40     +3   
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [93aca7b...dac50f6](https://codecov.io/gh/scikit-build/scikit-build/compare/93aca7b67f854ab2e9dfdcaaf99cdeaea0280413...dac50f68dde49069c66abcab94512547f3ed14f3?src=pr)
AppVeyorBot: :white_check_mark: [Build scikit-build 0.0.1.59 completed](https://ci.appveyor.com/project/scikit-build/scikit-build/build/0.0.1.59) (commit https://github.com/scikit-build/scikit-build/commit/847f829829 by @msmolens)
AppVeyorBot: :white_check_mark: [Build scikit-build 0.0.1.63 completed](https://ci.appveyor.com/project/scikit-build/scikit-build/build/0.0.1.63) (commit https://github.com/scikit-build/scikit-build/commit/331ee1542f by @msmolens)
AppVeyorBot: :white_check_mark: [Build scikit-build 0.0.1.70 completed](https://ci.appveyor.com/project/scikit-build/scikit-build/build/0.0.1.70) (commit https://github.com/scikit-build/scikit-build/commit/2cc9425fc2 by @msmolens)

Patch:
diff --git a/skbuild/cmaker.py b/skbuild/cmaker.py
index cdefc46..02a8e10 100644
--- a/skbuild/cmaker.py
+++ b/skbuild/cmaker.py
@@ -27,7 +27,7 @@ def pop_arg(arg, a, default=None):
     """Pops an arg(ument) from an argument list a and returns the new list
     and the value of the argument if present and a default otherwise.
     """
-    parser = argparse.ArgumentParser()
+    parser = argparse.ArgumentParser(add_help=False)
     parser.add_argument(arg)
     ns, a = parser.parse_known_args(a)
     ns = tuple(vars(ns).items())
@@ -365,7 +365,7 @@ class CMaker(object):
                                 "make?").format(CMAKE_BUILD_DIR))
 
         cmd = ["cmake", "--build", source_dir,
-               "--target", "install", "--config", config]
+               "--target", "install", "--config", config, "--"]
         cmd.extend(clargs)
         cmd.extend(
             filter(bool,
diff --git a/skbuild/command/egg_info.py b/skbuild/command/egg_info.py
new file mode 100644
index 0000000..3944aa3
--- /dev/null
+++ b/skbuild/command/egg_info.py
@@ -0,0 +1,14 @@
+
+import os
+
+from setuptools.command.egg_info import egg_info as _egg_info
+
+
+class egg_info(_egg_info):
+    def finalize_options(self):
+        if self.egg_base is not None:
+            script_path = os.path.abspath(self.distribution.script_name)
+            script_dir = os.path.dirname(script_path)
+            self.egg_base = os.path.join(script_dir, self.egg_base)
+
+        _egg_info.finalize_options(self)
diff --git a/skbuild/setuptools_wrap.py b/skbuild/setuptools_wrap.py
index 54efdb3..9a45d50 100644
--- a/skbuild/setuptools_wrap.py
+++ b/skbuild/setuptools_wrap.py
@@ -8,7 +8,7 @@ import sys
 import argparse
 
 from . import cmaker
-from .command import build, install, clean, bdist, bdist_wheel
+from .command import build, install, clean, bdist, bdist_wheel, egg_info
 from .exceptions import SKBuildError
 
 try:
@@ -17,24 +17,42 @@ except ImportError:
     from distutils.core import setup as upstream_setup
 
 
-def move_arg(arg, a, b, newarg=None, f=lambda x: x, concatenate_value=False):
-    """Moves an argument from a list to b list, possibly giving it a new name
-    and/or performing a transformation on the value. Returns a and b. The arg
-    need not be present in a.
+def create_skbuild_argparser():
+    """Create and return a scikit-build argument parser.
     """
-    newarg = newarg or arg
-    parser = argparse.ArgumentParser()
-    parser.add_argument(arg)
-    ns, a = parser.parse_known_args(a)
-    ns = tuple(vars(ns).items())
-    if len(ns) > 0 and ns[0][1] is not None:
-        key, value = ns[0]
-        newargs = [newarg, value]
-        if concatenate_value:
-            b.append("=".join(newargs))
-        elif value is not None:
-            b.extend(newargs)
-    return a, b
+    parser = argparse.ArgumentParser(add_help=False)
+    parser.add_argument(
+        '--build-type', default='Release', metavar='',
+        help='specify the CMake build type (e.g. Debug or Release)')
+    parser.add_argument(
+        '-G', '--generator', metavar='',
+        help='specify the CMake build system generator')
+    parser.add_argument(
+        '-j', metavar='N', type=int, dest='jobs',
+        help='allow N build jobs at once')
+    return parser
+
+
+def parse_skbuild_args(args, cmake_args, build_tool_args):
+    """
+    Parse arguments in the scikit-build argument set. Convert specified
+    arguments to proper format and append to cmake_args and build_tool_args.
+    Returns remaining arguments.
+    """
+    parser = create_skbuild_argparser()
+    ns, remaining_args = parser.parse_known_args(args)
+
+    # Construct CMake argument list
+    cmake_args.append('-DCMAKE_BUILD_TYPE:STRING=' + ns.build_type)
+    if ns.generator is not None:
+        cmake_args.extend(['-G', ns.generator])
+
+    # Construct build tool argument list
+    build_tool_args.extend(['--config', ns.build_type])
+    if ns.jobs is not None:
+        build_tool_args.extend(['-j', str(ns.jobs)])
+
+    return remaining_args
 
 
 def parse_args():
@@ -43,31 +61,20 @@ def parse_args():
     make = []
     argsets = [dutils, cmake, make]
     i = 0
+    separator = '--'
 
-    argv = list(sys.argv)
-    try:
-        argv.index("--build-type")
-    except ValueError:
-        argv.append("--build-type")
-        argv.append("Release")
-
-    for arg in argv:
-        if arg == '--':
+    for arg in sys.argv:
+        if arg == separator:
             i += 1
+            if i >= len(argsets):
+                sys.exit(
+                    "ERROR: Too many \"{}\" separators provided "
+                    "(expected at most {}).".format(separator,
+                                                    len(argsets) - 1))
         else:
             argsets[i].append(arg)
 
-    # handle argument transformations
-    dutils, cmake = move_arg('--build-type', dutils, cmake,
-                             newarg='-DCMAKE_BUILD_TYPE:STRING',
-                             concatenate_value=True)
-    dutils, cmake = move_arg('-G', dutils, cmake)
-    dutils, make = move_arg('-j', dutils, make)
-
-    def absappend(x):
-        return os.path.join(os.path.dirname(os.path.abspath(sys.argv[0])), x)
-
-    dutils, dutils = move_arg('--egg-base', dutils, dutils, f=absappend)
+    dutils = parse_skbuild_args(dutils, cmake, make)
 
     return dutils, cmake, make
 
@@ -79,6 +86,30 @@ def setup(*args, **kw):
     """
     sys.argv, cmake_args, make_args = parse_args()
 
+    # Skip running CMake when user requests help
+    help_parser = argparse.ArgumentParser(add_help=False)
+    help_parser.add_argument('-h', '--help', action='store_true')
+    help_parser.add_argument('--help-commands', action='store_true')
+    ns = help_parser.parse_known_args()[0]
+    if ns.help_commands:
+        return upstream_setup(*args, **kw)
+    if ns.help:
+        # Prepend scikit-build help. Generate option descriptions using
+        # argparse.
+        skbuild_parser = create_skbuild_argparser()
+        arg_descriptions = [line
+                            for line in skbuild_parser.format_help().split('\n')
+                            if line.startswith('  ')]
+        print('scikit-build options:')
+        print('\n'.join(arg_descriptions))
+        print()
+        print('Arguments following a "--" are passed directly to CMake '
+              '(e.g. -DMY_VAR:BOOL=TRUE).')
+        print('Arguments following a second "--" are passed directly to the '
+              'build tool.')
+        print()
+        return upstream_setup(*args, **kw)
+
     packages = kw.get('packages', [])
     package_dir = kw.get('package_dir', {})
     package_data = kw.get('package_data', {}).copy()
@@ -172,6 +203,7 @@ def setup(*args, **kw):
     cmdclass['bdist'] = cmdclass.get('bdist', bdist.bdist)
     cmdclass['bdist_wheel'] = cmdclass.get(
         'bdist_wheel', bdist_wheel.bdist_wheel)
+    cmdclass['egg_info'] = cmdclass.get('egg_info', egg_info.egg_info)
     kw['cmdclass'] = cmdclass
 
     return upstream_setup(*args, **kw)


Test patch:
diff --git a/tests/test_command_line.py b/tests/test_command_line.py
new file mode 100644
index 0000000..f48b68b
--- /dev/null
+++ b/tests/test_command_line.py
@@ -0,0 +1,51 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""test_command_line
+----------------------------------
+
+Tests for various command line functionality.
+"""
+
+from . import project_setup_py_test, push_dir
+
+
+@project_setup_py_test(("samples", "hello"), ["--help"])
+def test_help(capsys):
+    out, err = capsys.readouterr()
+    assert "scikit-build options" in out
+    assert "--build-type" in out
+    assert "Global options:" in out
+    assert "usage:" in out
+
+
+def test_no_command():
+    with push_dir():
+
+        @project_setup_py_test(("samples", "hello"), [""])
+        def run():
+            pass
+
+        failed = False
+        try:
+            run()
+        except SystemExit as e:
+            failed = e.args[0].startswith('invalid command name')
+
+        assert failed
+
+
+def test_too_many_separators():
+    with push_dir():
+
+        @project_setup_py_test(("samples", "hello"), ["--"] * 3)
+        def run():
+            pass
+
+        failed = False
+        try:
+            run()
+        except SystemExit as e:
+            failed = e.args[0].startswith('ERROR: Too many')
+
+        assert failed