On Mon, May 17, 2010 at 7:01 AM, Cedric Staniewski 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 ---

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