[pacman-dev] [PATCH] makepkg: refactor absolute filename detection
Dan McGee
dpmcgee at gmail.com
Mon May 24 22:53:28 EDT 2010
On Mon, May 17, 2010 at 7:01 AM, Cedric Staniewski <cedric at gmx.ca> wrote:
> Move the absolute filename detection to a new function to reduce code
> duplication.
>
> This patch also fixes the --allsource option that did not include remote
> source files if they reside in $startdir instead of $SRCDEST.
>
> Signed-off-by: Cedric Staniewski <cedric at gmx.ca>
> ---
>
> Finally finished this quiet old patch[1]. However, I am not completely happy
> with it:
>
> 1) error_source_file_not_found is an ugly name; need to find a better/shorter
> one. It's not possible to correctly exit makepkg from within a subshell anymore which
> why these file=... || error_source_file_not_found checks are necessary.
Yeah this is disgusting. :/
I am banging my head on this too trying to come up with something
better and nothing is coming to mind...
> 2) The part in download_sources is rather ugly too. Do we really need to
> distinguish between source files from build dir and from cache?
No, I don't think so, especially since we don't really do that anywhere else.
> I think I've tested every changed function, but please do further testing. This
> patch is rebased on master and Dieter's patch[2].
>
> [1] http://mailman.archlinux.org/pipermail/pacman-dev/2009-October/009755.html
> [2] http://mailman.archlinux.org/pipermail/pacman-dev/2010-May/010779.html
>
> scripts/makepkg.sh.in | 106 +++++++++++++++++++++++++-----------------------
> 1 files changed, 55 insertions(+), 51 deletions(-)
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 9741879..96c5d12 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -179,6 +179,32 @@ trap 'trap_exit "$(gettext "An unknown error has occurred. Exiting...")"' ERR
> # 1) "filename::http://path/to/file"
> # 2) "http://path/to/file"
>
> +# return the absolute filename of an source entry
> +#
> +# this function accepts a source entry or the already extracted filename of a
> +# source entry as input
> +get_absolute_filename() {
> + local file="$(get_filename "$1")"
> +
> + if [[ -f "$startdir/$file" ]] ; then
> + file="$startdir/$file"
> + else
> + if [[ -f "$SRCDEST/$file" ]] ; then
> + file="$SRCDEST/$file"
> + else
> + return 1
> + fi
> + fi
> +
> + echo "$file"
> +}
> +
> +error_source_file_not_found() {
> + error "$(gettext "Unable to find source file %s.")" "$file"
> + plain "$(gettext "Aborting...")"
> + exit 1 # $E_MISSING_FILE
> +}
> +
> # extract the filename from a source entry
> get_filename() {
> # if a filename is specified, use it
> @@ -458,20 +484,24 @@ download_sources() {
>
> local netfile
> for netfile in "${source[@]}"; do
> - local file=$(get_filename "$netfile")
> - local url=$(get_url "$netfile")
> - if [[ -f $startdir/$file ]]; then
> - msg2 "$(gettext "Found %s in build dir")" "$file"
> - rm -f "$srcdir/$file"
> - ln -s "$startdir/$file" "$srcdir/"
> - continue
> - elif [[ -f $SRCDEST/$file ]]; then
> - msg2 "$(gettext "Using cached copy of %s")" "$file"
> - rm -f "$srcdir/$file"
> - ln -s "$SRCDEST/$file" "$srcdir/"
> + local file
> + if file=$(get_absolute_filename "$netfile"); then
> + if [[ ${file%/*} = $startdir ]]; then
> + msg2 "$(gettext "Found %s in build dir")" "${file##*/}"
> + elif [[ ${file%/*} = $SRCDEST ]]; then
> + msg2 "$(gettext "Using cached copy of %s")" "${file##*/}"
> + else
> + # should never be reached
> + error 'Source file found but it neither exists in $startdir nor $SRCDEST.'
> + exit 1
> + fi
> + ln -sf "$file" "$srcdir/"
> continue
> fi
>
> + file=$(get_filename "$netfile")
> + local url=$(get_url "$netfile")
> +
> # if we get here, check to make sure it was a URL, else fail
> if [[ $file = $url ]]; then
> error "$(gettext "%s was not found in the build directory and is not a URL.")" "$file"
> @@ -553,18 +583,7 @@ generate_checksums() {
>
> local netfile
> for netfile in "${source[@]}"; do
> - local file="$(get_filename "$netfile")"
> -
> - if [[ ! -f $file ]] ; then
> - if [[ ! -f $SRCDEST/$file ]] ; then
> - error "$(gettext "Unable to find source file %s to generate checksum.")" "$file"
> - plain "$(gettext "Aborting...")"
> - exit 1
> - else
> - file="$SRCDEST/$file"
> - fi
> - fi
> -
> + local file="$(get_absolute_filename "$netfile")" || error_source_file_not_found
> local sum="$(openssl dgst -${integ} "$file")"
> sum=${sum##* }
> (( ct )) && echo -n "$indent"
> @@ -600,14 +619,10 @@ check_checksums() {
> file="$(get_filename "$file")"
> echo -n " $file ... " >&2
>
> - if [[ ! -f $file ]] ; then
> - if [[ ! -f $SRCDEST/$file ]] ; then
> - echo "$(gettext "NOT FOUND")" >&2
> - errors=1
> - found=0
> - else
> - file="$SRCDEST/$file"
> - fi
> + if ! file="$(get_absolute_filename "$file")"; then
> + echo "$(gettext "NOT FOUND")" >&2
> + errors=1
> + found=0
> fi
>
> if (( $found )) ; then
> @@ -645,25 +660,17 @@ extract_sources() {
> msg "$(gettext "Extracting Sources...")"
> local netfile
> for netfile in "${source[@]}"; do
> - file=$(get_filename "$netfile")
> + absfile="$(get_absolute_filename "$netfile")" || error_source_file_not_found
> + file=${absfile##*/}
> if in_array "$file" ${noextract[@]}; then
> #skip source files in the noextract=() array
> # these are marked explicitly to NOT be extracted
> continue
> fi
>
> - if [[ ! -f $file ]] ; then
> - if [[ ! -f $SRCDEST/$file ]] ; then
> - error "$(gettext "Unable to find source file %s for extraction.")" "$file"
> - plain "$(gettext "Aborting...")"
> - exit 1
> - else
> - file="$SRCDEST/$file"
> - fi
> - fi
>
> # fix flyspray #6246
> - local file_type=$(file -bizL "$file")
> + local file_type=$(file -bizL "$absfile")
> local ext=${file##*.}
> local cmd=''
> case "$file_type" in
> @@ -693,10 +700,10 @@ extract_sources() {
> local ret=0
> msg2 "$(gettext "Extracting %s with %s")" "$file" "$cmd"
> if [[ $cmd = bsdtar ]]; then
> - $cmd -xf "$file" || ret=$?
> + $cmd -xf "$absfile" || ret=$?
> else
> rm -f "${file%.*}"
> - $cmd -dcf "$file" > "${file%.*}" || ret=$?
> + $cmd -dcf "$absfile" > "${file%.*}" || ret=$?
> fi
> if (( ret )); then
> error "$(gettext "Failed to extract %s")" "$file"
> @@ -1091,13 +1098,10 @@ create_srcpackage() {
>
> local netfile
> for netfile in "${source[@]}"; do
> - local file=$(get_filename "$netfile")
> - if [[ -f $netfile ]]; then
> - msg2 "$(gettext "Adding %s...")" "$netfile"
> - ln -s "${startdir}/$netfile" "${srclinks}/${pkgbase}"
> - elif (( SOURCEONLY == 2 )) && [[ -f $SRCDEST/$file ]]; then
> - msg2 "$(gettext "Adding %s...")" "$file"
> - ln -s "$SRCDEST/$file" "${srclinks}/${pkgbase}/"
> + if [[ -f $netfile ]] || (( SOURCEONLY == 2 )); then
> + local file=$(get_absolute_filename "$netfile") || error_source_file_not_found
> + msg2 "$(gettext "Adding %s...")" "${file##*/}"
> + ln -s "$file" "${srclinks}/${pkgbase}"
> fi
> done
>
> --
> 1.7.1
>
>
>
More information about the pacman-dev
mailing list