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

poetry install fails when irrelevant extra is not installable #8548

Closed
4 tasks done
Pwuts opened this issue Oct 17, 2023 · 4 comments · Fixed by #8549
Closed
4 tasks done

poetry install fails when irrelevant extra is not installable #8548

Pwuts opened this issue Oct 17, 2023 · 4 comments · Fixed by #8549
Labels
area/installer Related to the dependency installer kind/bug Something isn't working as expected

Comments

@Pwuts
Copy link
Contributor

Pwuts commented Oct 17, 2023

  • I am on the latest stable Poetry version, installed using a recommended method.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • I have consulted the FAQ and blog for any relevant entries or release notes.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option) and have included the output below.

Issue

I have included agbenchmark as an optional dependency, which is only installed as part of the benchmark extras group if specified. However, running poetry install without --extras benchmark still fails if agbenchmark is not installable, in this case because its source path is not accessible in the docker build context.

In my opinion, poetry install should ignore non-installable packages when they are optional and not included in the set of packages to be installed.

Raw output

 > [autogpt-dev 1/1] RUN poetry install -vvv --no-root:                                                                                                                                                                                                              
#0 2.981 Path /benchmark for agbenchmark does not exist                                                                                                                                                                                                              
#0 2.985 Loading configuration file /root/.config/pypoetry/config.toml                                                                                                                                                                                               
#0 3.082 Creating virtualenv agpt-9TtSrW0h-py3.10 in /root/.cache/pypoetry/virtualenvs                                                                                                                                                                               
#0 5.175 Using virtualenv: /root/.cache/pypoetry/virtualenvs/agpt-9TtSrW0h-py3.10                                                                                                                                                                                    
#0 5.596 Installing dependencies from lock file
#0 6.638 
#0 6.638 Finding the necessary packages for the current system
#0 6.716 Path /benchmark for agbenchmark does not exist
#0 6.831 Path /benchmark for agbenchmark does not exist
#0 6.833 Path /benchmark for agbenchmark does not exist
#0 6.835 Path /benchmark for agbenchmark does not exist
#0 6.839 Path /benchmark for agbenchmark does not exist
#0 6.842 Path /benchmark for agbenchmark does not exist
#0 6.845 Path /benchmark for agbenchmark does not exist
#0 6.850 Path /benchmark for agbenchmark does not exist
#0 6.852 Path /benchmark for agbenchmark does not exist
#0 6.855 Path /benchmark for agbenchmark does not exist
#0 6.856 Path /benchmark for agbenchmark does not exist
#0 6.858 Path /benchmark for agbenchmark does not exist
#0 6.860 Path /benchmark for agbenchmark does not exist
#0 6.862 Path /benchmark for agbenchmark does not exist
#0 6.865 Path /benchmark for agbenchmark does not exist
#0 6.867 Path /benchmark for agbenchmark does not exist
#0 6.870 Path /benchmark for agbenchmark does not exist
#0 6.873 Path /benchmark for agbenchmark does not exist
#0 6.876 Path /benchmark for agbenchmark does not exist
#0 6.880 Path /benchmark for agbenchmark does not exist
#0 6.884 Path /benchmark for agbenchmark does not exist
#0 6.887 Path /benchmark for agbenchmark does not exist
#0 6.888 Path /benchmark for agbenchmark does not exist
#0 6.891 Path /benchmark for agbenchmark does not exist
#0 6.899 Path /benchmark for agbenchmark does not exist
#0 8.302 Path /benchmark for agbenchmark does not exist
#0 8.417 
#0 8.417   Stack trace:
#0 8.418 
#0 8.418   10  /opt/poetry/venv/lib/python3.10/site-packages/cleo/application.py:327 in run
#0 8.471        325│ 
#0 8.471        326│             try:
#0 8.471      → 327│                 exit_code = self._run(io)
#0 8.471        328│             except BrokenPipeError:
#0 8.471        329│                 # If we are piped to another process, it may close early and send a
#0 8.471 
#0 8.471    9  /opt/poetry/venv/lib/python3.10/site-packages/poetry/console/application.py:190 in _run
#0 8.496        188│         self._load_plugins(io)
#0 8.496        189│ 
#0 8.496      → 190│         exit_code: int = super()._run(io)
#0 8.496        191│         return exit_code
#0 8.496        192│ 
#0 8.497 
#0 8.497    8  /opt/poetry/venv/lib/python3.10/site-packages/cleo/application.py:431 in _run
#0 8.539        429│             io.input.interactive(interactive)
#0 8.539        430│ 
#0 8.539      → 431│         exit_code = self._run_command(command, io)
#0 8.539        432│         self._running_command = None
#0 8.539        433│ 
#0 8.539 
#0 8.540    7  /opt/poetry/venv/lib/python3.10/site-packages/cleo/application.py:473 in _run_command
#0 8.581        471│ 
#0 8.581        472│         if error is not None:
#0 8.582      → 473│             raise error
#0 8.582        474│ 
#0 8.582        475│         return terminate_event.exit_code
#0 8.583 
#0 8.583    6  /opt/poetry/venv/lib/python3.10/site-packages/cleo/application.py:457 in _run_command
#0 8.629        455│ 
#0 8.629        456│             if command_event.command_should_run():
#0 8.629      → 457│                 exit_code = command.run(io)
#0 8.629        458│             else:
#0 8.629        459│                 exit_code = ConsoleCommandEvent.RETURN_CODE_DISABLED
#0 8.630 
#0 8.630    5  /opt/poetry/venv/lib/python3.10/site-packages/cleo/commands/base_command.py:119 in run
#0 8.641        117│         io.input.validate()
#0 8.641        118│ 
#0 8.641      → 119│         status_code = self.execute(io)
#0 8.641        120│ 
#0 8.641        121│         if status_code is None:
#0 8.641 
#0 8.642    4  /opt/poetry/venv/lib/python3.10/site-packages/cleo/commands/command.py:62 in execute
#0 8.666         60│ 
#0 8.666         61│         try:
#0 8.666      →  62│             return self.handle()
#0 8.666         63│         except KeyboardInterrupt:
#0 8.666         64│             return 1
#0 8.667 
#0 8.667    3  /opt/poetry/venv/lib/python3.10/site-packages/poetry/console/commands/install.py:150 in handle
#0 8.680        148│         self.installer.verbose(self.io.is_verbose())
#0 8.681        149│ 
#0 8.682      → 150│         return_code = self.installer.run()
#0 8.682        151│ 
#0 8.683        152│         if return_code != 0:
#0 8.683 
#0 8.684    2  /opt/poetry/venv/lib/python3.10/site-packages/poetry/installation/installer.py:104 in run
#0 8.719        102│             self.verbose(True)
#0 8.719        103│ 
#0 8.719      → 104│         return self._do_install()
#0 8.720        105│ 
#0 8.720        106│     def dry_run(self, dry_run: bool = True) -> Installer:
#0 8.722 
#0 8.722    1  /opt/poetry/venv/lib/python3.10/site-packages/poetry/installation/installer.py:338 in _do_install
#0 8.762        336│             if dep.is_file() or dep.is_directory():
#0 8.762        337│                 dep = cast("PathDependency", dep)
#0 8.762      → 338│                 dep.validate(raise_error=True)
#0 8.762        339│ 
#0 8.762        340│         # Execute operations
#0 8.762 
#0 8.763   ValueError
#0 8.763 
#0 8.764   Path /benchmark for agbenchmark does not exist
#0 8.764 
#0 8.764   at /opt/poetry/venv/lib/python3.10/site-packages/poetry/core/packages/path_dependency.py:80 in validate
#0 8.774        76│     def validate(self, *, raise_error: bool) -> bool:
#0 8.774        77│         if not self._validation_error:
#0 8.775        78│             return True
#0 8.775        79│         if raise_error:
#0 8.775     →  80│             raise ValueError(self._validation_error)
#0 8.775        81│         logger.warning(self._validation_error)
#0 8.776        82│         return False
#0 8.776        83│ 
#0 8.777        84│     @property
------
Dockerfile:39
--------------------
  37 |     # dev build -> include everything
  38 |     FROM autogpt-base as autogpt-dev
  39 | >>> RUN poetry install -vvv --no-root
  40 |     ONBUILD COPY . ./
  41 |     
