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.
You mean "will be deleted 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.
The caller can't (properly) close an open file unless you give the
caller the (open) file handle, not just the file name.
> 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.
If you're going to hand someone an "open file", that either means an
(open) handle to the file, or it means a filename of a file that some
OTHER code path has an open handle to. If the former, where is the file
pointer (beginning, end, undefined)? If the latter, what is the sharing
mode (can the caller also open the file)?
In other words, it's just wierd. Either give the path of a (closed)
file, or give a stream (positioned at the beginning, ready to read
from).
- Julian
> > >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?
>
> Stefan
Received on 2010-03-12 01:07:24 CET