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

Re: svn commit: r919460 - filtering svn patch targets

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: Fri, 12 Mar 2010 20:06:35 +0100

On 11.03.2010 23:44, Stefan Sperling wrote:
> On Thu, Mar 11, 2010 at 07:07:04PM +0100, Stefan Küng wrote:
>> On 10.03.2010 23:22, Stefan Sperling wrote:
>>> svn_error_t *
>>> svn_client_patch(const char *abs_patch_path,
>>> const char *local_abspath,
>>> svn_boolean_t dry_run,
>>> int strip_count,
>>> svn_boolean_t reverse,
>>> apr_hash_t **tempfiles,
>>> svn_client_ctx_t *ctx,
>>> apr_pool_t *result_pool,
>>> apr_pool_t *scratch_pool);
>>>
>>> If you pass non-NULL for tempfiles, and TRUE for dry-run, you'd get back
>>> in *tempfiles a mapping {path as in patch file => tempfile containing patched
>>> result}, the tempfiles are left open by svn_client_patch() so you need to
>>> close them, and the working copy is not modified.
>>
>> Why are the files left open? If there's a good reason for leaving
>> them open, then of course I can close them in TSVN first before
>> using them. But it would be nice if the files were already closed.
>
> The tempfiles will eventually need to be deleted.
>
> Right now, the tempfiles will be closed on pool cleanup.
> My idea was to specify svn_io_file_del_on_close instead in case
> tempfiles is not NULL. Which would mean that we should leave them
> open when returning to the caller.
>
> But we can pick either or svn_io_file_del_on_pool_cleanup (depending
> on the result_pool) or svn_io_file_del_on_close. I don't really mind
> either way.
>
> Can you explain why you would want the files already closed? Just curious.

Because I need to open those files in TortoiseMerge again. Most likely
using a different API.

Also, the temp files shouldn't get deleted by the svn library but the
deletion should be left to the client. You don't know in the library
what a client will use the files for, so you should not delete them.

>
>>> What is not possible (without adding the --include-pattern option)
>>> is selecting which files to patch. Is selecting individual patch
>>> targets really that important?
>>
>> Yes, that's very important. I often find that when I get a patch, I
>> only want to use part of it because I found that when reviewing the
>> changes I have to reject some of those changes.
>
> I'm not sure if this use case is worth optimising for.
> You could easily apply the patch, and then selectively revert
> some of the patched files. What you are describing is a special
> case of "I want to apply this patch and also make custom modifications
> to the patched result." Why not just apply the patch and then make
> the necessary modifications? Or request a revised patch from the patch
> submitter?

The include pattern only has to filter by target path, not by individual
change chunks in the patch file.

Imagine you use a library, but you made some modifications to that
library for yourself. Now, the library gets an update and you receive a
patch for that update.
Do you really want to just apply the full patch without first knowing
whether it conflicts with your own modifications? And I'm not talking
here about conflicts that are detectable by svn and would lead to a
conflicted state, but subtle conflicts you would only notice later.
I often have this situation. So I apply the patch only to those files
that I haven't modified myself. Then I review carefully the result of
patching the files I've modified, by first create a temp file that is
the same as when the patch was applied directly to the real file (but
since it's a temp file, the real file isn't changed yet).

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
Received on 2010-03-12 20:07:10 CET

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