Skip to content

Conversation

stefanv
Copy link
Member

@stefanv stefanv commented Nov 13, 2024

Closes #242

@stefanv stefanv added the type: Enhancement New feature or request label Nov 13, 2024
@stefanv
Copy link
Member Author

stefanv commented Nov 13, 2024

@ngoldbaum Please take a look and see if this API covers your use cases.

Copy link
Contributor

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

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

Neat! I'll try to see if this solves the issues in NumPy soon. Just a quick drive-by comment I noticed looking at the docs.

@stefanv
Copy link
Member Author

stefanv commented Nov 14, 2024

I also added argument removal, a feature you previously requested.

@ngoldbaum
Copy link
Contributor

ngoldbaum commented Nov 14, 2024

This is awesome! I love deleting code and docs ending up with a better documented, more featureful result:

goldbaum at Nathans-MBP in ~/Documents/numpy on main!
± git diff
diff --git a/.spin/cmds.py b/.spin/cmds.py
index fea3565a5e..12ee17878c 100644
--- a/.spin/cmds.py
+++ b/.spin/cmds.py
@@ -1,8 +1,9 @@
+import importlib
 import os
-import shutil
 import pathlib
-import importlib
+import shutil
 import subprocess
+import sys

 import click
 from spin import util
@@ -72,46 +73,18 @@ def changelog(ctx, token, revision_range):
         )


-@click.command()
-@click.option(
-    "-j", "--jobs",
-    help="Number of parallel tasks to launch",
-    type=int
-)
-@click.option(
-    "--clean", is_flag=True,
-    help="Clean build directory before build"
-)
-@click.option(
-    "-v", "--verbose", is_flag=True,
-    help="Print all build output, even installation"
-)
 @click.option(
     "--with-scipy-openblas", type=click.Choice(["32", "64"]),
     default=None,
     help="Build with pre-installed scipy-openblas32 or scipy-openblas64 wheel"
 )
-@click.argument("meson_args", nargs=-1)
-@click.pass_context
-def build(ctx, meson_args, with_scipy_openblas, jobs=None, clean=False, verbose=False, quiet=False, *args, **kwargs):
-    """🔧 Build package with Meson/ninja and install
-
-    MESON_ARGS are passed through e.g.:
-
-    spin build -- -Dpkg_config_path=/lib64/pkgconfig
-
-    The package is installed to build-install
-
-    By default builds for release, to be able to use a debugger set CFLAGS
-    appropriately. For example, for linux use
-
-    CFLAGS="-O0 -g" spin build
-    """
-    # XXX keep in sync with upstream build
+@util.extend_command(meson.build)
+def build(
+        *, parent_callback, with_scipy_openblas=None, **kwargs
+):
     if with_scipy_openblas:
         _config_openblas(with_scipy_openblas)
-    ctx.params.pop("with_scipy_openblas", None)
-    ctx.forward(meson.build)
+    parent_callback(**kwargs)


 @click.command()

goldbaum at Nathans-MBP in ~/Documents/numpy on main!
± spin build --help
Usage: spin build [OPTIONS] [MESON_ARGS]...

  🔧 Build package with Meson/ninja

  The package is installed to `build-install` (unless a different build directory is specified with `-C`).

  MESON_ARGS are passed through e.g.:

    spin build -- -Dpkg_config_path=/lib64/pkgconfig

  By default meson-python does release builds. To be able to use a debugger, tell meson to build in debug mode:

    spin build -- -Dbuildtype=debug

  or set CFLAGS appropriately:

    CFLAGS="-O0 -g" spin build

  Build into a different build/build-install directory via the `-C/--build-dir` flag:

    spin build -C build-for-feature-x

  This feature is useful in combination with a shell alias, e.g.:

    $ alias spin-clang="SPIN_BUILD_DIR=build-clang CC=clang spin"

  Which can then be used to build (`spin-clang build`), to test (`spin-clang test ...`), etc.

Options:
  -j, --jobs INTEGER             Number of parallel tasks to launch
  --clean                        Clean build directory before build
  -v, --verbose                  Print detailed build and installation output
  --gcov                         Enable C code coverage using `gcov`. Use `spin test --gcov` to generate reports.
  --prefix TEXT                  The build prefix, passed directly to meson.  [default: /usr]
  -C, --build-dir TEXT           Meson build directory; package is installed into './{build-dir}-install'.  [env var: SPIN_BUILD_DIR; default: build]
  --with-scipy-openblas [32|64]  Build with pre-installed scipy-openblas32 or scipy-openblas64 wheel
  --help                         Show this message and exit.

I guess I could leave the old code in an if statement to support old versions of spin. Do you think that's worth doing in NumPy's spin config? Or should we just require spin 0.13 and substantially slim down our .spin/cmds.py?

@stefanv
Copy link
Member Author

stefanv commented Nov 14, 2024

Nice :)

I would just do a version check on spin (in .spin/cmds.py), and provide a helpful error message / instruction.

@stefanv
Copy link
Member Author

stefanv commented Nov 15, 2024

If this works well for you, I'm inclined to merge this and make a release.

@ngoldbaum
Copy link
Contributor

That would be great. I'd also appreciate it if you could handle updating NumPy's spin config in the context of your open PR. I'm also happy to take that over if you don't think you'll have time for a while.

@stefanv stefanv merged commit 5e91741 into scientific-python:main Nov 17, 2024
20 checks passed
@jarrodmillman jarrodmillman added this to the 0.13 milestone Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It should be easier to extend commands provided by spin
3 participants