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

python: move pip-cache location #13012

Closed
wants to merge 1 commit into from

Conversation

feckert
Copy link
Member

@feckert feckert commented Aug 3, 2020

Maintainer: @jefferyto @commodo
Compile tested: x86_64, Openwrt openwrt-19.07
Run tested: no

Description:
The pip-cache output ends up in the dl folder of openwrt.
Since the output is a cache, it should rather be in the tmp folder.

Other packages like NODE and GO also put their cache in the tmp folder.
To unify this and to keep the dl folder clean, move the cache from python to tmp opnwrt folder.

The pip-cache output ends up in the dl folder of openwrt. Since the
output is a cache, it should rather be in the tmp folder.

Other packages like NODE and GO also put their cache in the tmp folder.
To unify this and to keep the dl folder clean, move the cache from python
to tmp opnwrt folder.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
@aparcar
Copy link
Member

aparcar commented Aug 3, 2020

Isn't the DL folder ultimately a cache as well? It contains all kinds of package versions.

For a CI it seem to make sense caching the dl/ folder but not the tmp/ folder.

@feckert
Copy link
Member Author

feckert commented Aug 3, 2020

Isn't the DL folder ultimately a cache as well? It contains all kinds of package versions.

We could to that, but I run into a problem with this cache. I have found the following info about this.
https://stackoverflow.com/questions/9510474/removing-pips-cache

For a CI it seem to make sense caching the dl/ folder but not the tmp/ folder.

Of course you can do that as well, but in my opinion the dl folder should only contain packages with one version and not cached packages. A provocative question would be: If we do it this way why don't we write the build cache of c programs there as well?

@aparcar
Copy link
Member

aparcar commented Aug 3, 2020

I'm somewhat thinking about caching ccache within a CI, but that's another story 🙄

This PR looks reasonable to me.

@commodo
Copy link
Contributor

commodo commented Aug 3, 2020

i'd leave it to @jefferyto
i'm a bit neutral here; i looked through the dl/pip-cache, and it doesn't seem to be human-readable enough to keep in dl/ vs tmp/

@jefferyto
Copy link
Member

jefferyto commented Aug 3, 2020

The pip cache is not a build cache, it is a cache of packages downloaded from PyPI. If tmp is a temporary cache (for things that change) and dl is permanent cache (for things that do not change), then the pip cache belongs in dl.

We could to that, but I run into a problem with this cache. I have found the following info about this.
https://stackoverflow.com/questions/9510474/removing-pips-cache

I would rather want to understand the issue you are having with the pip cache rather than force every package build to re-download the same packages.

You are trying to install a host Python package for a package and are ending up with an incompatible version? Why not specify the version you want?

Edit: Part of the cache is for wheels, this does cache locally-built wheels (although I'm not sure if wheels are being built in our process). The main cache acts as a http cache.

Edit #2: Quoting from the pip maintainer's answer on the referenced SO question:

The specific issue of "installing the wrong version due to caching" issue mentioned in the question was fixed in pip 1.4 (back in 2013!):

Fix a number of issues related to cleaning up and not reusing build directories. (#413, #709, #634, #602, #939, #865, #948)

So again, I am more interested in what the real underlying issue is.

@jefferyto
Copy link
Member

Also, I plan on moving the Go cache into dl in the future. IIRC the main issue I have with it now is concurrent access, i.e. for parallel builds of Go packages.

@feckert
Copy link
Member Author

feckert commented Aug 4, 2020

So again, I am more interested in what the real underlying issue is.

Ok i understand that this is a download cache and not a build cache.
That was not clear to me.

My problem is that I have a central download folder (dl) where all my files I need for an openwrt build are stored. These files are saved every day, so I can build a build in the future, even if the package is not available upstream anymore. But now I have a problem with the backup of the dl folder, because it sets the file permissions to 600 and so another process running with a different user (backup collector) cannot backup these files, because it does not have the right permissions.
By the way all other files do have the permission set to 644.

By changing pip-cache location in the openwrt build tmp folder has solved my problem.
Since this is not the right way, can`t we set the permissions to 644?

@BKPepe
Copy link
Member

BKPepe commented Aug 6, 2020

This can be closed, right?

@BKPepe BKPepe requested a review from aparcar August 6, 2020 13:53
@feckert
Copy link
Member Author

feckert commented Aug 6, 2020

@BKPepe Not quite.
From my point of view, it's still about the wrong permissions in the pip cache.
I can't backup them, because they are not 644 but 600.

@jefferyto
Copy link
Member

I don't think there are "right" or "wrong" permissions in this case as AFAIK there is no expectation that files in dl would be readable by other users.

I would be open to adding a config option for the python3 package that, if enabled, would run chmod on the pip cache directory after every call to host pip. Would this work for you?

@feckert
Copy link
Member Author

feckert commented Aug 7, 2020 via email

@feckert feckert changed the title python: move pip-cache locattion python: move pip-cache location Aug 11, 2020
jefferyto added a commit to jefferyto/openwrt-packages that referenced this pull request Aug 15, 2020
This adds a config option PYTHON3_HOST_PIP_CACHE_WORLD_READABLE; if
enabled, chmod will be run after pip install to make all
files/directories in the host pip cache world-readable.

Supersedes openwrt#13012.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
@jefferyto
Copy link
Member

Please see #13135 - thanks.

@feckert
Copy link
Member Author

feckert commented Aug 17, 2020

@jefferyto thanks for your fix

@feckert feckert closed this Aug 17, 2020
1715173329 pushed a commit to immortalwrt/packages that referenced this pull request Aug 20, 2020
This adds a config option PYTHON3_HOST_PIP_CACHE_WORLD_READABLE; if
enabled, chmod will be run after pip install to make all
files/directories in the host pip cache world-readable.

Supersedes openwrt/packages#13012.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
farmergreg pushed a commit to farmergreg/packages that referenced this pull request Sep 8, 2020
This adds a config option PYTHON3_HOST_PIP_CACHE_WORLD_READABLE; if
enabled, chmod will be run after pip install to make all
files/directories in the host pip cache world-readable.

Supersedes openwrt#13012.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
farmergreg pushed a commit to farmergreg/packages that referenced this pull request Sep 8, 2020
This adds a config option PYTHON3_HOST_PIP_CACHE_WORLD_READABLE; if
enabled, chmod will be run after pip install to make all
files/directories in the host pip cache world-readable.

Supersedes openwrt#13012.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
@feckert feckert deleted the pr/20200803-python branch September 16, 2020 15:26
pprindeville pushed a commit to pprindeville/packages that referenced this pull request Dec 19, 2020
This adds a config option PYTHON3_HOST_PIP_CACHE_WORLD_READABLE; if
enabled, chmod will be run after pip install to make all
files/directories in the host pip cache world-readable.

Supersedes openwrt#13012.

Signed-off-by: Jeffery To <jeffery.to@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants