Re: [Quilt-dev] CVS / source control support

[Joe Green]

Dean Roehrich wrote:

scm_modify_patch $QUILT_PATCHES/$patch

That should imply the "edit".   Then...

  

scm_add_patch $QUILT_PATCHES/$patch

That should imply..."add" :)

I know my own version of this stuff also had this multiplexer but I think
we should move away from that.
  

Yes, I agree.  Especially if more hooks are added, it's better to just be able to override the hooks you need.  I've attached updated patches with this renaming and updates for some of the other changes I sent out today.

I'd also like to distinguish between pre-modify and post-modify steps, to accomodate
SCMs such as BK.  (I know, we've all migrated away from that by now...and I haven't
learned the git semantics yet, but I think that's post-modify, right?)  We could keep the scm_modify_patch() and then also have scm_pre_modify_patch().  The name "scm_modify_patch" has a finality to it so it doesn't need to be renamed scm_post_modify_patch.
  

It's been a while since I've used bitkeeper, but I don't remember a post-modify step.  Can you give me an example (in either BitKeeper or another source control system)?  I've been practicing YAGNI (You Aren't Gonna Need It) on this stuff.  There's a lot of hooks we *might* need; I'm waiting for proof :).

I think the more complicated issues are things like:

• is "add" done before or after the file is created?
• does the scm command want to delete removed files itself or not?
• what about renames?

It could get pretty messy to handle a lot of variations here, and it's especially tough if you want the implementation to be optional hooks (as opposed to assuming the SCM is there).

I'm also concerned about your use of "|| exit 1".  I think you need to unroll, in some
circumstances at least.  In the import newpatch=yes case, you've already successfully
executed a cp to replace the patch and it seems wrong to simply "exit 1" after the
scm_add_patch fails.  I think this will fail quite often, with people hitting ^C now and
again (you have given them more time to reconsider, and they will)...and they will expect the import itself to fail in this case.
  

Well, it does; it hasn't added the file to the series :).  It does leave the copied file in the patches directory, though; you're right that it would be better to clean that up.

Also, you wouldn't call scm_modify_patch if status=1.
  

Do you see this anywhere else except import?  That looks like an interesting case; why does it go on to add the patch to the series if it wasn't able to copy the file over?  It looks like there should be an exit or at least a continue after the bad status is set.

Hmm, does your workarea match what's currently at the top of the CVS tree?

The insert_in_series mismatch was the "first_patch" bug I sent mail about earlier.  I've included that patch this time.

The delete case was the "really_delete" patch I noted in the first email, which adds support for deleting patch files.  You need that patch applied if you want to apply the delete.in hunk.

I'll bet you'll want to look at some unroll steps.
  

Thanks for the notes; I'll look these over to see where I can do better.  I've generally been taking the approach (with knowledgeable users and a pretty reliable source control system) that it's easier for them to fix up a few problems than to do radical surgery on quilt.  As you noted, in some places doing perfect unrolling can get pretty messy, and may introduce multiple new opportunities for failure.

Ah, refresh.  Now here is where I become concerned that you'll be changing people's
habits--they're methods of work.  We've all learned over the years to save often, and
I've no doubt that quilt users also refresh often.  If this requires answering SCM
prompts then this will become unpopular mighty quick.  I think we need something other
than scm_edit_patch and scm_add_patch in this case.
  

It hasn't been a problem so far with our setup.  I have cvs set up to ssh into the server, and with these patches you don't hear anything about CVS unless there's a failure.  That's been extremely rare for me, and I refresh often.

The only known problem I can think of is that with CVS you can't remove a file and then add one with the same name without an intervening commit.  You could run into this if you decided to rename a patch and then changed your mind and tried to rename it back.

Since a couple of things have changed, here's the whole patch set again:

quilt-really_delete.patch

Add options to delete to remove the deleted patch files.  The scm_hooks patch is dependent on this patch because it adds a hook to the delete code.

quilt-first_patch.patch

Fixes a case where insert_in_series where it will add a new first patch before existing directives or comments in the series file.

quilt-realpath.patch

Adds patchfns "readlink" and "realpath" for dealing with symbolic links.  The CVS hooks depend on this, but I've also found these useful for other things, so I keep them in a separate patch.

quilt-scm_hooks.patch

Implements the generic source control hooks in the quilt code.  The default hooks just return success, so they don't do anything if not overridden by the user.

quiltrc.cvs

This is not a patch, but shows the functions that can be added to a user's quiltrc file to override the default hooks and add support for CVS.

-- 
Joe Green <address@hidden>
MontaVista Software, Inc.

Source: MontaVista Software, Inc. <address@hidden>
Type: Enhancement
Disposition: submitted to http://savannah.nongnu.org/projects/quilt

Add options to the delete command that will remove the deleted patch from
the QUILT_PATCHES directory and optionally create a backup file.

Index: quilt-0.42/quilt/delete.in
===================================================================
--- quilt-0.42.orig/quilt/delete.in
+++ quilt-0.42/quilt/delete.in
@@ -19,7 +19,7 @@ fi
 
 usage()
 {
-       printf $"Usage: quilt delete [patch | -n]\n"
+       printf $"Usage: quilt delete [-r] [--backup] [patch | -n]\n"
        if [ x$1 = x-h ]
        then
                printf $"
@@ -29,6 +29,12 @@ topmost patch can be removed right now.)
 
 -n     Delete the next patch after topmost, rather than the specified
        or topmost patch.
+
+-r     Remove the deleted patch file from the patches directory as well.
+
+--backup
+       Rename the patch file to patch~ rather than deleting it.
+       Ignored if not used with \"-r\".
 "
 
                exit 0
@@ -37,7 +43,7 @@ topmost patch can be removed right now.)
        fi
 }
 
