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