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

improved get-poetry.py and added POETRY_HOME support to poetry command #3550

Closed
wants to merge 1 commit into from

Conversation

mohan43u
Copy link
Contributor

@mohan43u mohan43u commented Jan 7, 2021

  • fixed prompting even after no-modify-path explicitly given in commandline
  • use POETRY_HOME if available instead of XDG_*_HOME variables in nix systems

Pull Request Check List

Resolves: #3542
Resolves: #2445

  • Added tests for changed code.
  • Updated documentation for changed code.

@mohan43u mohan43u force-pushed the master branch 3 times, most recently from dc1b0ec to 488552e Compare January 7, 2021 11:39
@mohan43u
Copy link
Contributor Author

mohan43u commented Jan 7, 2021

Can someone review this code?

The changes I made helps keep everything related to 'poetry' inside one place under POETRY_HOME

@TBBle
Copy link
Contributor

TBBle commented Jan 12, 2021

I don't see how this resolves #2445, unless the intention is to not support the cache-dir config option, or at least document that it only applies to the virtual-env default path and the 'artifacts' directory, but not other forms of caching done by Poetry.

@mohan43u
Copy link
Contributor Author

I don't see how this resolves #2445,

given reply

unless the intention is to not support the cache-dir config option, or at least document that it only applies to the virtual-env default path and the 'artifacts' directory, but not other forms of caching done by Poetry.

I'm not able to understand but not other forms of caching done by Poetry part. could you please elaborate?

@TBBle
Copy link
Contributor

TBBle commented Jan 12, 2021

Right now, the cache-dir config is used for 'virtualenv' (by default) and 'artifacts' (hard-coded), but not 'repositories' which ignores the setting and uses locations.CACHE_DIR. That's the problem seen in #2445, that sometimes it ignores the cache-dir config.

This PR doesn't change that behaviour, it instead adds POETRY_HOME as a preference over XDG_CACHE_HOME, which I separately think is the wrong approach, but certainly doesn't fix that the cache-dir config option should override this, and does not.

@mohan43u
Copy link
Contributor Author

This PR doesn't change that behaviour, it instead adds POETRY_HOME as a preference over XDG_CACHE_HOME, which I separately think is the wrong approach, but certainly doesn't fix that the cache-dir config option should override this, and does not.

This PR doesn't address #2445 directly, but it fixes the underlying problem which lead to #2445 issue. As I already mentioned, XDG_*_HOME dirs are suppose to be fallback dirs when user not explicitly specify the location to store files. But poetry is using these dirs as they are the one user has to use and expects user to change XDG_*_HOME variables rather than providing a way to explicitly specify where to store the files.

@TBBle
Copy link
Contributor

TBBle commented Jan 12, 2021

That's not the underlying problem in #2445. The underlying problem there is the surface problem:

poetry config cache-dir /tmp/poetrycache does not prevent Poetry trying to write to to $XDG_CACHE_HOME.

With this PR, the the same problem exists, except you add "or $POETRY_HOME/.cache" to the end of the problem description. It doesn't change the problem.

POETRY_HOME is not a way to "explicitly specify the location to store (cache) files". It's a way to explicitly specify the location to install Poetry's binaries when using get-poetry.py, which has no relationship with #2445.

@mohan43u
Copy link
Contributor Author

There is no solution in current poetry to address the problem this PR is addressing which is providing user a way to explicitly specify the location to use rather than asking user to change XDG_*_HOME dirs.

@TBBle
Copy link
Contributor

TBBle commented Jan 12, 2021

If by "location to use", you only mean "Poetry's cache files", then there is a documented solution, it just doesn't work: #2445.

If by "location to use" you include everything that Poetry touches (i.e. you want a "standalone" or "portable" install) then no, there isn't such a thing, but supporting that through POETRY_HOME doesn't fix #2445, which is a problem with redirecting Poetry's caches specifically, not producing a "standalone" install.

@mohan43u mohan43u force-pushed the master branch 2 times, most recently from 4f9bf2e to 0673c4e Compare March 8, 2021 13:39
* fixed prompting even after no-modify-path explicitly given in commandline
* use POETRY_HOME if available instead of XDG_*_HOME variables in nix systems
@neersighted
Copy link
Member

Closing this for a couple reasons: get-poetry.py is deprecated, and $POETRY_HOME has always been meant to control where poetry's isolated environment is created, but it was not intended to silo all of the files poetry generates.

I do think it's worth addressing the inconsistent cache dir in poetry -- but this PR is only tangentially related and not the way to go about solving that problem.

Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants