Poetry 1.2.0 doesn't seem to like shortened git commit hashes

[dasm-tmlt]

• 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: Mac OS 12.3

• Poetry version: 1.2.0

• Link of a Gist with the contents of your pyproject.toml file: [https://gist.github.com/dasm-tmlt/28b714320c663ae87df3f262a64916d1]

Issue

I'm the first person on my team using poetry 1.2.0, and noticed that one of our existing dependencies stopped working (I've verified that it works fine under 1.1.15). The gist above is a minimal example that demonstrates the problem - trying to run poetry lock returns Failed to clone https://github.com/ryan112358/private-pgm.git at 'e9ea5fc', verify ref exists on remote., but that commit definitely exists. Replacing the shortened version of the commit hash (e9ea5fc) with the full commit hash (e9ea5fcac62e2c5b92ae97f7afe2648c04432564) fixes the problem, so there is a workaround, but this does seem like a regression.

Note that debugging this problem was rather difficult because once the first failure has occurred, poetry doesn't attempt to re-download the dependency. Running poetry lock a second time will yield a different error:

Unable to determine package info for path: /Users/dasm/test/.venv/src/private-pgm

Command ['/var/folders/1g/g0wb8wcn2ys55jkc69z0xs3r0000gn/T/tmp53lmvcsc/.venv/bin/python', '-'] errored with the following return code 1, and output: 
Traceback (most recent call last):
  File "<stdin>", line 13, in <module>
  File "/var/folders/1g/g0wb8wcn2ys55jkc69z0xs3r0000gn/T/tmp53lmvcsc/.venv/lib/python3.7/site-packages/build/__init__.py", line 208, in __init__
    _validate_source_directory(srcdir)
  File "/var/folders/1g/g0wb8wcn2ys55jkc69z0xs3r0000gn/T/tmp53lmvcsc/.venv/lib/python3.7/site-packages/build/__init__.py", line 109, in _validate_source_directory
    raise BuildException(f'Source {srcdir} does not appear to be a Python project: no pyproject.toml or setup.py')
build.BuildException: Source /Users/dasm/test/.venv/src/private-pgm does not appear to be a Python project: no pyproject.toml or setup.py
input was : import build
import build.env
import pep517

source = '/Users/dasm/test/.venv/src/private-pgm'
dest = '/var/folders/1g/g0wb8wcn2ys55jkc69z0xs3r0000gn/T/tmp53lmvcsc/dist'

with build.env.IsolatedEnvBuilder() as env:
    builder = build.ProjectBuilder(
        srcdir=source,
        scripts_dir=env.scripts_dir,
        python_executable=env.executable,
        runner=pep517.quiet_subprocess_runner,
    )
    env.install(builder.build_system_requires)
    env.install(builder.get_requires_for_build('wheel'))
    builder.metadata_path(dest)

No fallback setup.py file was found to generate egg_info.

If instead you switch to the full commit hash after a failure with the short hash, poetry lock fails silently (there is error output when running with -vvv) rather than downloading the right dependency. Only if you remove the mostly empty directory (it contains a single .git file) at .venv/src/private-pgm does it start working properly.

[neersighted]

This is a dulwich issue -- there is currently a PR open to add a callout of this to the release notes, but if shortened hashes are important to your workflow you can work around the lack of them with poetry config experimental.system-git-client true.

[neersighted]

(the mentioned PR is python-poetry/website#82)

[dasm-tmlt]

I see. We've already switched to using longer hashes, so we should be okay.

If this is going to be a limitation of poetry going forward, it would be good to have a better error message (the current one suggesting that the ref might not exist is not that helpful). Additionally, the fact that trying to use a short hash lands you in a broken state where it's difficult to recover was really frustrating, and felt like a larger issue than lack of support for short hashes.

[neersighted]

I think the solution is to open a dulwich issue/request and see if this is something they intend to support. If they do, we can consider it a gotcha of Poetry 1.2.0 that future releases (or even an update of deps depending on dulwich semver) will resolve -- if they opt not to support it, we can add it to the docs and validate hash length to provide a useful error message/guards against creating a broken lock file.

[dasm-tmlt]

Got it, would you like me to open an issue with dulwich? I suspect I will not do a great job (since I don't know much about how poetry uses dulwich), but I can take a stab.

It's ultimately your call since you'd be doing the work, but I'd recommend doing the work to smooth the experience in the short term even if dulwich intends to fix it long-term. The current experience is not great.

At a minimum, I would recommend changing the documentation here so the examples don't use short hashes, and adding a note saying explicitly that they aren't supported. I would not have thought to check the changelog, but I did look at that documentation page.

[neersighted]

Dulwich is quite responsive so if they implement support I don't think we'll need to do much -- but if they don't, we certainly should add some error handling to catch this as part of the next patch release.

Please do open an issue -- feel free to link here and they should have enough context.

[dasm-tmlt]

Before I do that, I just want to run one thing by you: it looks like poetry is already attending to detect short hashes and replace them with full-length ones. From a cursory inspection, it seems like that should be preventing the problem I'm seeing. Are we sure this is a dulwich problem?

[neersighted]

Good call -- I think we might not be hitting that code path through the lock file, so we might need to generalize it and hoist it out, or ask Dulwich to support short hashes natively (if they don't already -- need to do more testing evidently).

[jelmer]

Dulwich does support short hashes, but only in its porcelain. The plumbing (which is what poetry uses) works with full hashes.

Poetry appears to have some code to resolve short hashes to long hashes in src/poetry/vcs/git/backend.py