-options=`getopt -o nh -- "$@"`
+options=`getopt -o nrh --long backup -- "$@"`
 
 if [ $? -ne 0 ]
 then
@@ -52,8 +58,14 @@ do
        -n)
                opt_next=1
                shift ;;
+       -r)
+               opt_remove=1
+               shift ;;
        -h)
                usage -h ;;
+       --backup)
+               QUILT_BACKUP=1
+               shift ;;
        --)
                shift
                break ;;
@@ -113,6 +125,28 @@ then
        printf $"Removed patch %s\n" "$(print_patch "$patch")"
 else
        printf $"Failed to remove patch %s\n" "$(print_patch "$patch")" >&2
+       exit 1
+fi
+
+patch_file=$(patch_file_name "$patch")
+if [ "$opt_remove" -a -e "$patch_file" ]
+then
+       if [ "$QUILT_BACKUP" ]
+       then
+               if ! mv -f "$patch_file" "$patch_file~"
+               then
+                       printf $"Failed to backup patch file \"%s\"\n" \
+                               "$patch_file" >&2
+                       exit 1
+               fi
+       else
+               if ! rm -f "$patch_file"
+               then
+                       printf $"Failed to remove patch file \"%s\"\n" \
+                               "$patch_file" >&2
+                       exit 1
+               fi
+       fi
 fi
 ### Local Variables:
 ### mode: shell-script
Index: quilt-0.42/bash_completion
===================================================================
--- quilt-0.42.orig/bash_completion
+++ quilt-0.42/bash_completion
@@ -139,7 +139,7 @@ _quilt_completion()
           COMPREPLY=( $( compgen -W "-h $(quilt applied)" -- $cur ) )
           ;;
        delete) 
-          COMPREPLY=( $( compgen -W "-n -h $(quilt series)" -- $cur ) )
+          COMPREPLY=( $( compgen -W "-n -r -h --backup $(quilt series)" -- 
$cur ) )
           ;;
        diff) 
           case $prev in

Source: MontaVista Software, Inc. <address@hidden>
Type: Defect Fix
Disposition: submit to http://savannah.nongnu.org/projects/quilt

Don't insert first patch before any initial directives or comments.

