reposync: Fix quoting, mention default destination, other nits

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

reposync: Fix quoting, mention default destination, other nits

Klemens Nanni-2
$destdir holds user input so quote it;  since ksh(1) not sh(1) is used,
use [[ instead of [ consistently to disable word splitting.

Mention /cvs in the manual being the default destination, move -p
without argument before -l and sync script usage with manual synopsis.

Delimit rm(1) options from paths just to be sure.

Feedback? OK?

While here, files/reposync has nothing to SUBSTitute, so use
INSTALL_SCRIPT in do-install directly.

If too much, I can also split up the diff/commit separately.


Index: Makefile
===================================================================
RCS file: /cvs/ports/sysutils/reposync/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile 10 Nov 2019 13:53:38 -0000 1.8
+++ Makefile 14 Nov 2019 11:39:13 -0000
@@ -2,7 +2,7 @@
 
 COMMENT= script to update an OpenBSD CVS repository via rsync
 
-PKGNAME= reposync-0.9
+PKGNAME= reposync-0.10
 
 CATEGORIES= sysutils
 DISTFILES=
@@ -16,11 +16,8 @@ NO_TEST= Yes
 
 RUN_DEPENDS= net/rsync
 
-do-extract:
- cp ${FILESDIR}/reposync* ${WRKDIR}/
-
 do-install:
- ${SUBST_PROGRAM} ${WRKSRC}/reposync ${PREFIX}/bin/reposync
- ${INSTALL_MAN} ${WRKSRC}/reposync.1 ${PREFIX}/man/man1/reposync.1
+ ${INSTALL_SCRIPT} ${FILESDIR}/reposync ${PREFIX}/bin/
+ ${INSTALL_MAN} ${FILESDIR}/reposync.1 ${PREFIX}/man/man1/
 
 .include <bsd.port.mk>
Index: files/reposync
===================================================================
RCS file: /cvs/ports/sysutils/reposync/files/reposync,v
retrieving revision 1.7
diff -u -p -r1.7 reposync
--- files/reposync 10 Nov 2019 13:53:39 -0000 1.7
+++ files/reposync 14 Nov 2019 11:38:27 -0000
@@ -24,7 +24,7 @@ err()
 
 usage()
 {
- echo "usage: ${0##*/} [-f] [-p | -l username] rsync://upstream/cvs [/path/to/cvs]" >&2
+ echo "usage: ${0##*/} [-f] [-p | -l username] rsync://upstream/path [destination]" >&2
  exit 1
 }
 
@@ -42,8 +42,7 @@ shift $((OPTIND-1))
 [ $# -gt 0 ] || usage
 
 synchost="$1"
-repodir=/cvs
-[[ -n $2 ]] && repodir=$2
+repodir=${2:-/cvs}
 
 run_rsync()
 {
@@ -57,8 +56,8 @@ run_rsync()
 
 rundir="/var/db/reposync"
 
-for i in $rundir $repodir; do
- [ ! -d $i ] || [ ! -w $i ] &&
+for i in "$rundir" "$repodir"; do
+ [[ ! -d $i ]] || [[ ! -w $i ]] &&
     err "$i must exist as a writable directory"
 done
 
@@ -71,7 +70,7 @@ hashfile="$rundir/reposync.hash"
 lockfile="$rundir/reposync.lock"
 cd $rundir
 
-if [ -h $lockfile ]; then
+if [[ -h $lockfile ]]; then
  # read the pid from $lockfile symlink target
  lockedpid=$(stat -f %Y $lockfile)
 
@@ -81,12 +80,12 @@ if [ -h $lockfile ]; then
  fi
 
  # not still running, the lock must be stale (machine panicked, etc) so zap it
- rm -f $lockfile
+ rm -f -- $lockfile
 fi
 
 ln -s $$ $lockfile || err "could not lock $lockfile"
 
-trap "rm -f $lockfile" 0 1 2 15
+trap "rm -f -- $lockfile" 0 1 2 15
 
 # check CVSROOT directory listing to identify updates; primarily for
 # ChangeLog but val-tags may also be updated after a checkout was done
@@ -98,12 +97,12 @@ _t="$(run_rsync ${synchost}/CVSROOT/ | g
 esac
 newhash="${synchost} $(echo $_t | sha256)"
 
-if [ -r $hashfile ]; then
+if [[ -r $hashfile ]]; then
  age=$((`date +%s` - `stat -t %s -f %m $hashfile`))
  # don't entirely rely on CVSROOT files; not all tree operations
  # result in a change there so also do a full update run at least
  # every 6h.
- if [ $age -lt $((6*60*60)) ]; then
+ if ((age < 6*60*60)); then
  oldhash=`cat $hashfile`
  fi
 fi
@@ -113,7 +112,7 @@ if $force || [[ $oldhash != $newhash ]];
  # the old one so sync is reattempted next run
  if run_rsync -rlptiz --omit-dir-times --delete \
     --exclude='#cvs.rfl.*' --exclude='CVSROOT/history*' \
-    ${synchost}/{CVSROOT,www,xenocara,ports,src} $repodir/; then
+    ${synchost}/{CVSROOT,www,xenocara,ports,src} "$repodir"/; then
  echo $newhash > $hashfile
  else
  err "rsync failed"
Index: files/reposync.1
===================================================================
RCS file: /cvs/ports/sysutils/reposync/files/reposync.1,v
retrieving revision 1.3
diff -u -p -r1.3 reposync.1
--- files/reposync.1 10 Nov 2019 13:36:32 -0000 1.3
+++ files/reposync.1 14 Nov 2019 11:38:18 -0000
@@ -23,7 +23,7 @@
 .Sh SYNOPSIS
 .Nm
 .Op Fl f
-.Op Fl l Ar username | Fl p
+.Op Fl p | Fl l Ar username
 .Ar rsync://upstream/path
 .Op Ar destination
 .Sh DESCRIPTION
@@ -40,6 +40,12 @@ If only a short time has elapsed since t
 directory on the upstream server is checked for changes and the update
 is skipped if none are detected.
 .Pp
+If
+.Ar destination
+is omitted,
+.Pa /cvs
+will be used.
+.Pp
 The directory
 .Pa /var/db/reposync
 must exist and be writable by the user running
@@ -71,6 +77,6 @@ update the local directory
 .Pa /home/cvs .
 .El
 .Sh SEE ALSO
-.Xr ssh 1 ,
+.Xr cvs 1 ,
 .Xr rsync 1 ,
-.Xr cvs 1 .
+.Xr ssh 1

Reply | Threaded
Open this post in threaded view
|

Re: reposync: Fix quoting, mention default destination, other nits

Klemens Nanni-2
On Thu, Nov 14, 2019 at 12:50:20PM +0100, Klemens Nanni wrote:

> $destdir holds user input so quote it;  since ksh(1) not sh(1) is used,
> use [[ instead of [ consistently to disable word splitting.
>
> Mention /cvs in the manual being the default destination, move -p
> without argument before -l and sync script usage with manual synopsis.
>
> Delimit rm(1) options from paths just to be sure.
>
> Feedback? OK?
>
> While here, files/reposync has nothing to SUBSTitute, so use
> INSTALL_SCRIPT in do-install directly.
>
> If too much, I can also split up the diff/commit separately.
New diff after espie's REVISION bump.


Index: Makefile
===================================================================
RCS file: /cvs/ports/sysutils/reposync/Makefile,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile
--- Makefile 14 Nov 2019 13:21:42 -0000 1.9
+++ Makefile 14 Nov 2019 19:20:59 -0000
@@ -2,8 +2,7 @@
 
 COMMENT= script to update an OpenBSD CVS repository via rsync
 
-PKGNAME= reposync-0.9
-REVISION = 0
+PKGNAME= reposync-0.10
 
 CATEGORIES= sysutils
 DISTFILES=
@@ -18,11 +17,8 @@ NO_TEST= Yes
 
 RUN_DEPENDS= net/rsync
 
-do-extract:
- cp ${FILESDIR}/reposync* ${WRKDIR}/
-
 do-install:
- ${SUBST_PROGRAM} ${WRKSRC}/reposync ${PREFIX}/bin/reposync
- ${INSTALL_MAN} ${WRKSRC}/reposync.1 ${PREFIX}/man/man1/reposync.1
+ ${INSTALL_SCRIPT} ${FILESDIR}/reposync ${PREFIX}/bin/
+ ${INSTALL_MAN} ${FILESDIR}/reposync.1 ${PREFIX}/man/man1/
 
 .include <bsd.port.mk>
Index: files/reposync
===================================================================
RCS file: /cvs/ports/sysutils/reposync/files/reposync,v
retrieving revision 1.7
diff -u -p -r1.7 reposync
--- files/reposync 10 Nov 2019 13:53:39 -0000 1.7
+++ files/reposync 14 Nov 2019 11:38:27 -0000
@@ -24,7 +24,7 @@ err()
 
 usage()
 {
- echo "usage: ${0##*/} [-f] [-p | -l username] rsync://upstream/cvs [/path/to/cvs]" >&2
+ echo "usage: ${0##*/} [-f] [-p | -l username] rsync://upstream/path [destination]" >&2
  exit 1
 }
 
@@ -42,8 +42,7 @@ shift $((OPTIND-1))
 [ $# -gt 0 ] || usage
 
 synchost="$1"
-repodir=/cvs
-[[ -n $2 ]] && repodir=$2
+repodir=${2:-/cvs}
 
 run_rsync()
 {
@@ -57,8 +56,8 @@ run_rsync()
 
 rundir="/var/db/reposync"
 
-for i in $rundir $repodir; do
- [ ! -d $i ] || [ ! -w $i ] &&
+for i in "$rundir" "$repodir"; do
+ [[ ! -d $i ]] || [[ ! -w $i ]] &&
     err "$i must exist as a writable directory"
 done
 
@@ -71,7 +70,7 @@ hashfile="$rundir/reposync.hash"
 lockfile="$rundir/reposync.lock"
 cd $rundir
 
-if [ -h $lockfile ]; then
+if [[ -h $lockfile ]]; then
  # read the pid from $lockfile symlink target
  lockedpid=$(stat -f %Y $lockfile)
 
@@ -81,12 +80,12 @@ if [ -h $lockfile ]; then
  fi
 
  # not still running, the lock must be stale (machine panicked, etc) so zap it
- rm -f $lockfile
+ rm -f -- $lockfile
 fi
 
 ln -s $$ $lockfile || err "could not lock $lockfile"
 
-trap "rm -f $lockfile" 0 1 2 15
+trap "rm -f -- $lockfile" 0 1 2 15
 
 # check CVSROOT directory listing to identify updates; primarily for
 # ChangeLog but val-tags may also be updated after a checkout was done
@@ -98,12 +97,12 @@ _t="$(run_rsync ${synchost}/CVSROOT/ | g
 esac
 newhash="${synchost} $(echo $_t | sha256)"
 
-if [ -r $hashfile ]; then
+if [[ -r $hashfile ]]; then
  age=$((`date +%s` - `stat -t %s -f %m $hashfile`))
  # don't entirely rely on CVSROOT files; not all tree operations
  # result in a change there so also do a full update run at least
  # every 6h.
- if [ $age -lt $((6*60*60)) ]; then
+ if ((age < 6*60*60)); then
  oldhash=`cat $hashfile`
  fi
 fi
@@ -113,7 +112,7 @@ if $force || [[ $oldhash != $newhash ]];
  # the old one so sync is reattempted next run
  if run_rsync -rlptiz --omit-dir-times --delete \
     --exclude='#cvs.rfl.*' --exclude='CVSROOT/history*' \
-    ${synchost}/{CVSROOT,www,xenocara,ports,src} $repodir/; then
+    ${synchost}/{CVSROOT,www,xenocara,ports,src} "$repodir"/; then
  echo $newhash > $hashfile
  else
  err "rsync failed"
Index: files/reposync.1
===================================================================
RCS file: /cvs/ports/sysutils/reposync/files/reposync.1,v
retrieving revision 1.3
diff -u -p -r1.3 reposync.1
--- files/reposync.1 10 Nov 2019 13:36:32 -0000 1.3
+++ files/reposync.1 14 Nov 2019 11:38:18 -0000
@@ -23,7 +23,7 @@
 .Sh SYNOPSIS
 .Nm
 .Op Fl f
-.Op Fl l Ar username | Fl p
+.Op Fl p | Fl l Ar username
 .Ar rsync://upstream/path
 .Op Ar destination
 .Sh DESCRIPTION
@@ -40,6 +40,12 @@ If only a short time has elapsed since t
 directory on the upstream server is checked for changes and the update
 is skipped if none are detected.
 .Pp
+If
+.Ar destination
+is omitted,
+.Pa /cvs
+will be used.
+.Pp
 The directory
 .Pa /var/db/reposync
 must exist and be writable by the user running
@@ -71,6 +77,6 @@ update the local directory
 .Pa /home/cvs .
 .El
 .Sh SEE ALSO
-.Xr ssh 1 ,
+.Xr cvs 1 ,
 .Xr rsync 1 ,
-.Xr cvs 1 .
+.Xr ssh 1

Reply | Threaded
Open this post in threaded view
|

Re: reposync: Fix quoting, mention default destination, other nits

Christian Weisgerber
In reply to this post by Klemens Nanni-2
On 2019-11-14, Klemens Nanni <[hidden email]> wrote:

> Feedback? OK?

Here's a bunch more script tweaks.  This is somewhat style/personal
preference territory.  --exclude can be used with listing mode.

--- reposync.orig Thu Nov 14 21:24:57 2019
+++ reposync Thu Nov 14 21:32:45 2019
@@ -31,30 +31,30 @@
 force=false
 fwduser=anoncvs
 while getopts "fl:p" c; do
- case "$c" in
+ case $c in
  f) force=true ;;
- l) fwduser="$OPTARG" ;;
- p) fwduser="" ;;
+ l) fwduser=$OPTARG ;;
+ p) fwduser= ;;
  *) usage ;;
  esac
 done
 shift $((OPTIND-1))
-[ $# -gt 0 ] || usage
+(( $# > 0 )) || usage
 
-synchost="$1"
+synchost=$1
 repodir=${2:-/cvs}
 
 run_rsync()
 {
  if [[ -n $fwduser ]]; then
  # reach rsync on the server via an ssh port-forward
- rsync -e "ssh -W localhost:rsync -l $fwduser" $* 2>&1
+ rsync -e "ssh -W localhost:rsync -l $fwduser" "$@" 2>&1
  else
- rsync $* 2>&1
+ rsync "$@" 2>&1
  fi
 }
 
-rundir="/var/db/reposync"
+rundir=/var/db/reposync
 
 for i in "$rundir" "$repodir"; do
  [[ ! -d $i ]] || [[ ! -w $i ]] &&
@@ -65,9 +65,9 @@
  err "should be run by the uid owning the repository"
 fi
 
-oldhash="invalid"
-hashfile="$rundir/reposync.hash"
-lockfile="$rundir/reposync.lock"
+oldhash=invalid
+hashfile=$rundir/reposync.hash
+lockfile=$rundir/reposync.lock
 cd $rundir
 
 if [[ -h $lockfile ]]; then
@@ -90,7 +90,7 @@
 # check CVSROOT directory listing to identify updates; primarily for
 # ChangeLog but val-tags may also be updated after a checkout was done
 # using a new tag. ignore "history" (lists read-only operations).
-_t="$(run_rsync ${synchost}/CVSROOT/ | grep -v history)"
+_t=$(run_rsync --exclude='history*' ${synchost}/CVSROOT/)
 [[ -n $fwduser ]] && case $_t in
  "stdio forwarding failed"*|"Stdio forwarding request failed"*)
  err "mirror does not support ssh port-forwarding" ;;
@@ -103,7 +103,7 @@
  # result in a change there so also do a full update run at least
  # every 6h.
  if ((age < 6*60*60)); then
- oldhash=`cat $hashfile`
+ oldhash=$(< $hashfile)
  fi
 fi
 

--
Christian "naddy" Weisgerber                          [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: reposync: Fix quoting, mention default destination, other nits

Klemens Nanni-2
On Thu, Nov 14, 2019 at 08:45:32PM -0000, Christian Weisgerber wrote:
> Here's a bunch more script tweaks.  This is somewhat style/personal
> preference territory.  --exclude can be used with listing mode.
Yup, I tried to stick to technical reasoning rather than personal style,
but your diff sure is finen with me.

I've also squeezed a syntax check into fake because why not bail out on
errors rather than packaging them.

Overall diff below.
OK?


Index: Makefile
===================================================================
RCS file: /cvs/ports/sysutils/reposync/Makefile,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile
--- Makefile 14 Nov 2019 13:21:42 -0000 1.9
+++ Makefile 14 Nov 2019 21:09:40 -0000
@@ -2,8 +2,7 @@
 
 COMMENT= script to update an OpenBSD CVS repository via rsync
 
-PKGNAME= reposync-0.9
-REVISION = 0
+PKGNAME= reposync-0.10
 
 CATEGORIES= sysutils
 DISTFILES=
@@ -18,11 +17,9 @@ NO_TEST= Yes
 
 RUN_DEPENDS= net/rsync
 
-do-extract:
- cp ${FILESDIR}/reposync* ${WRKDIR}/
-
 do-install:
- ${SUBST_PROGRAM} ${WRKSRC}/reposync ${PREFIX}/bin/reposync
- ${INSTALL_MAN} ${WRKSRC}/reposync.1 ${PREFIX}/man/man1/reposync.1
+ ksh -n ${FILESDIR}/reposync
+ ${INSTALL_SCRIPT} ${FILESDIR}/reposync ${PREFIX}/bin/
+ ${INSTALL_MAN} ${FILESDIR}/reposync.1 ${PREFIX}/man/man1/
 
 .include <bsd.port.mk>
Index: files/reposync
===================================================================
RCS file: /cvs/ports/sysutils/reposync/files/reposync,v
retrieving revision 1.7
diff -u -p -r1.7 reposync
--- files/reposync 10 Nov 2019 13:53:39 -0000 1.7
+++ files/reposync 14 Nov 2019 21:11:37 -0000
@@ -24,41 +24,40 @@ err()
 
 usage()
 {
- echo "usage: ${0##*/} [-f] [-p | -l username] rsync://upstream/cvs [/path/to/cvs]" >&2
+ echo "usage: ${0##*/} [-f] [-p | -l username] rsync://upstream/path [destination]" >&2
  exit 1
 }
 
 force=false
 fwduser=anoncvs
 while getopts "fl:p" c; do
- case "$c" in
+ case $c in
  f) force=true ;;
- l) fwduser="$OPTARG" ;;
- p) fwduser="" ;;
+ l) fwduser=$OPTARG ;;
+ p) fwduser= ;;
  *) usage ;;
  esac
 done
 shift $((OPTIND-1))
-[ $# -gt 0 ] || usage
+(( $# > 0 )) || usage
 
-synchost="$1"
-repodir=/cvs
-[[ -n $2 ]] && repodir=$2
+synchost=$1
+repodir=${2:-/cvs}
 
 run_rsync()
 {
  if [[ -n $fwduser ]]; then
  # reach rsync on the server via an ssh port-forward
- rsync -e "ssh -W localhost:rsync -l $fwduser" $* 2>&1
+ rsync -e "ssh -W localhost:rsync -l $fwduser" "$@" 2>&1
  else
- rsync $* 2>&1
+ rsync "$@" 2>&1
  fi
 }
 
-rundir="/var/db/reposync"
+rundir=/var/db/reposync
 
-for i in $rundir $repodir; do
- [ ! -d $i ] || [ ! -w $i ] &&
+for i in "$rundir" "$repodir"; do
+ [[ ! -d $i ]] || [[ ! -w $i ]] &&
     err "$i must exist as a writable directory"
 done
 
@@ -66,12 +65,12 @@ if [[ $(id -u) != $(stat -L -f "%u" "$re
  err "should be run by the uid owning the repository"
 fi
 
-oldhash="invalid"
-hashfile="$rundir/reposync.hash"
-lockfile="$rundir/reposync.lock"
+oldhash=invalid
+hashfile=$rundir/reposync.hash
+lockfile=$rundir/reposync.lock
 cd $rundir
 
-if [ -h $lockfile ]; then
+if [[ -h $lockfile ]]; then
  # read the pid from $lockfile symlink target
  lockedpid=$(stat -f %Y $lockfile)
 
@@ -81,30 +80,30 @@ if [ -h $lockfile ]; then
  fi
 
  # not still running, the lock must be stale (machine panicked, etc) so zap it
- rm -f $lockfile
+ rm -f -- $lockfile
 fi
 
 ln -s $$ $lockfile || err "could not lock $lockfile"
 
-trap "rm -f $lockfile" 0 1 2 15
+trap "rm -f -- $lockfile" 0 1 2 15
 
 # check CVSROOT directory listing to identify updates; primarily for
 # ChangeLog but val-tags may also be updated after a checkout was done
 # using a new tag. ignore "history" (lists read-only operations).
-_t="$(run_rsync ${synchost}/CVSROOT/ | grep -v history)"
+_t=$(run_rsync --exclude='history*' ${synchost}/CVSROOT/)
 [[ -n $fwduser ]] && case $_t in
  "stdio forwarding failed"*|"Stdio forwarding request failed"*)
  err "mirror does not support ssh port-forwarding" ;;
 esac
 newhash="${synchost} $(echo $_t | sha256)"
 
-if [ -r $hashfile ]; then
+if [[ -r $hashfile ]]; then
  age=$((`date +%s` - `stat -t %s -f %m $hashfile`))
  # don't entirely rely on CVSROOT files; not all tree operations
  # result in a change there so also do a full update run at least
  # every 6h.
- if [ $age -lt $((6*60*60)) ]; then
- oldhash=`cat $hashfile`
+ if ((age < 6*60*60)); then
+ oldhash=$(< $hashfile)
  fi
 fi
 
@@ -113,7 +112,7 @@ if $force || [[ $oldhash != $newhash ]];
  # the old one so sync is reattempted next run
  if run_rsync -rlptiz --omit-dir-times --delete \
     --exclude='#cvs.rfl.*' --exclude='CVSROOT/history*' \
-    ${synchost}/{CVSROOT,www,xenocara,ports,src} $repodir/; then
+    ${synchost}/{CVSROOT,www,xenocara,ports,src} "$repodir"/; then
  echo $newhash > $hashfile
  else
  err "rsync failed"
Index: files/reposync.1
===================================================================
RCS file: /cvs/ports/sysutils/reposync/files/reposync.1,v
retrieving revision 1.3
diff -u -p -r1.3 reposync.1
--- files/reposync.1 10 Nov 2019 13:36:32 -0000 1.3
+++ files/reposync.1 14 Nov 2019 11:38:18 -0000
@@ -23,7 +23,7 @@
 .Sh SYNOPSIS
 .Nm
 .Op Fl f
-.Op Fl l Ar username | Fl p
+.Op Fl p | Fl l Ar username
 .Ar rsync://upstream/path
 .Op Ar destination
 .Sh DESCRIPTION
@@ -40,6 +40,12 @@ If only a short time has elapsed since t
 directory on the upstream server is checked for changes and the update
 is skipped if none are detected.
 .Pp
+If
+.Ar destination
+is omitted,
+.Pa /cvs
+will be used.
+.Pp
 The directory
 .Pa /var/db/reposync
 must exist and be writable by the user running
@@ -71,6 +77,6 @@ update the local directory
 .Pa /home/cvs .
 .El
 .Sh SEE ALSO
-.Xr ssh 1 ,
+.Xr cvs 1 ,
 .Xr rsync 1 ,
-.Xr cvs 1 .
+.Xr ssh 1

Reply | Threaded
Open this post in threaded view
|

Re: reposync: Fix quoting, mention default destination, other nits

Christian Weisgerber
Klemens Nanni:

> I've also squeezed a syntax check into fake because why not bail out on
> errors rather than packaging them.
>
> Overall diff below.
> OK?

Obviously ok naddy@
But let's wait what the MAINTAINER... oh, never mind then. :-)

--
Christian "naddy" Weisgerber                          [hidden email]