Fix PEP508 formatting for subdirectory parameter

[marink]

Resolves: python-poetry#Fix for git subdirectories #288

There is a minor adjustment to fix #288. It was missing the '=' sign when specifying the Git subdirectory to load.

[finswimmer]

Thanks a lot 👍

Can you please add a test for it?

[aryarm]

just chiming in to say that I'm encountering this issue, as well

thanks for creating a fix for this!

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[AKuederle]

@marink @finswimmer is there anything I could do to help getting this merged?

As @finswimmer mentioned, this is missing a test. @marink I think the right place to add one would be here: https://github.com/python-poetry/poetry-core/blob/main/tests/packages/test_vcs_dependency.py

And if I understand the structure correctly, a valid test could look something like this:

def test_to_pep_508() -> None:
    dependency = VCSDependency(
        "poetry", "git", "https://github.com/python-poetry/poetry.git", directory="subdir"
    )

    expected = "poetry @ git+https://github.com/python-poetry/poetry.git#subdirectory=subdir"

    assert dependency.to_pep_508() == expected

[aryarm]

I think this PR can be closed, since #288 has been merged, and I believe it already implements this fix?

[AKuederle]

Actually no. #288 fixed parsing URLs that contain the sub-directory syntax. This PR fixes recreating the URL correctly so that other package managers can install a package that is managed by poetry and has a dependency with a sub-directory

The issue fixed here is still in master:

poetry-core/src/poetry/core/packages/vcs_dependency.py

Line 120 in d9ea915

requirement += f"#subdirectory{self._directory}"

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[AKuederle]

@marink Could you have a look at the example test that I posated a couple of comments up? I think that would be a sufficient test. If you add this we could move this PR forward.

[radoering]

It seems this one has been duplicated by #451, which got merged without a test. (Sorry about that.)

Later I added the missing test (and some more) in #453. Thus, nothing to do here.

[AKuederle]

Ah perfect! Thanks for handling that!