[arch-projects] [devtools] [PATCH 0/3] commitpkg: Version control check improvements

newer
[arch-projects] [devtools] [PATCH...

older
[arch-projects] [signoff] netcfg...

Lukas Fleischer

9 Aug 2011 9 Aug '11

8:53 p.m.

Some refactoring and bug fixes to the version control checks in commitpkg. The last patch is probably the most important one. I forgot to copy the install script from another build location to the local SVN checkout a couple of times in the last few months and commitpkg never warned me about that. Lukas Fleischer (3): commitpkg: Avoid redundant use of grep(1) commitpkg: Merge version control checks commitpkg: Use positive pattern in SVN checks commitpkg | 23 +++++++---------------- 1 files changed, 7 insertions(+), 16 deletions(-) -- 1.7.6

Show replies by date

Lukas Fleischer

9 Aug 9 Aug

8:53 p.m.

New subject: [arch-projects] [devtools] [PATCH 1/3] commitpkg: Avoid redundant use of grep(1)

We already sourced the PKGBUILD, so no need to grep the name of the install script and changelog file. Just use "$install" and "$changelog" instead. Also, use bash patterns instead of using grep(1) to check if a source file contains the string "://". Signed-off-by: Lukas Fleischer --- commitpkg | 19 +++++++------------ 1 files changed, 7 insertions(+), 12 deletions(-) diff --git a/commitpkg b/commitpkg index 9856df1..cfc2727 100755 --- a/commitpkg +++ b/commitpkg @@ -79,21 +79,16 @@ esac # check if all local source files are under version control for s in ${source[@]}; do - echo $s | grep -Fvq '://' && \ - svn status $s | grep -q '^\?' && \ - abort "$s is not under version control" + if [[ $s != *://* ]] && svn status $s | grep -q '^\?'; then + abort "$s is not under version control" + fi done # check if changelog and install files are under version control -for i in 'changelog' 'install'; do - filelist=$(sed -n "s/^[[:space:]]*$i=//p" PKGBUILD) - for file in $filelist; do - # evaluate any bash variables used - eval file=${file} - if svn status ${file} | grep -q '^\?'; then - abort "${file} is not under version control" - fi - done +for file in "$changelog" "$install"; do + if [[ -n ${file} ]] && svn status ${file} | grep -q '^\?'; then + abort "${file} is not under version control" + fi done # see if any limit options were passed, we'll send them to rsync -- 1.7.6

Allan McRae

9:17 p.m.

New subject: [arch-projects] [devtools] [PATCH 1/3] commitpkg: Avoid redundant use of grep(1)

On 10/08/11 06:53, Lukas Fleischer wrote:

...

We already sourced the PKGBUILD, so no need to grep the name of the install script and changelog file. Just use "$install" and "$changelog" instead.

Does this work with split packages each having their own install file?

...

Also, use bash patterns instead of using grep(1) to check if a source file contains the string "://".

Signed-off-by: Lukas Fleischer --- commitpkg | 19 +++++++------------ 1 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/commitpkg b/commitpkg index 9856df1..cfc2727 100755 --- a/commitpkg +++ b/commitpkg @@ -79,21 +79,16 @@ esac

# check if all local source files are under version control for s in ${source[@]}; do - echo $s | grep -Fvq '://'&& \ - svn status $s | grep -q '^\?'&& \ - abort "$s is not under version control" + if [[ $s != *://* ]]&& svn status $s | grep -q '^\?'; then + abort "$s is not under version control" + fi done

# check if changelog and install files are under version control -for i in 'changelog' 'install'; do - filelist=$(sed -n "s/^[[:space:]]*$i=//p" PKGBUILD) - for file in $filelist; do - # evaluate any bash variables used - eval file=${file} - if svn status ${file} | grep -q '^\?'; then - abort "${file} is not under version control" - fi - done +for file in "$changelog" "$install"; do + if [[ -n ${file} ]]&& svn status ${file} | grep -q '^\?'; then + abort "${file} is not under version control" + fi done

# see if any limit options were passed, we'll send them to rsync

Lukas Fleischer

9:36 p.m.

New subject: [arch-projects] [devtools] [PATCH 1/3] commitpkg: Avoid redundant use of grep(1)

On Wed, Aug 10, 2011 at 07:17:52AM +1000, Allan McRae wrote:

...

On 10/08/11 06:53, Lukas Fleischer wrote:

...
We already sourced the PKGBUILD, so no need to grep the name of the install script and changelog file. Just use "$install" and "$changelog" instead.

Does this work with split packages each having their own install file?

Crap. No, of course it doesn't. Still, I don't like the current way of grep'ing the PKGBUILD. While this will probably work in 99% of all cases, it doesn't cover all ways to set the "install" and "changelog" variables. My solution works fine for regular PKGBUILDs but doesn't work for split packages, yeah :/ If you want to keep the workaround, I'll discard this part of this patch and rebase the other ones.

Allan McRae

9:44 p.m.

New subject: [arch-projects] [devtools] [PATCH 1/3] commitpkg: Avoid redundant use of grep(1)

On 10/08/11 07:36, Lukas Fleischer wrote:

...

On Wed, Aug 10, 2011 at 07:17:52AM +1000, Allan McRae wrote:

...
On 10/08/11 06:53, Lukas Fleischer wrote:

...
We already sourced the PKGBUILD, so no need to grep the name of the install script and changelog file. Just use "$install" and "$changelog" instead.

Does this work with split packages each having their own install file?

Crap. No, of course it doesn't. Still, I don't like the current way of grep'ing the PKGBUILD. While this will probably work in 99% of all cases, it doesn't cover all ways to set the "install" and "changelog" variables. My solution works fine for regular PKGBUILDs but doesn't work for split packages, yeah :/

If you want to keep the workaround, I'll discard this part of this patch and rebase the other ones.

You could look at how it is done in makepkg in git currently as that is slightly more robust. And of course, the PKGBUILD has to get passed how makepkg parses it... Allan

Lukas Fleischer

10 Aug 10 Aug

12:53 p.m.

New subject: [arch-projects] [devtools] [PATCH 1/3] commitpkg: Avoid redundant use of grep(1)

On Wed, Aug 10, 2011 at 07:44:39AM +1000, Allan McRae wrote:

...

On 10/08/11 07:36, Lukas Fleischer wrote:

...
On Wed, Aug 10, 2011 at 07:17:52AM +1000, Allan McRae wrote:

...
On 10/08/11 06:53, Lukas Fleischer wrote:

...
We already sourced the PKGBUILD, so no need to grep the name of the install script and changelog file. Just use "$install" and "$changelog" instead.

Does this work with split packages each having their own install file?

Crap. No, of course it doesn't. Still, I don't like the current way of grep'ing the PKGBUILD. While this will probably work in 99% of all cases, it doesn't cover all ways to set the "install" and "changelog" variables. My solution works fine for regular PKGBUILDs but doesn't work for split packages, yeah :/

If you want to keep the workaround, I'll discard this part of this patch and rebase the other ones.

You could look at how it is done in makepkg in git currently as that is slightly more robust. And of course, the PKGBUILD has to get passed how makepkg parses it...

Well, as far as I can see, makepkg(8) currently uses exactly the same method that we currently use in commitpkg, so I'll revert that change, fix up and resubmit my patch set.

Lukas Fleischer

9 Aug 9 Aug

8:53 p.m.

New subject: [arch-projects] [devtools] [PATCH 2/3] commitpkg: Merge version control checks

Local source files, as well as install and changelog files used to have separate version control checks. Move them into a single loop. Signed-off-by: Lukas Fleischer --- commitpkg | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-) diff --git a/commitpkg b/commitpkg index cfc2727..36d018b 100755 --- a/commitpkg +++ b/commitpkg @@ -77,17 +77,11 @@ case "$repo" in echo "Non-standard repository $repo in use, defaulting to server $server" ;; esac -# check if all local source files are under version control -for s in ${source[@]}; do - if [[ $s != *://* ]] && svn status $s | grep -q '^\?'; then - abort "$s is not under version control" - fi -done - -# check if changelog and install files are under version control -for file in "$changelog" "$install"; do - if [[ -n ${file} ]] && svn status ${file} | grep -q '^\?'; then - abort "${file} is not under version control" +# check if all local source files, as well as changelog and install files are +# under version control +for i in ${source[@]} "$changelog" "$install"; do + if [[ -n $i && $i != *://* ]] && svn status $i | grep -q '^\?'; then + abort "$i is not under version control" fi done -- 1.7.6

Dave Reisner

9:03 p.m.

New subject: [arch-projects] [devtools] [PATCH 2/3] commitpkg: Merge version control checks

On Tue, Aug 09, 2011 at 10:53:44PM +0200, Lukas Fleischer wrote:

...

Local source files, as well as install and changelog files used to have separate version control checks. Move them into a single loop.

Signed-off-by: Lukas Fleischer --- commitpkg | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/commitpkg b/commitpkg index cfc2727..36d018b 100755 --- a/commitpkg +++ b/commitpkg @@ -77,17 +77,11 @@ case "$repo" in echo "Non-standard repository $repo in use, defaulting to server $server" ;; esac

-# check if all local source files are under version control -for s in ${source[@]}; do - if [[ $s != *://* ]] && svn status $s | grep -q '^\?'; then - abort "$s is not under version control" - fi -done - -# check if changelog and install files are under version control -for file in "$changelog" "$install"; do - if [[ -n ${file} ]] && svn status ${file} | grep -q '^\?'; then - abort "${file} is not under version control" +# check if all local source files, as well as changelog and install files are +# under version control +for i in ${source[@]} "$changelog" "$install"; do

Proper quoting here please: "${source[@]}"

...

+ if [[ -n $i && $i != *://* ]] && svn status $i | grep -q '^\?'; then

Same here: "$i"

...

+ abort "$i is not under version control" fi done

-- 1.7.6

Lukas Fleischer

9:28 p.m.

New subject: [arch-projects] [devtools] [PATCH 2/3] commitpkg: Merge version control checks

On Tue, Aug 09, 2011 at 05:03:08PM -0400, Dave Reisner wrote:

...

On Tue, Aug 09, 2011 at 10:53:44PM +0200, Lukas Fleischer wrote:

...
Local source files, as well as install and changelog files used to have separate version control checks. Move them into a single loop.

Signed-off-by: Lukas Fleischer --- commitpkg | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/commitpkg b/commitpkg index cfc2727..36d018b 100755 --- a/commitpkg +++ b/commitpkg @@ -77,17 +77,11 @@ case "$repo" in echo "Non-standard repository $repo in use, defaulting to server $server" ;; esac

-# check if all local source files are under version control -for s in ${source[@]}; do - if [[ $s != *://* ]] && svn status $s | grep -q '^\?'; then - abort "$s is not under version control" - fi -done - -# check if changelog and install files are under version control -for file in "$changelog" "$install"; do - if [[ -n ${file} ]] && svn status ${file} | grep -q '^\?'; then - abort "${file} is not under version control" +# check if all local source files, as well as changelog and install files are +# under version control +for i in ${source[@]} "$changelog" "$install"; do

Proper quoting here please: "${source[@]}"

Hm, I just copy-pasted that from our current code. You're obviously right though. Will fix that.

...

...
+ if [[ -n $i && $i != *://* ]] && svn status $i | grep -q '^\?'; then

Same here: "$i"

Same here :)

...

...
+ abort "$i is not under version control" fi done

-- 1.7.6

Lukas Fleischer

8:53 p.m.

New subject: [arch-projects] [devtools] [PATCH 3/3] commitpkg: Use positive pattern in SVN checks

In addition to what we had before, this will also detect: * Non-existent files. * Files that are missing or scheduled for deletion. * Ignored files. Signed-off-by: Lukas Fleischer --- commitpkg | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/commitpkg b/commitpkg index 36d018b..223f078 100755 --- a/commitpkg +++ b/commitpkg @@ -80,8 +80,10 @@ esac # check if all local source files, as well as changelog and install files are # under version control for i in ${source[@]} "$changelog" "$install"; do - if [[ -n $i && $i != *://* ]] && svn status $i | grep -q '^\?'; then - abort "$i is not under version control" + if [[ -n $i && $i != *://* ]]; then + if ! svn status -v $i | grep -q '^[ AMRX~]'; then + abort "$i is not under version control" + fi fi done -- 1.7.6

4649

Age (days ago)

4650

Last active (days ago)

List overview

Download

9 comments

3 participants

participants (3)

Allan McRae
Dave Reisner
Lukas Fleischer

[arch-projects] [devtools] [PATCH 0/3] commitpkg: Version control check improvements

tags

participants (3)