Re: gnupload improvements

[Ralf Wildenhues]

Hello Sergey,

again, apologies for the delay.

* Sergey Poznyakoff wrote on Tue, Feb 17, 2009 at 09:56:09AM CET:
> Ralf Wildenhues <address@hidden> ha escrit:
> 
> > >   --symlink a b c d
> > > 
> > > will create two symbolic links: a -> b and c -> d.
> > 
> > This API looks rather unusual.
> 
> Well, I make it this way for two reasons: first, it is consistent with
> another options (--rmsymlink, --delete)

Hmm, then I guess I don't like those options either; but also I don't
like backward incompatibilities.  Sigh.

> and secondly it allows to spare
> some typing when several links need be created.  However, if you
> think it is too error-prone, I can easily switch it back to a two
> argument form. 


> > Also, why is --rmsymlink not --delete?  (This is me wondering about the
> > file format, not a question about your patch.)
> 
> They are two different commands. The --delete option causes removal of
> a regular file (actually, archivation of it, but the result is the
> same), whereas --rmsymlink removes a symbolic link.  

Yes I understand that.  I don't think it would have needed two different
options for that, though.

> > > gnupload --to alpha.gnu.org:tar \
> > >          --delete tar-1.20.90.tar.gz tar-1.20.90.tar.bz2 \
> > >          --rmsymlink tar-latest.tar.gz tar-latest.tar.gz2 \
> > >          -- tar-1.20.91.tar.gz
> > > 
> > > (double-dash in this case is needed to separate files to upload
> > > from --rmsymlink arguments).       
> > 
> > Do we want to make guarantees about the ordering in which actions are
> > taken?
> 
> The directives are created in the order of their appearance in the
> command line.  The description of the upload directive file in
> maintain.info says that "if you use more than one directive, the
> directives are executed in the sequence they are specified in." So, it
> is guaranteed, that the actions are executed in the order they are listed
> in the command line.

OK.  I should have been more precise in my comment: if we want to
guarantee the ordering of actions (which is important when .gnupload
comes into play), then the --help output should explicitly say that
ordering is guaranteed.  I have added that in the patch below.

Bike shedding question: .gnupload or .gnuploadrc?

> > > +  echo "$0: Reading configuration file .gnupload"
> > > +  eval set -- "`sed 's/#.*$//;/^$/d' .gnupload | tr '\n' ' '` $*"
> > 
> > A couple of portability nits:
> > - 'set x ...; shift'
> 
> Hmm, not quite sure where it can be used in the above statement?

Like this:
  eval set x "`sed 's/#.*$//;/^$/d' .gnupload | tr '\012\015' '  '` 
\"address@hidden""
  shift

> > - \"address@hidden" instead of $*
> 
> $@ maybe, but \"address@hidden" will coalesce the actual command line 
> arguments
> into one,

Sorry, I forgot to escape the $, too.

> As far as I can tell, directive file format does not allow file names
> containing white space, so the distinction between $@ and $*
> is probably of little concern here.

Maybe; but I dislike unnecessary quoting bugs in code that may be
reused elsewhere later.  And these two lines are otherwise perfectly
reusable.

> > If you agree with my comments, and answer the implied questions, I can
> > make those changes; if you want to resubmit the patch, even better.  :-)
> 
> Sure, find attached a modified version :^) 

Thanks.

> > I do with we had some testsuite exposure for gnupload though, this has
> > grown so complicated that bugs are likely.  Is there test upload space
> > on some of these hosts?
> 
> I plan to implement this on gnu.org.ua. I'll let you know when it is
> operational.

Thanks.  That'd be interesting to hear.

In your other mail, you write:

> Just in case, attached is a modified patch, that removes the leftover
> uses of ${parameter%%word} and ${parameter##word} expansions.

Yes, but that updated patch you sent is less reliable in the sense that
the expr expressions return empty strings when there is no separator.

I am thus committing your second-newest iteration of the patch (the one
from the message this one replies to) and adding you to THANKS, and on
top of that, committing the patch below, and then another
whitespace-only patch to reindent the argument parsing loop.

With the patch below, --dry-run also does not invoke gpg but only echoes
the gpg commands that would be called.

It would be nice if you could try the resulting script.  Thanks.

Committing to both branches.

Cheers, and sorry for the delay again,
Ralf

2009-03-04  Sergey Poznyakoff  <address@hidden>

        Various gnupload improvements.
        Add support for uploading to download.gnu.org.ua.
        Add support for a .gnupload configuration file.
        Support creating and removing symlinks.
        Allow for several operations in a single invocation.
        Add debugging features.
        * lib/gnupload: New options --delete, --symlink, --rmsymlink,
        --symlink-regex, --dry-run; support `--' to separate options and
        commands from files.  New target download.gnu.org.ua.  Expand
        `.gnupload' file contents before command line arguments.
        (usage): Expand.
        (dprint, mkdirective, mksymlink, upload): New functions.
        * THANKS: Update.

(patch as in your mail)


2009-03-04  Ralf Wildenhues  <address@hidden>

    gnupload touchups.
    
    * lib/gnupload (conffile): New variable.  Use throughout.
    (dry_run): Use `false' and `:' values.  Show, don't execute,
    gpg signing commands upon --dry-run.
    (usage): Typo and formatting fixes.  Mention that commands are
    applied in order.
    (argument loop): Factorize a bit, fix quoting.
    (dprint, mkdirective, mksymlink, upload): Formatting and
    portability fixes.

diff --git a/lib/gnupload b/lib/gnupload
index a87c13f..e600bb7 100755
--- a/lib/gnupload
+++ b/lib/gnupload
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Sign files and upload them.
 
-scriptversion=2009-03-04.20
+scriptversion=2009-03-04.21
 
 # Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009  Free Software Foundation
 #
@@ -23,8 +23,9 @@ scriptversion=2009-03-04.20
 set -e
 
 GPG='gpg --batch --no-tty'
+conffile=.gnupload
 to=
-dry_run=
+dry_run=false
 symlink_files=
 delete_files=
 delete_symlinks=
@@ -47,11 +48,11 @@ Options:
   --to DEST                specify one destination for FILES
                            (multiple --to options are allowed)
   --user NAME              sign with key NAME
-  --symlink-regex[=SED-EXPR]  use SED-EXPR to create symbolic links
+  --symlink-regex[=EXPR]   use sed script EXPR to compute symbolic link names
   --dry-run                do nothing, show what would have been done
   --version                output version information and exit
 
-If --symlink-re is given without SED-EXPR, then the link target name
+If --symlink-regex is given without EXPR, then the link target name
 is created by replacing the version information with \`-latest', e.g.:
 
   foo-1.3.4.tar.gz -> foo-latest.tar.gz
@@ -66,9 +67,10 @@ Recognized destinations are:
                            build directive files and upload files by SFTP
   address@hidden:DIRECTORY    upload files with scp
 
-If the file .gnupload exists in the current working directory, its contents
-are prepended to the actual command line options.  Use this to keep your
-defaults.  Comments (#) and empty lines in .gnupload are allowed.
+Options and commands are applied in order.  If the file $conffile exists
+in the current working directory, its contents are prepended to the
+actual command line options.  Use this to keep your defaults.  Comments
+(#) and empty lines in $conffile are allowed.
 
 Examples:
 1. Upload automake-1.8.2b.tar.gz and automake-1.8.2b.tar.bz2 to two sites:
@@ -79,7 +81,7 @@ Examples:
 2. Same as above, but also create symbolic links to automake-latest.tar.*:
   gnupload --to sources.redhat.com:~ftp/pub/automake \\
            --to alpha.gnu.org:automake \\
-          --symlink-regex \\
+           --symlink-regex \\
            automake-1.8.2b.tar.gz automake-1.8.2b.tar.bz2
 
 3. Symlink automake-1.8.2b.tar.gz to automake-latest.tar.gz and
@@ -87,26 +89,27 @@ automake-1.8.2b.tar.bz2 to automake-latest.tar.bz2 on both 
sites:
 
   gnupload --to sources.redhat.com:~ftp/pub/automake \\
            --to alpha.gnu.org:automake \\
-          --symlink automake-1.8.2b.tar.gz automake-latest.tar.gz \\
-                    automake-1.8.2b.tar.bz2 automake-latest.tar.bz2
+           --symlink automake-1.8.2b.tar.gz automake-latest.tar.gz \\
+                     automake-1.8.2b.tar.bz2 automake-latest.tar.bz2
 
 4. Delete automake-1.8.2a.tar.gz and .bz2, remove symlink
 automake-latest.tar.gz and upload automake-1.8.2b.tar.gz:
 
   gnupload --to sources.redhat.com:~ftp/pub/automake \\
            --to alpha.gnu.org:automake \\
-          --delete automake-1.8.2a.tar.gz automake-1.8.2a.tar.bz2 \\
-          --rmsymlink automake-latest.tar.gz \\
-          -- \\
+           --delete automake-1.8.2a.tar.gz automake-1.8.2a.tar.bz2 \\
+           --rmsymlink automake-latest.tar.gz \\
+           -- \\
            automake-1.8.2b.tar.gz automake-1.8.2b.tar.bz2
 
 Report bugs to <address@hidden>.
 Send patches to <address@hidden>."
 
 # Read local configuration file
-if test -r .gnupload; then
-  echo "$0: Reading configuration file .gnupload"
-  eval set -- "`sed 's/#.*$//;/^$/d' .gnupload | tr '\012\015' ' '` $@"
+if test -r "$conffile"; then
+  echo "$0: Reading configuration file $conffile"
+  eval set x "`sed 's/#.*$//;/^$/d' \"$conffile\" | tr '\012\015' '  '` 
\"address@hidden""
+  shift
 fi
 
 while test -n "$1"; do
@@ -123,7 +126,7 @@ while test -n "$1"; do
             exit 1
           else
             to="$to $2"
-            shift 2
+            shift
           fi
           ;;
         --user)
