[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.

Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2369487
Received on 2009-07-09 22:05:45 CEST

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