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

Implement --devenv #1326

Merged
merged 1 commit into from
Jun 1, 2019
Merged

Implement --devenv #1326

merged 1 commit into from
Jun 1, 2019

Conversation

asottile
Copy link
Contributor

@asottile asottile commented May 28, 2019

This implements an option which makes it very easy to set up development virtualenvs based on the existing [testenv] configurations.

Contribution checklist:

(also see CONTRIBUTING.rst for details)

  • wrote descriptive pull request text
  • added/updated test(s)
  • updated/extended the documentation
  • added relevant issue keyword
    in message body
  • added news fragment in changelog folder
    • fragment name: <issue number>.<type>.rst for example (588.bugfix.rst)
    • <type> is must be one of bugfix, feature, deprecation,breaking, doc, misc
    • if PR has no issue: consider creating one first or change it to the PR number after creating the PR
    • "sign" fragment with "by :user:<your username>"
    • please use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files - by :user:superuser."
    • also see examples
  • added yourself to CONTRIBUTORS (preserving alphabetical order)

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this better than tox -e py37 --develop --notest, it's much more expressive and gives the user the choice of python interpreter to use? If you want to create the env at root level rather than in .tox I would much prefer to expose the envdir via a CLI option. So maybe do then tox -e py37 --develop --notest --envdir=venv, which could be aliased in the user shell for a quick go.

@asottile
Copy link
Contributor Author

My initial goal was to replace this (incorrect) copypasta in all of my projects: https://github.com/pre-commit/pre-commit/blob/0b6a39768aa1eeee937b5146d35eb69edbc4715b/tox.ini#L17-L19 (it should have usedevelop=true, what if I want a devenv of py27? etc. etc.)

But then I realized I could make it much easier to use and support all of the various toxenvs. I also wanted a command that would be easy to type (without needing to remember three options). I settled on --devenv ENVNAME. I considered --envdir as a cli option but typing tox --envdir venv --develop --notest -e py37 is really really clunky.

The main reason not to use .tox/XXX environments directly is they're hidden files and feel like private implementation detail. A non-hidden environment at . is much more "let's make a virtualenv and poke around with it". Also it's very easy to break the testenvs if you mess with .tox/pyXX directly :)

--devenv does support all of the various tox environments, for instance:

$ tox --devenv venv37 -e py37
py37 create: /tmp/tox/venv37
py37 installdeps: pip == 19.1.1
py37 develop-inst: /tmp/tox
py37 installed: apipkg==1.5,atomicwrites==1.3.0,attrs==19.1.0,coverage==4.5.3,execnet==1.6.0,filelock==3.0.12,flaky==3.5.3,freezegun==0.3.11,importlib-metadata==0.15,more-itertools==7.0.0,packaging==19.0,pathlib2==2.3.3,pluggy==0.12.0,psutil==5.6.2,py==1.8.0,pyparsing==2.4.0,pytest==4.5.0,pytest-cov==2.7.1,pytest-forked==1.0.2,pytest-mock==1.10.4,pytest-randomly==1.2.3,pytest-xdist==1.28.0,python-dateutil==2.8.0,six==1.12.0,toml==0.10.0,-e git+git@github.com:tox-dev/tox@2a7f94068afeaef90ea6a00f980dc04b012c21a6#egg=tox,virtualenv==16.6.0,wcwidth==0.1.7,zipp==0.5.1
___________________________________ summary ____________________________________
  py37: skipped tests
  congratulations :)
$ ./venv37/bin/python --version
Python 3.7.3

$ tox --devenv venv35 -e py35
py35 create: /tmp/tox/venv35
py35 installdeps: pip == 19.1.1
py35 develop-inst: /tmp/tox
py35 installed: apipkg==1.5,atomicwrites==1.3.0,attrs==19.1.0,coverage==4.5.3,execnet==1.6.0,filelock==3.0.12,flaky==3.5.3,freezegun==0.3.11,importlib-metadata==0.15,more-itertools==7.0.0,packaging==19.0,pathlib2==2.3.3,pluggy==0.12.0,psutil==5.6.2,py==1.8.0,pyparsing==2.4.0,pytest==4.5.0,pytest-cov==2.7.1,pytest-forked==1.0.2,pytest-mock==1.10.4,pytest-randomly==1.2.3,pytest-xdist==1.28.0,python-dateutil==2.8.0,six==1.12.0,toml==0.10.0,-e git+git@github.com:tox-dev/tox@2a7f94068afeaef90ea6a00f980dc04b012c21a6#egg=tox,virtualenv==16.6.0,wcwidth==0.1.7,zipp==0.5.1
___________________________________ summary ____________________________________
  py35: skipped tests
  congratulations :)