@@ -132,41 +135,35 @@ while test -n "$1"; do
             exit 1
           else
             GPG="$GPG --local-user $2"
-            shift 2
+            shift
           fi
           ;;
-       --delete)
+        --delete)
           collect_var=delete_files
-          shift
           ;;
-       --rmsymlink)
-         collect_var=delete_symlinks
-         shift
-         ;;
+        --rmsymlink)
+          collect_var=delete_symlinks
+          ;;
         --symlink-regex=*)
           symlink_expr=`expr "$1" : '[^=]*=\(.*\)'`
-          shift
           ;;
         --symlink-regex)
           symlink_expr='s|-[0-9][0-9\.]*\(-[0-9][0-9]*\)\{0,1\}\.|-latest.|'
-          shift
           ;;
         --symlink)
           collect_var=symlink_files
-          shift
           ;;
         --dry-run|-n)
-          dry_run=1
-          shift
+          dry_run=:
           ;;
         --version)
           echo "gnupload $scriptversion"
           exit $?
           ;;
-       --)
-         shift
-         break
-         ;;
+        --)
+          shift
+          break
+          ;;
         -*)
           echo "$0: Unknown option \`$1', try \`$0 --help'" 1>&2
           exit 1
@@ -176,11 +173,11 @@ while test -n "$1"; do
   *)  if test -z "$collect_var"; then
         break
       else
-        eval $collect_var=\"\$$collect_var \"$1\"\"
-       shift
+        eval "$collect_var=\"\$$collect_var $1\""
       fi
       ;;
   esac
+  shift
 done
 
 dprint()
@@ -188,7 +185,7 @@ dprint()
   echo "Running $*..."
 }
 
-if test -n "$dry_run"; then
+if $dry_run; then
   dbg=dprint
 fi
 
@@ -223,7 +220,7 @@ else
       if test -z "$linkname"; then
         echo "$0: symlink expression produces empty results" >&2
         exit 1
-      elif test $linkname = $file; then
+      elif test "$linkname" = $file; then
         echo "$0: symlink expression does not alter file name" >&2
         exit 1
       fi
@@ -251,13 +248,15 @@ if test $# -ne 0; then
   do
     echo "Signing $file..."
     rm -f $file.sig
