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

libreelec: use build project directory for ccache #433

Merged
merged 3 commits into from Jun 25, 2016

Conversation

zalaare
Copy link
Contributor

@zalaare zalaare commented Jun 8, 2016

Saw this communicated elsewhere, and thought I'd take a dig at it.

Moved ccache for given project to its build.project directory.
make clean will blow it up along with the project.

No changes to current CCACHE_CACHE_SIZE="30G"

@MilhouseVH
Copy link
Contributor

MilhouseVH commented Jun 8, 2016

-100 - this would mean individual .ccache directories per build - eg. one for RPi2, one for RPI2-debug, another for Generic etc. Anyone building multiple projects (and variations of those projects) will have numerous .ccache directories (which will take up more space, not to mention be less effective).

@MilhouseVH
Copy link
Contributor

If this does happen, at least change CCACHE_CACHE_SIZE to something more reasonable, such as 5GB.

@zalaare
Copy link
Contributor Author

zalaare commented Jun 8, 2016

Various conversations taking place; indicated a change to CCACHE_CACHE_SIZE was wanted. Lowering to a "safe" limit of 10GB for now. If we decide less or more, this commit be squashed after that change.

@popcornmix
Copy link
Contributor

I'm not keen on this. I make clean or delete a build directory reasonably often, knowing the .ccache will make the next build less painfully slow.
It's pretty rare that I actually need or want to delete .ccache. I would assume that if compile options or compiler version changes then ccache handles that without needed to purge.

What is the advantage of this?

@gdachs
Copy link
Contributor

gdachs commented Jun 8, 2016

Building for RPi and Wetek Play/Core can give you false cache hits per example. The compiler command line is the same, but the kernel headers changed and you get an object file that was build for the wrong ABI.

@gdachs
Copy link
Contributor

gdachs commented Jun 8, 2016

Oh, now I see what @popcornmix means. To put the cache dir into the build directory is wrong!
It should go into a project specific cache dir, but outside of the build dir. If I build 2 different distributions with the same project these builds should share the same cache dir.

@MilhouseVH
Copy link
Contributor

What about CCACHE_DIR=$ROOT/.ccache/$PROJECT?

Add /.ccache/ to .gitignore.

If you're really paranoid... CCACHE_DIR=$ROOT/.ccache/$PROJECT-$LIBREELEC_VERSION (or some variation).

@popcornmix
Copy link
Contributor

The compiler command line is the same, but the kernel headers changed and you get an object file that was build for the wrong ABI.

You mean the compilers for the two targets are built differently, but are named the same?
Perhaps giving them unique names would work?

@zalaare
Copy link
Contributor Author

zalaare commented Jun 8, 2016

@MilhouseVH @popcornmix I see what your saying now. $ROOT/.ccache/$PROJECT.$ARCH.$LIBREELEC_VERSION or other seems easy enough

@MilhouseVH
Copy link
Contributor

Next question is whether make clean should clobber the .ccache - I'd say it shouldn't (rm -fr .ccache if you want this), but others might disagree. Or add a make really-clean which clobbers the .ccache. :)

@popcornmix
Copy link
Contributor

I'd say make clean shouldn't clobber .ccache. That would just add a couple of hours to build time...

@zalaare
Copy link
Contributor Author

zalaare commented Jun 8, 2016

Should I add a make /distclean/really-clean/ variant? I know I'd use it, but would anyone else?

@MilhouseVH
Copy link
Contributor

Add the variant, as it's a small addition - people don't have to use it, but it might be useful as a last resort when dealing with weird build issues (particularly on the forum or github - "run make distclean and build again" etc.)

@zalaare
Copy link
Contributor Author

zalaare commented Jun 8, 2016

Would it make sense to rm -rf ./.ccache ./build.* ./sources for distclean? Maybe even ./target?
Most probably won't use this, but it seems it should attempt to get back to a clean slate if it is used right?

@MilhouseVH
Copy link
Contributor

Definitely don't touch sources or target

@zalaare
Copy link
Contributor Author

zalaare commented Jun 8, 2016

I think I've met all current conditions.

@gdachs
Copy link
Contributor

gdachs commented Jun 9, 2016

@popcornmix: that the compilers are different, but named identically is not the problem.
Imagine you are building the glibc for project A and project B. Project A uses kernel 1 and project B uses kernel 2. After you have build one object file for glibc for project A and kernel 1 you do the same for project B and kernel 2. Both compiles use the same commandline, so ccache uses the object file from project A that was built against a different kernel. That will work in most cases, but one mismatch is already one to many and I had this problem already in the past.

@stefansaraev
Copy link
Contributor

this feels wrong. what I have in my fork is ccache folders inside build directories (like build.*/.ccache)
ccache size limit of 5 - 10G is perfectly ok in that case. and ccache is NOT wiped on make clean

@MilhouseVH
Copy link
Contributor

I suppose a per-build .ccache is the other extreme, having started with a system wide .ccache.

As this PR stands, implementing a per project .ccache seems like an acceptable compromise, if it reduces the chance of compiler confusion, while still delivering some of the benefits of a shared .ccache.

Users can always override CCACHE_DIR in $DISTRO/options. if they want or need a more restrictive .ccache.

@stefansaraev
Copy link
Contributor

to me, it makes no sense to spread temp dirs (.ccache/*) everywhere with no real benefit. but as you wish

@chewitt
Copy link
Member

chewitt commented Jun 9, 2016

If you make the cache per-project it doesn't fully resolve scenarios building from branches with different kernels. A per-build cache solves that. I also value reliable build over speed of build - particularly as we start to add DevOps things behind the scenes.

@MilhouseVH
Copy link
Contributor

MilhouseVH commented Jun 9, 2016

Just my opinion, but a system-wide .ccache was actually fine for most builders. This PR however is a compromise and should still be OK for most users, and may avoid some (but probably not all) issues. However I'm not entirely sure it was needed, as more advanced users building multiple different kernels can easily implement a more restrictive ccache by adding one line to $DISTRO/options.

As such I don't think it would be right to implement the most restrictive (and least beneficial) form of ccache as the default (as that's where this discussion seems to be headed), as this would only benefit a small fraction of builders who should know enough to add the required line in $DISTRO/options, while penalising the majority.

@chewitt chewitt merged commit ec5e0bc into LibreELEC:master Jun 25, 2016
@zalaare zalaare deleted the pr_ccache branch August 29, 2017 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants