[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: thoughts about svnpatch

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: Thu, 09 Jul 2009 22:05:22 +0200

Stefan Sperling wrote:
> On Mon, Jun 22, 2009 at 10:03:58PM +0200, Stefan Küng wrote:
>> Stefan Sperling wrote:
>>> - a strip-count option (-p) for stripping leading directory
>>> components from target paths specified in the patch file
>> This might even be possible to do automatically: the svn_client_patch()
>> function knows the target path, and it knows the paths inside the
>> svnpatch file. Stripping path parts from the paths in the svnpatch file
>> until it 'matches' should be not very hard to implement?
> I thought about this again. Consider the following scenario:
> A patch wants to modify zzz/foo/x and zzz/bar/blob/y.
> The target working copy contains:
> foo/x
> zzz/bar/blob/y
> So there is not correct set of targets for the entire patch.
> If we start stripping path components we'll end up matching foo/x
> for the first target, and then we'll either find no match for
> zzz/bar/blob/y after stripping zzz from it, or we'll match
> zzz/bar/blob/y without stripping (depending on how we implement it).
> In either case, the patch cannot be applied correctly, so we could
> just say that if we don't find a match for all paths, we don't
> apply the patch at all.

Yes, that's what I thought how it should work:
string path parts from *all* targets and succeed as soon as there's a
match for all targets. If none can be found until there can't be any
part stripped from the target paths anymore, the function should fail.

> But if the user specifies how many components should be stripped,
> there is no ambiguity anymore, and we can allow at least part of
> the patch to be applied correctly:
> -p0 --> skip foo/x, patch zzz/bar/blob/y
> -p1 --> patch foo/x, skip zzz/bar/blob/y
> So I don't want to do it automatically without providing a way to
> override the automatic decision. Because the match can be ambiguous,
> and if the user can't override it then the result will either be
> incorrect or the application will fail entirely. Even if the user
> might in fact be fine with applying just a subset of the patch.
> The user would then rightfully say that Subversion is trying to
> be too smart and is getting in the way rather than being helpful.
> So we'll need a strip count option anyway, regardless of whether
> or not we'll support automatic stripping of path components.

Sure, that would be needed anyway: imagine a patch that adds a new file
(all lines added). That target doesn't exist yet so the automatic part
wouldn't work.

> I'll add a -p flag and make it default to -p0. Then we can see about
> adding some way of stripping path components automatically if you
> really think it's necessary (I don't).

If we have a dry-run option to have the patch function return
information on how many targets fail/succeed, a client could just call
that function repeatedly with -p x until either x results in a full
match or results in empty target paths.

>> TortoiseMerge currently does that before giving up if it can't find a match.
> Right, and does it have the above problem?
> If not, or if you don't think it's a problem, would you be willing
> to implement it for Subversion, too?

Yes, it has that problem. But it first tries without stripping anything.
Only if that fails, it loops through by stripping paths until it finds a
match. But of course, it won't automatically do the patching but shows
the user the list of possible targets and then the user can choose which
paths to actually patch.


Received on 2009-07-09 22:05:45 CEST

This is an archived mail posted to the Subversion Dev mailing list.