python-poetry / poetry

Python packaging and dependency management made easy
https://python-poetry.org
MIT License
31.43k stars 2.27k forks source link

Do not validate path dependencies when they are not selected for installation #668

Closed spither closed 1 year ago

spither commented 5 years ago

Issue

I cannot run poetry install --no-dev away from my development environment because I have dev-only path based dependencies.

> poetry install --no-dev

[ValueError]             
Directory ../dev-only does not exist  

The error is correct, the directory doesn't exist, but it's only listed as a dev-dependency and I'm trying to install --no-dev. I was not expecting any dev-dependency checks to take place.

In my case I'm trying to install the non-dev dependencies for a Django site deployment. The path based dev-dependencies only exist in development, not on the deployment system.

sdispater commented 5 years ago

Poetry locks everything, including dev dependencies, before installing to ensure that there is no conflict between dev and runtime dependencies, so every dependency must be resolvable.

spither commented 5 years ago

This happens with an existing lock file in place.

Is resolving dev dependencies (especially with an existing lock file) really needed when --no-dev is specified? Is there anything I can do to avoid it?

I don't have a need or desire to copy these dependencies to my deployment system. They wouldn't/shouldn't be used for anything there and might add confusion by being present.

rphes commented 5 years ago

I've also run into this issue, my use case being that I want to use a local (and editable) version of a package during development and a commit from a git repository for deployment situations.

jjuris commented 5 years ago

Hello,

I also found in this situation when I used poetry in my project. As a workaround, I simply commented out lines under development dependencies section in the project, but it is not nice solution -> un/commenting lines in configuration file is pretty annoying.

@sdispater, I would like to help you solve this problem.

The simplest solution would be "Ignore dev dependencies when --no-dev option is used."

If it is ok for you let me know and I will try to find it and fix it. If you have better idea, let me know and we can discuss it.

Have a nice

Day

Jozef Juris

jjuris commented 5 years ago

Hello,

I think I found a solution for this case. It is pretty simple. The idea to ignore dev-dependencies seems to be fine. Existing tests are passing and the poetry is acting in desired manner.

Unfortunately I am not sure if this solution is elegant enough, but I did not find other way to work with --no-dev flag without hacking a lot of code. What do you think @sdispater ?

Best regards

Jozef

1119 commit

poetry.py

        import sys
        if "dev-dependencies" in local_config and not "--no-dev" in sys.argv:
            for name, constraint in local_config["dev-dependencies"].items():
                if isinstance(constraint, list):
                    for _constraint in constraint:
                        package.add_dependency(name, _constraint, category="dev")

                    continue

                package.add_dependency(name, constraint, category="dev")
hozn commented 5 years ago

I'm also interested in a solution for this. My scenario may be slightly different from OP, but like @rphes , I'd like to be able to use local path dependencies during development (so that I can make associated library changes concurrently) but have the final image builds pull in only real tagged dependencies. I assume that path dependencies is the answer here, though admittedly I'm not sure how I'd have the same dep exist as both a path dependency (for development) and a real verisoned dep (for packaging/production) -- @rphes , how do you solve that aspect?

This feels like a key missing feature in poetry for handling large-scale project development (where a project consists of many related libraries).

LouisTrezzini commented 4 years ago

hello, thanks for the great work on Poetry

we are also facing this issue, is there any plan to solve it in an upcoming release?

reem commented 4 years ago

Also facing this issue, makes it incredibly inconvenient to use path based dependencies in development and external dependencies in production - "solutions" include maintaining two duplicate pyproject.toml files or commenting/uncommenting constantly, neither of which is really acceptable.

tall-josh commented 4 years ago

I know it's a MASSIVE hack but this was killing my docker build and I didn't want two pyproject.toml so I just put the [tool.poetry.dev-dependencies] heading at the bottom of my pyproject.toml and used RUN sed -i -n '/tool.poetry.dev-dependencies/q;p' pyproject.toml before poetry install.

I know. I feel dirty, but I regret nothing!

pwoolvett commented 3 years ago

