Fix: Misconfiguration in ask dependency interactive

[PabloEmidio]

Fixing a misunderstand of buitin str function: maxsplit can return maxsplit+1 items

Resolves: #7567

---

Although, this alteration resolves the broken message reported, we'd be better to treat the string using regex and validating the received result.

[radoering]

Can you add a test, please?

[PabloEmidio]

@radoering debbuging the source code, I find the follow behavior as used this PR change:

Package to add or search for (leave blank to skip): now we crash
> /home/utrape/Documents/programming/contributions/poetry/src/poetry/console/commands/init.py(484)_validate_package()
-> if package and len(package.split()) > 2:
(Pdb) c
Invalid package definition.
Package to add or search for (leave blank to skip): about now
> /home/utrape/Documents/programming/contributions/poetry/src/poetry/console/commands/init.py(484)_validate_package()
-> if package and len(package.split()) > 2:
(Pdb) c
Adding about now

Add a package (leave blank to skip): here we go
Adding here we go

Clearly the validation has not been called at some asking.

Working on it.

[PabloEmidio]

I modify an existing test to add the scenary reported. When runned against master generate the follow output:

=========================================================================== FAILURES ============================================================================
_________________________________________________________ test_interactive_with_wrong_dependency_inputs _________________________________________________________
[gw3] linux -- Python 3.11.2 /home/utrape/.cache/pypoetry/virtualenvs/poetry-X0uBKQkB-py3.11/bin/python

tester = <cleo.testers.command_tester.CommandTester object at 0x7f2628c88ed0>, repo = <tests.helpers.TestRepository object at 0x7f2628c49950>

    def test_interactive_with_wrong_dependency_inputs(
        tester: CommandTester, repo: TestRepository
    ):
        repo.add_package(get_package("pendulum", "2.0.0"))
        repo.add_package(get_package("pytest", "3.6.0"))
    
        inputs = [
            "my-package",  # Package name
            "1.2.3",  # Version
            "This is a description",  # Description
            "n",  # Author
            "MIT",  # License
            "^3.8",  # Python
            "",  # Interactive packages
            "foo 1.19.2",
            "pendulum 2.0.0 foo",  # Package name and constraint (invalid)
            "pendulum 2.0.0",  # Package name and constraint (invalid)
            "pendulum 2.0.0",  # Package name and constraint (invalid)
            "pendulum 2.0.0",  # Package name and constraint (invalid)
            "pendulum@^2.0.0",  # Package name and constraint (valid)
            "",  # End package selection
            "",  # Interactive dev packages
            "pytest 3.6.0 foo",  # Dev package name and constraint (invalid)
            "pytest 3.6.0",  # Dev package name and constraint (invalid)
            "pytest@3.6.0",  # Dev package name and constraint (valid)
            "",  # End package selection
            "\n",  # Generate
        ]
>       tester.execute(inputs="\n".join(inputs))

/home/utrape/Documents/programming/contributions/poetry/tests/console/commands/test_init.py:623: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/home/utrape/.cache/pypoetry/virtualenvs/poetry-X0uBKQkB-py3.11/lib/python3.11/site-packages/cleo/testers/command_tester.py:88: in execute
    self._status_code = self._command.run(self._io)
/home/utrape/.cache/pypoetry/virtualenvs/poetry-X0uBKQkB-py3.11/lib/python3.11/site-packages/cleo/commands/base_command.py:119: in run
    status_code = self.execute(io)
/home/utrape/.cache/pypoetry/virtualenvs/poetry-X0uBKQkB-py3.11/lib/python3.11/site-packages/cleo/commands/command.py:62: in execute
    return self.handle()
/home/utrape/Documents/programming/contributions/poetry/src/poetry/console/commands/init.py:207: in handle
    self._format_requirements(self._determine_requirements([]))
/home/utrape/Documents/programming/contributions/poetry/src/poetry/console/commands/init.py:297: in _determine_requirements
    constraint = self._parse_requirements([package])[0]
/home/utrape/Documents/programming/contributions/poetry/src/poetry/console/commands/init.py:438: in _parse_requirements
    return [
/home/utrape/Documents/programming/contributions/poetry/src/poetry/console/commands/init.py:439: in <listcomp>
    parse_dependency_specification(
/home/utrape/Documents/programming/contributions/poetry/src/poetry/utils/dependency_specification.py:218: in parse_dependency_specification
    or _parse_dependency_specification_simple(requirement)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

requirement = 'pendulum 2.0.0 foo'

    def _parse_dependency_specification_simple(
        requirement: str,
    ) -> DependencySpec | None:
        extras: list[str] = []
        pair = re.sub("^([^@=: ]+)(?:@|==|(?<![<>~!])=|:| )(.*)$", "\\1 \\2", requirement)
        pair = pair.strip()
    
        require: DependencySpec = {}
    
        if " " in pair:
>           name, version = pair.split(" ", 2)
E           ValueError: too many values to unpack (expected 2)

/home/utrape/Documents/programming/contributions/poetry/src/poetry/utils/dependency_specification.py:117: ValueError

[PabloEmidio]

Could anyone give any thought about it's something missing or it can be merged?

[radoering]

Since there is a fix in parse_dependency_specification it might make sense to extend test_parse_dependency_specification.

[PabloEmidio]

Since there is a fix in parse_dependency_specification it might make sense to extend test_parse_dependency_specification.

@radoering the fix in parse_dependency_specification was to prevent the error too many values to unpack (expected 2) saw in the issues to happen. But the way it was handle allows the follow behavior (using the pytest.mark.parametrize of test_parse_dependency_specification to illustrate)

        ("demo 1.0.0", {"name": "demo", "version": "1.0.0"}),
        ("demo 1.0.0 foo", {"name": "demo", "version": "1.0.0 foo"}),

Maybe it'll be better to check the requirement against the number of argument passed in the shell:

name, version, *_excess = pair.split(" ", 2)
if _excess:
    raise ValueError(f"Not recognize argument(s) {_excess}")
...

And add some test to validate this raise given the context.

---

What do you think?

[radoering]

I think poetry should not crash with a ValueError. (I didn't check if such an error is caught somewhere.) The best response would be to tell the user that the dependency specification is invalid and ask again for a package.

[PabloEmidio]

I think poetry should not crash with a ValueError. (I didn't check if such an error is caught somewhere.) The best response would be to tell the user that the dependency specification is invalid and ask again for a package.

@radoering, I suggest the ValueError because the functions that called this code raise ValueError in x scenarios.

We could catch the ValueError in the _determine_requirements and ask again:

# src/poetry/console/commands/init.py:L302
try:
    constraint = self._parse_requirements([package])[0]
except ValueError as err:
    self.line_error(f"<error>{str(err)}</error>")
    package = self.ask(question)
    continue

[radoering]

It seems _validate_package already handles this. If I provoke an Invalid package definition., poetry asks for the next package. Maybe, we can just do the complete parsing, i.e. call _parse_requirements, in _validate_package?

[PabloEmidio]

@radoering exactly. The central idea is the requirement already was validate before been passed to parse_dependency_specification. But, it has been the central idea for while and we got 2 issue about it. And anyhow, the quote bellow continue to be true and the right way to split in max 2 items is passing maxsplit=1.

Fixing a misunderstand of buitin str function: maxsplit can return maxsplit+1 items

[PabloEmidio]

looking the application as a whole, the currently code is going to work property. But looking only to _parse_dependency_specification_simple and knowing what return expect, the function does not attend its purpose.

[radoering]

Now, I realize that due to the fix in init.py the fix in dependency_specification.py does not matter anymore (but is more correct nevertheless) because there will always be just one space.

With that knowledge I think this PR is fine as is.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.