Skip to content

Use $XDG_CACHE_HOME by default for distshare #535

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

Closed
wants to merge 1 commit into from
Closed

Use $XDG_CACHE_HOME by default for distshare #535

wants to merge 1 commit into from

Conversation

jleclanche
Copy link

@jleclanche jleclanche commented Jun 21, 2017

Closes #346

https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html

This defaults distshare to $XDG_CACHE_HOME. Per the spec, if that variable is not defined, $HOME/cache is used (os.path.expanduser("~") is windows-compatible as well). All this removes extraneous clutter in $HOME.


  • Make sure to include one or more tests for your change;
  • if an enhancement PR please create docs and at best an example
  • Add yourself to CONTRIBUTORS;
  • make a descriptive Pull Request text (it will be used for changelog)

@@ -19,7 +19,7 @@ List of optional global options::
toxworkdir=path # tox working directory, defaults to {toxinidir}/.tox
setupdir=path # defaults to {toxinidir}
distdir=path # defaults to {toxworkdir}/dist
distshare=path # (DEPRECATED) defaults to {homedir}/.tox/distshare
distshare=path # (DEPRECATED) defaults to $XDG_CACHE_HOME/tox/distshare
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the docs say: this feature is deprecated, so why should we put extra work into this - or is the deprecation deprecated?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... or am I missing something?

Copy link
Author

@jleclanche jleclanche Jun 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea about the feature, but its default has been updated due to how the PR works. I wanted the documentation to be consistent.

Copy link
Member

@obestwalter obestwalter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jleclanche thanks for the PR anyway, but I am really not sure if this makes sense at all to touch, as the distshare option in tox as such is marked deprecated - so I will leave this PR hang around for now and wait until we made a decision in. #346

@@ -19,7 +19,7 @@ List of optional global options::
toxworkdir=path # tox working directory, defaults to {toxinidir}/.tox
setupdir=path # defaults to {toxinidir}
distdir=path # defaults to {toxworkdir}/dist
distshare=path # (DEPRECATED) defaults to {homedir}/.tox/distshare
distshare=path # (DEPRECATED) defaults to $XDG_CACHE_HOME/tox/distshare
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... or am I missing something?

@jleclanche
Copy link
Author

My understanding is that it's the distshare config option that is deprecated, not the concept of a distshare.

@obestwalter
Copy link
Member

That might well be - I simply don't know, as I don't use that feature, I'll dig around a bit.

@obestwalter
Copy link
Member

obestwalter commented Jun 30, 2017

In the CHANGELOG it also looks like distshare as such is deprecated:

DEPRECATE distshare in documentation

There is not much about it - looks like the original use case was for Jenkins to share builds between different runs, and the only other mention is in tips and tricks, so I guess this is really deprecated.

Maybe someone else can shed some light on this ...

@jleclanche
Copy link
Author

I find that as far as personal usage goes, tox does a bit of a poor job in caching wheels and updating venvs when the requirements update (eg. on travis, caching the .tox directory is a huge speedup, but then causes build failures when the requirements are updated).

As far as distshare goes, I think it should at the very least be disabled by default if tox no longer cares about it and it's truly deprecated.

@obestwalter
Copy link
Member

obestwalter commented Jun 30, 2017

As far as distshare goes, I think it should at the very least be disabled by default if tox no longer cares about it and it's truly deprecated.

That's what I am trying to find out now. I personally don't care about it (at least not until now), but I don't know why it's deprecated either.

I find that as far as personal usage goes, tox does a bit of a poor job in caching wheels and updating venvs when the requirements update

If you look around in the issues a bit, you'll find out that you are not the only one who noticed that and that there are several efforts to improve that.

e.g:
#149
#93
#296

copying in and copying out artifacts (i.e. Python packages).
By default, the distshare directory is located in
``$XDG_CACHE_HOME/tox/distshare``, where ``$XDG_CACHE_HOME`` defaults to
``{homedir}`` if not set. This directory will be used for copying in and
Copy link

@merwok merwok Jun 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be {homedir}/.cache/

@obestwalter
Copy link
Member

@jleclanche I Won't merge this for this release as #346 is actually not clarified yet, if understand it correctly (windows behaviour). Also @merwok suggested a fix that has not been integerated yet. For 2.8 there is more than enough already, so this will hopefully go into 2.9 once it's clarified.

@obestwalter obestwalter added the needs:discussion It's not quite clear if and how this should be done label Aug 22, 2017
@obestwalter obestwalter added needs:review somebody who thinks they know what they are doing should have a look at this and removed needs:discussion It's not quite clear if and how this should be done labels Sep 4, 2017
@gaborbernat gaborbernat closed this Dec 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:review somebody who thinks they know what they are doing should have a look at this
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants