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

Allow pass_env/set_env in packaging/building/installing stage #2543

Closed
henryborchers opened this issue Nov 22, 2022 · 10 comments · Fixed by #2813
Closed

Allow pass_env/set_env in packaging/building/installing stage #2543

henryborchers opened this issue Nov 22, 2022 · 10 comments · Fixed by #2813
Assignees
Labels
area:documentation enhancement help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.

Comments

@henryborchers
Copy link

As a developer who creates a number of Python packages with C and C++ extensions, I would love to my packages to have access to environment variables during the building/installing stage so that I can used custom cached libraries on my computer to speed up my builds.

More info:
I currently have a number of Setuptools projects that use the Conan C and C++ package manager to build/gather the libraries that my extensions need to link to. However, the only way that I know that I can tell Conan where to look for a cache is with either a "--config-setting" with a pep517 builder or an environment variable. Neither of these are possible to pass at build/install time with Tox to my knowledge. This means, that when I run Tox on these projects, they end up compiling the entire dependency tree of the 3rd party libraries acquired from Conan if there aren't binaries available for my platform (usually the case) for each Tox environment. I have no way to reuse these already compiled libraries.

For example:
I have a project that links to Google's Tesseract library. Binaries available for every platform that I'm running on, so I have to have Conan build it first which could take about 20 minutes + 20 seconds for my own C++ wrapper code. Now if I'm testing this with Tox with 4 versions of Python, this could take over an hour.

What I would love is a way to do something like this:

[testenv]
envlist = py37, py38, py39, py310,py311
isolated_build = true
passenv =
    CONAN_USER_HOME

That way I could set CONAN_USER_HOME to a single directory and have each environment reuse the same dependencies.

@henryborchers henryborchers added the feature:new something does not exist yet, but should label Nov 22, 2022
@henryborchers henryborchers changed the title Allow passenv in packaging stage Allow passenv in packaging/building/installing stage Nov 22, 2022
@gaborbernat
Copy link
Member

This should work 👍

@henryborchers
Copy link
Author

henryborchers commented Nov 22, 2022

@gaborbernat, what do you mean? Do you mean "this should already work" or this is something that "This should be improved to work like this"

The environment variables are visible during testing but it doesn't seem to be accessible to my custom pep517 build-backend which is pretty much just inherited everything from Setuptools.

@gaborbernat
Copy link
Member

It should be accessible in the build back ends too. If it doesn't, that's a bug 🤣

@crwood
Copy link

crwood commented Dec 7, 2022

This, indeed, appears to be broken in tox 4.0.0; environment variables passed down to testenvs via passenv are no longer visible to setup.py.

@gaborbernat gaborbernat added this to the 4.0.x milestone Dec 8, 2022
@gaborbernat gaborbernat added bug:normal affects many people or has quite an impact help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. and removed feature:new something does not exist yet, but should labels Dec 8, 2022
@gaborbernat
Copy link
Member

Seems I was wrong. This is an intended breaking behavior. We need to document it. We had many people commenting that it's confusing that setting values in testenv does changes with the packaging environment too. And makes sense, just because you want CONAN_USER_HOME set for packaging that might not be desirable for the test environment. With tox 4 now, packaging environments no longer inherit from the testenv. You can still make these changes by doing the changes in the .pkg env (which is the default packaging environment for source distributions, if you select wheel mode it will be .pkg-{python_flavor}{pthon_version}), e.g. as a migration, you can do:

[testenv:.pkg]
passenv = CONAN_USER_HOME

We need to document this.

And also see the need for packaging environments to set their base to packaging section to allow setting these configurations for all packaging environments. This feature is not yet implemented.

@ThomasWaldmann
Copy link

ThomasWaldmann commented Dec 10, 2022

@gaborbernat thanks for the hint!

I needed add this to get borgbackup macOS CI tox 4.0.x working:

[testenv:.pkg]
passenv = *  # needed by tox4, so env vars are visible for building borg

Otherwise it would not see PKG_CONFIG_PATH and thus not find openssl stuff on macOS homebrew.

@miettal
Copy link

miettal commented Dec 11, 2022

I faced same issue.

I'm using ssh-agent to install python package from github(I wrote "git+ssh://git@github.com/..." as dependency in setup.py).
In this case, I could use tox successfully until 3.0, but cannot from 4.0.

// this is just introducing use case, I understand this issue mechanism and workaround.

@gaborbernat gaborbernat changed the title Allow passenv in packaging/building/installing stage Allow pass_env/set_env in packaging/building/installing stage Dec 28, 2022
@jonathanunderwood
Copy link

Capturing here a use case that may not be covered currently by the [testenv:.pkg] recipe above. In #2784 I needed to pass an environment variable through to the package build step for just one of the test environments. So, to get back to the pre-4.0 capabilities, I think the capability to specify .pkg per test environment is needed, not just for testenv.

@jonathanunderwood
Copy link

jonathanunderwood commented Dec 30, 2022

Capturing here a use case that may not be covered currently by the [testenv:.pkg] recipe above. In #2784 I needed to pass an environment variable through to the package build step for just one of the test environments. So, to get back to the pre-4.0 capabilities, I think the capability to specify .pkg per test environment is needed, not just for testenv.

In actual fact the capability is already there to deal with this use case - see @gaborbernat 's reply here.

@gaborbernat
Copy link
Member

gaborbernat commented Jan 3, 2023

Ideally we could do something like:

[tox]
package_env_inherit_section = package

[package]
pass_env = 
     magic

So that there's one central package configuration section, similar to how we have testenv for test environments. This could be configurable, but perhaps we can just default to pkgenv 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:documentation enhancement help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants