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

Add pre_install configuration option #2886

Closed
Czaki opened this issue Jan 20, 2023 · 8 comments
Closed

Add pre_install configuration option #2886

Czaki opened this issue Jan 20, 2023 · 8 comments

Comments

@Czaki
Copy link
Contributor

Czaki commented Jan 20, 2023

What's the problem this feature will solve?

Next to the normal test, I would have the oldest-supported test. I test my code on the oldest supported version of packages based on requirements constrains from setup.cfg.

Currently, it requires me to run separate commands before calling tox (or make tox calling tox) to extract minimal requirements from package metadata. So time to time, I have false negative runs when executing it manually. I also need to have a more complex CI configuration.

Describe the solution you'd like

Add pre_install command that will be executed before the installation of package. To allow modify metadata/requirements file.

Manual curated list of minimal requirements to be passed to deps:
https://github.com/4DNucleome/PartSeg/blob/develop/build_utils/minimal-req.txt
https://github.com/4DNucleome/PartSeg/blob/develop/tox.ini#L91

Special call of script that modifies setup.cfg for testing minimal requirements:
https://github.com/napari/napari/blob/20319df0b397f85054e1c7e728d393eb7a7a7b38/.github/workflows/test_pull_requests.yml#L129

Alternative Solutions

  1. run a separate command before tox call
  2. run command and reinstall package in commadn_pre - additional maintenance need to keep consistency with other envs
  3. modify install_command to execute pinning before call pip install - additional maintenance need to keep consistency with other envs

All works but all looks like a dirty solutions.

Additional context

@gaborbernat
Copy link
Member

Add pre_install command that will be executed before the installation of package. To allow modify metadata/requirements file.

I'm not sure I follow how this work. What kind of modification would happen, and how would this trickle back to the installation command for tox?

@masenf
Copy link
Collaborator

masenf commented Jan 20, 2023

@Czaki If you're looking for something "simple" to get off the ground, consider an inline plugin, toxfile.py.

The plugin api exposes tox_on_install which is essentially a pre_install hook. It's not exposed via the ini config, but it sounds like you'd be calling out to an external script anyway, so maybe that's okay.

Here's a possible example:

import os
from typing import Any

from tox.config.cli.parser import DEFAULT_VERBOSITY
from tox.execute.request import StdinSource
from tox.plugin import impl
from tox.tox_env.api import ToxEnv


HERE = os.path.dirname(__file__)


@impl
def tox_on_install(tox_env: ToxEnv, arguments: Any, section: str, of_type: str):
    if of_type == "deps":
        cmd = ["python", os.path.join(HERE, "build_utils", "create_minimal_reqs.py")]
        result = tox_env.execute(
            cmd=cmd,
            stdin=StdinSource.user_only(),
            run_id="create_minimal_requirements",
            show=tox_env.options.verbosity > DEFAULT_VERBOSITY,
        )

I've been developing a plugin recently, tox-pin-deps, which creates pip-compile lock files from the deps list and then uses the lock files to install the packages. It's still early development at this point, but the ideas in that plugin sound similar to what you're trying to do with the lock file here.

While I don't think tox-pin-deps directly solves your use case as it is, if you're interested in pushing some of this "upstream" I'd be willing to work with you to make it more suitable for this use case.

@Czaki
Copy link
Contributor Author

Czaki commented Jan 20, 2023

I'm not sure I follow how this work. What kind of modification would happen, and how would this trickle back to the installation command for tox?

in the second example napari/napari@20319df/.github/workflows/test_pull_requests.yml#L129

the called scripts modifies >= to == in the setup.cfg install_requires section,
so when pip install . is called, then not the most current requirements, but the oldest possible one will be installed (unfortunately only direct dependencies will be oldest possible).

@Czaki If you're looking for something "simple" to get off the ground, consider an inline plugin, toxfile.py.

Wow, it looks promising.

I've been developing a plugin recently, tox-pin-deps, which creates pip-compile lock files from the deps list and then uses the lock files to install the packages. It's still early development at this point, but the ideas in that plugin sound similar to what you're trying to do with the lock file here.

It looks interesting, but pip-compile selects the newest possible package version. So will be really nice for the reproducible test of the application, but maybe not for the testing library that should provide a wide range of versions compatibility.

@gaborbernat
Copy link
Member

gaborbernat commented Jan 20, 2023

he called scripts modifies >= to == in the setup.cfg install_requires section,

This sounds destructive, and probably there are better ways to go. Using constraint files is a better solution. https://tox.wiki/en/latest/faq.html#using-constraint-files

@masenf
Copy link
Collaborator

masenf commented Jan 20, 2023

Oh yikes. I guess I was only looking at the example that built a requirements file with pip-compile...

Munging the setup.cfg dependencies is a messy solution and ensures that tested package is NOT the same as the published package.

I think using a pip-compile lock file is okay, but installing any requirements file via deps does NOT guarantee those exact deps are used, as the install_pkg_deps step (and transitives) can override deps, see #2386.

Ultimately I think constraint files are the answer here, so the question remains as to where those constraint files come from and how tox can know about them and apply them at the correct time.

Again, even specifying -cconstraints.txt in deps could still be overridden by the package dependencies, as that occurs in a different pip invocation. Will need a plug-in or proper fix for #2386 to apply constraints at package_deps install time.

@Czaki
Copy link
Contributor Author

Czaki commented Jan 20, 2023

This sounds destructive, and probably there are better ways to go. Using constraint files is a better solution. tox.wiki/en/latest/faq.html#using-constraint-files

I agree, but it provides a single source of truth. MAybe it could be better to use a constraints file, but it still is nice to have the option to generate it from setup.cfg. And it will be nice if running a minimum version test suite constarains will be automatically refreshed.

@gaborbernat
Copy link
Member

I'd say I won't do part of the core. You can create a plugin for it, though. Core only intends to support constraints files.

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

4 participants