Index: quilt-0.42/scripts/patchfns.in
===================================================================
--- quilt-0.42.orig/scripts/patchfns.in
+++ quilt-0.42/scripts/patchfns.in
@@ -191,7 +191,7 @@ patch_in_series()
 insert_in_series()
 {
        local patch=$1 patch_args=$2
-       local top=$(top_patch) tmpfile
+       local next=$(patches_after "$(top_patch)" | head -n 1) tmpfile
 
        if [ -n "$patch_args" ]
        then
@@ -200,12 +200,12 @@ insert_in_series()
 
        tmpfile=$(gen_tempfile) || return 1
        mkdir -p $(dirname $SERIES)
-       if [ -n "$top" ]
+       if [ -n "$next" ]
        then
                @AWK@ '
-                               { print }
-               /^'"$(quote_re $top)"'([ \t]|$)/ \
+               /^'"$(quote_re $next)"'([ \t]|$)/ \
                                { print "'"$patch$patch_args"'" }
+                               { print }
                ' $SERIES > $tmpfile
                status=$?
                if [ $status -ne 0 ]
@@ -214,11 +214,11 @@ insert_in_series()
                        return 1
                fi
        else
-               echo "$patch$patch_args" > $tmpfile
                if [ -e $SERIES ]
                then
-                       cat $SERIES >> $tmpfile
+                       cat $SERIES > $tmpfile
                fi
+               echo "$patch$patch_args" >> $tmpfile
        fi
        cat $tmpfile > $SERIES
        rm -f $tmpfile

Source: MontaVista Software, Inc. <address@hidden>
Type: Enhancement
Disposition: submit to http://savannah.nongnu.org/projects/quilt

Add "readlink" and "realpath" functions for following symlinks.

Index: quilt-0.42/scripts/patchfns.in
===================================================================
--- quilt-0.42.orig/scripts/patchfns.in
+++ quilt-0.42/scripts/patchfns.in
@@ -125,6 +125,29 @@ dirname()
        echo "${path:-.}"
 }
 
