[pacman-dev] [PATCH] makepkg: use basename instead of a sed command

older
[pacman-dev] Is -Suu supposed to...

Cedric Staniewski

17 Oct 2009 17 Oct '09

1:41 p.m.

The basename command is easier to understand than a sed command and it is even slightly faster than sed. Signed-off-by: Cedric Staniewski --- or if you prefer bashisms, see next patch scripts/makepkg.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 24fddf6..62d0923 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -190,7 +190,7 @@ get_filename() { # if a filename is specified, use it local filename=$(echo $1 | sed 's|::.*||') # if it is just an URL, we only keep the last component - echo "$filename" | sed 's|^.*://.*/||g' + basename "$filename" } # extract the URL from a source entry @@ -321,7 +321,7 @@ download_file() { # destination file local file=$3 # temporary download file, default to last component of the URL - local dlfile=$(echo "$url" | sed 's|^.*://.*/||g') + local dlfile=$(basename "$url") # replace %o by the temporary dlfile if it exists if echo "$dlcmd" | grep -q "%o" ; then -- 1.6.5

Show replies by date

Cedric Staniewski

17 Oct 17 Oct

1:41 p.m.

New subject: [pacman-dev] [PATCH] makepkg: replace basename emulation via sed with a bash substitution

Signed-off-by: Cedric Staniewski --- scripts/makepkg.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 24fddf6..ad8b6da 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -190,7 +190,7 @@ get_filename() { # if a filename is specified, use it local filename=$(echo $1 | sed 's|::.*||') # if it is just an URL, we only keep the last component - echo "$filename" | sed 's|^.*://.*/||g' + echo "${filename##*/}" } # extract the URL from a source entry @@ -282,7 +282,7 @@ in_array() { get_downloadclient() { # $1 = URL with valid protocol prefix local url=$1 - local proto=$(echo "$url" | sed 's|://.*||') + local proto=$(echo ${url##*/}) # loop through DOWNLOAD_AGENTS variable looking for protocol local i -- 1.6.5

Allan McRae

18 Oct 18 Oct

1:26 a.m.

New subject: [pacman-dev] [PATCH] makepkg: replace basename emulation via sed with a bash substitution

Cedric Staniewski wrote:

...

Signed-off-by: Cedric Staniewski --- scripts/makepkg.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 24fddf6..ad8b6da 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -190,7 +190,7 @@ get_filename() { # if a filename is specified, use it local filename=$(echo $1 | sed 's|::.*||')

We could also use bash substitution for that line.

...

# if it is just an URL, we only keep the last component - echo "$filename" | sed 's|^.*://.*/||g' + echo "${filename##*/}" }

# extract the URL from a source entry

and about two lines below here...

...

@@ -282,7 +282,7 @@ in_array() { get_downloadclient() { # $1 = URL with valid protocol prefix local url=$1 - local proto=$(echo "$url" | sed 's|://.*||') + local proto=$(echo ${url##*/})

# loop through DOWNLOAD_AGENTS variable looking for protocol local i

Cedric Staniewski

10:52 a.m.

New subject: [pacman-dev] [PATCH] makepkg: replace basename emulation via sed with a bash substitution

Allan McRae wrote:

...

Cedric Staniewski wrote:

...
Signed-off-by: Cedric Staniewski --- scripts/makepkg.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 24fddf6..ad8b6da 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -190,7 +190,7 @@ get_filename() { # if a filename is specified, use it local filename=$(echo $1 | sed 's|::.*||')

We could also use bash substitution for that line.

...
# if it is just an URL, we only keep the last component - echo "$filename" | sed 's|^.*://.*/||g' + echo "${filename##*/}" }

# extract the URL from a source entry

and about two lines below here...

...
@@ -282,7 +282,7 @@ in_array() { get_downloadclient() { # $1 = URL with valid protocol prefix local url=$1 - local proto=$(echo "$url" | sed 's|://.*||') + local proto=$(echo ${url##*/})

# loop through DOWNLOAD_AGENTS variable looking for protocol local i

There are several commands which could be replaced by bash substitutions. I attached a patch which also contains the first patch of this thread. So, if we decide for the 'bash way' rather than using basename, this one could be used.

...

From 23b7c7346e58293e403b00a8ddff0c7e37498f46 Mon Sep 17 00:00:00 2001 From: Cedric Staniewski Date: Sun, 18 Oct 2009 16:32:51 +0200 Subject: [PATCH] makepkg, repo-add: replace external commands with bash substitutions where possible

This also removes the awk dependency from makepkg and repo-add. Signed-off-by: Cedric Staniewski --- Probably, it is better to keep the sed command for extracting the latest svn revision. The 'bash way' is imo more complex, and the slow part of this command is anyway svn info. Besides, I'm not sure about the change in write_pkginfo. `du` separates size and file/directory name by a tab and the output looks like this: 346616 . Currently, I only remove the period at the end and bash strips the trailing tab by itself. Of course, it would be possible to use size=${size%% *} with the whitespace being a tab (bash does not support \t in substitutions as it seems) which I do not really like, so the remaining possibility would be: local size="$(du -sk | tr '\t' ' ')" size="$(( ${size%% *} * 1024 ))" which is not that good either. scripts/makepkg.sh.in | 43 ++++++++++++++++++++++++------------------- scripts/repo-add.sh.in | 6 ++++-- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 24fddf6..41078e9 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -27,7 +27,7 @@ # makepkg uses quite a few external programs during its execution. You # need to have at least the following installed for makepkg to function: -# awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils), +# bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils), # getopt (util-linux), gettext, grep, gzip, openssl, sed # gettext initialization @@ -188,15 +188,15 @@ trap 'trap_exit "$(gettext "An unknown error has occurred. Exiting...")"' ERR # extract the filename from a source entry get_filename() { # if a filename is specified, use it - local filename=$(echo $1 | sed 's|::.*||') + local filename="${1%%::*}" # if it is just an URL, we only keep the last component - echo "$filename" | sed 's|^.*://.*/||g' + echo "${filename##*/}" } # extract the URL from a source entry get_url() { # strip an eventual filename - echo $1 | sed 's|.*::||' + echo "${1#*::}" } ## @@ -246,11 +246,11 @@ check_buildenv() { # ? - not found ## in_opt_array() { - local needle=$(echo $1 | tr '[:upper:]' '[:lower:]'); shift + local needle="${1,,}"; shift local opt for opt in "$@"; do - opt=$(echo $opt | tr '[:upper:]' '[:lower:]') + opt="${opt,,}" if [ "$opt" = "$needle" ]; then echo 'y' # Enabled return @@ -282,14 +282,14 @@ in_array() { get_downloadclient() { # $1 = URL with valid protocol prefix local url=$1 - local proto=$(echo "$url" | sed 's|://.*||') + local proto="${url%%://*}" # loop through DOWNLOAD_AGENTS variable looking for protocol local i for i in "${DLAGENTS[@]}"; do - local handler=$(echo $i | sed 's|::.*||') + local handler="${i%%::*}" if [ "$proto" = "$handler" ]; then - agent=$(echo $i | sed 's|^.*::||') + agent="${i##*::}" break fi done @@ -302,7 +302,7 @@ get_downloadclient() { fi # ensure specified program is installed - local program="$(echo $agent | awk '{print $1 }')" + local program="${agent%% *}" if [ ! -x "$program" ]; then local baseprog=$(basename $program) error "$(gettext "The download program %s is not installed.")" "$baseprog" @@ -321,7 +321,7 @@ download_file() { # destination file local file=$3 # temporary download file, default to last component of the URL - local dlfile=$(echo "$url" | sed 's|^.*://.*/||g') + local dlfile="${url##*/}" # replace %o by the temporary dlfile if it exists if echo "$dlcmd" | grep -q "%o" ; then @@ -431,7 +431,7 @@ remove_deps() { local dep depstrip deplist deplist="" for dep in $pkgdeps; do - depstrip=$(echo $dep | sed -e 's|=.*$||' -e 's|>.*$||' -e 's|<.*$||') + depstrip="${dep%%[<=>]*}" deplist="$deplist $depstrip" done @@ -513,7 +513,7 @@ generate_checksums() { local integ for integ in ${INTEGRITY_CHECK[@]}; do - integ="$(echo $integ | tr '[:upper:]' '[:lower:]')" + integ="${integ,,}" case "$integ" in md5|sha1|sha256|sha384|sha512) : ;; *) @@ -546,7 +546,8 @@ generate_checksums() { fi fi - local sum="$(openssl dgst -${integ} "$file" | awk '{print $NF}')" + local sum="$(openssl dgst -${integ} "$file")" + sum=${sum##* } [ $ct -gt 0 ] && echo -n "$indent" echo -n "'$sum'" ct=$(($ct+1)) @@ -591,8 +592,9 @@ check_checksums() { fi if [ $found -gt 0 ] ; then - local expectedsum="$(echo ${integrity_sums[$idx]} | tr '[A-F]' '[a-f]')" - local realsum="$(openssl dgst -${integ} "$file" | awk '{print $NF}')" + local expectedsum="${integrity_sums[$idx],,}" + local realsum="$(openssl dgst -${integ} "$file")" + realsum="${realsum##* }" if [ "$expectedsum" = "$realsum" ]; then echo "$(gettext "Passed")" >&2 else @@ -882,7 +884,8 @@ write_pkginfo() { else local packager="Unknown Packager" fi - local size=$(du -sk | awk '{print $1 * 1024}') + local size="$(du -sk)" + size="$(( ${size%%.*} * 1024 ))" msg2 "$(gettext "Generating .PKGINFO file...")" echo "# Generated by makepkg $myver" >.PKGINFO @@ -1265,7 +1268,9 @@ devel_check() { elif [ -n "${_svntrunk}" -a -n "${_svnmod}" ] ; then [ $(type -p svn) ] || return 0 msg "$(gettext "Determining latest svn revision...")" - newpkgver=$(LC_ALL=C svn info $_svntrunk | sed -n 's/^Last Changed Rev: $[0-9]*$$/\1/p') + newpkgver="$(LC_ALL=C svn info $_svntrunk)" + newpkgver="${newpkgver##*Last Changed Rev: }" + newpkgver=${newpkgver%%[A-Z]*} elif [ -n "${_bzrtrunk}" -a -n "${_bzrmod}" ] ; then [ $(type -p bzr) ] || return 0 msg "$(gettext "Determining latest bzr revision...")" @@ -1588,7 +1593,7 @@ if [ "$CLEANCACHE" -eq 1 ]; then echo -n "$(gettext " Are you sure you wish to do this? ")" echo -n "$(gettext "[Y/n]")" read answer - answer=$(echo $answer | tr '[:lower:]' '[:upper:]') + answer="${answer^^}" if [ "$answer" = "$(gettext "YES")" -o "$answer" = "$(gettext "Y")" ]; then rm "$SRCDEST"/* if [ $? -ne 0 ]; then diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 2a62d2b..3f00441 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -136,7 +136,8 @@ db_write_delta() echo -e "%DELTAS%" >>$deltas fi # get md5sum and compressed size of package - md5sum="$(openssl dgst -md5 "$deltafile" | awk '{print $NF}')" + md5sum="$(openssl dgst -md5 "$deltafile")" + md5sum="${md5sum##* }" csize=$(@SIZECMD@ "$deltafile") oldfile=$(xdelta3 printhdr $deltafile | grep "XDELTA filename (source)" | sed 's/.*: *//') @@ -213,7 +214,8 @@ db_write_entry() IFS=$OLDIFS # get md5sum and compressed size of package - md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" + md5sum="$(openssl dgst -md5 "$pkgfile")" + md5sum="${md5sum##* }" csize=$(@SIZECMD@ "$pkgfile") # ensure $pkgname and $pkgver variables were found -- 1.6.5

Dan McGee

8:49 p.m.

New subject: [pacman-dev] [PATCH] makepkg: replace basename emulation via sed with a bash substitution

On Sun, Oct 18, 2009 at 9:52 AM, Cedric Staniewski wrote:

...

Allan McRae wrote:

...
Cedric Staniewski wrote:

...
Signed-off-by: Cedric Staniewski --- scripts/makepkg.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 24fddf6..ad8b6da 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -190,7 +190,7 @@ get_filename() { # if a filename is specified, use it local filename=$(echo $1 | sed 's|::.*||')

We could also use bash substitution for that line.

...
# if it is just an URL, we only keep the last component - echo "$filename" | sed 's|^.*://.*/||g' + echo "${filename##*/}" }

# extract the URL from a source entry

and about two lines below here...

...
@@ -282,7 +282,7 @@ in_array() { get_downloadclient() { # $1 = URL with valid protocol prefix local url=$1 - local proto=$(echo "$url" | sed 's|://.*||') + local proto=$(echo ${url##*/})

# loop through DOWNLOAD_AGENTS variable looking for protocol local i

There are several commands which could be replaced by bash substitutions. I attached a patch which also contains the first patch of this thread. So, if we decide for the 'bash way' rather than using basename, this one could be used.

...
From 23b7c7346e58293e403b00a8ddff0c7e37498f46 Mon Sep 17 00:00:00 2001 From: Cedric Staniewski Date: Sun, 18 Oct 2009 16:32:51 +0200 Subject: [PATCH] makepkg, repo-add: replace external commands with bash substitutions where possible

This also removes the awk dependency from makepkg and repo-add.

Signed-off-by: Cedric Staniewski ---

Probably, it is better to keep the sed command for extracting the latest svn revision. The 'bash way' is imo more complex, and the slow part of this command is anyway svn info.

Besides, I'm not sure about the change in write_pkginfo. `du` separates size and file/directory name by a tab and the output looks like this: 346616 . Currently, I only remove the period at the end and bash strips the trailing tab by itself. Of course, it would be possible to use size=${size%% *} with the whitespace being a tab (bash does not support \t in substitutions as it seems) which I do not really like, so the remaining possibility would be:

local size="$(du -sk | tr '\t' ' ')" size="$(( ${size%% *} * 1024 ))"

which is not that good either.

This patch seems pretty reasonable. I think leaving the svn command as-is would be a bit more clear, but the rest seems relatively reasonable. I'll let Allan comment as well though. -Dan

Allan McRae

19 Oct 19 Oct

8:25 a.m.

New subject: [pacman-dev] [PATCH] makepkg: replace basename emulation via sed with a bash substitution

Dan McGee wrote:

...

On Sun, Oct 18, 2009 at 9:52 AM, Cedric Staniewski wrote:

...
Allan McRae wrote:

...
Cedric Staniewski wrote:

...
Signed-off-by: Cedric Staniewski --- scripts/makepkg.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 24fddf6..ad8b6da 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -190,7 +190,7 @@ get_filename() { # if a filename is specified, use it local filename=$(echo $1 | sed 's|::.*||')

We could also use bash substitution for that line.

...
# if it is just an URL, we only keep the last component - echo "$filename" | sed 's|^.*://.*/||g' + echo "${filename##*/}" }

# extract the URL from a source entry

and about two lines below here...

...
@@ -282,7 +282,7 @@ in_array() { get_downloadclient() { # $1 = URL with valid protocol prefix local url=$1 - local proto=$(echo "$url" | sed 's|://.*||') + local proto=$(echo ${url##*/})

# loop through DOWNLOAD_AGENTS variable looking for protocol local i

There are several commands which could be replaced by bash substitutions. I attached a patch which also contains the first patch of this thread. So, if we decide for the 'bash way' rather than using basename, this one could be used.

...
From 23b7c7346e58293e403b00a8ddff0c7e37498f46 Mon Sep 17 00:00:00 2001 From: Cedric Staniewski Date: Sun, 18 Oct 2009 16:32:51 +0200 Subject: [PATCH] makepkg, repo-add: replace external commands with bash substitutions where possible

This also removes the awk dependency from makepkg and repo-add.

Signed-off-by: Cedric Staniewski ---

Probably, it is better to keep the sed command for extracting the latest svn revision. The 'bash way' is imo more complex, and the slow part of this command is anyway svn info.

Besides, I'm not sure about the change in write_pkginfo. `du` separates size and file/directory name by a tab and the output looks like this: 346616 . Currently, I only remove the period at the end and bash strips the trailing tab by itself. Of course, it would be possible to use size=${size%% *} with the whitespace being a tab (bash does not support \t in substitutions as it seems) which I do not really like, so the remaining possibility would be:

local size="$(du -sk | tr '\t' ' ')" size="$(( ${size%% *} * 1024 ))"

which is not that good either.

This patch seems pretty reasonable. I think leaving the svn command as-is would be a bit more clear, but the rest seems relatively reasonable. I'll let Allan comment as well though.

On a brief review, it looks good to me too. I will have some time to review this in more detail later this week and will comment or pull to my working branch as appropriate. Allan

Allan McRae

20 Oct 20 Oct

1:21 a.m.

New subject: [pacman-dev] [PATCH] makepkg: replace basename emulation via sed with a bash substitution

Cedric Staniewski wrote:

...

Allan McRae wrote:

...
Cedric Staniewski wrote:

...
Signed-off-by: Cedric Staniewski --- scripts/makepkg.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 24fddf6..ad8b6da 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -190,7 +190,7 @@ get_filename() { # if a filename is specified, use it local filename=$(echo $1 | sed 's|::.*||')

We could also use bash substitution for that line.

...
# if it is just an URL, we only keep the last component - echo "$filename" | sed 's|^.*://.*/||g' + echo "${filename##*/}" }

# extract the URL from a source entry

and about two lines below here...

...
@@ -282,7 +282,7 @@ in_array() { get_downloadclient() { # $1 = URL with valid protocol prefix local url=$1 - local proto=$(echo "$url" | sed 's|://.*||') + local proto=$(echo ${url##*/})

# loop through DOWNLOAD_AGENTS variable looking for protocol local i

There are several commands which could be replaced by bash substitutions. I attached a patch which also contains the first patch of this thread. So, if we decide for the 'bash way' rather than using basename, this one could be used.

...
From 23b7c7346e58293e403b00a8ddff0c7e37498f46 Mon Sep 17 00:00:00 2001 From: Cedric Staniewski Date: Sun, 18 Oct 2009 16:32:51 +0200 Subject: [PATCH] makepkg, repo-add: replace external commands with bash substitutions where possible

This also removes the awk dependency from makepkg and repo-add.

Signed-off-by: Cedric Staniewski ---

Probably, it is better to keep the sed command for extracting the latest svn revision. The 'bash way' is imo more complex, and the slow part of this command is anyway svn info.

I agree, keep the svn revision extraction as is.

...

Besides, I'm not sure about the change in write_pkginfo. `du` separates size and file/directory name by a tab and the output looks like this: 346616 . Currently, I only remove the period at the end and bash strips the trailing tab by itself. Of course, it would be possible to use size=${size%% *} with the whitespace being a tab (bash does not support \t in substitutions as it seems) which I do not really like, so the remaining possibility would be:

local size="$(du -sk | tr '\t' ' ')" size="$(( ${size%% *} * 1024 ))"

which is not that good either.

This is what you are looking for: size=${size%%[^0-9]*} I will pull this patch with those two minor changes to my working branch in the next couple of days. Allan

Cedric Staniewski

9:44 a.m.

New subject: [pacman-dev] [PATCH] makepkg: replace basename emulation via sed with a bash substitution

Allan McRae wrote:

...

This is what you are looking for: size=${size%%[^0-9]*}

Wow, thanks a lot! Sometimes, it's just that easy. ;)

...

I will pull this patch with those two minor changes to my working branch in the next couple of days.

I attached a new patch containing these two changes. The former one did not apply cleanly anyway due to a formatting error in its header.

...

From d7b100ad2f58b1b39a5c42e24225c4b67122b010 Mon Sep 17 00:00:00 2001 From: Cedric Staniewski Date: Sun, 18 Oct 2009 16:52:30 +0200 Subject: [PATCH] makepkg, repo-add: replace external commands with bash substitutions where possible

This also removes the awk dependency from makepkg and repo-add. Signed-off-by: Cedric Staniewski --- scripts/makepkg.sh.in | 39 +++++++++++++++++++++------------------ scripts/repo-add.sh.in | 6 ++++-- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 24fddf6..c48bb34 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -27,7 +27,7 @@ # makepkg uses quite a few external programs during its execution. You # need to have at least the following installed for makepkg to function: -# awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils), +# bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils), # getopt (util-linux), gettext, grep, gzip, openssl, sed # gettext initialization @@ -188,15 +188,15 @@ trap 'trap_exit "$(gettext "An unknown error has occurred. Exiting...")"' ERR # extract the filename from a source entry get_filename() { # if a filename is specified, use it - local filename=$(echo $1 | sed 's|::.*||') + local filename="${1%%::*}" # if it is just an URL, we only keep the last component - echo "$filename" | sed 's|^.*://.*/||g' + echo "${filename##*/}" } # extract the URL from a source entry get_url() { # strip an eventual filename - echo $1 | sed 's|.*::||' + echo "${1#*::}" } ## @@ -246,11 +246,11 @@ check_buildenv() { # ? - not found ## in_opt_array() { - local needle=$(echo $1 | tr '[:upper:]' '[:lower:]'); shift + local needle="${1,,}"; shift local opt for opt in "$@"; do - opt=$(echo $opt | tr '[:upper:]' '[:lower:]') + opt="${opt,,}" if [ "$opt" = "$needle" ]; then echo 'y' # Enabled return @@ -282,14 +282,14 @@ in_array() { get_downloadclient() { # $1 = URL with valid protocol prefix local url=$1 - local proto=$(echo "$url" | sed 's|://.*||') + local proto="${url%%://*}" # loop through DOWNLOAD_AGENTS variable looking for protocol local i for i in "${DLAGENTS[@]}"; do - local handler=$(echo $i | sed 's|::.*||') + local handler="${i%%::*}" if [ "$proto" = "$handler" ]; then - agent=$(echo $i | sed 's|^.*::||') + agent="${i##*::}" break fi done @@ -302,7 +302,7 @@ get_downloadclient() { fi # ensure specified program is installed - local program="$(echo $agent | awk '{print $1 }')" + local program="${agent%% *}" if [ ! -x "$program" ]; then local baseprog=$(basename $program) error "$(gettext "The download program %s is not installed.")" "$baseprog" @@ -321,7 +321,7 @@ download_file() { # destination file local file=$3 # temporary download file, default to last component of the URL - local dlfile=$(echo "$url" | sed 's|^.*://.*/||g') + local dlfile="${url##*/}" # replace %o by the temporary dlfile if it exists if echo "$dlcmd" | grep -q "%o" ; then @@ -431,7 +431,7 @@ remove_deps() { local dep depstrip deplist deplist="" for dep in $pkgdeps; do - depstrip=$(echo $dep | sed -e 's|=.*$||' -e 's|>.*$||' -e 's|<.*$||') + depstrip="${dep%%[<=>]*}" deplist="$deplist $depstrip" done @@ -513,7 +513,7 @@ generate_checksums() { local integ for integ in ${INTEGRITY_CHECK[@]}; do - integ="$(echo $integ | tr '[:upper:]' '[:lower:]')" + integ="${integ,,}" case "$integ" in md5|sha1|sha256|sha384|sha512) : ;; *) @@ -546,7 +546,8 @@ generate_checksums() { fi fi - local sum="$(openssl dgst -${integ} "$file" | awk '{print $NF}')" + local sum="$(openssl dgst -${integ} "$file")" + sum=${sum##* } [ $ct -gt 0 ] && echo -n "$indent" echo -n "'$sum'" ct=$(($ct+1)) @@ -591,8 +592,9 @@ check_checksums() { fi if [ $found -gt 0 ] ; then - local expectedsum="$(echo ${integrity_sums[$idx]} | tr '[A-F]' '[a-f]')" - local realsum="$(openssl dgst -${integ} "$file" | awk '{print $NF}')" + local expectedsum="${integrity_sums[$idx],,}" + local realsum="$(openssl dgst -${integ} "$file")" + realsum="${realsum##* }" if [ "$expectedsum" = "$realsum" ]; then echo "$(gettext "Passed")" >&2 else @@ -882,7 +884,8 @@ write_pkginfo() { else local packager="Unknown Packager" fi - local size=$(du -sk | awk '{print $1 * 1024}') + local size="$(du -sk)" + size="$(( ${size%%[^0-9]*} * 1024 ))" msg2 "$(gettext "Generating .PKGINFO file...")" echo "# Generated by makepkg $myver" >.PKGINFO @@ -1588,7 +1591,7 @@ if [ "$CLEANCACHE" -eq 1 ]; then echo -n "$(gettext " Are you sure you wish to do this? ")" echo -n "$(gettext "[Y/n]")" read answer - answer=$(echo $answer | tr '[:lower:]' '[:upper:]') + answer="${answer^^}" if [ "$answer" = "$(gettext "YES")" -o "$answer" = "$(gettext "Y")" ]; then rm "$SRCDEST"/* if [ $? -ne 0 ]; then diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 2a62d2b..3f00441 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -136,7 +136,8 @@ db_write_delta() echo -e "%DELTAS%" >>$deltas fi # get md5sum and compressed size of package - md5sum="$(openssl dgst -md5 "$deltafile" | awk '{print $NF}')" + md5sum="$(openssl dgst -md5 "$deltafile")" + md5sum="${md5sum##* }" csize=$(@SIZECMD@ "$deltafile") oldfile=$(xdelta3 printhdr $deltafile | grep "XDELTA filename (source)" | sed 's/.*: *//') @@ -213,7 +214,8 @@ db_write_entry() IFS=$OLDIFS # get md5sum and compressed size of package - md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" + md5sum="$(openssl dgst -md5 "$pkgfile")" + md5sum="${md5sum##* }" csize=$(@SIZECMD@ "$pkgfile") # ensure $pkgname and $pkgver variables were found -- 1.6.5.1

Allan McRae

9:59 a.m.

New subject: [pacman-dev] [PATCH] makepkg: replace basename emulation via sed with a bash substitution

Cedric Staniewski wrote:

...

Allan McRae wrote:

...
This is what you are looking for: size=${size%%[^0-9]*}

Wow, thanks a lot! Sometimes, it's just that easy. ;)

...
I will pull this patch with those two minor changes to my working branch in the next couple of days.

I attached a new patch containing these two changes. The former one did not apply cleanly anyway due to a formatting error in its header.

Thanks, Applied and pushed to my working branch. Allan

Xavier

17 Oct 17 Oct

2:16 p.m.

New subject: [pacman-dev] [PATCH] makepkg: use basename instead of a sed command

On Sat, Oct 17, 2009 at 7:41 PM, Cedric Staniewski wrote:

...

The basename command is easier to understand than a sed command and it is even slightly faster than sed.

Signed-off-by: Cedric Staniewski --- or if you prefer bashisms, see next patch

I actually saw that code recently and thought exactly the same. I prefer the bash way for code that is executed many times (for performance reason), but it's not really the case here, so basename should be fine. Is basename available on all the os we support ?

Cedric Staniewski

3:09 p.m.

New subject: [pacman-dev] [PATCH] makepkg: use basename instead of a sed command

Xavier wrote:

...

On Sat, Oct 17, 2009 at 7:41 PM, Cedric Staniewski wrote:

...
The basename command is easier to understand than a sed command and it is even slightly faster than sed.

Signed-off-by: Cedric Staniewski --- or if you prefer bashisms, see next patch

I actually saw that code recently and thought exactly the same. I prefer the bash way for code that is executed many times (for performance reason), but it's not really the case here, so basename should be fine. Is basename available on all the os we support ?

I do not know if it is available on all the required os, but it was already used before in makepkg and makepkg's header states:

...

# makepkg uses quite a few external programs during its execution. You # need to have at least the following installed for makepkg to function: # awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils), # getopt (util-linux), gettext, grep, gzip, openssl, sed

with basename being part of coreutils.

Allan McRae

18 Oct 18 Oct

1:24 a.m.

New subject: [pacman-dev] [PATCH] makepkg: use basename instead of a sed command

Cedric Staniewski wrote:

...

Xavier wrote:

...
On Sat, Oct 17, 2009 at 7:41 PM, Cedric Staniewski wrote:

...
The basename command is easier to understand than a sed command and it is even slightly faster than sed.

Signed-off-by: Cedric Staniewski --- or if you prefer bashisms, see next patch

I actually saw that code recently and thought exactly the same. I prefer the bash way for code that is executed many times (for performance reason), but it's not really the case here, so basename should be fine. Is basename available on all the os we support ?

I do not know if it is available on all the required os, but it was already used before in makepkg and makepkg's header states:

...
# makepkg uses quite a few external programs during its execution. You # need to have at least the following installed for makepkg to function: # awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils), # getopt (util-linux), gettext, grep, gzip, openssl, sed

with basename being part of coreutils.

I'd prefer the bash substituion for 1) it is a bash internal so does not require starting another process 2) the only use of basename so far in makepkg is in an error message which I doubt many people ever hit. So there is no real guarantee this is portable across platforms (though it should be...) Allan

Cedric Staniewski

11:05 a.m.

New subject: [pacman-dev] [PATCH] makepkg: use basename instead of a sed command

Allan McRae wrote:

...

Cedric Staniewski wrote:

...
Xavier wrote:

...
On Sat, Oct 17, 2009 at 7:41 PM, Cedric Staniewski wrote:

...
The basename command is easier to understand than a sed command and it is even slightly faster than sed.

Signed-off-by: Cedric Staniewski --- or if you prefer bashisms, see next patch

I actually saw that code recently and thought exactly the same. I prefer the bash way for code that is executed many times (for performance reason), but it's not really the case here, so basename should be fine. Is basename available on all the os we support ?

I do not know if it is available on all the required os, but it was already used before in makepkg and makepkg's header states:

...
# makepkg uses quite a few external programs during its execution. You # need to have at least the following installed for makepkg to function: # awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils), # getopt (util-linux), gettext, grep, gzip, openssl, sed

with basename being part of coreutils.

I'd prefer the bash substituion for

1) it is a bash internal so does not require starting another process

That's sure, but for basename it makes not so much difference: $ url="http://mailman.archlinux.org/mailman/listinfo/pacman-dev" $ time echo $url | sed 's|^.*://.*/||g' pacman-dev real 0m0.012s user 0m0.010s sys 0m0.003s $ time echo $(basename "$url") pacman-dev real 0m0.003s user 0m0.003s sys 0m0.000s $ time echo ${url##*/} pacman-dev real 0m0.000s user 0m0.000s sys 0m0.000s In my opinion, the use of basename would make the code a little bit easier to understand, but if we use bash substitutions for anything else, it would not make much sense to use basename here.

...

2) the only use of basename so far in makepkg is in an error message which I doubt many people ever hit. So there is no real guarantee this is portable across platforms (though it should be...)

If you thing this could be an issue, we should use bash substitutions.

...

Allan

5307

Age (days ago)

5310

Last active (days ago)

List overview

Download

12 comments

4 participants

participants (4)

Allan McRae
Cedric Staniewski
Dan McGee
Xavier