Regression / behaviour change: install_command with empty evaluated value #2695
Comments
gaborbernat
added
bug:minor
|
Feel free to put in a PR to fix this 👍 |
|
A couple of clarifying questions for a pull request (either myself or anyone else helping out before I do):
(I couldn't determine answers to these from the documentation, but it's possible that I missed something) |
We no longer have custom parsing logic per key, so this custom logic from v3 is no longer supported. In the case, you mention, the install_command will be set to None. So breaking change, but intended. This is to support: pass_env =
A
c: DGenerates A, D on C, and A otherwise.
All would match, and all would be inserted. This is a breaking change we should probably document, to be fair. |
Thanks, that makes sense. One more question: would it seem reasonable to add a The use case (as kinda-but-not-really-demonstrated by If so, a configuration like this could be the result (not valid currently): [tox]
isolated_build = true
[testenv]
platform =
beos: beos
install_command =
{default_install_command}
beos: --require-hashes |
|
That wouldn't work because that would imply two separate commands because of the new line 🤔 We don't join lines for install_command... |
|
That being said if all you want is to include set_env =
beos: PIP_REQUIRE_HASHES=1And that's our recommendation going ahead rather than overwriting install_command. |
|
The fix I'm considering is to raise a |
Not really, that was an attempt at a small example case, and maybe not a good one 😄 In practice: at the moment there's a bug with compiled-from-source So I'm wondering how to do that in |
|
I reiterate all pip configurations should be passed through environment variables. I plan to deprecate support for |
|
Ok, sorry - I didn't understand that all pip configuration should begin using environment variables instead. That should work in the binary-wheel selection case. Thank you. Edit: ...and thanks also for the correction :) |
To clarify all pip configurations 😊 The env var configuration for pip is more generic and easier to maintain. |
jayaddison
changed the title
|
Re-prompting this question (that I shouldn't have embedded in a bunch of unrelated text..):
|
I'm -1 on this; I don't see how that would be valuable. Better just overwrite it entirely in the spirit of explicit better than implicit. |
Ok, yep - after a bit more consideration, I agree; thanks. (in particular: if the default command did change in future, then codebases using a replacement pattern like that might experience unexpected and tricky-to-detect ripple effects) |
That sounds like a breaking change requiring transition documentation and a major version bump either way. Unlikely we'd do something so crazy. |
gaborbernat
added
area:documentation
and removed
bug:minor
|
A PR explaining the new behavior in the breaking change section will resolve this issue. |
Issue
Somewhere between
toxversions 3.21.4 and 4.0.8, runningtoxbegan raising anIndexErrorexception when using thetox.inifile provided as a minimal repro case.Environment
Output of running tox
Provide the output of
tox -rvv:Minimal example
If possible, provide a minimal reproducer for the issue:
tox.ini
(note: a valid
pyproject.toml/setup.pyfile is required alongside this -- I've been using the examplepyproject.tomlprovided below)pyproject.toml
The text was updated successfully, but these errors were encountered: