svn patch pitfalls

I seem to meet lots of unexpected traps with 'svn patch'
these days,
especially with copy and move operations since this is what
I've been
working on more or less recently.  I have two of them in
mind:

* dry-run patching with copy and move operations

This one is rather because of the way we use GNU patch. 
Basically,
'svn patch' calls the external tool on any given output. 
That is,
even when dealing with copy and move operations, which the
tool
doesn't understand/speak.  Consider the following unidiff
patch
created after iota2 was copied from iota (svnpatch bytes
aren't
displayed):

[[[
Index: iota2
============================================================
=======
--- iota2       (revision 1)
+++ iota2       (working copy)
 -1 +1,2

 This is the file 'iota'.
+bla
]]]

Now run 'svn patch --dry-run patchfile' (or even 'patch -p0
--dry-run
< patchfile'), and GNU patch bails out it can't find such
a file.
Why?  Because it's dry-run, and thus 'svn patch' didn't copy
the file
it would have copied if not in dry-run mode.  Let's remind
that 'svn
patch' *first* operates on serialized editor commands and
*second*
sends unidiff to an external program, precisely to avoid
this kind of
problem (when not in dry-run).  But with dry-run, we have a
problem,
iota2 is missing when the external tool is invoked and
expects to see
it.  I would hate to have to execute editor commands as if
it wasn't
dry-run, let the external tool run in dry-run, and then
revert, to
solve this problem.

(Note this isn't a problem when dealing with new files, i.e.
add-file
operation, since GNU patch knows (because of lack of context
I guess)
the file is new and doesn't expect it to be on disk.)

Thoughts?

* Should svn diff --svnpatch imply --no-diff-deleted as
well?

When a file is schedule-delete, either because of a delete
or a move,
the unidiff appears with deletion lines of the file (diffs
against
/dev/null).  Nothing new.  The point is, when applying the
patch,
delete-entry will first remove the file from disk, then the
external
tool will try to apply unidiff and won't find the file and
prompt the
user.  So should we turn on --no-diff-deleted when
--svnpatch is?

Help welcome 

Charles

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

"Charles Acknin" <charlesackningmail.com> writes:
> I seem to meet lots of unexpected traps with 'svn
patch' these days,
> especially with copy and move operations since this is
what I've been
> working on more or less recently.  I have two of them
in mind:
>
> * dry-run patching with copy and move operations
>
> This one is rather because of the way we use GNU patch.
 Basically,
> 'svn patch' calls the external tool on any given
output.  That is,
> even when dealing with copy and move operations, which
the tool
> doesn't understand/speak.  Consider the following
unidiff patch
> created after iota2 was copied from iota (svnpatch
bytes aren't
> displayed):
>
> [[[
> Index: iota2
>
============================================================
=======
> --- iota2       (revision 1)
> +++ iota2       (working copy)
>  -1 +1,2 
>  This is the file 'iota'.
> +bla
> ]]]
>
> Now run 'svn patch --dry-run patchfile' (or even 'patch
-p0 --dry-run
> < patchfile'), and GNU patch bails out it can't find
such a file.
> Why?  Because it's dry-run, and thus 'svn patch' didn't
copy the file
> it would have copied if not in dry-run mode.  Let's
remind that 'svn
> patch' *first* operates on serialized editor commands
and *second*
> sends unidiff to an external program, precisely to
avoid this kind of
> problem (when not in dry-run).  But with dry-run, we
have a problem,
> iota2 is missing when the external tool is invoked and
expects to see
> it.  I would hate to have to execute editor commands as
if it wasn't
> dry-run, let the external tool run in dry-run, and then
revert, to
> solve this problem.
>
> (Note this isn't a problem when dealing with new files,
i.e. add-file
> operation, since GNU patch knows (because of lack of
context I guess)
> the file is new and doesn't expect it to be on disk.)
>
> Thoughts?

Ugh :-(.  That's a tough one.  The simplest answer for now
might be to
notice whether there were copies/moves during the dry run,
trap the
error from the external patch program, see if the error
applies to any
of the putatively copied/moved items, and transform it into
an
"application deferred" warning instead.  I realize
that's lame, but
it's the best I can come up with right now.

-K

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On Sun, 21 Oct 2007, Karl Fogel wrote:

> "Charles Acknin" <charlesackningmail.com> writes:
> > I seem to meet lots of unexpected traps with 'svn
patch' these days,
> > especially with copy and move operations since
this is what I've been
> > working on more or less recently.  I have two of
them in mind:
> >
> > * dry-run patching with copy and move operations
> >
> > This one is rather because of the way we use GNU
patch.  Basically,
> > 'svn patch' calls the external tool on any given
output.  That is,
> > even when dealing with copy and move operations,
which the tool
> > doesn't understand/speak.  Consider the following
unidiff patch
> > created after iota2 was copied from iota (svnpatch
bytes aren't
> > displayed):
> >
> > [[[
> > Index: iota2
> >
============================================================
=======
> > --- iota2       (revision 1)
> > +++ iota2       (working copy)
> >  -1 +1,2 
> >  This is the file 'iota'.
> > +bla
> > ]]]
> >
> > Now run 'svn patch --dry-run patchfile' (or even
'patch -p0 --dry-run
> > < patchfile'), and GNU patch bails out it can't
find such a file.
> > Why?  Because it's dry-run, and thus 'svn patch'
didn't copy the file
> > it would have copied if not in dry-run mode. 
Let's remind that 'svn
> > patch' *first* operates on serialized editor
commands and *second*
> > sends unidiff to an external program, precisely to
avoid this kind of
> > problem (when not in dry-run).  But with dry-run,
we have a problem,
> > iota2 is missing when the external tool is invoked
and expects to see
> > it.  I would hate to have to execute editor
commands as if it wasn't
> > dry-run, let the external tool run in dry-run, and
then revert, to
> > solve this problem.
> >
> > (Note this isn't a problem when dealing with new
files, i.e. add-file
> > operation, since GNU patch knows (because of lack
of context I guess)
> > the file is new and doesn't expect it to be on
disk.)
> >
> > Thoughts?
> 
> Ugh :-(.  That's a tough one.  The simplest answer for
now might be to
> notice whether there were copies/moves during the dry
run, trap the
> error from the external patch program, see if the error
applies to any
> of the putatively copied/moved items, and transform it
into an
> "application deferred" warning instead.  I
realize that's lame, but
> it's the best I can come up with right now.

This is a similar approach to what we use for dry-run
merges.  It's lame,
but better than nothing.
-- 

Daniel Rall

On 10/22/07, Charles Acknin <charlesackningmail.com> wrote:
> On 10/22/07, Daniel L. Rall <dlrfinemaltcoding.com> wrote:
> > > Ugh :-(.  That's a tough one.  The simplest
answer for now might be to
> > > notice whether there were copies/moves during
the dry run, trap the
> > > error from the external patch program, see if
the error applies to any
> > > of the putatively copied/moved items, and
transform it into an
> > > "application deferred" warning
instead.  I realize that's lame, but
> > > it's the best I can come up with right now.
> >
> > This is a similar approach to what we use for
dry-run merges.  It's lame,
> > but better than nothing.
>
> That sounds very tricky.  For example, GNU patch
prompts the user for
> a direction to take when such thing happen.  Does that
mean we're also
> going to wrap the program's input stream and answer on
behalf of the
> user when the prompting questions match a particular
pattern?  That
> sounds like a lot of code specific to one external
program and again
> very hairy.
>
> How about temporarily spawning the files the external
tool needs for
> the time it runs?  That could involve spawning a whole
tree.

Writing to the wc during --dry-run seems like a bad idea. 
I'd rather
drop dry-run functionality than need to do this.  (In fact
in the
interest of actually finishing this increasingly hairy
feature, maybe
you should punt --dry-run until later?)

--dave

-- 
David Glasser | glasserdavidglasser.net | http://www.davidglasser.
net/

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On 10/23/07, Daniel Rall <dlrcollab.net> wrote:
> On Mon, 22 Oct 2007, David Glasser wrote:
> > Writing to the wc during --dry-run seems like a
bad idea.  I'd rather
> > drop dry-run functionality than need to do this. 
(In fact in the
> > interest of actually finishing this increasingly
hairy feature, maybe
> > you should punt --dry-run until later?)
>
> That sounds okay to me, too (I had actually been
thinking the same thing).

Ok let's put dry-run back.  We can still leave it ON for
the
serialized commands though.  I'm just going to disable the
call to the
external tool when in dry-run mode.  How does that sound? 
(I have to
admit, a bit like a half-baked feature, but this saves some
code
refactoring and API changes.)

Charles

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org