-    echo $passphrase | $GPG --passphrase-fd 0 -ba -o $file.sig $file
+    echo "$passphrase" | $dbg $GPG --passphrase-fd 0 -ba -o $file.sig $file
   done
 fi
 
+
 # mkdirective DESTDIR BASE FILE STMT
 # Arguments: See upload, below
-mkdirective() {
+mkdirective ()
+{
   stmt="$4"
   if test -n "$3"; then
     stmt="
@@ -269,14 +268,15 @@ version: 1.1
 directory: $1
 comment: gnupload v. $scriptversion$stmt
 EOF
-  if test -n "$dry_run"; then
+  if $dry_run; then
     echo "File ${2}.directive:"
     cat ${2}.directive
     echo "File ${2}.directive:" | sed 's/./-/g'
   fi
 }
 
-mksymlink() {
+mksymlink ()
+{
   while test $# -ne 0
   do
     echo "symlink: $1 $2"
@@ -293,7 +293,8 @@ mksymlink() {
 #  FILE     Name of the file to distribute (may be empty);
 #  STMT     Additional statements for the directive file;
 #  FILES    List of files to upload.
-upload() {
+upload ()
+{
   dest=$1
   destdir=$2
   base=$3
@@ -305,12 +306,12 @@ upload() {
   case $dest in
     alpha.gnu.org:*)
       mkdirective "$destdir" "$base" "$file" "$stmt"
-      echo "$passphrase" | $GPG --passphrase-fd 0 --clearsign $base.directive
+      echo "$passphrase" | $dbg $GPG --passphrase-fd 0 --clearsign 
$base.directive
       $dbg ncftpput ftp-upload.gnu.org /incoming/alpha $files 
$base.directive.asc
       ;;
     ftp.gnu.org:*)
       mkdirective "$destdir" "$base" "$file" "$stmt"
-      echo "$passphrase" | $GPG --passphrase-fd 0 --clearsign $base.directive
+      echo "$passphrase" | $dbg $GPG --passphrase-fd 0 --clearsign 
$base.directive
       $dbg ncftpput ftp-upload.gnu.org /incoming/ftp $files $base.directive.asc
       ;;
     savannah.gnu.org:*)
@@ -326,17 +327,20 @@ upload() {
       $dbg ncftpput savannah.nongnu.org /incoming/savannah/$destdir $files
       ;;
     download.gnu.org.ua:alpha/*|download.gnu.org.ua:ftp/*)
-      mkdirective "${destdir#*/}" "$base" "$file" "$stmt"
-      echo "$passphrase" | $GPG --passphrase-fd 0 --clearsign $base.directive
+      destdir_p1=`echo "$destdir" | sed 's,^[^/]*/,,'`
+      destdir_topdir=`echo "$destdir" | sed 's,/.*,,'`
+      mkdirective "$destdir_p1" "$base" "$file" "$stmt"
+      echo "$passphrase" | $dbg $GPG --passphrase-fd 0 --clearsign 
$base.directive
       for f in $files $base.directive.asc
       do
         echo put $f
-      done | $dbg sftp -b - puszcza.gnu.org.ua:/incoming/${destdir%%/*}
+      done | $dbg sftp -b - puszcza.gnu.org.ua:/incoming/$destdir_topdir
       ;;
     /*)
+      dest_host=`echo "$dest" | sed 's,:.*,,'`
       mkdirective "$destdir" "$base" "$file" "$stmt"
-      echo "$passphrase" | $GPG --passphrase-fd 0 --clearsign $base.directive
-      $dbg cp $files $base.directive.asc ${dest%%:*}
+      echo "$passphrase" | $dbg $GPG --passphrase-fd 0 --clearsign 
$base.directive
+      $dbg cp $files $base.directive.asc $dest_host
       ;;
     *)
       if test -z "$files"; then
@@ -395,6 +399,8 @@ symlink: $file.sig $linkname.sig"
   done
 done
 
+exit 0
+
 # Local variables:
 # eval: (add-hook 'write-file-hooks 'time-stamp)
 # time-stamp-start: "scriptversion="