Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Fix build_ccache_wrappers: #14631

Merged
merged 1 commit into from
Jun 24, 2019
Merged

Fix build_ccache_wrappers: #14631

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 20 additions & 29 deletions ci/docker/runtime_functions.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,37 +86,29 @@ build_ccache_wrappers() {
# But in the beginning, we'll make this opt-in. In future, loads of processes like
# the scala make step or numpy compilation and other pip package generations
# could be heavily sped up by using ccache as well.
mkdir /tmp/ccache-redirects
mkdir -p /tmp/ccache-redirects
export PATH=/tmp/ccache-redirects:$PATH
ln -s ccache /tmp/ccache-redirects/gcc
ln -s ccache /tmp/ccache-redirects/gcc-8
ln -s ccache /tmp/ccache-redirects/g++
ln -s ccache /tmp/ccache-redirects/g++-8
ln -s ccache /tmp/ccache-redirects/nvcc
ln -s ccache /tmp/ccache-redirects/clang++-3.9
ln -s ccache /tmp/ccache-redirects/clang-3.9
ln -s ccache /tmp/ccache-redirects/clang++-5.0
ln -s ccache /tmp/ccache-redirects/clang-5.0
ln -s ccache /tmp/ccache-redirects/clang++-6.0
ln -s ccache /tmp/ccache-redirects/clang-6.0
ln -s ccache /usr/local/bin/gcc
ln -s ccache /usr/local/bin/gcc-8
ln -s ccache /usr/local/bin/g++
ln -s ccache /usr/local/bin/g++-8
ln -s ccache /usr/local/bin/nvcc
ln -s ccache /usr/local/bin/clang++-3.9
ln -s ccache /usr/local/bin/clang-3.9
ln -s ccache /usr/local/bin/clang++-5.0
ln -s ccache /usr/local/bin/clang-5.0
ln -s ccache /usr/local/bin/clang++-6.0
ln -s ccache /usr/local/bin/clang-6.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also restore the links in /usr/local/bin/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no need, also they fail with permission denied when executing locally or without root privileges.

No need because they are added to the path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you aware that the links you created previously were dangling? (basically didn't work)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do you think the links to /usr/local/bin should be restored? Could you explain?

Copy link
Contributor

@marcoabreu marcoabreu Apr 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We spoke about this already a dozen times, so I will only link the official guide: https://ccache.dev/manual/latest.html#_run_modes

Back when I developed this, I tested it only by relaying on the PATH (hence I introduced /tmp/ccache-redirects) and it didn't work. Thus, I added the /usr/local/bin method.

The links did work - why shouldn't they?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't work because your links were broken. So with my changes it works in /tmp/ccache-redirects . there's no need to touch /usr/local/bin this is not allowed for users, requires sudo privileges, it's not necessary, etc. Please elaborate why you don't accept my fix, which uses the ccache wrappers in the /tmp folder. The guide says "something like this" you are taking the guide too seriously, having in any other folder which is then injected first in the path is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you elaborate on why are you requesting changes when my changes actually fix the problem and make ccache work? or do you think there is something that is not working with my changes? Is not clear from your comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are not answering my comments, can you get into the container and see that this PR is fixing your broken links? That's the reason your approach didn't work, and you still left the /tmp/ccache-redirect broken links. I fixed this, thus no need to write in /usr/local/bin which prevents the build functions from being used locally or without root permissions.


export NVCC=ccache
CCACHE=`which ccache`
ln -sf $CCACHE /tmp/ccache-redirects/gcc
ln -sf $CCACHE /tmp/ccache-redirects/gcc-8
ln -sf $CCACHE /tmp/ccache-redirects/g++
ln -sf $CCACHE /tmp/ccache-redirects/g++-8
ln -sf $CCACHE /tmp/ccache-redirects/clang++-3.9
ln -sf $CCACHE /tmp/ccache-redirects/clang-3.9
ln -sf $CCACHE /tmp/ccache-redirects/clang++-5.0
ln -sf $CCACHE /tmp/ccache-redirects/clang-5.0
ln -sf $CCACHE /tmp/ccache-redirects/clang++-6.0
ln -sf $CCACHE /tmp/ccache-redirects/clang-6.0
#Doesn't work: https://github.com/ccache/ccache/issues/373
# ln -sf $CCACHE /tmp/ccache-redirects/nvcc
# ln -sf $CCACHE /tmp/ccache-redirects/nvcc
# export NVCC="/tmp/ccache-redirects/nvcc"

# Uncomment if you would like to debug CCache hit rates.
# You can monitor using tail -f ccache-log
# export CCACHE_LOGFILE=/work/mxnet/ccache-log
# export CCACHE_DEBUG=1
#export CCACHE_LOGFILE=/work/mxnet/ccache-log
#export CCACHE_LOGFILE=/tmp/ccache-log
#export CCACHE_DEBUG=1
}

build_wheel() {
Expand Down Expand Up @@ -689,8 +681,7 @@ build_ubuntu_gpu_mkldnn_nocudnn() {

build_ubuntu_gpu_cuda100_cudnn7() {
set -ex
# unfortunately this build has problems in 3rdparty dependencies with ccache and make
# build_ccache_wrappers
build_ccache_wrappers
make \
DEV=1 \
ENABLE_TESTCOVERAGE=1 \
Expand Down