[pacman-dev] [PATCH 1/2] makepkg: less code rep with single redirection

newer
Re: [pacman-dev] [arch-dev-public]...

older
[pacman-dev] [GIT] The official...

Andres P

23 May 2010 23 May '10

11:20 a.m.

Signed-off-by: Andres P --- scripts/makepkg.sh.in | 50 ++++++++++++++++++++++++------------------------ 1 files changed, 25 insertions(+), 25 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 1707245..7f27361 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -904,56 +904,56 @@ write_pkginfo() { size="$(( ${size%%[^0-9]*} * 1024 ))" msg2 "$(gettext "Generating .PKGINFO file...")" - echo "# Generated by makepkg $myver" >.PKGINFO + echo "# Generated by makepkg $myver" if (( INFAKEROOT )); then - echo "# using $(fakeroot -v)" >>.PKGINFO - fi - echo "# $(LC_ALL=C date -u)" >>.PKGINFO - echo "pkgname = $1" >>.PKGINFO - (( SPLITPKG )) && echo pkgbase = $pkgbase >>.PKGINFO - echo "pkgver = $pkgver-$pkgrel" >>.PKGINFO - echo "pkgdesc = $pkgdesc" >>.PKGINFO - echo "url = $url" >>.PKGINFO - echo "builddate = $builddate" >>.PKGINFO - echo "packager = $packager" >>.PKGINFO - echo "size = $size" >>.PKGINFO - echo "arch = $PKGARCH" >>.PKGINFO + echo "# using $(fakeroot -v)" + fi + echo "# $(LC_ALL=C date -u)" + echo "pkgname = $1" + (( SPLITPKG )) && echo pkgbase = $pkgbase + echo "pkgver = $pkgver-$pkgrel" + echo "pkgdesc = $pkgdesc" + echo "url = $url" + echo "builddate = $builddate" + echo "packager = $packager" + echo "size = $size" + echo "arch = $PKGARCH" if [[ $(check_option force) = "y" ]]; then - echo "force = true" >> .PKGINFO + echo "force = true" fi local it for it in "${license[@]}"; do - echo "license = $it" >>.PKGINFO + echo "license = $it" done for it in "${replaces[@]}"; do - echo "replaces = $it" >>.PKGINFO + echo "replaces = $it" done for it in "${groups[@]}"; do - echo "group = $it" >>.PKGINFO + echo "group = $it" done for it in "${depends[@]}"; do - echo "depend = $it" >>.PKGINFO + echo "depend = $it" done for it in "${optdepends[@]}"; do - echo "optdepend = $it" >>.PKGINFO + echo "optdepend = $it" done for it in "${conflicts[@]}"; do - echo "conflict = $it" >>.PKGINFO + echo "conflict = $it" done for it in "${provides[@]}"; do - echo "provides = $it" >>.PKGINFO + echo "provides = $it" done for it in "${backup[@]}"; do - echo "backup = $it" >>.PKGINFO + echo "backup = $it" done for it in "${packaging_options[@]}"; do local ret="$(check_option $it)" if [[ $ret != "?" ]]; then if [[ $ret = y ]]; then - echo "makepkgopt = $it" >>.PKGINFO + echo "makepkgopt = $it" else - echo "makepkgopt = !$it" >>.PKGINFO + echo "makepkgopt = !$it" fi fi done @@ -964,7 +964,7 @@ write_pkginfo() { warning "$(gettext "Please add a license line to your %s!")" "$BUILDSCRIPT" plain "$(gettext "Example for GPL\'ed software: license=('GPL').")" fi -} +} > .PKGINFO check_package() { cd "$pkgdir" -- 1.7.1

Show replies by date

Andres P

23 May 23 May

11:20 a.m.

New subject: [pacman-dev] [PATCH 2/2] makepkg: printf exists

Signed-off-by: Andres P --- Without a function, this would look like: local fmt="= %s\n" [[ $license ]] && printf "license $fmt" "${license[@]}" [[ $replaces ]] && printf "replaces $fmt" "${replaces[@]}" [[ $groups ]] && printf "group $fmt" "${groups[@]}" [[ $depends ]] && printf "depend $fmt" "${depends[@]}" [[ $optdepend ]] && printf "optdepend $fmt" "${optdepends[@]}" [[ $conflict ]] && printf "conflict $fmt" "${conflicts[@]}" [[ $provides ]] && printf "provides $fmt" "${provides[@]}" [[ $backup ]] && printf "backup $fmt" "${backup[@]}" ... scripts/makepkg.sh.in | 38 +++++++++++++------------------------- 1 files changed, 13 insertions(+), 25 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 7f27361..18bbb3f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -893,6 +893,10 @@ tidy_install() { fi } +multiopt() { + [[ $2 ]] && printf "$1 = %s\n" "${@:1}" +} + write_pkginfo() { local builddate=$(date -u "+%s") if [[ -n $PACKAGER ]]; then @@ -922,31 +926,15 @@ write_pkginfo() { echo "force = true" fi - local it - for it in "${license[@]}"; do - echo "license = $it" - done - for it in "${replaces[@]}"; do - echo "replaces = $it" - done - for it in "${groups[@]}"; do - echo "group = $it" - done - for it in "${depends[@]}"; do - echo "depend = $it" - done - for it in "${optdepends[@]}"; do - echo "optdepend = $it" - done - for it in "${conflicts[@]}"; do - echo "conflict = $it" - done - for it in "${provides[@]}"; do - echo "provides = $it" - done - for it in "${backup[@]}"; do - echo "backup = $it" - done + multiopt "license" "${license[@]}" + multiopt "replaces" "${replaces[@]}" + multiopt "group" "${groups[@]}" + multiopt "depend" "${depends[@]}" + multiopt "optdepend" "${optdepends[@]}" + multiopt "conflict" "${conflicts[@]}" + multiopt "provides" "${provides[@]}" + multiopt "backup" "${backup[@]}" + for it in "${packaging_options[@]}"; do local ret="$(check_option $it)" if [[ $ret != "?" ]]; then -- 1.7.1

Allan McRae

12:58 p.m.

New subject: [pacman-dev] [PATCH 2/2] makepkg: printf exists

On 23/05/10 21:20, Andres P wrote:

...

Signed-off-by: Andres P ---

Without a function, this would look like: local fmt="= %s\n" [[ $license ]]&& printf "license $fmt" "${license[@]}" [[ $replaces ]]&& printf "replaces $fmt" "${replaces[@]}" [[ $groups ]]&& printf "group $fmt" "${groups[@]}" [[ $depends ]]&& printf "depend $fmt" "${depends[@]}" [[ $optdepend ]]&& printf "optdepend $fmt" "${optdepends[@]}" [[ $conflict ]]&& printf "conflict $fmt" "${conflicts[@]}" [[ $provides ]]&& printf "provides $fmt" "${provides[@]}" [[ $backup ]]&& printf "backup $fmt" "${backup[@]}"

I'd prefer this version. In fact, removing the $fmt stuff would be better as it is only two characters longer and makes it a lot more readable. Again... commit messages... Allan

Andres P

1:32 p.m.

New subject: [pacman-dev] [PATCH] makepkg: use printf when writing pkginfo

Signed-off-by: Andres P --- scripts/makepkg.sh.in | 34 +++++++++------------------------- 1 files changed, 9 insertions(+), 25 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index fd1b68b..efd3d80 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -947,31 +947,15 @@ write_pkginfo() { echo "force = true" fi - local it - for it in "${license[@]}"; do - echo "license = $it" - done - for it in "${replaces[@]}"; do - echo "replaces = $it" - done - for it in "${groups[@]}"; do - echo "group = $it" - done - for it in "${depends[@]}"; do - echo "depend = $it" - done - for it in "${optdepends[@]}"; do - echo "optdepend = $it" - done - for it in "${conflicts[@]}"; do - echo "conflict = $it" - done - for it in "${provides[@]}"; do - echo "provides = $it" - done - for it in "${backup[@]}"; do - echo "backup = $it" - done + [[ $license ]] && printf "license = %s\n" "${license[@]}" + [[ $replaces ]] && printf "replaces = %s\n" "${replaces[@]}" + [[ $groups ]] && printf "group = %s\n" "${groups[@]}" + [[ $depends ]] && printf "depend = %s\n" "${depends[@]}" + [[ $optdepend ]] && printf "optdepend = %s\n" "${optdepends[@]}" + [[ $conflict ]] && printf "conflict = %s\n" "${conflicts[@]}" + [[ $provides ]] && printf "provides = %s\n" "${provides[@]}" + [[ $backup ]] && printf "backup = %s\n" "${backup[@]}" + for it in "${packaging_options[@]}"; do local ret="$(check_option $it)" if [[ $ret != "?" ]]; then -- 1.7.1

Dan McGee

25 May 25 May

2:22 a.m.

New subject: [pacman-dev] [PATCH] makepkg: use printf when writing pkginfo

On Sun, May 23, 2010 at 8:32 AM, Andres P wrote:

...

Signed-off-by: Andres P --- scripts/makepkg.sh.in | 34 +++++++++------------------------- 1 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index fd1b68b..efd3d80 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -947,31 +947,15 @@ write_pkginfo() { echo "force = true" fi

- local it - for it in "${license[@]}"; do - echo "license = $it" - done - for it in "${replaces[@]}"; do - echo "replaces = $it" - done - for it in "${groups[@]}"; do - echo "group = $it" - done - for it in "${depends[@]}"; do - echo "depend = $it" - done - for it in "${optdepends[@]}"; do - echo "optdepend = $it" - done - for it in "${conflicts[@]}"; do - echo "conflict = $it" - done - for it in "${provides[@]}"; do - echo "provides = $it" - done - for it in "${backup[@]}"; do - echo "backup = $it" - done + [[ $license ]] && printf "license = %s\n" "${license[@]}" + [[ $replaces ]] && printf "replaces = %s\n" "${replaces[@]}" + [[ $groups ]] && printf "group = %s\n" "${groups[@]}" + [[ $depends ]] && printf "depend = %s\n" "${depends[@]}" + [[ $optdepend ]] && printf "optdepend = %s\n" "${optdepends[@]}" + [[ $conflict ]] && printf "conflict = %s\n" "${conflicts[@]}" + [[ $provides ]] && printf "provides = %s\n" "${provides[@]}" + [[ $backup ]] && printf "backup = %s\n" "${backup[@]}" + for it in "${packaging_options[@]}"; do local ret="$(check_option $it)" if [[ $ret != "?" ]]; then --

This looks really cool, but where is this printf behavior documented? I can't find it at all in the bash manpage. Is it portable to different (read: older) versions of bash? -Dan

Allan McRae

3:20 a.m.

New subject: [pacman-dev] [PATCH] makepkg: use printf when writing pkginfo

On 25/05/10 12:22, Dan McGee wrote:

...

On Sun, May 23, 2010 at 8:32 AM, Andres P wrote:

...
Signed-off-by: Andres P --- scripts/makepkg.sh.in | 34 +++++++++------------------------- 1 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index fd1b68b..efd3d80 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -947,31 +947,15 @@ write_pkginfo() { echo "force = true" fi

- local it - for it in "${license[@]}"; do - echo "license = $it" - done - for it in "${replaces[@]}"; do - echo "replaces = $it" - done - for it in "${groups[@]}"; do - echo "group = $it" - done - for it in "${depends[@]}"; do - echo "depend = $it" - done - for it in "${optdepends[@]}"; do - echo "optdepend = $it" - done - for it in "${conflicts[@]}"; do - echo "conflict = $it" - done - for it in "${provides[@]}"; do - echo "provides = $it" - done - for it in "${backup[@]}"; do - echo "backup = $it" - done + [[ $license ]]&& printf "license = %s\n" "${license[@]}" + [[ $replaces ]]&& printf "replaces = %s\n" "${replaces[@]}" + [[ $groups ]]&& printf "group = %s\n" "${groups[@]}" + [[ $depends ]]&& printf "depend = %s\n" "${depends[@]}" + [[ $optdepend ]]&& printf "optdepend = %s\n" "${optdepends[@]}" + [[ $conflict ]]&& printf "conflict = %s\n" "${conflicts[@]}" + [[ $provides ]]&& printf "provides = %s\n" "${provides[@]}" + [[ $backup ]]&& printf "backup = %s\n" "${backup[@]}" + for it in "${packaging_options[@]}"; do local ret="$(check_option $it)" if [[ $ret != "?" ]]; then --

This looks really cool, but where is this printf behavior documented? I can't find it at all in the bash manpage. Is it portable to different (read: older) versions of bash?

I can confirm that this works in bash-3.1.17 which is from 2005. I think that is old enough. Remember that this patch relies on the one removing all the >.PKGINFO redirections when it comes time to commit. Allan

Allan McRae

27 May 27 May

10:58 p.m.

New subject: [pacman-dev] [PATCH] makepkg: use printf when writing pkginfo

On 23/05/10 23:32, Andres P wrote:

...

Signed-off-by: Andres P

Signoff. On my post-3.4 branch. Allan

Allan McRae

23 May 23 May

12:43 p.m.

New subject: [pacman-dev] [PATCH 1/2] makepkg: less code rep with single redirection

On 23/05/10 21:20, Andres P wrote:

...

Signed-off-by: Andres P ---

Please use more informative commit messages. From the commit message I should know exactly what your patch is doing. With a decent commit message this patch gets a +1. Allan

Andres P

27 May 27 May

4:34 p.m.

New subject: [pacman-dev] [PATCH] makepkg: use single redirection when writing PKGINFO

Instead of specifying the output file on every echo, leave it to the caller of write_pkginfo to specify the target. Signed-off-by: Andres P --- This is a resubmit of an old patch No bug fixes on top master (5dffef787df72cc4) On Sun, May 23, 2010 at 8:13 AM, Allan McRae wrote:

...

On 23/05/10 21:20, Andres P wrote:

...
Signed-off-by: Andres P ---

Please use more informative commit messages. From the commit message I should know exactly what your patch is doing.

With a decent commit message this patch gets a +1.

Allan

Besides changing the commit message, I decided that putting the redirection on the function's ending bracket is an unusual construct. In works in lots of shells, even dash, but it has the tendency of throwing people off. ... scripts/makepkg.sh.in | 50 ++++++++++++++++++++++++------------------------ 1 files changed, 25 insertions(+), 25 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 76b6183..e7a080b 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -902,56 +902,56 @@ write_pkginfo() { size="$(( ${size%%[^0-9]*} * 1024 ))" msg2 "$(gettext "Generating .PKGINFO file...")" - echo "# Generated by makepkg $myver" >.PKGINFO + echo "# Generated by makepkg $myver" if (( INFAKEROOT )); then - echo "# using $(fakeroot -v)" >>.PKGINFO - fi - echo "# $(LC_ALL=C date -u)" >>.PKGINFO - echo "pkgname = $1" >>.PKGINFO - (( SPLITPKG )) && echo pkgbase = $pkgbase >>.PKGINFO - echo "pkgver = $pkgver-$pkgrel" >>.PKGINFO - echo "pkgdesc = $pkgdesc" >>.PKGINFO - echo "url = $url" >>.PKGINFO - echo "builddate = $builddate" >>.PKGINFO - echo "packager = $packager" >>.PKGINFO - echo "size = $size" >>.PKGINFO - echo "arch = $PKGARCH" >>.PKGINFO + echo "# using $(fakeroot -v)" + fi + echo "# $(LC_ALL=C date -u)" + echo "pkgname = $1" + (( SPLITPKG )) && echo pkgbase = $pkgbase + echo "pkgver = $pkgver-$pkgrel" + echo "pkgdesc = $pkgdesc" + echo "url = $url" + echo "builddate = $builddate" + echo "packager = $packager" + echo "size = $size" + echo "arch = $PKGARCH" if [[ $(check_option force) = "y" ]]; then - echo "force = true" >> .PKGINFO + echo "force = true" fi local it for it in "${license[@]}"; do - echo "license = $it" >>.PKGINFO + echo "license = $it" done for it in "${replaces[@]}"; do - echo "replaces = $it" >>.PKGINFO + echo "replaces = $it" done for it in "${groups[@]}"; do - echo "group = $it" >>.PKGINFO + echo "group = $it" done for it in "${depends[@]}"; do - echo "depend = $it" >>.PKGINFO + echo "depend = $it" done for it in "${optdepends[@]}"; do - echo "optdepend = $it" >>.PKGINFO + echo "optdepend = $it" done for it in "${conflicts[@]}"; do - echo "conflict = $it" >>.PKGINFO + echo "conflict = $it" done for it in "${provides[@]}"; do - echo "provides = $it" >>.PKGINFO + echo "provides = $it" done for it in "${backup[@]}"; do - echo "backup = $it" >>.PKGINFO + echo "backup = $it" done for it in "${packaging_options[@]}"; do local ret="$(check_option $it)" if [[ $ret != "?" ]]; then if [[ $ret = y ]]; then - echo "makepkgopt = $it" >>.PKGINFO + echo "makepkgopt = $it" else - echo "makepkgopt = !$it" >>.PKGINFO + echo "makepkgopt = !$it" fi fi done @@ -1005,7 +1005,7 @@ create_package() { PKGARCH=$CARCH fi - write_pkginfo $nameofpkg + write_pkginfo $nameofpkg > .PKGINFO local comp_files=".PKGINFO" -- 1.7.1

Allan McRae

10:58 p.m.

New subject: [pacman-dev] [PATCH] makepkg: use single redirection when writing PKGINFO

On 28/05/10 02:34, Andres P wrote:

...

Instead of specifying the output file on every echo, leave it to the caller of write_pkginfo to specify the target.

Signed-off-by: Andres P ---

This is a resubmit of an old patch

No bug fixes on top master (5dffef787df72cc4)

On Sun, May 23, 2010 at 8:13 AM, Allan McRae wrote:

...
On 23/05/10 21:20, Andres P wrote:

...
Signed-off-by: Andres P ---

Please use more informative commit messages. Â From the commit message I should know exactly what your patch is doing.

With a decent commit message this patch gets a +1.

Allan

Besides changing the commit message, I decided that putting the redirection on the function's ending bracket is an unusual construct.

In works in lots of shells, even dash, but it has the tendency of throwing people off.

Good. I prefer this way. Signoff. Pushed to my post-3.4 branch Allan

5083

Age (days ago)

5087

Last active (days ago)

List overview

Download

9 comments

3 participants

participants (3)

Allan McRae
Andres P
Dan McGee

[pacman-dev] [PATCH 1/2] makepkg: less code rep with single redirection

tags

participants (3)