Pull requests

#61 Merged
Repository
carljm carljm
Branch
default
Repository
hpk42 hpk42
Branch
default

issue #35: custom install command

Author
  1. Carl Meyer
Reviewers
Description

This pull request implements a custom install command for installing packages into a testenv, both for dependencies and package-under-test sdists, as described in issue #35.

For simplicity I initially implemented just a single install_command option which is used for all installs. Personally I think this might be adequate as-is. If we want to split this out more, I'm not sure if there should be two options (install_deps_command and install_package_command) or three (install_deps_command, install_sdist_command, and install_develop_command)? The use of -e in the args set by developpkg is specific to pip, and so perhaps should be part of the default value of a separate install_develop_command option, rather than hardcoded? If we did split these out, I would probably have each accept a different substitution var: something like {deps} for install_deps_command, {sdistpath} for install_sdist_command, and {setupdir} for install_develop_command.

I don't like the is_pip argument to _installopts to decide whether to add --download-cache to the install command; it would be simpler IMO to just document "don't use the downloadcache testenv option if your chosen install command can't accept --download-cache". The problem with this is that tox currently explicitly sets envconfig.downloadcache if it finds PIP_DOWNLOAD_CACHE in the environ, which means people can't easily avoid having envconfig.downloadcache set in tox by just avoiding the config option. I'm not sure why tox does this; pip will respect PIP_DOWNLOAD_CACHE anyway itself, so why does tox need to explicitly pass --download-cache when PIP_DOWNLOAD_CACHE is set?

Comments and concerns welcome! :-)

Comments (9)

  1. Carl Meyer author

    Oh, and another note: some of the uses of :confval: that I added in the documentation don't actually create working links because they would need to be e.g. :confval:'deps=MULTI-LINE-LIST' rather than :confval:'deps', and I didn't want to use the former for readability reasons. Not sure how you'd like to address this; it would be nice if :confval: were smarter so you could reference one by name alone, even when a sample value placeholder is present.

  2. holger krekel repo owner

    Thanks for the good PR. I think:

    • download-cache can be stripped back, i.e. tox does not set it from the env var and only honours it if it is explicitely set. Are you up for an update to your PR for that?

    • develop/deps/pkg install separation: i think i'd like to start with a generic command and see how far we get. If one only uses the defaults, 1.6 would be backward-compatible to 1.5, right? In that case, we can later introduce differentiated installer_deps etc. if really needed. More specific installer_X would replace the generic installer_command definition. This should be manageable, right? As to developpkg - the feature was never released. I think it's fine to check if we are using pip and otherwise bail out with a clear error message (this can be checked at configuration time, i guess).

    • i don't know a good solution for the confval referencing other than the enhancement to sphinx you suggest.

    Would like to merge your PR along my suggested updates over the weekend or monday morning, as i am heading for tox and devpi releases very early next week. cheers and thanks, holger

  3. Carl Meyer author

    Yes, I can also update the pull request to avoid setting envconfig.downloadcache based on PIP_DOWNLOAD_CACHE.

    And yes, this should be backwards-compatible; the default install_command value results in the same install command as was previously used. There should be no problem splitting out the different install command options later if we need to. I like that plan.

    I would like to avoid all attempts to "check if we are using pip" if possible and stick with "duck-typing" rather than "type-checking" installers, because I know people use things like wrapper scripts around pip that may cause false negatives on such checks. For download cache I think I can remove this check if we no longer check the env var, because the documentation for the downloadcache option can be clear that your installer command (whatever it is) must support --download-cache, and if it doesn't the failure mode should be clear enough. I think we can take the same approach for developpkg.

    I'll make these updates today, and can make further updates tomorrow as needed.

  4. Carl Meyer author

    I guess the reason tox checks PIP_DOWNLOAD_CACHE is so that the env var can override a tox.ini-configured value. I can see why this makes sense, and in any case it would be backwards-incompatible to change it. So I didn't remove the checking of PIP_DOWNLOAD_CACHE entirely, I just rearranged the logic slightly so that if no downloadcache is present in the config, the --download-cache option is never passed to the install command. This is fully backwards-compatible, since pip will respect the env value in that case anyway.

    There was only one test checking the behavior of PIP_DOWNLOAD_CACHE and it was in test_venv.py instead of test_config.py. I found that test a bit confusing as it seemed to be checking multiple different things, so I just removed its assertions about config values (which were no longer correct) and added several new tests to test_config.py regarding envconfig.downloadcache.

  5. holger krekel repo owner

    Carl Meyer all looks good and i am basically fine to proceed to merge. Could you add two examples to the docs (doc/example/basic.txt) for using "easy_install" and for using "--find-links" (marked with .. versionadded: 1.6)?

    1. Carl Meyer author

      Unfortunately I am camping and out of internet range until Wednesday. Id be happy to add these examples when I get back but it sounds like that may not be in time for your release plans.

    2. Carl Meyer author

      Unfortunately I am camping and out of internet range until Wednesday. Id be happy to add these examples when I get back but it sounds like that may not be in time for your release plans.

      1. holger krekel

        Hi Carl,

        i'd very much appreciate if could you care for the docs tomorrow (wednesday)?

        Also, i think we should default to "--pre" with pip>=1.4. With tox i, and i guess others, test with development packages and i'd rather like to set a install_command to not use --pre when really needed. Could you care for this change?

        thanks, holger