Signed-off-by: Dominik Heidler dheidler@gmail.com --- 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 / +if [[ "$passedccdir" != "" ]]; then + ccachedir=$(readlink -e "$passedccdir") + [[ ! -d $ccachedir ]] && die "Invalid path '$passedccdir' given for ccache directory" +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"} \ /chrootbuild then move_products
On Tue, Jul 30, 2013 at 03:55:26PM +0200, Dominik Heidler wrote:
Signed-off-by: Dominik Heidler dheidler@gmail.com
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" ;;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
On Tue, Jul 30, 2013 at 4:08 PM, Dave Reisner d@falconindy.com wrote:
On Tue, Jul 30, 2013 at 03:55:26PM +0200, Dominik Heidler wrote:
Signed-off-by: Dominik Heidler dheidler@gmail.com
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" ;; esacdone @@ -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' givenfor ccache directory"
die takes a format string and arguments. please don't inject code into the format string.
doesn't seem to work here - and is not done that in the script at all
+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.
The build directory is created by makearchrootpkg and the .ccache subdir is automaticly created by systemd-nspawn.
The paths are different, because nobody can't use root's .ccache dir and also this allows using different .ccache directories for different architectures.
/chrootbuildthen move_products -- 1.8.3.4
On Tue, Jul 30, 2013 at 04:37:50PM +0200, Dominik Heidler wrote:
On Tue, Jul 30, 2013 at 4:08 PM, Dave Reisner d@falconindy.com wrote:
On Tue, Jul 30, 2013 at 03:55:26PM +0200, Dominik Heidler wrote: > Signed-off-by: Dominik Heidler <dheidler@gmail.com> > --- > 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.doesn't seem to work here - and is not done that in the script at all
Hrmm... makechrootpkg.in has "m4_include(lib/common.sh)". lib/common.sh has:
error() { local mesg=$1; shift printf "${RED}==> ERROR:${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 }
...
die() { error "$*" cleanup 1 }
Sigh. die's definition is wrong and must use "$@" instead of "$*". This *should* work.
> +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.The build directory is created by makearchrootpkg and the .ccache subdir is automaticly created by systemd-nspawn.
The paths are different, because nobody can't use root's .ccache dir and also this allows using different .ccache directories for different architectures.
Sorry, I misread this and mentally passed over the colon in the --bind argument.
> /chrootbuild > then > move_products > -- > 1.8.3.4 >
Users can set a ccache directory (which must be writeable by the user nobody) to be mapped to /build/.ccache (which is the directory, that ccache will use in the chroot)
Signed-off-by: Dominik Heidler dheidler@gmail.com --- makechrootpkg.in | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/makechrootpkg.in b/makechrootpkg.in index d7d3ecf..f04e715 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -21,6 +21,7 @@ run_namcap=false temp_chroot=false chrootdir= passeddir= +ccachedir= declare -a install_pkgs declare -i ret=0
@@ -58,10 +59,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 +73,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) ccachedir="$OPTARG" ;; *) makepkg_args="$makepkg_args -$arg $OPTARG" ;; esac done @@ -84,6 +87,11 @@ 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 / +if [[ "$ccachedir" != "" ]]; then + [[ ! -d $ccachedir ]] && die "Invalid path '%s' given for ccache directory" "$ccachedir" +fi + # Detect chrootdir filesystem type chroottype=$(stat -f -c %T "$chrootdir")
@@ -349,6 +357,7 @@ download_sources if arch-nspawn "$copydir" \ --bind-ro="$PWD:/startdir_host" \ --bind-ro="$SRCDEST:/srcdest_host" \ + ${ccachedir:+"--bind=$ccachedir:/build/.ccache"} \ /chrootbuild then move_products
Users can set a ccache directory (which must be writeable by the user nobody) to be mapped to /build/.ccache (which is the directory, that ccache will use in the chroot)
Signed-off-by: Dominik Heidler dheidler@gmail.com --- makechrootpkg.in | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/makechrootpkg.in b/makechrootpkg.in index d7d3ecf..3d12d1c 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -21,6 +21,7 @@ run_namcap=false temp_chroot=false chrootdir= passeddir= +ccachedir= declare -a install_pkgs declare -i ret=0
@@ -58,10 +59,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 +73,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) ccachedir="$OPTARG" ;; *) makepkg_args="$makepkg_args -$arg $OPTARG" ;; esac done @@ -84,6 +87,11 @@ 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 / +if [[ "$ccachedir" != "" ]]; then + [[ ! -d $ccachedir ]] && die "Invalid path '$ccachedir' given for ccache directory" +fi + # Detect chrootdir filesystem type chroottype=$(stat -f -c %T "$chrootdir")
@@ -349,6 +357,7 @@ download_sources if arch-nspawn "$copydir" \ --bind-ro="$PWD:/startdir_host" \ --bind-ro="$SRCDEST:/srcdest_host" \ + ${ccachedir:+"--bind=$ccachedir:/build/.ccache"} \ /chrootbuild then move_products
arch-projects@lists.archlinux.org