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

Build packages with pinned dependencies from .lock file #3341

Closed
wants to merge 4 commits into from

Conversation

mrijken
Copy link

@mrijken mrijken commented Nov 8, 2020

Pull Request Check List

Resolves: #2778

  • Added tests for changed code.
  • Updated documentation for changed code.

@sinoroc
Copy link

sinoroc commented Nov 8, 2020

@mrijken
Is that the same feature request as has been discussed in #3334 ?

@finswimmer finswimmer requested a review from a team November 9, 2020 05:58
@finswimmer finswimmer added the kind/feature Feature requests/implementations label Nov 9, 2020
@mrijken
Copy link
Author

mrijken commented Nov 9, 2020

Yes, the use case is the same as in #3334.

@mrijken
Copy link
Author

mrijken commented Dec 1, 2020

What is needed to get this PR reviewed? Or is it just time ;-)

@sinoroc
Copy link

sinoroc commented Dec 1, 2020

What is needed to get this PR reviewed? Or is it just time ;-)

Maintainers need to find time, I guess. I asked some people who seem to want a similar feature to look at your PR, test it if possible.

Also note: python-poetry/poetry-core#71

@MartinThoma
Copy link

Thank you for taking the time and doing it! The --lock option makes sense to me, although it for sure means that people should install such "locked" applications in their own (isolated) environment. As @sinoroc phrased it in another context: "this seems like one nice way to shoot yourself in the foot". Without more tooling support and more documentation, I fear that this might be used in a wrong way: Either by people locking dependencies in libraries or users installing applications in the global environment.

Also, how could you now upgrade the dependencies? Would this locking basically lose all information about version restrictions in the pyproject.toml?

@mrijken
Copy link
Author

mrijken commented Dec 1, 2020

For sure the people who uses this, needs to know the consequences. In our company we want to be 100% sure that the app and the dependencies build and checked by Jenkins is the same as the one installed. We check regular whether updates of dependencies are present and can rebuild the app with the locked dependencies again. When we install that new version of the app, we know for sure that it passes all tests with the upgraded dependencies. Are there other methods / best practices to achieve that without a lock?

@sinoroc
Copy link

sinoroc commented Dec 1, 2020

Maybe an alternative solution is to export the lockfile of the application (App) to a requirements.txt and the install in staging/production as pip install App -r requirements.txt.
[Off the top of my head, I have not tested this.]

@abn
Copy link
Member

abn commented Mar 15, 2021

@mrijken sorry it took a while for me to get to this one. I do not think that this is the right level at which we should support this feature. The main reason why I say this is because this introduces 2 different kinds of wheels when (a) built via poetry as the front end using the --lock option and (b) when built via another PEP-517 compatible front-end. This is, imho, is a dangerous inconsistency. This is why I proposed the original change in poetry-core as this allows for a consistent behaviour.

In the current situation, the common solution for relying on pinned version in an environment is to use the poetry install --no-dev command.

@artslob
Copy link

artslob commented Apr 25, 2021

@abn can you please clarify about what inconsistency are you talking?

common solution for relying on pinned version in an environment is to use the poetry install --no-dev command

But --no-dev flag just allows to install package without dependencies. Also for this flag to work you need download source code.

Flag that was requested in #2778 is about building packages with dependecnies as specified in poetry.lock file. If maintainer decided to build package with --lock flag it means package should be installed with same dependencies as in lock file. I assume this flag would be primary used for private package management, not for public libraries.

@sinoroc
Copy link

sinoroc commented Apr 25, 2021

One thing that came to my mind, while seeing this topic pop up again:

This has to be done carefully. If there is a way to build 2 wheels (or 2 sdist) of the same project, 1 with pinned dependencies and 1 without pinned dependencies, then they definitely should have different file names. Because most of the packaging tools are built around the idea that if 2 distributions have the same file name then they are the exact same thing. So those 2 distributions (pinned and unpinned) should be differentiable by comparing the file names.

I do not know what is the best way to go about this, maybe by adding something to the version string (see PEP 440).

If only the pinned wheel of the project is distributed ever, then I guess it is all fine. But if it is intended to publish the 2 variants (even privately), then I think there might be issues.

[I did not check if it has been said already or not, apologies if it has]

@artslob
Copy link

artslob commented Apr 25, 2021

From PEP 440

Specific build information may also be included in local version labels

Maybe add +lock local label to builded version?

But

local version identifiers MUST NOT be permitted in version specifiers

Also

