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.

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

Modify noxfile.py::_cmake so it works with nox --install-only #224

Closed
dhermes opened this issue May 20, 2020 · 5 comments
Closed

Modify noxfile.py::_cmake so it works with nox --install-only #224

dhermes opened this issue May 20, 2020 · 5 comments

Comments

@dhermes
Copy link
Owner

dhermes commented May 20, 2020

The usage of session.run

    session.run(
        "cmake",
        "--build",
        build_dir,
        "--config",
        build_type,
        "--target",
        "install",
        external=cmake_external,
    )

means the later session.install(".", env=env) in install_bezier() doesn't work as expected when --install-only is used.

@dhermes
Copy link
Owner Author

dhermes commented May 20, 2020

I'm (sort of) happy to see that the --install-only check is hyper-isolated to Session.run():

if self._runner.global_config.install_only:
    logger.info("Skipping {} run, as --install-only is set.".format(args[0]))
    return None

@dhermes
Copy link
Owner Author

dhermes commented May 20, 2020

I can think of at least two options here:

  1. Just invoke session._run() directly
  2. Use a context manager (probably via @contextlib.contextmanager) to temporarily set self._runner.global_config.install_only to False

I don't like either one. Both assume some level of knowledge of implementation details, so they can bitrot over time if nox internals change.

The second one somehow seems more appealing, however by monkey-patching a global value, we ruin the ability for the config to be used concurrently by other nox sessions.

@theacodes WDYT?

@theacodes
Copy link

tbh I'd be happy to see a run_always for this exact use case.

@dhermes dhermes changed the title Modify noxfile.py::_cmake to it works with nox --install-only Modify noxfile.py::_cmake so it works with nox --install-only May 27, 2020
@dhermes
Copy link
Owner Author

dhermes commented Jun 4, 2020

@theacodes Good idea. I love when teamwork works (i.e. my brain didn't go there at all)! Will send a PR soon.

@dhermes
Copy link
Owner Author

dhermes commented Jun 7, 2020

Sent: wntrblm/nox#331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants