search

python-poetry / poetry

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

Git dependencies are not resolved to revisions #1331

Closed bibz closed 5 years ago

bibz commented 5 years ago

Issue

Git dependencies explicitly pinned (through the branch, rev, or tag arguments) are not resolved to the revision (SHA-1) when poetry locking.

Steps to reproduce

Note: The referenced gist lists one dependency pinned with a tag. Using a branch name (branch = "master") or a short revision (rev = "f68740f") leads to the same result.

% poetry lock

Behaviour

The poetry.lock file freezes the git dependency to a named reference (e.g. the tag name):

…
[[package]]
category = "main"
description = "Fast serialization framework on top of dataclasses"
name = "mashumaro"
optional = false
python-versions = ">=3.6"
version = "1.7"

[package.dependencies]
msgpack = ">=0.5.6"
pyyaml = ">=3.13"

[package.source]
reference = "v1.7"
type = "git"
url = "git://github.com/Fatal1ty/mashumaro.git"
…

Considering that branches and tags are mutable, this is not a proper locking mechanism. A simple fast-forward (or overwrite) is enough to change the reference being pointed to.

The short revision is not mutable but can eventually become non-unique (might not be a problem is there is a deterministic decision process in git).

Expected behaviour

The pinned reference (package.source.reference) gets resolved to a full SHA-1 revision:

…
[[package]]
category = "main"
description = "Fast serialization framework on top of dataclasses"
name = "mashumaro"
optional = false
python-versions = ">=3.6"
version = "1.7"

[package.dependencies]
msgpack = ">=0.5.6"
pyyaml = ">=3.13"

[package.source]
reference = "f68740f0e344a99e849419da99e96ae5b0bc5ec8"
type = "git"
url = "git://github.com/Fatal1ty/mashumaro.git"
…

Related issues

The issue raised here was also mentioned in #691 during analysis of a troublesome behaviour with git dependencies.

abn commented 5 years ago

@bibz @brycedrennan I think we should consider the proposed fix (#1337) a breaking change as I believe there are cases where this has been intentionally used to point to a mutable named branches. Because of that I would say this should not go out as part of a bug fix but a feature realese instead if merged.

@bibz while I agree that using the full sha-1 revision is the preferred locking solution here for reasons you have mentioned above, I do think that the decision to do so should be left upto the project maintainer. We will have to provide a mechanism such that the maintainer can opt-out or opt-in to enforcing the use of revesion even if a branch/tag is specified, according to their needs. The current implementation (master and develop) allows that implicitly if a tag or branch is specified as reference resolves to branch or tag or rev, in that order.

That said, I do think using a tagged revision explicitly (rev = 'f68740f', in this case) should not use the tag reference (v1.7) in the lock file, this is a bug. Note that this only happens if there was a pre-existing lock file when pyproject.toml specified tag = 'v1.7. If a new lock file is created the referece correctly says reference = "f68740f".

bibz commented 5 years ago

@abn I see your point. Even though it might have been undefined or broken behaviour, it makes sense to not break the current flow.

Put differently the following use-cases should be supported:

For which something as simple as flag could work, e.g. editable. And if the current behaviour is kept the default, then the fix would not even be a breaking change :-) .

Are there other use-cases you were thinking of?

abn commented 5 years ago

@bibz I think those cases cover the what I had in mind.

We should probably avoid the the terms 'editable' as that can get confused with editable installs (pip -e), which a git dependency might not be. Same applies for 'development' as it is overloaded at the moment. I, personally, like this to be an opt-in behavior if a tag/branch is specified (when rev is used it is implicit), and hence an option like 'enforce-revisionorlock-revision` would be more appropriate.

Should we split this issue into two separate ones?

  1. Bug: Lock file does not use revision specified for git dependency if previously a named tag/branch at the same revision was specified.
  2. Feature: Support locking revision for git dependencies where named tag/branch are used.
bibz commented 5 years ago

Yes you are right, one bug and one feature. Would you recommend splitting this issue into two and then closing it? Otherwise I can edit this bug report and make it more constrained, and create a new feature request.

abn commented 5 years ago

:+1: I think this can stay as the bug; the PR just needs update I guess. A new feature request might be appropriate for the feature.

bibz commented 5 years ago

This bug has been transformed to a feature request instead (#1348).

@abn: I could not reproduce the bug when locking from a tag to a revision. If you can, feel free to reopen.

github-actions[bot] commented 6 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.