"local version label" concepts to allow system integrators ... as well as to allow the incorporation of build tags into the versioning of binary distributions

@artslob
Copy link

artslob commented Apr 25, 2021

If there is a way to build 2 wheels (or 2 sdist) of the same project, 1 with pinned dependencies and 1 without pinned dependencies, then they definitely should have different file names

Is it really need 2 different names? From version convention (PEP 440) there is no standartizied way to differentiate locked and ususal versions, so maybe it should be responsibility of package maintainer to choose to build package as locked or unlocked versions?

@sinoroc
Copy link

sinoroc commented Apr 25, 2021

@artslob

If there is a way to build 2 wheels (or 2 sdist) of the same project, 1 with pinned dependencies and 1 without pinned dependencies, then they definitely should have different file names

Is it really need 2 different names? From version convention (PEP 440) there is no standartizied way to differentiate locked and ususal versions, so maybe it should be responsibility of package maintainer to choose to build package as locked or unlocked versions?

If the project maintainers only ever publish the distribution variant with pinned dependencies, then fair enough, no problem. Same if they only ever publish the distribution with unpinned dependencies. BUT, if project maintainers publish both a distribution with pinned dependencies and another with unpinned, then they should have different file names. So, maybe poetry should also offer a way to do this name differentiation (if possible according to PEP440 of course).

Maybe this is a case where there should be an option in pyproject.toml to say: "by default all distributions of this project should be created with pinned dependencies", reason most likely is that the project is an application and not a library. The point would be to reduce the risk of having both pinned and unpinned being published. But this is very risky from my point of view, what gets pinned today, might not be the same thing that will get pinned tomorrow. The results of the dependency resolution (the lock file) depends on what is available on the index (PyPI), and it changes constantly.

To be honest, my personal point of view right now, is that I don't think it makes sense to have pinned dependencies in a distribution. There are other ways to solve what we are trying to solve here. On of the possible solutions is to look towards things like zipapp, pex, shiv, etc. those are much better suited for applications with pinned dependencies. Note that there is also a general intention to formalize/standardise a "requirements" format (the next requirements.txt) but from what I understood, it is very challenging task to get this right.

@artslob
Copy link

artslob commented Apr 27, 2021

BUT, if project maintainers publish both a distribution with pinned dependencies and another with unpinned, then they should have different file names

Why?


Also, if we always build packages with poetry using requirement constraints from pyproject.toml (usually in form of pkg >= 1.0 < 2.0) what point of having .lock. file with pinned deps? I think .lock file has sense for application type projects.

I think idea of specifing type of python project (library or application) is good, so it will have consistency with builder and explains existance of .lock file.

@abn
Copy link
Member

abn commented Apr 28, 2021

@abn can you please clarify about what inconsistency are you talking?

If we follow this proposal, you have 2 different behaviours here for building project wheels.

  1. Using poetry build --lock.
  2. Using poetry build or using a PEP-517 frontend (eg: pip wheel).

Since poetry is not always involved in the (2) case there is no way to reproduce the "locked" builds consistently. It is important to note that there are a lot of cases where, even when a wheel exists on pypi, due to system compatibilities or end user choices (eg: pip install --no-binary :all:), you end up having to rebuild the package wheel from source relying on PEP-517 builds.

But --no-dev flag just allows to install package without dependencies. Also for this flag to work you need download source code.

Correct.

Flag that was requested in #2778 is about building packages with dependecnies as specified in poetry.lock file. If maintainer decided to build package with --lock flag it means package should be installed with same dependencies as in lock file. I assume this flag would be primary used for private package management, not for public libraries.

Sure. But we cannot really introduce a new option with this assumption unfortunately. Addtiionally, as noted above, we cannot guarantee that a wheel will always be installed either. This is particularly why we need to ensure both cases mentioend above behave consistently.

Another point of note here is that wheels may not necessarily be the right solution for a "locked" solution. It might be that a zipapp might serve the use case better especially in the "private" package use cases.

@ghost
Copy link

ghost commented Apr 29, 2021

I have an alternative idea for this problem, which still allows the building of wheels with locked dependencies. I would really love to continue allowing the distribution of applications via wheels - whilst zipapp or docker might currently provide more conducive features for distributing applications, wheels are still just something "installable" - there's nothing about them that restricts them to be used as a library, it's just that, IMO, current build backends (such as poetry-core and setuptools) don't lend themselves to creating wheels for applications. There even exist some installation tools (e.g. pipx) which are specifically designed to install wheels as applications.

