Environment variable POETRY_EXPERIMENTAL_SYSTEM_GIT_CLIENT not recognised during poetry install

[lhutzelmann]

• Poetry version: 1.2.1

• Python version: 3.9.10

• OS version and name: Ubuntu 22.04

• pyproject.toml: See below

• I am on the latest stable Poetry version, installed using a recommended method.

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

• I have consulted the FAQ and blog for any relevant entries or release notes.

• If an exception occurs when executing a command, I executed it again in debug mode (-vvv option) and have included the output below.

Issue

My pyproject.toml contains a dependency to a company internal git repository such as following example:

[tool.poetry.dependencies]
mylib = {git = "https://SOME_GIT_CLONE_URL"}

This is only relevant to ensure that the configuration is asked whether the system git client should be used or not.
When setting the environment variable POETRY_EXPERIMENTAL_SYSTEM_GIT_CLIENT to true and then running a poetry install the environment variable is ignored and the internal git client library is used.
When setting the configuration experimental.system-git-client via poetry configto true and then running the same installation command the system git client is used now.
It seems that in the method is_using_legacy_client() of module poetry.vcs.git.backend the following line does not catch the value from the environment variable (due to the two separate .get() calls that do not fit to the single environment variable name).

Config.create().get("experimental", {}).get("system-git-client", False)

At other places the following line is used to get the "new-installer" from "experimental" values instead (which works AFAIK):

poetry.config.get("experimental.new-installer", False)

Maybe this can be fixed in a similar way?
Best regards
Lars

[ramvikrams]

can i take up this issue and do the changes

[ramvikrams]

this Config.create().get("experimental", {}).get("system-git-client", False) has to be changed to poetry.config.get("experimental.new-installer", False) ,am I correct?

[neersighted]

can i take up this issue and do the changes

Nobody is going to tell you you can't -- the worst that could happen is someone else beats you to the punch after you've started work.

this Config.create().get("experimental", {}).get("system-git-client", False) has to be changed to poetry.config.get("experimental.new-installer", False) ,am I correct?

This diagnosis looks correct, but only you will be able to figure that out for sure. If someone else knew for sure that was the fix, they would have already written the code, yeah? 😄

[neersighted]

As an aside, since I see you are participating in Hacktoberfest by making PRs to a "free t-shirt" repo -- please realize that Poetry is not participating in Hacktoberfest this year and PRs do need to be useful/are held to a high standard (e.g. tests are usually required).

[ramvikrams]

My main reason for coming here was not for hscktoberfest or any t-shirt now I wanted to do contribute to poetry

[neersighted]

My main reason for coming here was not for hscktoberfest or any t-shirt now I wanted to do contribute to poetry

No problem! The t-shirt thing has just lead to overload for a lot of volunteers in the past and seeing anybody chase a shirt can make people a little nervous 😔

Hopefully we see a PR from you!

[ramvikrams]

thank you I will start working on it and the test cases

[ramvikrams]

can i take up this issue and do the changes

Nobody is going to tell you you can't -- the worst that could happen is someone else beats you to the punch after you've started work.

this Config.create().get("experimental", {}).get("system-git-client", False) has to be changed to poetry.config.get("experimental.new-installer", False) ,am I correct?

This diagnosis looks correct, but only you will be able to figure that out for sure. If someone else knew for sure that was the fix, they would have already written the code, yeah? 😄

if the diagnosis is correct can you please tell how to write the test cases for it

[neersighted]

if the diagnosis is correct can you please tell how to write the test cases for it

I don't know whether this diagnosis is correct, but it is very likely from my understanding of the code. Whether it is possible to regression test for this (and if it is worth doing) is in large part a matter of discovery/experimentation by the author of the PR fixing it -- you would generally be expected to explain why it's not possible/a good idea to test for in your PR if that's the way you go.

I can't tell you how to write tests either, other than to look for the existing config tests to see how we test the environment variable equivalence -- writing a test is similar to the process of fixing the problem, and I would generally suggest writing a test for a bug before you try to fix it so that you can tell when it is fixed.

[ramvikrams]

if the diagnosis is correct can you please tell how to write the test cases for it

I don't know whether this diagnosis is correct, but it is very likely from my understanding of the code. Whether it is possible to regression test for this (and if it is worth doing) is in large part a matter of discovery/experimentation by the author of the PR fixing it -- you would generally be expected to explain why it's not possible/a good idea to test for in your PR if that's the way you go.

I can't tell you how to write tests either, other than to look for the existing config tests to see how we test the environment variable equivalence -- writing a test is similar to the process of fixing the problem, and I would generally suggest writing a test for a bug before you try to fix it so that you can tell when it is fixed.

thanks you for the explanation I will get on the work right away

[lhutzelmann]

Sorry if I caused some confusion by my examples.

@ramvikrams: Your fix uses the examples I extracted from other places of the code where a value of "experimental.*" are used. In the example this was the "new-installer" configuration sub value. It should illustrate an analoguous case.
This ticket is about the "system-git-client" sub value where the retrieval differs by getting "experimental" followed by "system-git-client" instead of doing it in one go.
Therefore the fix should be:
poetry.config.get("experimental.system-git-client", False)

[lhutzelmann]

Thus the answer to your question

this Config.create().get("experimental", {}).get("system-git-client", False) has to be changed to poetry.config.get("experimental.new-installer", False) ,am I correct?

Should have been "Sorry, no, it should refer to the experimental.system-git-client insted".

[github-actions]

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.