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 does not pull latest commit from branch #6046

Closed
3 tasks done
mgkwill opened this issue Jul 21, 2022 · 17 comments
Closed
3 tasks done

Poetry does not pull latest commit from branch #6046

mgkwill opened this issue Jul 21, 2022 · 17 comments
Labels
kind/bug Something isn't working as expected

Comments

@mgkwill
Copy link

mgkwill commented Jul 21, 2022

  • I am on the latest Poetry version.

  • I have searched the issues of this repo and believe that this is not a duplicate.

  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).

  • OS version and name: Ubuntu 20.04.4 LTS (Focal Fossa)

  • Poetry version: 1.14

  • pyproject.toml file:

[tool.poetry.dependencies]
lava-nc = { git = "https://github.com/lava-nc/lava.git", branch = "main", develop = true}

Issue

According to the docs a project can get the latest commit from a branch using the requests = { git = "https://github.com/kennethreitz/requests.git", branch = "next" } dependency specification.

However when using this specification, poetry pins the dependency to a specific commit hash. The poetry.lock after poetry update using the above pyproject.toml dependency declaration:

[package.source]
type = "git"
url = "https://github.com/lava-nc/lava.git"
reference = "main"
resolved_reference = "af84234734afe6eda9214b432bd4a3af6d282ec2"

The expectation based on the docs (see below) is that using a branch would pull the latest from that branch on each install unless a rev or tag is specified, however what poetry does is pull the latest at the time of update and then pin to that rev using resolved_reference = <commit hash>. Either the documentation needs to be clearer about pinning to the version at the time of poetry update or there is a bug in how poetry is handling the git dependency specification. If the docs need to be updated because this is intended behavior perhaps this could be a feature request:

  • poetry pulls the latest when a branch is specified, unless there is a rev or tag specified also, or some flag is used to tell poetry to not pin to a specific rev

If this is a feature that sounds reasonable for poetry, I would be happy to submit a PR with the needed functionality. If this is a bug or docs issues, I would be happy to submit a PR with the needed fix.

See docs https://python-poetry.org/docs/dependency-specification/#git-dependencies, reproduced below:

"To depend on a library located in a git repository, the minimum information you need to specify is the location of the repository with the git key:

[tool.poetry.dependencies]
requests = { git = "https://github.com/requests/requests.git" }

Since we haven’t specified any other information, Poetry assumes that we intend to use the latest commit on the master branch to build our project.

You can combine the git key with the branch key to use another branch. Alternatively, use rev or tag to pin a dependency to a specific commit hash or tagged ref"

@mgkwill mgkwill added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Jul 21, 2022
@mgkwill
Copy link
Author

mgkwill commented Jul 21, 2022

If you remove the resolved_reference from the poetry.lock file and poetry install the expected behavior is present.

The latest from <branch> is installed.

@dimbleby
Copy link
Contributor

It would defeat the point of a lockfile if it didn't actually lock.

Docs MR clarifying that this is deliberate sounds like the way to go.

@mgkwill
Copy link
Author

mgkwill commented Jul 21, 2022

It would defeat the point of a lockfile if it didn't actually lock.

Docs MR clarifying that this is deliberate sounds like the way to go.

There are some instances (CI and dev) where it would be useful to be able use a branch and not pin to a specific revision. For release pinning makes sense.

I can work around the above behavior. But it would make poetry more adaptable to make this pinning optional.

@mgkwill
Copy link
Author

mgkwill commented Jul 21, 2022

Also, we use main instead of master, so we have to specify a branch and cannot use bare git repository string. It's not clear to me if this behavior works the same specifying a branch or not.

@dimbleby
Copy link
Contributor

'adaptable' is not the goal: the purpose of the lockfile is to lock your dependencies. If you want to update them, that's what 'poetry update' is for.

@mgkwill
Copy link
Author

mgkwill commented Jul 21, 2022

If this turns into a docs issue, this is what we have to do in CI to work around:

# After poetry is installed in earlier step and dependencies are installed, before testing,
# reinstall after removing  resolved_reference
poetry update
<sed command or other command removing resolved_reference from package.source of correct package in poetry.lock>
poetry install

This seems super hacky, and if poetry followed what the docs appear to say I could just updated to main branch after each release, and on release pin to a tag, or use an artifact.

There are other undesirable behaviors that happen, that we have to fix like remapping absolute path specified in the pyproject.toml for artifact install from relative path back to absolute, but that is a different issue.

@dimbleby
Copy link
Contributor

dimbleby commented Jul 21, 2022

poetry assumes that we intend to use the latest commit

does not specify whether this assumption is made at lock time (which is actually the case) or at install time (which you wish for).

So I agree that there's room for improvement there, but I don't agree that the behaviour and the docs are actually different.

# reinstall after removing  resolved_reference
poetry update

simply poetry update should be enough, no need to hack around with the lockfile though I seem to recall that this was bugged in the 1.1 series and so you will need a 1.2 beta to do this. edit: seems to work fine at 1.1.14

@mgkwill
Copy link
Author

mgkwill commented Jul 21, 2022

'adaptable' is not the goal: the purpose of the lockfile is to lock your dependencies. If you want to update them, that's what 'poetry update' is for.

I appreciate the purpose of a lock file. However when you have projects that depend on projects and are developing in a tiered fashion, some deps can be useful to have them unlocked during development and locked during release.

@dimbleby
Copy link
Contributor

fyi #3875 fixed this: poetry used to do what you would like it to do but that was considered to be a bug

@mgkwill
Copy link
Author

mgkwill commented Jul 21, 2022

fyi #3875 fixed this: poetry used to do what you would like it to do but that was considered to be a bug

Yeah I'm not sure it's exactly my issue, in that issue the lock file was updated but not the src where git dependencies had been cloned. I.E. <venv>/src/<git dir> was not updated. See #2921

I'm suggesting a way to allow the latest to be pulled when calling poetry install on a specific git dependency. The docs definitely need to be clearer if that is not the current functionality when specifying a bare git repo and or a branch.

I guess we could just delete the lock file between releases, which will be recreated with the latest on each poetry install but also this is not ideal for all the other dependencies.

@dimbleby
Copy link
Contributor

this had the unintended side effect of pulling the latest revision of VCS dependencies regardless of the locked version.

pretty clear that what you are asking for was an unintended side effect, deliberately fixed.

@mgkwill
Copy link
Author

mgkwill commented Jul 21, 2022

this had the unintended side effect of pulling the latest revision of VCS dependencies regardless of the locked version.

pretty clear that what you are asking for was an unintended side effect, deliberately fixed.

Actually I'm suggesting it does not put a resolved_ref in poetry.lock under certain circumstances and then updated to the latest based on that poetry.lock file.

This bug was about updating only the poetry.lock and then not updating the src at all. Read the bug:
"poetry update successfully updates the poetry.lock file with the correct latest commit, but the library in the virtual environment remains checked-out at the commit that was latest when poetry install was last run." #2921

@dimbleby
Copy link
Contributor

I'm suggesting it does not put a resolved_ref in poetry.lock under certain circumstances

pretty sure this just isn't going to happen and you're better off looking for a workflow that works for you with poetry as it is - or perhaps not using poetry at all. You "appreciate the purpose of a lockfile" but are literally asking for it not to work as a lockfile.

I'm sure a docs MR clarifying poetry behaviour would be welcome.

@mgkwill
Copy link
Author

mgkwill commented Jul 21, 2022

Is the poetry community open to discussing issues users have or new features that allows others to use poetry in their particular env or use-case?

I was following another bug about paths for installed artifacts and the community didn't seem to care that, despite the fact an absolute path is used in the pyproject.toml, poetry writes a relative path to the poetry.lock. This assumes the project installation from the exact same place each time. In many circumstances this will fail. But there was pushback on fixing it despite multiple people saying it was an issue in CI.

poetry frankly has the ability to do both, just like what I'm asking here.

@mgkwill
Copy link
Author

mgkwill commented Jul 21, 2022

I'm suggesting it does not put a resolved_ref in poetry.lock under certain circumstances

pretty sure this just isn't going to happen and you're better off looking for a workflow that works for you with poetry as it is - or perhaps not using poetry at all. You "appreciate the purpose of a lockfile" but are literally asking for it not to work as a lockfile.

I'm sure a docs MR clarifying poetry behaviour would be welcome.

"you're better off.. perhaps not using poetry at all"

Wow. OK.

@dimbleby
Copy link
Contributor

Wow. OK.

I simply mean that you should use the right tool for the job. Poetry is a tool, among other things, for locking dependencies and giving repeatable builds. If that's not what you want, then perhaps it's not the right tool for you.

I don't speak for "the poetry community", only for myself. I don't know anything about that other conversation.

Feel free to submit an MR, others may take a different view. I wouldn't expect so, but I've been wrong before!

@neersighted
Copy link
Member

I'm going to close this issue as wont-fix since it seems to be getting a bit heated -- it's always been Poetry's explicit intention to provide reproducible installs. I don't think an explicit poetry update <dep> in CI if you want to update a git dep is unreasonable, but I would be happy to consider a PR adding an option to the pyproject.toml entry.

Likewise, a docs PR clarifying any deficiency in explaining expected behavior would also be welcome.

@neersighted neersighted closed this as not planned Won't fix, can't repro, duplicate, stale Jul 21, 2022
@python-poetry python-poetry locked as too heated and limited conversation to collaborators Jul 21, 2022
@mkniewallner mkniewallner removed the status/triage This issue needs to be triaged label Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

No branches or pull requests

4 participants