[arch-projects] [devtools] [PATCH 0/3] commitpkg: Version control check improvements
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
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 <archlinux@cryptocrack.de> --- 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
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<archlinux@cryptocrack.de> --- 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
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.
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
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.
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 <archlinux@cryptocrack.de> --- 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
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 <archlinux@cryptocrack.de> --- 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
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 <archlinux@cryptocrack.de> --- 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
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 <archlinux@cryptocrack.de> --- 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
participants (3)
-
Allan McRae
-
Dave Reisner
-
Lukas Fleischer