Why wouldn't the tox see the environment variable during install_command?

Question

Given the tox==3.13.2, see details

++pip install tox
Requirement already satisfied: tox in /home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages (3.13.2)

and the tox.ini:

[tox]
envlist =
    py-cythonized


[testenv]
; simplify numpy installation
setenv =
    LAPACK=
    ATLAS=None
    PYTHONWARNINGS=ignore

; Copy all environment variables to the tox test environment
passenv = *

deps =
    py-cythonized: Cython >= 0.28.5

changedir = nltk/test
commands =
    ; scipy and scikit-learn requires numpy even to run setup.py so
    ; they can't be installed in one command
    pip install scipy scikit-learn

    ; python runtests.py --with-coverage --cover-inclusive --cover-package=nltk --cover-html --cover-html-dir={envdir}/docs []
    python runtests.py []

And specifically:

# Test Cython compiled installation.
[testenv:py-cythonized]
setenv =
    CYTHONIZE_NLTK = "true"
    NLTK_DATA = {homedir}/nltk_data/
extras = all
passenv =
    CYTHONIZE_NLTK
commands =
    {toxinidir}/tools/travis/coverage-pylint.sh

There's the CYTHONIZE_NLTK variable that should be passed when testing with tox -e py-cythonize tox.ini -vv

The CYTHONIZE_NLTK variable is checked by my setup.py to add extended modules ext_modules:

MODULES_TO_COMPILE = [
    #'nltk.ccg.*', # Fails on https://travis-ci.org/nltk/nltk/jobs/529589821#L2077
    'nltk.chat.*',
    'nltk.chunk.*',

def compile_modules(modules):
    """
    Compile the named modules using Cython, using the clearer Python 3 semantics.
    """
    import Cython
    from Cython.Build import cythonize
    files = [name.replace('.', os.path.sep) + '.py' for name in modules]
    print("Compiling %d modules using Cython %s" % (len(modules), Cython.__version__))
    return cythonize(files, language_level=3)


print('PRINTING ENVIRON...')
print(os.environ)
if os.getenv('CYTHONIZE_NLTK') == 'true':
    ext_modules = compile_modules(MODULES_TO_COMPILE)
else:
    ext_modules = None

setup(
    name="nltk",
    extras_require=extras_require,
    packages=find_packages(),
    ext_modules=ext_modules,
    zip_safe=False,  # since normal files will be present too?
    entry_points=console_scripts,
)

But from the CI, it looks like the CYTHONIZE_NLTK wasn't recognized when tox does the install_command:

[5305] /home/travis/build/nltk/nltk$ /home/travis/build/nltk/nltk/.tox/py-cythonized/bin/python -m pip install --exists-action w '/home/travis/build/nltk/nltk/.tox/.tmp/package/1/nltk-3.5.zip[all]'

More details on https://travis-ci.org/nltk/nltk/jobs/578224159#L1988

It looks like a quotes escaping issue to me. `CYTHONIZE_NLTK = "true"` sets `"true"` and not `true` string as variable's value. Either change `CYTHONIZE_NLTK = true` in `tox.ini` or modify the check in the setup script: `if os.getenv('CYTHONIZE_NLTK') == '"true"':`, or use `"\"true\""` etc. — hoefling, Aug 31 '19 at 20:24
Yep, I think this was it - tried it out myself today with changed `CYTHONIZE_NLTK = true` in `tox.ini`. Here's a [passing job example](https://travis-ci.org/hoefling/nltk/jobs/579443921) and here's the [installation log with cythonizing](https://travis-ci.org/hoefling/nltk/jobs/579446413#L13546) (warning: this one is HUGE because I ran `pip` in verbose mode to reveal the build logs). — hoefling, Sep 01 '19 at 15:17
What I would also suggest is to move the `cythonize` call from the top of the setup script to the build command. Cythonizing only makes sense in binary builds and is useless when making source dists. But again, no big deal in the current state either. — hoefling, Sep 01 '19 at 16:12
The current state forces the cythonize before the build at the pre-install https://github.com/alvations/nltk/blob/cythonize/tools/travis/install.sh#L12 and the build passed too https://travis-ci.org/nltk/nltk/jobs/578737247#L1039 (Not sure if I'm doing the right thing though) — alvas, Sep 01 '19 at 22:15
That's (partially) the reason I wrote the above comment. Right now, you are doing the following: 1. add `CYTHONIZE_NLTK` to the env; 2. package a `.zip` source dist, transpiling Python modules to C extensions on the way (b/c of 1), but it's useless since the transpiled result is ignored; 3. create a new `tox` env where `CYTHONIZE_NLTK` is set to `"true"` and not `true` (b/c of `setenv`, `passenv` will not invoke here) and install the zip dist, so no building of C extensions will be invoked. — hoefling, Sep 05 '19 at 13:09
The build you mention passed, but it doesn't look like the C extensions were built; I bet you have got the pure python installation again.IMO there are two changes necessary: 1.unquote the env var value in `tox.ini` ([first patch](https://github.com/hoefling/nltk/commit/496403731e88c05d8418b7419d28a5c04fbdd8eb)) and 2.clean up `tox.ini` and `.travis.yml`, removing the manipulations with `CYTHONIZE_NLTK` ([second patch](https://github.com/hoefling/nltk/commit/e2b49a3ab3d5e7e0b5ac570951b890c08dfce8f8)). With the two patches, the `py-cythonized` job gets the correct installation (job logs above). — hoefling, Sep 05 '19 at 13:14
`passenv` only passes environment variables "from the tox invocation environment to the test environment when executing test commands": https://tox.readthedocs.io/en/latest/config.html#conf-passenv. No mention of passenv affecting how dependencies are installed. — Johan Walles, Sep 16 '19 at 12:41

Why wouldn't the tox see the environment variable during install_command?

0 Answers0