$ ./venv35/bin/python --version
Python 3.5.7

@gaborbernat
Copy link
Member

gaborbernat commented May 28, 2019

I'm worried a bit the --develop will be confusing alongside --devenv 🤔 otherwise sounds okay-ish. I use https://github.com/nokia/PyVenvManage to assign interpreters to my open project, so never ran into the issue of feeling .tox being private. I'm in the end not too much against this but would like some nice documentation for this 👍

GitHub
PyVenvManage is a plugin for managing the Python interpreter of Pycharm Projects - nokia/PyVenvManage

@asottile
Copy link
Contributor Author

yeah happy to fill out the docs on this -- wanted some feedback before I poured work into that though 😆 👍

src/tox/session/__init__.py Outdated Show resolved Hide resolved
@rpkilby
Copy link
Member

rpkilby commented May 28, 2019

Just my two cents, but my initial reaction is -0. tox is fundamentally a test/CI tool and isn't really intended to manage the development environment. My main concern is that this feels like scope creep and will add unexpected complexity in how various features/options interact. I'd argue that this should exist as a plugin. But again, -0 so not strongly opinionated on this.

@gaborbernat
Copy link
Member

@asottile is this still on? 😄

@asottile
Copy link
Contributor Author

Yeah this week has been Hella busy, going to work on this on the weekend

@gaborbernat
Copy link
Member

cool

@obestwalter
Copy link
Member

obestwalter commented May 31, 2019

I really like the idea except for having any tox created environments outside of <project>/.tox by default.

The main reason not to use .tox/XXX environments directly is they're hidden files and feel like private implementation detail.

