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

tox 3.8 crashes on Appveyor #1227

Closed
takluyver opened this issue Mar 29, 2019 · 23 comments
Closed

tox 3.8 crashes on Appveyor #1227

takluyver opened this issue Mar 29, 2019 · 23 comments

Comments

@takluyver
Copy link
Contributor

@takluyver takluyver commented Mar 29, 2019

In the h5py project, all the builds on Appveyor recently started failing. This seems to coincide with the release of tox 3.8.0, and it still occurs with 3.8.1. I'm still investigating this, but here's the error we're getting:

ci\appveyor\build.cmd py -3.5 -m tox
Using default MSVC build environment
C:\Python35-x64\lib\site-packages\tox\config\__init__.py:576: UserWarning: conflicting basepython version (set 36, should be 27) for env 'py27-test-deps';resolve conflict or set ignore_basepython_conflict
  proposed_version, implied_version, testenv_config.envname
C:\Python35-x64\lib\site-packages\tox\config\__init__.py:576: UserWarning: conflicting basepython version (set 36, should be 27) for env 'py27-test-mindeps';resolve conflict or set ignore_basepython_conflict
  proposed_version, implied_version, testenv_config.envname
C:\Python35-x64\lib\site-packages\tox\config\__init__.py:576: UserWarning: conflicting basepython version (set 36, should be 34) for env 'py34-test-deps';resolve conflict or set ignore_basepython_conflict
  proposed_version, implied_version, testenv_config.envname
C:\Python35-x64\lib\site-packages\tox\config\__init__.py:576: UserWarning: conflicting basepython version (set 36, should be 34) for env 'py34-test-mindeps';resolve conflict or set ignore_basepython_conflict
  proposed_version, implied_version, testenv_config.envname
C:\Python35-x64\lib\site-packages\tox\config\__init__.py:576: UserWarning: conflicting basepython version (set 36, should be 35) for env 'py35-test-deps';resolve conflict or set ignore_basepython_conflict
  proposed_version, implied_version, testenv_config.envname
C:\Python35-x64\lib\site-packages\tox\config\__init__.py:576: UserWarning: conflicting basepython version (set 36, should be 35) for env 'py35-test-mindeps';resolve conflict or set ignore_basepython_conflict
  proposed_version, implied_version, testenv_config.envname
C:\Python35-x64\lib\site-packages\tox\config\__init__.py:576: UserWarning: conflicting basepython version (set 36, should be 37) for env 'py37-test-deps';resolve conflict or set ignore_basepython_conflict
  proposed_version, implied_version, testenv_config.envname
C:\Python35-x64\lib\site-packages\tox\config\__init__.py:576: UserWarning: conflicting basepython version (set 36, should be 37) for env 'py37-test-mindeps';resolve conflict or set ignore_basepython_conflict
  proposed_version, implied_version, testenv_config.envname
Traceback (most recent call last):
  File "C:\Python35-x64\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Python35-x64\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\Python35-x64\lib\site-packages\tox\__main__.py", line 4, in <module>
    tox.cmdline()
  File "C:\Python35-x64\lib\site-packages\tox\session\__init__.py", line 42, in cmdline
    main(args)
  File "C:\Python35-x64\lib\site-packages\tox\session\__init__.py", line 58, in main
    config = load_config(args)
  File "C:\Python35-x64\lib\site-packages\tox\session\__init__.py", line 75, in load_config
    config = parseconfig(args)
  File "C:\Python35-x64\lib\site-packages\tox\config\__init__.py", line 249, in parseconfig
    ParseIni(config, config_file, content)
  File "C:\Python35-x64\lib\site-packages\tox\config\__init__.py", line 1077, in __init__
    config.envconfigs[name] = self.make_envconfig(name, section, reader._subs, config)
  File "C:\Python35-x64\lib\site-packages\tox\config\__init__.py", line 1181, in make_envconfig
    res = env_attr.postprocess(testenv_config=tc, value=res)
  File "C:\Python35-x64\lib\site-packages\tox\config\__init__.py", line 571, in basepython_default
    if not proposed_version.startswith(implied_version):
TypeError: startswith first arg must be str or a tuple of str, not NoneType

Example failure log: https://ci.appveyor.com/project/h5py/h5py/builds/23449770/job/itchqco453midako

I'll update this with anything I find.

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

Can you provide a two verbosity log? Feels like somehow 3.6 gets forced as version for all envs 🤔

FYI: tox is always lowercase t 👍

@gaborbernat gaborbernat changed the title Tox 3.8 crashes on Appveyor tox 3.8 crashes on Appveyor Mar 29, 2019
@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

