Adopt use of module invocation method for tools

[ssbarnea]

Avoid annoying warnigns raised by tox about externals when sitepackages
is enabled, mainly because these tools where already found installed
as system packages.

PR Type

• Bugfix Pull Request

[webknjaz]

I assume this suggests using things outside of tox-managed env which is wrong.

[decentral1se]

I assume this suggests using things outside of tox-managed env which is wrong.

But I thought we'd already gone down that path with https://tox.readthedocs.io/en/latest/config.html#conf-sitepackages and this is only reducing warnings because of this (I don't make any comment whether this is "right" but it makes sense based on the existing tox configuration).

And it wasn't removed in #1895? See https://github.com/ansible/molecule/pull/1895/files#diff-b91f3d5bd63fcd17221b267e851608e8R59. Oh wait, that selinux bug ...

[webknjaz]

@decentral1se that's just to facilitate selinux but I think that relying on something outside of tox envs (except for POSIX binaries like rm) is bad. If we say "tox doesn't manage this", install it yourself, then we have no way of controlling what user will set up and they will get back to us asking to "fix" their 100% custom env just because they have a different global flake8 version or so.
Why even have tox if we have to manually set up each bit outside of it? Also, encouraging people to have something global for all their projects doesn't sound right either.
Currently, tox is a glue for standardizing everything and it should remain this way. I still think that if users want site-packages, they'd better use a CLI arg for that. This feature comes with a price and even tox maintainers were talking about the possibility to remove it...

[webknjaz]

We need to check whether we can inject something like --force-reinstall into the deps section...

[ssbarnea]

@webknjaz We cannot inject force reinstall and even if we could I would be against as I do not want to reinstall deps on each execution. The lack of dependency refresh on tox is a well known issue which is not limited to use of system site packages. Lucky for us the current maintainer started to work on a solution but it will take many months before we would be able to consuime it (it is a very complex issue related to tox design).

For the moment I see no real reason for not merging this as the only side effect it has is to avoid some really annoying warnings.

[webknjaz]

@ssbarnea solve this with python -m style invocations then.

[ssbarnea]

@webknjaz I used the python -m for years, spending a lot of time explaining people why was better or making PRs on lots of tools in order to implement module calling convention (you cannot guess how many authors are totally unaware of this feature).

Still, pur cannot be called as a module and worse it was somethign I read yesterday on https://discuss.python.org/t/pep-582-python-local-packages-directory/963/11 probbly which said that current implementation of -m calling method does a lookup in current folder first, which means that if you happen to have some files/modules there with the same name they will be loaded instead of the tool you want. Never happened to me but.

To summarize, I will use module calling whenever is possible.

[webknjaz]

@ssbarnea local folder lookup is kinda expected and I use it all the time. You can even use python -m . which is pretty handy. I don't see any problem here.

Oh, and pur has been removed. I was hoping to go forward now and enable some bot for that.

[webknjaz]

Why did you do this?

[webknjaz]

Only this env needs sh so there's no need to move it to the global one.

[ssbarnea]

To simplify setup. What is the great feature of keeping these in several places? ... unless we do take a special pleasure in having more complex tox.ini files. Clearly is not related to security or so, in the end any code change would need to be reviewed.

[webknjaz]

This is related to envs being obvious and avoiding a godobject antipattern.

When you look at the env you shouldn't have to look up all the other envs to guess what else may influence it. Explicit is better than implicit.

Also, dumping everything into one env makes it unreadable/unmaintainable as I've explained somewhere else already.

[webknjaz]

Hey, I've changed my mind (again, sorry) about whitelist_externals.
rm is only used in build-dists and shouldn't be put into the base env therefore.

There's been some refactoring by @themr0c where I explained why it should move in this direction.