+if ! type readlink &> /dev/null ; then
+readlink()
+{
+       expr "$(/bin/ls -ld $1)" : ".*-> \(.*\)$"
+}
+fi
+
+# If input file names a symbolic link, print path to real file.
+# If non-existent or not a link, print the input path.
+realpath()
+{
+       local path="$1"
+       while [ -L "$path" ] ; do
+               local link=$(readlink "$path")
+               if [ "${link#/}" = "$link" ] ; then
+                       path=$(dirname "$path")"/$link"
+               else
+                       path="$link"
+               fi
+       done
+       echo "$path"
+}
+
 patch_file_name()
 {
        echo "$QUILT_PATCHES/$1"

Source: MontaVista Software, Inc. (address@hidden)
Type: Enhancement
Disposition: submit to http://savannah.nongnu.org/projects/quilt

Add hooks for Sofware Configuration Management systems.  These have
been tested with CVS hooks.

Index: quilt-0.42/quilt/import.in
===================================================================
--- quilt-0.42.orig/quilt/import.in
+++ quilt-0.42/quilt/import.in
@@ -104,10 +104,13 @@ do
                fi
                printf $"Replacing patch %s with new version\n" \
                       "$(print_patch $patch)" >&2
+               scm_modify_patch "$QUILT_PATCHES/$patch" || exit 1
+               newpatch=no
        else
                printf $"Importing patch %s (stored as %s)\n" \
                       "$(print_patch $patch_file)" \
                       "$(print_patch $patch)"
+               newpatch=yes
        fi
        dest=$QUILT_PATCHES/$patch
        mkdir -p "${dest%/*}"
@@ -117,6 +120,11 @@ do
                status=1
        fi
 
+       if [ "$newpatch" = "yes" ]
+       then
+               scm_add_patch "$QUILT_PATCHES/$patch" || exit 1
+       fi
+
        if ! patch_in_series $patch &&
           ! insert_in_series $patch "$patch_args"
        then
Index: quilt-0.42/scripts/patchfns.in
===================================================================
--- quilt-0.42.orig/scripts/patchfns.in
+++ quilt-0.42/scripts/patchfns.in
@@ -40,6 +40,55 @@ fi
 
 # ========================================================
 
+# Overridable hooks to enable source control
+
+if [ "$(type -t scm_add_patch 2> /dev/null)" != function ]; then
+       # Add patch_file to source control
+       scm_modify_patch()
+       {
+               # local patch_file="$1"
+               return 0
+       }
+fi
+
+if [ "$(type -t scm_delete_patch 2> /dev/null)" != function ]; then
+       # Remove patch_file from source control
+       scm_delete_patch()
+       {
+               # local patch_file="$1"
+               return 0
+       }
+fi
+
+if [ "$(type -t scm_modify_patch 2> /dev/null)" != function ]; then
+       # Enable modification to existing patch_file
+       scm_modify_patch()
+       {
+               # local patch_file="$1"
+               return 0
+       }
+fi
+
+if [ "$(type -t scm_add_series 2> /dev/null)" != function ]; then
+       # Add series_file to source control
+       scm_add_series()
+       {
+               # local series_file="$1"
+               return 0
+       }
+fi
+
+if [ "$(type -t scm_modify_series 2> /dev/null)" != function ]; then
+       # Enable modification to existing series_file
+       scm_modify_series()
+       {
+               # local series_file="$1"
+               return 0
+       }
+fi
+
+# ========================================================
+
 #declare -a exit_handlers
 #
 #add_exit_handler() {
@@ -190,6 +239,11 @@ change_db_strip_level()
                ' $SERIES > $tmpfile
                if ! cmp $SERIES $tmpfile >/dev/null 2>/dev/null
                then
+                       if ! scm_modify_series "$SERIES"
+                       then
+                               rm -f $tmpfile
+                               return 1
+                       fi
                        cat $tmpfile > $SERIES
                fi
                rm -f $tmpfile
@@ -215,6 +269,7 @@ insert_in_series()
 {
        local patch=$1 patch_args=$2
        local next=$(patches_after "$(top_patch)" | head -n 1) tmpfile
+       local new_series=no
 
        if [ -n "$patch_args" ]
        then
@@ -243,8 +298,24 @@ insert_in_series()
                fi
                echo "$patch$patch_args" >> $tmpfile
        fi
+
+       [ -e "$SERIES" ] || new_series=yes
+
+       if [ "$new_series" = "no" ] && ! scm_modify_series "$SERIES"
+       then
+               rm -f $tmpfile
+               return 1
+       fi
+
        cat $tmpfile > $SERIES
        rm -f $tmpfile
+
+       if [ "$new_series" = "yes" ]
+       then
+               scm_add_series "$SERIES" || return 1
+       fi
+
+       return 0
 }
 
 remove_from_series()
@@ -256,7 +327,7 @@ remove_from_series()
        ! /^'"$(quote_re $patch)"'([ \t]|$)/ \
                                { print }
        ' $SERIES > $tmpfile
-       if [ $? -ne 0 ]
+       if [ $? -ne 0 ] || ! scm_modify_series "$SERIES"
        then
                rm -f $tmpfile
                return 1
@@ -277,7 +348,7 @@ rename_in_series()
                { print }
        END     { exit(! good) }
        ' $SERIES > $tmpfile
-       if [ $? -ne 0 ]
+       if [ $? -ne 0 ] || ! scm_modify_series "$SERIES"
        then
                rm -f $tmpfile
                return 1
Index: quilt-0.42/quilt/delete.in
===================================================================
--- quilt-0.42.orig/quilt/delete.in
+++ quilt-0.42/quilt/delete.in
@@ -146,6 +146,7 @@ then
                        exit 1
                fi
        fi
+       scm_delete_patch "$patch_file" || exit 1
 fi
 ### Local Variables:
 ### mode: shell-script
Index: quilt-0.42/quilt/fork.in
===================================================================
--- quilt-0.42.orig/quilt/fork.in
+++ quilt-0.42/quilt/fork.in
@@ -118,6 +118,13 @@ printf $"Fork of patch %s created as %s\
        "$(print_patch $top_patch)" \
        "$(print_patch $new_patch)"
 
+patch_file=$(patch_file_name "$new_patch")
+if [ -e "$patch_file" ]
+then
+       scm_add_patch "$patch_file" || exit 1
+fi
+
+exit 0
 ### Local Variables:
 ### mode: shell-script
 ### End:
Index: quilt-0.42/quilt/refresh.in
===================================================================
--- quilt-0.42.orig/quilt/refresh.in
+++ quilt-0.42/quilt/refresh.in
@@ -290,6 +290,14 @@ fi
 
 cat $tmp_patch >> $tmp_result
 
+if [ -e "$patch_file" ]
+then
+       scm_modify_patch "$patch_file" || die 1
+       newpatch=no
+else
+       newpatch=yes
+fi
+
 if [ -e $patch_file ] && \
    @DIFF@ -q $patch_file $tmp_result > /dev/null
 then
@@ -305,6 +313,11 @@ fi
 
 touch $QUILT_PC/$patch/.timestamp
 
+if [ "$newpatch" = "yes" ]
+then
+       scm_add_patch "$patch_file" || die 1
+fi
+
 rm -f $QUILT_PC/$patch~refresh
 if ! change_db_strip_level -p$opt_strip_level $patch
 then
Index: quilt-0.42/quilt/header.in
===================================================================
--- quilt-0.42.orig/quilt/header.in
+++ quilt-0.42/quilt/header.in
@@ -192,7 +192,11 @@ then
        | maybe_strip_trailing_whitespace
 else
        patch_file_or_null=/dev/null
-       [ -e "$patch_file" ] && patch_file_or_null=$patch_file
+       if [ -e "$patch_file" ]
+       then
+               patch_file_or_null=$patch_file
+               scm_modify_patch "$patch_file"
+       fi
 
        tmp=$(gen_tempfile) || exit 1
        tmp2=$(gen_tempfile) || exit 1
@@ -250,6 +254,11 @@ $"Replaced header of patch %s\n" "$(prin
                        printf \
 $"Appended text to header of patch %s\n" "$(print_patch $patch)"
                fi
+
+               if [ "$patch_file_or_null" = "/dev/null" ]
+               then
+                       scm_add_patch "$patch_file"
+               fi
        else
                exit 1
        fi
Index: quilt-0.42/quilt/rename.in
===================================================================
--- quilt-0.42.orig/quilt/rename.in
+++ quilt-0.42/quilt/rename.in
@@ -40,6 +40,8 @@ move_file()
 
        [ -d "$newdir" ] || mkdir -p "$newdir" || return 1
        mv "$old" "$new" || return 1
+       scm_add_patch "$new" || return 1
+       scm_delete_patch "$old" || return 1
        rmdir -p "$(dirname "$old")" 2> /dev/null
 
        return 0

scm_add_patch()
{
        local patch_file="$1"
        local dir=$(dirname "$patch_file") name=$(basename "$patch_file")

        if [ -d "$dir/CVS" ]; then
                # Add file to source control
                if ! (cd "$dir" && cvs -Q add -ko "$name"); then
                        printf $"CVS add failed for file \"%s\".\n" \
                                "$dir/$name" >&2
                        return 1
                fi
        fi
        return 0
}

scm_delete_patch()
{
        local patch_file="$1"
        local dir=$(dirname "$patch_file") name=$(basename "$patch_file")

        if [ -d "$dir/CVS" ]; then
                # Remove file from source control
                if ! (cd "$dir" && cvs -Q remove "$name") ; then
                        printf $"CVS remove failed for file \"%s\".\n" \
                                "$dir/$name" >&2
                        return 1
                fi
        fi
        return 0
}

scm_modify_patch()
{
        local patch_file="$1"
        local dir=$(dirname "$patch_file") name=$(basename "$patch_file")

        if [ -d "$dir/CVS" ]; then
                # Enable modification to existing file
                if [ ! -w "$dir/$name" ] && ! (cd "$dir" && cvs edit "$name")
                then
                        printf $"CVS edit failed for file \"%s\".\n" \
                                "$dir/$name" >&2
                        return 1
                fi
        fi
        return 0
}

scm_add_series()
{
        local series_file=$(realpath "$1")
        local dir=$(dirname "$series_file") name=$(basename "$series_file")

        if [ -d "$dir/CVS" ]; then
                # Add file to source control
                if ! (cd "$dir" && cvs -Q add -ko "$name"); then
                        printf $"CVS add failed for file \"%s\".\n" \
                                "$dir/$name" >&2
                        return 1
                fi
        fi
        return 0
}

scm_modify_series()
{
        local series_file=$(realpath "$1")
        local dir=$(dirname "$series_file") name=$(basename "$series_file")

        if [ -d "$dir/CVS" ]; then
                # Enable modification to existing file
                if [ ! -w "$dir/$name" ] && ! (cd "$dir" && cvs edit "$name")
                then
                        printf $"CVS edit failed for file \"%s\".\n" \
                                "$dir/$name" >&2
                        return 1
                fi
        fi
        return 0
}