FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 05-12-2008, 02:35 PM
Allan McRae
 
Default makepkg - add check for valid options in PKGBUILD

This patch removes the code block in makepkg that checked for
depreciated options in a PKGBUILD provided a workaround.
Unknown and depreciated options are upgraded to error conditions.

Also, removed TODO regarding including install script if exists and
$install is unset. That should never happen.

Signed-off-by: Allan McRae <mcrae_allan@hotmail.com>
---
scripts/makepkg.sh.in | 38 ++++++++++++++++++--------------------
1 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 53d7f98..690ab9c 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -182,25 +182,6 @@ check_option() {
return
fi

- # BEGIN DEPRECATED
- # TODO: This code should be removed in the next release of makepkg.
- local needle=$(echo $1 | tr [:upper:] [:lower:])
- local opt
- for opt in ${options[@]}; do
- opt=$(echo $opt | tr [:upper:] [:lower:])
- if [ "$opt" = "no$needle" ]; then
- warning "$(gettext "Options beginning with 'no' will be deprecated in the next version of makepkg!")"
- plain "$(gettext "Please replace 'no' with '!': %s -> %s.")" "no$needle" "!$needle"
- echo 'n' # Disabled
- return
- elif [ "$opt" = "keepdocs" -a "$needle" = "docs" ]; then
- warning "$(gettext "Option 'keepdocs' may not work as intended. Please replace with 'docs'.")"
- echo 'y' # Enabled
- return
- fi
- done
- # END DEPRECATED
-
# fall back to makepkg.conf options
ret=$(in_opt_array "$1" ${OPTIONS[@]})
if [ "$ret" != '?' ]; then
@@ -870,7 +851,6 @@ create_package() {
local comp_files=".PKGINFO"

# check for an install script
- # TODO: should we include ${pkgname}.install if it exists and $install is unset?
if [ "$install" != "" ]; then
msg2 "$(gettext "Adding install script...")"
cp "$startdir/$install" .INSTALL
@@ -1369,6 +1349,24 @@ if [ "$install" -a ! -f "$install" ]; then
exit 1
fi

+known_options=('strip' 'docs' 'libtool' 'emptydirs' 'ccache' 'distcc' 'makeflags' 'force')
+valid_options=0
+for opt in ${options[@]}; do
+ known=1
+ for kopt in ${known_options[@]}; do
+ if [ "${opt}" = "${kopt}" -o "${opt}" = "!${kopt}" ]; then
+ known=0
+ fi
+ done
+ if [ $known -eq 1 ]; then
+ error "$(gettext "Unknown option '%s'")" "$opt"
+ valid_options=1
+ fi
+done
+if [ $valid_options -eq 1 ]; then
+ exit 1
+fi
+
# We need to run devel_update regardless of whether we are in the fakeroot
# build process so that if the user runs makepkg --forcever manually, we
# 1) output the correct pkgver, and 2) use the correct filename when
--
1.5.5.1


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-12-2008, 02:50 PM
Xavier
 
Default makepkg - add check for valid options in PKGBUILD

On Mon, May 12, 2008 at 4:35 PM, Allan McRae <mcrae_allan@hotmail.com> wrote:
>
> Unknown and depreciated options are upgraded to error conditions.
>
> +known_options=('strip' 'docs' 'libtool' 'emptydirs' 'ccache' 'distcc' 'makeflags' 'force')
> +valid_options=0
> +for opt in ${options[@]}; do
> + known=1
> + for kopt in ${known_options[@]}; do
> + if [ "${opt}" = "${kopt}" -o "${opt}" = "!${kopt}" ]; then
> + known=0
> + fi
> + done
> + if [ $known -eq 1 ]; then
> + error "$(gettext "Unknown option '%s'")" "$opt"
> + valid_options=1
> + fi
> +done
> +if [ $valid_options -eq 1 ]; then
> + exit 1
> +fi
> +

I see how this can be useful. I still find it a bit disappointing to
have to maintain a list of valid options but I don't know..
Btw, if this is for master, you forgot that one:
http://projects.archlinux.org/?p=pacman.git;a=commitdiff;h=dae3f9deefdb86f726a68 dc89a7391e9df7517df

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-13-2008, 01:51 AM
Allan McRae
 
Default makepkg - add check for valid options in PKGBUILD

This patch removes the code block in makepkg that checked for
depreciated options in a PKGBUILD and provided a workaround.
Unknown and depreciated options are upgraded to error conditions.

Also, removed TODO regarding including install script if exists and
$install is unset. That should never happen.

Signed-off-by: Allan McRae <mcrae_allan@hotmail.com>
---
scripts/makepkg.sh.in | 38 ++++++++++++++++++--------------------
1 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 53d7f98..c81282f 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -182,25 +182,6 @@ check_option() {
return
fi

- # BEGIN DEPRECATED
- # TODO: This code should be removed in the next release of makepkg.
- local needle=$(echo $1 | tr [:upper:] [:lower:])
- local opt
- for opt in ${options[@]}; do
- opt=$(echo $opt | tr [:upper:] [:lower:])
- if [ "$opt" = "no$needle" ]; then
- warning "$(gettext "Options beginning with 'no' will be deprecated in the next version of makepkg!")"
- plain "$(gettext "Please replace 'no' with '!': %s -> %s.")" "no$needle" "!$needle"
- echo 'n' # Disabled
- return
- elif [ "$opt" = "keepdocs" -a "$needle" = "docs" ]; then
- warning "$(gettext "Option 'keepdocs' may not work as intended. Please replace with 'docs'.")"
- echo 'y' # Enabled
- return
- fi
- done
- # END DEPRECATED
-
# fall back to makepkg.conf options
ret=$(in_opt_array "$1" ${OPTIONS[@]})
if [ "$ret" != '?' ]; then
@@ -870,7 +851,6 @@ create_package() {
local comp_files=".PKGINFO"

# check for an install script
- # TODO: should we include ${pkgname}.install if it exists and $install is unset?
if [ "$install" != "" ]; then
msg2 "$(gettext "Adding install script...")"
cp "$startdir/$install" .INSTALL
@@ -1369,6 +1349,24 @@ if [ "$install" -a ! -f "$install" ]; then
exit 1
fi

+known_options=('strip' 'docs' 'libtool' 'emptydirs' 'zipman' 'ccache' 'distcc' 'makeflags' 'force')
+valid_options=0
+for opt in ${options[@]}; do
+ known=1
+ for kopt in ${known_options[@]}; do
+ if [ "${opt}" = "${kopt}" -o "${opt}" = "!${kopt}" ]; then
+ known=0
+ fi
+ done
+ if [ $known -eq 1 ]; then
+ error "$(gettext "Unknown option '%s'")" "$opt"
+ valid_options=1
+ fi
+done
+if [ $valid_options -eq 1 ]; then
+ exit 1
+fi
+
# We need to run devel_update regardless of whether we are in the fakeroot
# build process so that if the user runs makepkg --forcever manually, we
# 1) output the correct pkgver, and 2) use the correct filename when
--
1.5.5.1


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-13-2008, 02:40 AM
"Dan McGee"
 
Default makepkg - add check for valid options in PKGBUILD

On Mon, May 12, 2008 at 8:51 PM, Allan McRae <mcrae_allan@hotmail.com> wrote:
> This patch removes the code block in makepkg that checked for
> depreciated options in a PKGBUILD and provided a workaround.
>
> Unknown and depreciated options are upgraded to error conditions.
>
>
> Also, removed TODO regarding including install script if exists and
> $install is unset. That should never happen.
>
> Signed-off-by: Allan McRae <mcrae_allan@hotmail.com>
> ---
> scripts/makepkg.sh.in | 38 ++++++++++++++++++--------------------
> 1 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 53d7f98..c81282f 100644
>
>
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -182,25 +182,6 @@ check_option() {
> return
> fi
>
> - # BEGIN DEPRECATED
> - # TODO: This code should be removed in the next release of makepkg.
> - local needle=$(echo $1 | tr [:upper:] [:lower:])
> - local opt
> - for opt in ${options[@]}; do
> - opt=$(echo $opt | tr [:upper:] [:lower:])
> - if [ "$opt" = "no$needle" ]; then
> - warning "$(gettext "Options beginning with 'no' will be deprecated in the next version of makepkg!")"
> - plain "$(gettext "Please replace 'no' with '!': %s -> %s.")" "no$needle" "!$needle"
> - echo 'n' # Disabled
> - return
> - elif [ "$opt" = "keepdocs" -a "$needle" = "docs" ]; then
> - warning "$(gettext "Option 'keepdocs' may not work as intended. Please replace with 'docs'.")"
> - echo 'y' # Enabled
> - return
> - fi
> - done
> - # END DEPRECATED
> -
> # fall back to makepkg.conf options
> ret=$(in_opt_array "$1" ${OPTIONS[@]})
> if [ "$ret" != '?' ]; then
> @@ -870,7 +851,6 @@ create_package() {
> local comp_files=".PKGINFO"
>
> # check for an install script
> - # TODO: should we include ${pkgname}.install if it exists and $install is unset?
> if [ "$install" != "" ]; then
> msg2 "$(gettext "Adding install script...")"
> cp "$startdir/$install" .INSTALL
> @@ -1369,6 +1349,24 @@ if [ "$install" -a ! -f "$install" ]; then
> exit 1
> fi
>
> +known_options=('strip' 'docs' 'libtool' 'emptydirs' 'zipman' 'ccache' 'distcc' 'makeflags' 'force')
Perhaps we should throw this somewhere at the top of the script, since
it is a constant? I'm thinking somewhere right after pkgdir & srcdir.
In additoin, we can probably mark with either readonly or readonly -a
as we don't want any scripts mucking with it.

> +valid_options=0
> +for opt in ${options[@]}; do
> + known=1
> + for kopt in ${known_options[@]}; do
I missed the ! in the line below here for a second so I wondered why
you were doing double the work.
Maybe add a comment: # check if option matches a known option or its inverse
> + if [ "${opt}" = "${kopt}" -o "${opt}" = "!${kopt}" ]; then
> + known=0
> + fi
> + done
> + if [ $known -eq 1 ]; then
> + error "$(gettext "Unknown option '%s'")" "$opt"
Just so it is clear it is coming from the options array, and to match
the other error messages (where they state the field name first),
maybe this?
options array contains unknown option '%s'
> + valid_options=1
> + fi
> +done
> +if [ $valid_options -eq 1 ]; then
> + exit 1
> +fi
> +
Add an unset for valid_options, opt, known, and kopt here once we are
done with them- it helps keep our variables as local as possible.
unset valid_options opt known kopt

Otherwise this looks great, thanks for actually reading the TODO
markers in makepkg.

-Dan

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-13-2008, 10:26 AM
Allan McRae
 
Default makepkg - add check for valid options in PKGBUILD

This patch removes the code block in makepkg that checked for
depreciated options in a PKGBUILD and provided a workaround.
Unknown and depreciated options are upgraded to error conditions.

Also, removed TODO regarding including install script if exists and
$install is unset. That should never happen.

Signed-off-by: Allan McRae <mcrae_allan@hotmail.com>
---
scripts/makepkg.sh.in | 41 +++++++++++++++++++++--------------------
1 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 53d7f98..72d740f 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -38,6 +38,8 @@ confdir='@sysconfdir@'
startdir="$PWD"
srcdir="$startdir/src"
pkgdir="$startdir/pkg"
+known_options=('strip' 'docs' 'libtool' 'emptydirs' 'zipman' 'ccache' 'distcc' 'makeflags' 'force')
+readonly -a known_options

# Options
ASROOT=0
@@ -182,25 +184,6 @@ check_option() {
return
fi

- # BEGIN DEPRECATED
- # TODO: This code should be removed in the next release of makepkg.
- local needle=$(echo $1 | tr [:upper:] [:lower:])
- local opt
- for opt in ${options[@]}; do
- opt=$(echo $opt | tr [:upper:] [:lower:])
- if [ "$opt" = "no$needle" ]; then
- warning "$(gettext "Options beginning with 'no' will be deprecated in the next version of makepkg!")"
- plain "$(gettext "Please replace 'no' with '!': %s -> %s.")" "no$needle" "!$needle"
- echo 'n' # Disabled
- return
- elif [ "$opt" = "keepdocs" -a "$needle" = "docs" ]; then
- warning "$(gettext "Option 'keepdocs' may not work as intended. Please replace with 'docs'.")"
- echo 'y' # Enabled
- return
- fi
- done
- # END DEPRECATED
-
# fall back to makepkg.conf options
ret=$(in_opt_array "$1" ${OPTIONS[@]})
if [ "$ret" != '?' ]; then
@@ -870,7 +853,6 @@ create_package() {
local comp_files=".PKGINFO"

# check for an install script
- # TODO: should we include ${pkgname}.install if it exists and $install is unset?
if [ "$install" != "" ]; then
msg2 "$(gettext "Adding install script...")"
cp "$startdir/$install" .INSTALL
@@ -1369,6 +1351,25 @@ if [ "$install" -a ! -f "$install" ]; then
exit 1
fi

+valid_options=0
+for opt in ${options[@]}; do
+ known=1
+ # check if option matches a known option or its inverse
+ for kopt in ${known_options[@]}; do
+ if [ "${opt}" = "${kopt}" -o "${opt}" = "!${kopt}" ]; then
+ known=0
+ fi
+ done
+ if [ $known -eq 1 ]; then
+ error "$(gettext "options array contains unknown option '%s'")" "$opt"
+ valid_options=1
+ fi
+done
+if [ $valid_options -eq 1 ]; then
+ exit 1
+fi
+unset valid_options opt known kopt
+
# We need to run devel_update regardless of whether we are in the fakeroot
# build process so that if the user runs makepkg --forcever manually, we
# 1) output the correct pkgver, and 2) use the correct filename when
--
1.5.5.1


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-20-2008, 10:53 PM
Xavier
 
Default makepkg - add check for valid options in PKGBUILD

On Tue, May 13, 2008 at 12:26 PM, Allan McRae <mcrae_allan@hotmail.com> wrote:
> +valid_options=0
> +for opt in ${options[@]}; do
> + known=1
> + # check if option matches a known option or its inverse
> + for kopt in ${known_options[@]}; do
> + if [ "${opt}" = "${kopt}" -o "${opt}" = "!${kopt}" ]; then
> + known=0
> + fi
> + done
> + if [ $known -eq 1 ]; then
> + error "$(gettext "options array contains unknown option '%s'")" "$opt"
> + valid_options=1
> + fi
> +done
> +if [ $valid_options -eq 1 ]; then
> + exit 1
> +fi

No big deal, but in my mind, the boolean value should be reversed.
known == 1 == true, or known == 0 == false.
So inverting known and valid_options make more sense to me
Other than that, the patch looks fine so I pulled it with that change :
http://shining.toofishes.net/gitweb/gitweb.cgi?p=pacman.git;a=commitdiff;h=824469c2e51 22c4689747736dae944a43c8b84e0

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 

Thread Tools




All times are GMT. The time now is 06:24 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org