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

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

Open
webknjaz opened this issue Apr 28, 2020 · 6 comments
Open

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

webknjaz opened this issue Apr 28, 2020 · 6 comments
Labels
feature:new something does not exist yet, but should help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.

Comments

@webknjaz
Copy link
Contributor

webknjaz commented Apr 28, 2020

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 webknjaz added the feature:new something does not exist yet, but should label Apr 28, 2020
@webknjaz
Copy link
Contributor Author

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
Copy link
Member

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
Copy link

jayvdb commented Oct 19, 2020

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 2db7902. 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. #594 (unsolved) and #1695 (very conservatively solved).

@jayvdb
Copy link

jayvdb commented Oct 21, 2020

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
Copy link
Contributor Author

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
Copy link

jayvdb commented Oct 23, 2020

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. #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}?

manuq added a commit to endlessm/kolibri-explore-plugin that referenced this issue Feb 16, 2021
From the github-actions configuration.

We are disabling this command for now, since tox.ini doesn't
support expanding globs. tox-dev/tox#1571

https://phabricator.endlessm.com/T31414
@gaborbernat gaborbernat added this to the 4.1 milestone Mar 1, 2021
@gaborbernat gaborbernat added the help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. label Jun 16, 2023
@gaborbernat gaborbernat removed this from the P-1 milestone Jun 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature:new something does not exist yet, but should help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.
Projects
None yet
Development

No branches or pull requests

3 participants