--------------------
ERROR: failed to solve: process "/bin/sh -c poetry install -vvv --no-root" did not complete successfully: exit code: 1
@Pwuts Pwuts added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Oct 17, 2023
@dimbleby
Copy link
Contributor

not a bug, poetry needs to consider the optional dependencies to be sure that a consistent solution is available

@dimbleby
Copy link
Contributor

dimbleby commented Oct 17, 2023

actually I've changed my mind, this can probably sensibly be addressed with something like:

diff --git a/src/poetry/installation/installer.py b/src/poetry/installation/installer.py
index c30dd1b4..98fe4d43 100644
--- a/src/poetry/installation/installer.py
+++ b/src/poetry/installation/installer.py
@@ -332,6 +332,8 @@ class Installer:

         # Validate the dependencies
         for op in ops:
+            if op.skipped:
+                continue
             dep = op.package.to_dependency()
             if dep.is_file() or dep.is_directory():
                 dep = cast("PathDependency", dep)

(plus corresponding unit test)

or maybe it would be better to use the skipped property to decide whether or not dep.validate() should raise an error:

diff --git a/src/poetry/installation/installer.py b/src/poetry/installation/installer.py
index c30dd1b4..4c8d6bfd 100644
--- a/src/poetry/installation/installer.py
+++ b/src/poetry/installation/installer.py
@@ -335,7 +335,7 @@ class Installer:
             dep = op.package.to_dependency()
             if dep.is_file() or dep.is_directory():
                 dep = cast("PathDependency", dep)
-                dep.validate(raise_error=True)
+                dep.validate(raise_error=not op.skipped)

         # Execute operations
         status = self._execute(ops)

merge request welcome I expect

@Pwuts
Copy link
Contributor Author

Pwuts commented Oct 17, 2023

Thanks @dimbleby, I'll make a PR with the suggested changes

@radoering radoering added area/installer Related to the dependency installer and removed status/triage This issue needs to be triaged labels Nov 19, 2023
Copy link

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/installer Related to the dependency installer kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants