On Tue, Jul 30, 2013 at 03:55:26PM +0200, Dominik Heidler wrote:
> Signed-off-by: Dominik Heidler <[email protected]>
> ---
> makechrootpkg.in | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/makechrootpkg.in b/makechrootpkg.in
> index d7d3ecf..d4c6abe 100644
> --- a/makechrootpkg.in
> +++ b/makechrootpkg.in
> @@ -21,6 +21,8 @@ run_namcap=false
> temp_chroot=false
> chrootdir=
> passeddir=
> +ccachedir=
> +passedccdir=
> declare -a install_pkgs
> declare -i ret=0
>
> @@ -58,10 +60,11 @@ usage() {
> echo " Default: $copy"
> echo '-n Run namcap on the package'
> echo '-T Build in a temporary directory'
> + echo '-C <dir> The ccache directory to use'
> exit 1
> }
>
> -while getopts 'hcur:I:l:nT' arg; do
> +while getopts 'hcur:I:l:C:nT' arg; do
> case "$arg" in
> h) usage ;;
> c) clean_first=true ;;
> @@ -71,6 +74,7 @@ while getopts 'hcur:I:l:nT' arg; do
> l) copy="$OPTARG" ;;
> n) run_namcap=true; makepkg_args="$makepkg_args -i" ;;
> T) temp_chroot=true; copy+="-$$" ;;
> + C) passedccdir="$OPTARG" ;;
> *) makepkg_args="$makepkg_args -$arg $OPTARG" ;;
> esac
> done
> @@ -84,6 +88,12 @@ chrootdir=$(readlink -e "$passeddir")
> [[ ! -d $chrootdir ]] && die "No chroot dir defined, or invalid path
> '$passeddir'"
> [[ ! -d $chrootdir/root ]] && die "Missing chroot dir root directory. Try
> using: mkarchroot $chrootdir/root base-devel"
>
> +# Canonicalize ccachedir, getting rid of trailing /
Ok, but why is this necessary? Based on the comment, I'd say you just
want to make the assignment dir=${dir%/} and readlink is overkill. But,
I still don't understand why this is needed.
Ranty/semi-offtopic: Comments should explain why, not what. If the
"what" needs explaining, then your code is probably too complicated. I
realize bash is an odd case and sometimes the syntax itself needs
clarification, but that's clearly not the case here.
> +if [[ "$passedccdir" != "" ]]; then
if [[ -z $passedccdir ]]; then ...
> + ccachedir=$(readlink -e "$passedccdir")
> + [[ ! -d $ccachedir ]] && die "Invalid path '$passedccdir' given for
> ccache directory"
die takes a format string and arguments. please don't inject code into
the format string.
> +fi
> +
> # Detect chrootdir filesystem type
> chroottype=$(stat -f -c %T "$chrootdir")
>
> @@ -349,6 +359,7 @@ download_sources
> if arch-nspawn "$copydir" \
> --bind-ro="$PWD:/startdir_host" \
> --bind-ro="$SRCDEST:/srcdest_host" \
> + ${ccachedir:+"--bind=$ccachedir:/build/.ccache"} \
Why do you modify the ccachedir path? What guarantee is there that this
extra path exists on top of the user provided directory? I'm not a
ccache user, but this seems rather unexpected at a glance.
> /chrootbuild
> then
> move_products
> --
> 1.8.3.4
>
