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 DELETED ALL OF MY CODE #352

Closed
pytoxbot opened this Issue Sep 17, 2016 · 9 comments

Comments

Projects
None yet
2 participants
@pytoxbot

pytoxbot commented Sep 17, 2016

TL;DR: due to a (slightly) wrong tox invocation... tox punished me by deleting all of the files in my project.

Transcript of my shell session:

$ pwd
/home/andrew/tmp/dataplanet
$ ls -l
total 44
4 -rw-r--r-- 1 andrew andrew  311 Jul 27 10:47 bitbucket-pipelines.yml
4 drwxr-xr-x 2 andrew andrew 4096 Jul 27 10:47 docs
4 -rw-r--r-- 1 andrew andrew  167 Jul 27 10:47 MANIFEST.in
8 -rw-r--r-- 1 andrew andrew 4308 Jul 27 10:47 README
4 -rw-r--r-- 1 andrew andrew 1021 Jul 27 10:47 setup.cfg
4 -rw-r--r-- 1 andrew andrew 1564 Jul 27 10:47 setup.py
4 drwxr-xr-x 4 andrew andrew 4096 Jul 27 10:47 src
4 drwxr-xr-x 2 andrew andrew 4096 Jul 27 10:47 tests
4 -rw-r--r-- 1 andrew andrew  998 Jul 27 10:47 TODO
4 -rw-r--r-- 1 andrew andrew 1147 Jul 27 10:47 tox.ini
$ tox -e tests
GLOB sdist-make: /home/andrew/tmp/dataplanet/setup.py
tests create: /home/andrew/tmp/dataplanet
ERROR: invocation failed (exit code 1), logfile: /home/andrew/tmp/dataplanet/log/tests-0.log
ERROR: actionid: tests
msg: getenv
cmdargs: ['/usr/bin/python3', '-m', 'virtualenv', '--python', '/usr/bin/python3', 'dataplanet']
ERROR: InvocationError: /usr/bin/python3 -m virtualenv --python /usr/bin/python3 dataplanet (see /home/andrew/tmp/dataplanet/log/tests-0.log)
env: {... environment variables omitted ...}

Traceback (most recent call last):
  File "/home/andrew/.local/lib/python3.4/site-packages/virtualenv.py", line 840, in install_wheel
    from urlparse import urljoin
ImportError: No module named 'urlparse'

_________________________________________________________________ summary __________________________________________________________________
ERROR:   tests: InvocationError: /usr/bin/python3 -m virtualenv --python /usr/bin/python3 dataplanet (see /home/andrew/tmp/dataplanet/log/tests-0.log)                                                                                                                                  
andrew@ip-10-0-20-167:~/tmp/dataplanet>
total 0
$ pwd
/home/andrew/tmp/dataplanet
$ cd ../dataplanet
$ ls -l
total 16
4 drwxr-xr-x 2 andrew andrew 4096 Jul 27 10:39 bin
4 drwxr-xr-x 2 andrew andrew 4096 Jul 27 10:39 include
4 drwxr-xr-x 3 andrew andrew 4096 Jul 27 10:39 lib
4 drwxr-xr-x 2 andrew andrew 4096 Jul 27 10:39 log
$ pwd
/home/andrew/tmp/dataplanet

Here's a minimal tox.ini that will reproduce the problem:

[tox]
envlist = check, py34-tests


[testenv]
envdir=
    check:          {toxworkdir}/py
    py34:           {toxworkdir}/py34


skip_install=
    check:          true
    tests:          false


deps=
    check:          flake8
    tests:          pytest


commands=
    check:          flake8 --show-source {toxinidir}/src
    tests:          py.test --strict --doctest-modules {toxinidir}/tests

Did you see what happened there? I meant to run tox -e py34-tests, but instead ran tox -e tests. This somehow resulted in {toxworkdir} getting set to {toxinidir} (!!!) and as a result tox helpfully deleted my project's code and replaced it with a tox working directory....

THIS IS NOT ACCEPTABLE BEHAVIOUR.