I would say this feature belongs in the build backend (poetry-core), and should be configurable from the pyproject.toml file. E.g. if we add an optional key in the [build-system] table such as locked_dependencies = true|false, then poetry-core would build the wheel with locked dependencies. I believe this would remove that inconsistency which @abn is (rightfully) concerned about, as it becomes frontend-agnostic. Whilst this design might not be as "flexible" for the person building the wheel, I think it captures the real use case of #3334 correctly - this key would be set to true for applications, false otherwise.

@sinoroc
Copy link

sinoroc commented Apr 29, 2021

if we add an optional key in the [build-system] table such as locked_dependencies = true|false, then poetry-core would build the wheel with locked dependencies.

This was my suggestion here: #3341 (comment)

Anyway, I believe this whole thing of having locked dependencies in the wheel is a bad idea. With this you are kind of making it hard to install security fixes of the dependencies and things like that.

If I am not mistaken, if you were to try to update a dependency in such a case, pip would complain. Maybe it would not prevent the update, but it would at least display some message.


I recall seeing a couple of times, that the whole Python packaging is mostly designed towards the concept of libraries and not so much applications.

There are other ways to explore:

  • publish a requirements.txt and instruct: "Please install with python -m pip install --requirement https://github.com/ping/pong/blob/v1.2.3/requirements.txt to have the recommended set of dependencies"
  • distribute zipapps (shiv, pex, etc.)
  • this one is also not recommended since it might be even worse than pinning dependencies in the wheel metadata, but well... vendoring is an option.

@bneijt
Copy link

bneijt commented May 1, 2021

I'm using a separate script to achieve this functionality and I'm interested in getting this to work as a poetry plugin in the future. For now, just using poetry add --dev poetry-lock-package works for all my build requirements.

I've done a write-up here: https://bneijt.nl/pr/poetry-lock-package/

@tom-bowles
Copy link

tom-bowles commented Jun 22, 2021

Anyway, I believe this whole thing of having locked dependencies in the wheel is a bad idea. With this you are kind of making it hard to install security fixes of the dependencies and things like that.

If I am not mistaken, if you were to try to update a dependency in such a case, pip would complain. Maybe it would not prevent the update, but it would at least display some message.

My use case is for an internal application that will always be deloyed using pipx. Pipx creates a separate virtualenv just for the application and installs only that application in it. Adopting a security fix for a dependency would be up to the application owners, who would update the lock file, test and publish a new version of the application distribution.

Applications are inherently different to libraries. An application is the root node of the dependency graph, which means it can and arguably should enforce specific versions across that entire graph, thus ensuring that the dependency versions used are consistent throughout development, testing and production deployment. This is the primary purpose of lock files as I see it - they're less critical for libraries, though still very useful.

@sinoroc
Copy link

sinoroc commented Jun 22, 2021

@tom-bowles Applications and libraries have to be handled differently, yes. I still believe pinning all dependencies in the application's wheel metadata is counter-productive. If a dependency gets a security fix released, I don't want to wait until the maintainers of the application update the pinned dependencies and release a new wheel of the application, I want to update the dependency right now. But for internal applications, like in your case, then yes, do whatever you feel like.

@richardwhitefoot
Copy link

I think it would be good for this to became a core feature and give users the choice whether to use it or not. The use-case for building internal applications is good, and when deploying the same application to multiple environments (ie. internet & secure extranet) you really want to be sure dependencies are consistent and without bypassing a lot of the good features of Poetry you can't do that at present.

@bneijt
Copy link

bneijt commented Jun 25, 2021

I think this PR will not make it, meanwhile there has been the initiation of the export plugin for the newer poetry releases and if this PR is going to be accepted it probably needs to land there.

For the closely related "build a wheel file with pinned dependencies" I have already created an issue there: python-poetry/poetry-plugin-export#1

@fish-face
Copy link

@mrijken it looks like this does not honour markers. I am not sure exactly how markers are represented in the lockfile, but it looks like they don't show up in the entry of the package itself, only the things that depend on it. Therefore if you have something in the lockfile which is depended on with a marker, your change will always add them to the package dependencies even when they shouldn't be. You can see this if you depend on something like jupyter-core which depends on pywin32 which has a sys_platform marker.

@neersighted
Copy link
Member

neersighted commented Nov 13, 2021

Per all the discussion above, I'm going to close this for now as this implementation is a likely dead end. I'd suggest discussion occur in #2778, and we can migrate further to a Github discussion if desired.

Copy link

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.

@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
kind/feature Feature requests/implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build packages with pinned dependencies from .lock file