I added -vv, but it doesn't seem to make much difference to the output. Do you want more vs?

https://ci.appveyor.com/project/h5py/h5py/builds/23451295/job/r9es7qlpgxee3r44

(Thanks, I'll try to remember the spelling. Having worked on IPython (not iPython), I know how frustrating mis-capitalisation can be).

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

Do -v -v instead 😄 I know parse_known_args is a bit dumb 😆 👍 Though that not give more 🤕 will need to try to set up an Appveyor tester project 😄

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

Related recent changes 6cb0949

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

Still not much difference: https://ci.appveyor.com/project/h5py/h5py/builds/23451490/job/nsia3mda8krmjydq

Looking at the code, could it be an issue that one of our env names is mpi4py? To get a None, something must be matching the PY_FACTORS_RE without matching the numeric part:

implied_version = tox.PYTHON.PY_FACTORS_RE.match(factor).group(2)
python_info_for_proposed = testenv_config.python_info
if not isinstance(python_info_for_proposed, NoInterpreterInfo):
proposed_version = "".join(
str(i) for i in python_info_for_proposed.version_info[0:2]
)
# '27'.startswith('2') or '27'.startswith('27')
if not proposed_version.startswith(implied_version):

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

That well could be the case 🤔 but why only on Windows? 🤔

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

Possibly because we're setting an environment variable TOXPYTHON which the config file refers to:

https://github.com/h5py/h5py/blob/2b6ec062baad266b74982b9a3ecd231a6736b242/appveyor.yml#L66-L71

https://github.com/h5py/h5py/blob/2b6ec062baad266b74982b9a3ecd231a6736b242/tox.ini#L31-L38

GitHub
HDF5 for Python -- The h5py package is a Pythonic interface to the HDF5 binary data format. - h5py/h5py
GitHub
HDF5 for Python -- The h5py package is a Pythonic interface to the HDF5 binary data format. - h5py/h5py

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

Why this is needed? 🤔

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

env TOXPYTHON=python python3.7 -m tox -av replicates to me locally too on MacOs.

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

I'm not sure.

Guesswork: Tox appears to expect interpreters with the version in the name, like python3.7, as is common on Unix systems. Appveyor doesn't appear to do this, putting the version in a folder name instead (C:\Python37\Python.exe). Possibly the TOXPYTHON environment variable was meant to be a way to bridge that gap. Presumably not the best way, though.

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

#1124 may be related.

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

@takluyver but we fallback to py - which would discover correctly your interpreters 👍

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

#1124 mentions doing this to test on both 32-bit and 64-bit Python on Appveyor, which h5py is also doing. I'm puzzled why it thinks the basepython is 2.7 in all cases, though.

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

Ah, I know. I'm looking at a run that happened on 2.7. It checks all the specified envs, not just the one it's going to use.

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

@takluyver that's by design, as we try to warn upfront if the config is bad, and in this case setting python (which is 3.5) is bad for a lot of environments. I would not advise using such env-var base-python override, the crash itself has been fixed via #1229.

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

Thanks!

I don't know tox well enough to see how to pass in the path to Python for each test job without overriding it for all environments like that. I'll keep on looking.

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

@takluyver the expectation here is that you don't need to set it, tox should be able to discover it, and if it fails then please describe the case and we'll advise further on it 👍

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

I think the issue is that we want to distinguish between 32 and 64-bit Python interpreters. As far as I know, tox doesn't have any specific support for distinguishing 32-bit vs 64-bit interpreters (but please correct me if I'm wrong!). So specifying explicit paths to Python seems like the clearest way to do what we want.

I've been looking at the docs. Unfortunately, I still don't see a much better way than what we are currently doing.

One option would be to have a separate tox INI file for Appveyor, which doesn't list different Python versions, so each Appveyor job can give it the Python version to run. But that's not easy either, because some of the dependencies differ according to the Python version.

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

Fair enough, we probably should support the {32,64} factors, but the question remains how do you discover 32 and 64-bit interpreters; on Windows, you can do based on their path, on Unix/Mac though?

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

I don't know of any easy way to do that, and at least on Linux I think it's unusual to have 32-bit Python installed on a 64-bit system.

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

BTW, now that the crash is fixed, I think my remaining question is the same as #1124. I've subscribed to that issue, so why don't we continue the discussion there.

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 29, 2019

We released tox 3.8.3 that should now fix it 👍

@takluyver
Copy link
Contributor Author

@takluyver takluyver commented Mar 29, 2019

Thanks!

@tox-dev tox-dev locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants