tox-dev / tox

Command line driven CI frontend and development task automation tool.
https://tox.wiki
MIT License
3.67k stars 517 forks source link

Support expanding globs/path wildcards in `tox.ini` #1571

Open webknjaz opened 4 years ago

webknjaz commented 4 years ago

Currently, if you put a command like delocate-wheel dist/*.whl in tox.ini, that command (delocate-wheel) will get "dist/*.whl" string as its arg. But if you run it in shell, it'll get the list of matching paths.

Some programs are okay with that and apply glob internally while others don't know how to do this. Or, in case of delocate, it does it partially but has a bug where it outputs a file with an invalid filename called *.whl.

So in some cases, it's reasonable to want to expand a glob explicitly.

I suggest adding a new substitution syntax. This is what I have in mind:

commands =
  {envpython} -m some-command {glob:dist/*.whl} {glob:**/*.tar.gz:fallback/path/without/substitutions.txt} {env:VAR:{glob:fallback/*.zip}}

Refs:

webknjaz commented 4 years ago

In my case, I implemented a workaround for my case but it's sort of ugly because it requires running a separate command for this env and doesn't allow taking advantage of depends and putting all related envs into one TOXENV to let tox figure out the sequence internally:

commands =
    {envpython} -m \
      delocate.cmd.delocate_wheel \
      -v \
      {posargs:"{env:PEP517_OUT_DIR}"/*.whl}
gaborbernat commented 4 years ago

I think this is a good idea of a feature, note I personally don't plan to do work on this before tox 4... so if you want it before feel free to put in a PR.

jayvdb commented 4 years ago

Note that test_install_deps_wildcard shows the following works

[tox]
  distshare = {toxworkdir}/distshare
[testenv:py123]
deps=
  {distshare}/dep1-*

Possibly this could be used with {[testenv:py123]deps} to achieve roughly what this issue wants.

That test was added in v1.3 2db7902e. However, I cant find "wildcard" or "glob" in the source. Test test_matchingdependencies_latest also does similar glob support.

Possibly that only allowed the * to go through to pip which might do the globbing.

In any case, implementing {packages} in the config substitution engine would require bringing any globbing into the config engine as requested by this issue. c.f. https://github.com/tox-dev/tox/issues/594 (unsolved) and https://github.com/tox-dev/tox/issues/1695 (very conservatively solved).

jayvdb commented 4 years ago

Checked, and the below does not work. It also results in a literal *. Globbing must be happening in tox/venv.py.

[tox]
distshare = {toxworkdir}/distshare

[testenv:py123]
deps=
  {distshare}/dep1-*

[testenv:deps_glob]
commands = ls {[testenv:py123]deps}

pip also isnt doing the globbing.

pip install '/path/to/.tox/distshare/dep1-*'
Defaulting to user installation because normal site-packages is not writeable
ERROR: Invalid requirement: '/path/to/.tox/distshare/dep1-*'
Hint: It looks like a path. File '/path/to/.tox/distshare/dep1-*' does not exist.

I do know of one way to achieve it, using my alpha grade plugin tox-backticks which allows the following

[tox]
envlist=deps_glob
requires=tox-backticks

[testenv:py123]
deps=
  dist/*.whl

[testenv:deps_glob]
setenv =
  FOO=`python -c 'import glob,sys; sys.stdout.write(" ".join(glob.glob(sys.argv[1])))' {[testenv:py123]deps}`
commands = echo {env:FOO} 

That globbing can be done using another python package.

webknjaz commented 4 years ago

No need for any third-party packages. This task is quite straightforward — we just need {glob:...} substitution implemented in tox that essentially would inject ' '.join(glob.glob(...)) in-place.

jayvdb commented 3 years ago

Dont get me wrong (edited for clarity), I'm broadly in favour of adding explicit syntax for globbing. But it isnt in core yet.

There are also significant problems with syntax in general that IMO should be fixed first, especially wrt to the replacement engine handling of characters like \ and } (c.f. https://github.com/tox-dev/tox/issues/1708) which occur in paths

{glob:..} implementation specifics could make this worse. The problem is not the simple case of {glob:dir/*.whl}. The problem is that whatever is inside {..} is expected to support other {..} syntax, such as {glob:{toxworkdir}/*.whl}, where toxworkdir could be any other substitution syntax recursively.

If we can prevent all special characters in the {glob:..}, allowing only glob char *, ? and (new) {/} (os.pathsep), it is much safer to introduce and shouldnt prevent many valid use cases.

Something to think through is that Python module glob introduces an extra layer of escaping:

For a literal match, wrap the meta-characters in brackets. For example, '[?]' matches the character '?'.

c.f. https://docs.python.org/2/library/glob.html

fnmatch also supports !; I dont recall if glob also supports !. I dont forsee any problems with ! in glob syntax, as that is only used in factor negation.

A much simpler implementation detail: Which directory setting should it be relative to, if given a relative path ("{glob:dist/*.whl}")? {toxinidir}?