In case you're wondering "why configure things in this particular manner?", my full configuration has additional environments e.g. py27-tests, py34-docs, py34-build, etc. and I want to speed things up by sharing the working directory between these different targets. (That is, I want exactly one toxworkdir per Python version.)

@pytoxbot

This comment has been minimized.

pytoxbot commented Sep 17, 2016

Original comment by @obestwalter

It's stunningly non-obvious that the actual semantics are "set envdir to a blank value (i.e. remove the default value but do not replace it with anything else) if the environment name doesn't contain either check or py34."

I agree and I would venture a guess that this is a bug and not a feature.

@pytoxbot

This comment has been minimized.

pytoxbot commented Sep 17, 2016

Original comment by @obestwalter

Hi Andrew,

I have not much experience with the codebase and the backwards compatibility implications mentioned by @pjdelport so I went for the easy way out, to make sure that it's simply not possible to delete the project dir, whatever happens, as I am sure nobody wants that.

@pytoxbot

This comment has been minimized.

pytoxbot commented Sep 17, 2016

Original comment by andrew-tipton-cgg

Oliver, thanks for the PR. I think it's a good idea to have a sanity check for this specific situation.

I still believe that the tox.ini parsing needs to be fixed, however. Recall my original configuration:

envdir=
    check:          {toxworkdir}/py
    py34:           {toxworkdir}/py34

The "principle of least surprise" suggests that this snippet should be interpreted as "if the environment name contains check, set envdir={toxworkdir}/py; if the environment name contains py34, set envdir={toxworkdir}/py34; otherwise, leave envdir at its default value."

It's stunningly non-obvious that the actual semantics are "set envdir to a blank value (i.e. remove the default value but do not replace it with anything else) if the environment name doesn't contain either check or py34."

@pytoxbot

This comment has been minimized.

pytoxbot commented Sep 17, 2016

(NOTE: tox just moved from bitbucket and the automatic links are not resolved correctly (yet) - there are also some PR leftovers on Bitbucket. This PR is still open here: https://bitbucket.org/hpk42/tox/pull-requests/204/catch-project-deletion-if-envdir-toxinidir/diff instead).

Original comment by @obestwalter

See pull request #204

@pytoxbot

This comment has been minimized.

pytoxbot commented Sep 17, 2016

Original comment by @obestwalter

I agree. This should never happen. This is basically the worst thing that can happen during development, especially because it makes it very probable that you lose hours of work because you are testing the changes locally and have not pushed them to the remote yet.

@pytoxbot

This comment has been minimized.

pytoxbot commented Sep 17, 2016

Original comment by andrew-tipton-cgg

My understanding was that if the "environment" didn't match either check or py34 then the envdir setting would remain the default.

Note that the following does not work:

envdir=
    {toxworkdir}/default
    check:  {toxworkdir}/py
    py34:   {toxworkdir}/py34

Again, let me reiterate that UNDER NO CIRCUMSTANCES SHOULD TOX EVER DELETE MY PROJECT DIRECTORY. There is absolutely no reason for anyone to ever intentionally configure tox in such a manner, and it should therefore refuse to proceed if such a situation arises due to misconfiguration. Are we 100% clear on this particular aspect of the issue?

@pytoxbot

This comment has been minimized.

pytoxbot commented Sep 17, 2016

Original comment by @pjdelport

The problem here is that the tox.ini sets envdir to empty by default, which overrides the default value of {toxworkdir}/{envname}:

envdir=
    check:          {toxworkdir}/py
    py34:           {toxworkdir}/py34

I don't think there's anything that Tox can reasonably do about this misconfiguration except maybe raising an overridable warning if envdir is empty? This would mean breaking backward compatibility a bit, but it may be justifiable in this case?

@obestwalter

This comment has been minimized.

Member

obestwalter commented Sep 18, 2016

@pjdelport that's what I thought. We should make sure at the very least, that a user can't delete the whole project as a result of a misconfiguration. See #352 (comment)

@obestwalter

This comment has been minimized.

Member

obestwalter commented Oct 13, 2016

fixed in 3e66f01 - part of 2.4.0 release.

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