Yet another workaround:

  1. create a "virtual" setup.py with a redirect depending on the dependency existence

    $ cat /path/to/virtual_dependency/setup.py
    from pathlib import Path
    import sys
    
    path = Path("/path/to/real/dependency/setup.py")
    if path.exists():
      sys.path.insert(0, str(path.parent))
      from setup import setup
    else:
      from distutils.core import setup
    
       setup(
           name="real_dependency",
           version="0.0.1",
           description="""Mock real_dependency""",
           packages=[""],
           package_data={"": []},
       )
  2. poetry add --optional /path/to/virtual_dependency

tested with Poetry version 1.1.4

sinoroc commented 3 years ago

@pwoolvett Maybe something like the following could work as well:

#!/usr/bin/env python3

import pathlib

import setuptools

def main():
    install_requires = []
    dep_name = 'Schroedinger'
    dep_path = pathlib.Path(f'../{dep_name}')
    if dep_path.is_dir():
        install_requires.append(f'{dep_name} @ {dep_path}')
    #
    setuptools.setup(
        install_requires=install_requires,
    )

if __name__ == '__main__':
    main()
dermidgen commented 3 years ago

I've created an example repo that shows how to get around this; without additional poetry support.
https://github.com/dermidgen/python-monorepo/

JTarball commented 3 years ago

any updates on a fix inside poetry?

ride90 commented 3 years ago

Here is a PR

marowu commented 2 years ago

Hi, are there any chances of reviewing the PR of @ride90 ? In my case, the issue is blocking usage of multi-layer docker build. My approach would be to first build a layer with the external dependencies which don't change that often (poetry install --no-dev). Next copy my packaged development code and than to install it.

abuchmueller commented 2 years ago

Hi there, can anyone of the maintainers kindly take a look at https://github.com/python-poetry/poetry-core/pull/210? Seems like a rather trivial fix, which would benefit poetry greatly. It's annoying having to circumvent this bug every time. Plus this issue has been around for ages.

lc0rp commented 2 years ago

RE PR comments, what if this was implemented using conditional dependencies? So that the path must exist only if the condition passes?

acme = {develop = true, path = "./../acme", markers = "sys_platform == 'macos'"}

radoering commented 2 years ago

That may be true for installing if a lock file exists. However, if there is no lock file, the path must exist in any case because poetry creates a lock file valid for all platforms no matter on which platform locking takes place.

TBBle commented 2 years ago

I would think if you have no Poetry.lock file, then there's no way to expect a path-based dependency to resolve correctly, i.e. install parameters like --no-dev should not affect the implicit Poetry.lock generation triggered by poetry install if you do not already have a Poetry.lock.

Conversely, if you have a Poetry.lock, you should not need to go querying repos, paths, etc for project metadata unless performing an operation that edits the lock file, i.e. poetry lock or poetry update.

So I think the use-case of this issue only makes sense if a Poetry.lock is already in-place, c.f. https://github.com/python-poetry/poetry/issues/668#issuecomment-441604727. This also relates to #1168, where the use-case is locking a dev and non-dev version of a dependency project, where the former is pathed in a monorepo or similar but the latter is not-pathed and so is pulled from a pip index at install-time, and so install --no-dev should use the non-dev dependency which would not care that the locked dev-dependency is not present.

Same rationale applies with slightly more complex details for the Poetry 1.2 group support which replaces --no-dev. Basically, the idea is that if your selected groups for poetry install do not select a pathed dependency which exists in the lock file, being unable to resolve that pathed dependency should not present any difficulties.

kbzowski commented 1 year ago

As of today, this is still a huge problems with poetry 1.3

radoering commented 1 year ago

There is already a core PR (python-poetry/poetry-core#520) with a corresponding change and a companion PR (#6844) to make sure that poetry still fails gracefully if a non-existent path dependency is really required.

Especially the core PR still has to be reviewed. If you feel comfortable enough, you can even help without reviewing the code by testing the change and commenting at python-poetry/poetry-core#520 if you find any regressions.

If you are using pipx, you can install a version of poetry based on the current master as of today including the concerning change via:

pipx install --suffix _test git+https://github.com/python-poetry/poetry@refs/pull/6844/head

and then use this special version by typing poetry_test instead of poetry (_test is the suffix, you can choose another one).

marowu commented 1 year ago

Great news!

MythicManiac commented 1 year ago

Still facing this with poetry 1.4.2 for optional dependency groups extras.

TBBle commented 1 year ago

Are you getting the new error message (Path {self._full_path} for {self.pretty_name} does not exist), or the old one (Directory/File {self._path} does not exist)?

Poetry 1.4.2 should contain poetry-core 1.5.2, so the fix is supposed to be there, so I'd suggest more information (or even an entirely new bug report, with repro etc) may be needed to diagnose this, in case it's not the same problem.

MythicManiac commented 1 year ago

I was getting the new error after swapping to 1.4.X, but I've since changed the configurations several times and am no longer sure what the setup was exactly.

What I think happened is that dependencies with optional = true in the main dependency list are validated, even though they're optional. I tried to initially use that approach and declare them as a group in [tool.poetry.extras] so that by default they would not get installed, but it would be possible to select them for installing as an extras group.

After finding out the above doesn't work, I moved the optional dependency into a new dependency group (i.e. [tool.poetry.group.extras.dependencies]), which made the error disappear, but would cause lockfile regeneration to omit the entire group from extras, presumably because they're being treated as dev dependencies when not in the main group. This might be a separate bug? To be clear, installation behavior worked as expected as long as the extras weren't empty in the lockfile, but it would get emptied by poetry.

I then moved away from extras entirely, opting to use poetry's groups directly. At first I didn't realize to declare the group itself as optional (given that the dependency within the group was already declared as optional) which lead to issues. I then attempted to declare the group optional, which removed the error, but also made the dependency impossible to install.

Finally once I marked the dependency itself as required (optional = false) but kept the group optional, everything started to work as expected. The caveat is that the dependencies are treated as dev dependencies and aren't possible to use as extras, not to mention they're poetry specific.

I can try re-creating some of the scenarios described above if that'd help, just let me know.

Summary

For the sake of clarity & potential future readers, this setup works for optional path-based local dependencies:

[tool.poetry.group.foo]
optional = true

[tool.poetry.group.foo.dependencies]
bar = { path = "../python-packages/bar", develop = true }

The above should be ignored by validation unless explicitly selected for installing:

poetry install --with foo
TBBle commented 1 year ago

To be clear, installation behavior worked as expected as long as the extras weren't empty in the lockfile, but it would get emptied by poetry.

After finding out the above doesn't work, I moved the optional dependency into a new dependency group (i.e. [tool.poetry.group.extras.dependencies]), which made the error disappear, but would cause lockfile regeneration to omit the entire group from extras

Interesting. I'd expected that lockfile generation would fail if a pathed dependency doesn't exist; per https://github.com/python-poetry/poetry/issues/668#issuecomment-1201356401 and my comment after, it's only when installing with an already-existing lockfile that we should ignore missing dependencies that are not activated.

The test suite appears to cover this case and expects an error.

So this seems like a new bug, if you're regenerating the lockfile with missing pathed dependencies, and it's skipping the dependencies instead of failing with an error.

MythicManiac commented 1 year ago

I'm not 100% right if I came across correctly so clarifying just in case:

but would cause lockfile regeneration to omit the entire group from extras

With this I meant that the [extras] section in the lockfile would map the foo extra-group to an empty array instead of including the dependencies that were declared in [tool.poetry.extras] in pyproject.toml. The dependency itself still gets emitted to the lockfile appropriately, it just gets omitted from the extra group's dependency list if it's not declared under main dependencies, and this doesn't seem to produce a warning or error of any kind.

DanGrizzly commented 1 year ago

Still an issue on poetry 1.5. Can't poetry update, install, or anything really, because I don't have all of the platform specific wheels in a dependency already marked as extra. Please fix this, it makes poetry near unusable for multiple platform development and CI actions on github.

royp-larium commented 1 year ago

Same here. still an issue

github-actions[bot] commented 7 months ago

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.