-
Notifications
You must be signed in to change notification settings - Fork 13
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
ccache: either bind-mount or use named volume per-architecture #56
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Richard Mortier <mort@cantab.net>
Signed-off-by: Richard Mortier <mort@cantab.net>
Signed-off-by: Richard Mortier <mort@cantab.net>
dabuild.in
Outdated
@@ -73,6 +73,26 @@ ABUILD_VOLUMES="-v ${HOME}/.abuild:/home/builder/.abuild \ | |||
-v ${PWD%/aports/*}/aports:/home/builder/aports \ | |||
-v ${ABUILD_PACKAGES}:/home/builder/packages" | |||
|
|||
## if we have ~/.ccache on host, bind-mount it; else use named per-arch volumes | |||
if [ -w "$HOME/.ccache" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should we support bind mounting ccache directory? Are there use cases where you want to share ccache? or does it make sense to have direct access to it? It will also start using my local ccache directory if it existed even if I don't want it to.
dabuild.in
Outdated
## if we have ~/.ccache on host, bind-mount it; else use named per-arch volumes | ||
if [ -w "$HOME/.ccache" ]; then | ||
ccvol=$HOME/.ccache/$DABUILD_ARCH | ||
if [ ! \( -d "$ccvol" -a -w "$ccvol" \) ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason to test for directory existence after which you check if the file/directory is writable, isn't that exactly what [ ! -w "$ccvol" ]
will do?
Even more simple is to use mkdir -p which will create the directory if needed or returns 0 if already exists
dabuild.in
Outdated
@@ -73,6 +73,19 @@ ABUILD_VOLUMES="-v ${HOME}/.abuild:/home/builder/.abuild \ | |||
-v ${PWD%/aports/*}/aports:/home/builder/aports \ | |||
-v ${ABUILD_PACKAGES}:/home/builder/packages" | |||
|
|||
dabuild_mkvol () { | |||
vol=$1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variables in functions should be assigned local
dabuild.in
Outdated
mkdir $ccvol | ||
fi | ||
else | ||
ccvol=abuild-$ABUILD_VERSION-$DABUILD_ARCH-ccache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ABUILD_VERSION is a kind of confusing. Can we globally rename this to ALPINE_BRANCH or ALPINE_RELEASE of which the later sounds preferable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, not sure you noticed but i switched this repo to gitlab.
Please dont push to git.a.o anymore.
@clandmeter does this do what you were thinking?
Signed-off-by: Richard Mortier mort@cantab.net