I understand that, but I think this is a matter of communicating what the .tox folder is for clearer in the docs then (I'll volunteer to help with that part). I think .tox inside of the project is a great established container which already has the advantage that many IDEs and tools are aware of it and do the right thing (e.g. ignoring everything in .tox for code searches etc). Like @gaborbernat said: having all the envs conveniently accessible inside the project makes it easy for tools like pyvenvmanage to pour some user friendly sugar over the whole thing.

So how about completely taking away the freedom of naming the devenv and placing it in .tox with a devenv pre- or postfix?

One of the above examples would look something like that then:

$ tox --devenv -e py37
py37 create: <toxinidir>.tox/devenv-py37

@asottile
Copy link
Contributor Author

putting it in .tox defeats my goals. I want it to replace the current directions of virtualenv venv or tox -e venv # which has envdir=venv`)

@gaborbernat
Copy link
Member

I'm fine in the end with Anthonys proposal. Both Visual Studio code and Pycharm doesn't handle well .tox folder, but do great with a venv folder. So let's go with this 👌 ahead

@asottile asottile changed the title WIP: implement --devenv Implement --devenv Jun 1, 2019
@obestwalter
Copy link
Member

@rpkilby

Just my two cents, but my initial reaction is -0. tox is fundamentally a test/CI tool and isn't really intended to manage the development environment.

FWIW that's exactly what I routinely use tox for. It is also recommended in the docs. So the scope has already crept :)

You have a point though in questioning if we really need a different way of doing this, or if the existing explicit, project specific approach is good enough.

Speaking for myself: for most projects I am involved in, the way it is documented is good enough and also more explicit as the --devenv approach. Other developers can discover all that is important for the project (including creating a devenv) by calling tox -av or looking into the tox.ini for the project. So I also don't feel the need of growing another command line switch that does something that is already possible only with a bit more work (unless I am still missing something central, then pleasae correct me @asottile).

Maybe it might also be worth considering #634 if more of these kinds of scenarios pop up?

@asottile

A non-hidden environment at . is much more "let's make a virtualenv and poke around with it". Also it's very easy to break the testenvs if you mess with .tox/pyXX directly :).

[...]

putting it in .tox defeats my goals. I want it to replace the current directions of virtualenv venv or tox -e venv # which has envdir=venv`)

Like I said: I really feel like I am still missing something.

Everything in .tox can be automatically regenerated with a simple call and poking around in there is perfectly o.k. - breaking things in there are not a problem that a simple tox -re whatever can't fix. So I still think that this is still a question of communicating that clearer rather than starting to have two "classes" of of tox generated environments (the throwaway "test" type and the "precious" devenv type).

So what is the advantage of having a tox generated environment outside of .tox by default (because explicitly it is already possible with overriding envdir)?

@gaborbernat
Copy link
Member

I know @ambv uses a wrapper script to achieve similar effect, I'm fine with offering it out of box, especially as requires little maintainence, and I feel will make easier to use tox for many people (we could also fight it with documentation, but why bother if this makes it easier to use and understand).

@asottile
Copy link
Contributor Author

asottile commented Jun 1, 2019

The hidden .tox directory is very much not-beginner-friendly, even as a relatively experienced developer I see hidden file and it screams "please don't touch me" (not going to touch .coverage, .cache, .pytest_cache, .git, etc,) -- I myself however am comfortable poking around in .tox but I don't expect that for anyone else. Being able to see a development environment as a non-hidden directory in the working directory is much much easier, and in many cases IDEs will auto detect and use those environments (for example, pycharm will auto-find anything named venv* in your project root).

But that's all fine, I could accomplish that today, however it would require an absolute ton of boilerplate.

Consider the following tox.ini which is ~basically the standard one I use everywhere -- I've reduced it to almost no boilerplate for discussion:

[tox]
envlist = py27,py36,py37,py38,pypy,pypy3,pre-commit

[testenv]
deps = -rrequirements-dev.txt
commands =
    coverage erase
    coverage run -m pytest {posargs:tests}
    coverage report --fail-under 100
    pre-commit install

[testenv:pre-commit]
skip_install = true
deps = pre-commit
commands = pre-commit run --all-files --show-diff-on-failure

Now let's say I wanted an easy way to make those environments into devenvs, I would need to make 7 additional environments, each with basepython, envdir, commands=, usedevelop=true (and I'd have to carefully name them, ensure that the basepython is in line with the original env they're trying to mirror, and make sure to continually sync them with envlist):

[testenv:devenv-py27]
basepython=python2.7
envdir=venv-py27
usedevelop=true
commands=

That's an absolute boatload of error prone duplication and boilerplate (assuming I separate each by a newline that adds 42 lines to my 14 line tox.ini (!!!). Most of which I wouldn't even use! Whereas with --devenv, I can delete all that boilerplate and get the functionality from a short easy to remember command.


technically with --devenv there's nothing that says it has to be outside of .tox -- @obestwalter you could use tox --devenv .tox/devenv-py27 -e py27 if you're uncomfortable with envdirs outside of there.

@gaborbernat gaborbernat merged commit 1554104 into tox-dev:master Jun 1, 2019
@asottile asottile deleted the devenv branch June 1, 2019 15:56
@obestwalter
Copy link
Member

obestwalter commented Jun 1, 2019

I still don't agree with the idea that anything tox related should be created outside of .tox|~/.tox by default. Let me try again.

If the default of --devenv is to create environments outside of tox a long standing promise is broken and I for one really don't like the thought of that.

Let's enter the mind of a hypothetical developer that is mildly experienced in the use of tox and has built up a set of (reasonable - I might add) assumptions about how the tool works:

megacalc dev: So I see that new switch popping up with the new release of tox. When I type --help I see

--devenv DEVENV sets up a development environment based on the tox configuration. (default: None)

megacalc dev: Sounds interesting. "based on the tox configuration" ... whatever that means ... I guess that means I just give it some name and then it creates a development environment for me and if I don't say anything extra it will use the first environment in my envlist? Shall I have a look at the documentation? Nah I'll just try it. What can go wrong? From my experience with tox I know that the environment will be placed in the .tox folder just like it is the default for every other environment that tox creates. My project is called megacalc and the package I am working on lives in the megacalc folder in my poject root. I like the name megacalc. I could say it all day. I will also name my development environment megacalc because is is the development environment for megacalc. It's a good name.

megacalc dev: So I type ...

tox --devenv megacalc

megacalc dev: Where are my megacalc source files? Oh ... I just replaced them with my development environment ... when was the last time I pushed my changes?

I don't think the potential for damage is worth the extra convenience of not having to explicitly request the creation outside of its managed folder.

Next thing that might surprise a mildly experienced tox user:

... sets up a development environment based on the tox configuration

legacy dev: I guess that means that all my default factors and such works like I am used to by now, so I can create a development environment based on python2.7 by typing

tox --devenv py27

legacy dev: ok the name is "py27" but if I use it I get syntax errors regarding my print statements ... I guess I am understanding something wrong.

That's why I still prefer describing the development environment(s) explicitly in the tox.ini rather than hiding it behind a command line switch that might be very surprising for the naive user.

There are also project where development environments as such make no sense at all, but now it's an option for all projects.

@asottile
Copy link
Contributor Author

asottile commented Jun 1, 2019

sounds like an easy docs fix, metavar='ENVDIR' is probably an easy oneline patch that clarifies that

to be fair the envdir setting suffers from exactly the same "usability problem" (envdir=megacalc) but that hasn't been an issue

@obestwalter
Copy link
Member

The difference is: one is default and one is explicitly configured, so it's not a docs fix - it's a real risk in the default usage, so I am really strongly -1 on shipping this like that. We neutered envdir to not be able to delete the whole project due to #352 btw.

I also hope that before someone sets envdir=megacalc they will have had a closer look at what they are doing because they are in the process of writing the automation config for a project. I keep repeating myself but this is why I think it as a GoodThing to encode things like that in the project configuration rather than exposing them as command line options that might behave in surprising ways if they are used.

@asottile
Copy link
Contributor Author

asottile commented Jun 1, 2019

I added a test to ensure that doesn't happen: #1333

(it currently was ignoring --devenv when running --devenv '' so I switched all the checks to devenv is not None so it triggers the proper error)

@gaborbernat
Copy link
Member

I don't share your worries Oliver on this to be honest and by talking to some users I feel this will make people worry less about using .tox hidden folder. Yes we can try to document it but people reading it is unlikely, as opposed to doing a --help. Making it easier to use for newcomers is something that is going to help us with adoption.

@obestwalter
Copy link
Member

obestwalter commented Jun 2, 2019

Making it easier to use for newcomers is something that is going to help us with adoption.

That is where we differ. I think setting the default rootdir for --devenv to {toxinidir} instead of the "natural" {toxworkdir} does not help newcomers. I understand where it's coming from, but I still think that in the long run we're better off keeping the defaults like they are. At the moment it's simple: all environments that tox creates end up in {toxworkdir} by default. That's why it's called toxworkdir after all.

I think sensible defaults are very important. ~/.virtualenvs is also a very common place to put venvs but I would also not recommend that as a sensible default for tox generated venvs just like I wouldn't recommend {toxinidir} as sensible default (especially not if the name is freely choosable and will overwrite any existing folder or file of the same name without warning).

If we are talking about making it easy for newcomers we should care about consistent behaviour to provide a clear mental model about the tool. So again: if you can say: "every virtual environment that tox creates ends up in .tox by default": that is easy to explain to newcomers. As opposed to: "some virtual environments that tox creates end up in .tox by default some don't - it's complicated".

@gaborbernat
Copy link
Member

gaborbernat commented Jun 2, 2019 via email

@obestwalter
Copy link
Member

Well, it's all a guessing game anyway. We all see different slices of the user base and we don't really know who's using what, why and how. Let's just see how this plays out then 👍

@carltongibson
Copy link
Contributor

So, a year late to the party, but I just want to say AWESOME! 💃 Thanks for adding this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants