| Message ID | 20170924225632.7084-1-lukeshu@parabola.nu |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [devtools,1/1] use makepkg library instead of local function copies | expand |
On 09/24/2017 06:56 PM, Luke Shumaker wrote: > This mirrors dbscripts commit > 625fa02 by Pierre Schmitz <pierre@archlinux.de> at 2017-04-18 14:20:49 > --- > lib/common.sh | 100 ++++------------------------------------------------------ > 1 file changed, 7 insertions(+), 93 deletions(-) > > diff --git a/lib/common.sh b/lib/common.sh > index 0fb93d9..a3c2ec2 100644 > --- a/lib/common.sh > +++ b/lib/common.sh > @@ -6,62 +6,21 @@ > [[ -z ${_INCLUDE_COMMON_SH:-} ]] || return 0 > _INCLUDE_COMMON_SH=true > > +# shellcheck disable=1091 > +. /usr/share/makepkg/util.sh Please just source the actual parts we need, the same way https://git.archlinux.org/dbscripts.git/commit/?id=625fa02 also only uses /usr/share/makepkg/util/message.sh /usr/share/makepkg/util/util.sh /usr/share/makepkg/util/pkgbuild.sh
On Sun, 24 Sep 2017 20:08:51 -0400, Eli Schwartz wrote: > On 09/24/2017 06:56 PM, Luke Shumaker wrote: > > This mirrors dbscripts commit > > 625fa02 by Pierre Schmitz <pierre@archlinux.de> at 2017-04-18 14:20:49 > > --- > > +. /usr/share/makepkg/util.sh > > Please just source the actual parts we need, the same way > https://git.archlinux.org/dbscripts.git/commit/?id=625fa02 also only uses > /usr/share/makepkg/util/message.sh > /usr/share/makepkg/util/util.sh > /usr/share/makepkg/util/pkgbuild.sh I was wondering if someone was going to call me on that. I suppose I should have noted in the commit message that 2 days later, in commit 76f95dd, Pierre changed it to use /usr/share/makepkg/util.sh instead; so I mimicked that. AFAICT, that change was unrelated to the rest of 76f95dd, but it's quite a large patch, and I didn't look at it too closely. https://git.archlinux.org/dbscripts.git/commit/?id=76f95dd
On 09/24/2017 08:52 PM, Luke Shumaker wrote: > On Sun, 24 Sep 2017 20:08:51 -0400, > Eli Schwartz wrote: >> On 09/24/2017 06:56 PM, Luke Shumaker wrote: >>> This mirrors dbscripts commit >>> 625fa02 by Pierre Schmitz <pierre@archlinux.de> at 2017-04-18 14:20:49 >>> --- > >>> +. /usr/share/makepkg/util.sh >> >> Please just source the actual parts we need, the same way >> https://git.archlinux.org/dbscripts.git/commit/?id=625fa02 also only uses >> /usr/share/makepkg/util/message.sh >> /usr/share/makepkg/util/util.sh >> /usr/share/makepkg/util/pkgbuild.sh > > I was wondering if someone was going to call me on that. I suppose I > should have noted in the commit message that 2 days later, in commit > 76f95dd, Pierre changed it to use /usr/share/makepkg/util.sh instead; > so I mimicked that. AFAICT, that change was unrelated to the rest of > 76f95dd, but it's quite a large patch, and I didn't look at it too > closely. > > https://git.archlinux.org/dbscripts.git/commit/?id=76f95dd Hah! :p Well, this seems like bloat anyway, maybe I should ask Pierre to change it back...
On Mon, Sep 25, 2017 at 2:59 AM Eli Schwartz <eschwartz@archlinux.org> wrote: > On 09/24/2017 08:52 PM, Luke Shumaker wrote: > > On Sun, 24 Sep 2017 20:08:51 -0400, > > Eli Schwartz wrote: > >> On 09/24/2017 06:56 PM, Luke Shumaker wrote: > >>> This mirrors dbscripts commit > >>> 625fa02 by Pierre Schmitz <pierre@archlinux.de> at 2017-04-18 14:20:49 > >>> --- > > > >>> +. /usr/share/makepkg/util.sh > >> > >> Please just source the actual parts we need, the same way > >> https://git.archlinux.org/dbscripts.git/commit/?id=625fa02 also only > uses > >> /usr/share/makepkg/util/message.sh > >> /usr/share/makepkg/util/util.sh > >> /usr/share/makepkg/util/pkgbuild.sh > > > > I was wondering if someone was going to call me on that. I suppose I > > should have noted in the commit message that 2 days later, in commit > > 76f95dd, Pierre changed it to use /usr/share/makepkg/util.sh instead; > > so I mimicked that. AFAICT, that change was unrelated to the rest of > > 76f95dd, but it's quite a large patch, and I didn't look at it too > > closely. > > > > https://git.archlinux.org/dbscripts.git/commit/?id=76f95dd > > Hah! :p > > Well, this seems like bloat anyway, maybe I should ask Pierre to change > it back... > It seems simpler to me, even if it loads more code than needed. If everyone sources util.sh instead of its components directly, makepkg can shuffle the internals around without breaking consumers. This is also a common pattern seen in C includes. I'll apply this patch as-is now. We can still change it afterwards. Luke, thanks for your continued work on devtools. <div dir="ltr">On Mon, Sep 25, 2017 at 2:59 AM Eli Schwartz <<a href="mailto:eschwartz@archlinux.org">eschwartz@archlinux.org</a>> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 09/24/2017 08:52 PM, Luke Shumaker wrote:<br> > On Sun, 24 Sep 2017 20:08:51 -0400,<br> > Eli Schwartz wrote:<br> >> On 09/24/2017 06:56 PM, Luke Shumaker wrote:<br> >>> This mirrors dbscripts commit<br> >>> 625fa02 by Pierre Schmitz <<a href="mailto:pierre@archlinux.de" target="_blank">pierre@archlinux.de</a>> at 2017-04-18 14:20:49<br> >>> ---<br> ><br> >>> +. /usr/share/makepkg/util.sh<br> >><br> >> Please just source the actual parts we need, the same way<br> >> <a href="https://git.archlinux.org/dbscripts.git/commit/?id=625fa02" rel="noreferrer" target="_blank">https://git.archlinux.org/dbscripts.git/commit/?id=625fa02</a> also only uses<br> >> /usr/share/makepkg/util/message.sh<br> >> /usr/share/makepkg/util/util.sh<br> >> /usr/share/makepkg/util/pkgbuild.sh<br> ><br> > I was wondering if someone was going to call me on that. I suppose I<br> > should have noted in the commit message that 2 days later, in commit<br> > 76f95dd, Pierre changed it to use /usr/share/makepkg/util.sh instead;<br> > so I mimicked that. AFAICT, that change was unrelated to the rest of<br> > 76f95dd, but it's quite a large patch, and I didn't look at it too<br> > closely.<br> ><br> > <a href="https://git.archlinux.org/dbscripts.git/commit/?id=76f95dd" rel="noreferrer" target="_blank">https://git.archlinux.org/dbscripts.git/commit/?id=76f95dd</a><br> <br> Hah! :p<br> <br> Well, this seems like bloat anyway, maybe I should ask Pierre to change<br> it back...<br></blockquote><div><br></div><div>It seems simpler to me, even if it loads more code than needed. If everyone sources util.sh instead of its components directly, makepkg can shuffle the internals around without breaking consumers. This is also a common pattern seen in C includes.</div><div><br></div><div>I'll apply this patch as-is now. We can still change it afterwards.<br></div><div><br></div><div>Luke, thanks for your continued work on devtools.<br></div></div></div>
diff --git a/lib/common.sh b/lib/common.sh index 0fb93d9..a3c2ec2 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -6,62 +6,21 @@ [[ -z ${_INCLUDE_COMMON_SH:-} ]] || return 0 _INCLUDE_COMMON_SH=true +# shellcheck disable=1091 +. /usr/share/makepkg/util.sh + # Avoid any encoding problems export LANG=C shopt -s extglob # check if messages are to be printed using color -declare ALL_OFF='' BOLD='' BLUE='' GREEN='' RED='' YELLOW='' if [[ -t 2 ]]; then - # prefer terminal safe colored and bold text when tput is supported - if tput setaf 0 &>/dev/null; then - ALL_OFF="$(tput sgr0)" - BOLD="$(tput bold)" - BLUE="${BOLD}$(tput setaf 4)" - GREEN="${BOLD}$(tput setaf 2)" - RED="${BOLD}$(tput setaf 1)" - YELLOW="${BOLD}$(tput setaf 3)" - else - ALL_OFF="\e[1;0m" - BOLD="\e[1;1m" - BLUE="${BOLD}\e[1;34m" - GREEN="${BOLD}\e[1;32m" - RED="${BOLD}\e[1;31m" - YELLOW="${BOLD}\e[1;33m" - fi + colorize +else + # shellcheck disable=2034 + declare -gr ALL_OFF='' BOLD='' BLUE='' GREEN='' RED='' YELLOW='' fi -readonly ALL_OFF BOLD BLUE GREEN RED YELLOW - -plain() { - local mesg=$1; shift - # shellcheck disable=2059 - printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 -} - -msg() { - local mesg=$1; shift - # shellcheck disable=2059 - printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 -} - -msg2() { - local mesg=$1; shift - # shellcheck disable=2059 - printf "${BLUE} ->${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 -} - -warning() { - local mesg=$1; shift - # shellcheck disable=2059 - printf "${YELLOW}==> WARNING:${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 -} - -error() { - local mesg=$1; shift - # shellcheck disable=2059 - printf "${RED}==> ERROR:${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 -} stat_busy() { local mesg=$1; shift @@ -110,51 +69,6 @@ die() { cleanup 255 } -## -# usage : in_array( $needle, $haystack ) -# return : 0 - found -# 1 - not found -## -in_array() { - local needle=$1; shift - local item - for item in "$@"; do - [[ $item = "$needle" ]] && return 0 # Found - done - return 1 # Not Found -} - -## -# usage : get_full_version( [$pkgname] ) -# return : full version spec, including epoch (if necessary), pkgver, pkgrel -## -get_full_version() { - # set defaults if they weren't specified in buildfile - local pkgbase=${pkgbase:-${pkgname[0]}} - local epoch=${epoch:-0} - local pkgver=${pkgver} - local pkgrel=${pkgrel} - if [[ -z $1 ]]; then - if (( ! epoch )); then - printf '%s\n' "$pkgver-$pkgrel" - else - printf '%s\n' "$epoch:$pkgver-$pkgrel" - fi - else - local pkgver_override='' pkgrel_override='' epoch_override='' - for i in pkgver pkgrel epoch; do - local indirect="${i}_override" - eval "$(declare -f "package_$1" | sed -n "s/\(^[[:space:]]*$i=\)/${i}_override=/p")" - [[ -z ${!indirect} ]] && eval ${indirect}=\"${!i}\" - done - if (( ! epoch_override )); then - printf '%s\n' "$pkgver_override-$pkgrel_override" - else - printf '%s\n' "$epoch_override:$pkgver_override-$pkgrel_override" - fi - fi -} - ## # usage : lock( $fd, $file, $message, [ $message_arguments... ] ) ##