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

Re: svn commit: r27222 - branches/svnpatch-diff/subversion/libsvn_client

From: Charles Acknin <charlesacknin_at_gmail.com>
Date: 2007-10-17 00:48:05 CEST

On 10/17/07, David Glasser <glasser@davidglasser.net> wrote:
> create_empty_file doesn't call anything else in this file... maybe
> just move it up itself? (Or does that throw off the organization of
> the whole file?)

I just figured it would be more convenient to have related helper
functions at the same place in the file. get_empty_file() calls
create_empty_file() for example. I'm trying to keep this file as
clear as possible since it contains both diff callbacks (first half)
and editor functions (second half).

Now I'm just realizing the add_file_with_history *is* at the wrong
place, in the midst of diff callbacks although it is called by
merge_file_added(). Going to move it up.

> It doesn't actually look like the local-mods issues are dealt with
> here... or does svn_wc_copy2 handle that? (Is this situation tested?
> In general, are any of the corner cases tested?)

Why? Local-mods implies file-not-deleted implies call to
svn_wc_copy2, which does handle that gracefully. Were you talking
about the docstring or the function body itself?

> > + /* The file we're about to add needs a copyfrom_url along with
> > + * a copyfrom_rev, which we both retrieve through the working
> > + * copy administrative area of the source file. */
> > + SVN_ERR(svn_wc_adm_probe_open3(&copyfrom_access, NULL,
> > + copyfrom_path, FALSE, -1,
> > + patch_b->ctx->cancel_func,
> > + patch_b->ctx->cancel_baton, pool));
> > + SVN_ERR(svn_wc_get_ancestry(&copyfrom_url, &copyfrom_rev,
> > + copyfrom_path, copyfrom_access, pool));
> > + SVN_ERR(svn_wc_add_repos_file2(path, adm_access,
> > + copyfrom_src, NULL,
> > + apr_hash_make(pool), NULL,
>
> Hmm, looks like you're throwing away all the props there... that can't
> be right.

Well not exactly. I'm throwing away the source file's unmodified
properties, which is surely wrong, yes. On the other hand, the patch
does bring modified properties. I'll have to work this a bit more,
the problem is: do we have an API to get only unmodified properties,
so that the file-add would bring unmodified properties and the
svnpatch the modified ones? I wouldn't want the modified property
diff not to show up and be buried in the property file like this is
the case with merge.

> > err = svn_client__wc_delete(mine, parent_access, patch_b->force,
> > - patch_b->dry_run, FALSE, NULL, NULL,
> > + patch_b->dry_run,
> > + has_local_mods ? TRUE : FALSE,
> > + NULL, NULL,
>
> Hmm, so a delete (whether or not in a move) coming in against a file
> with local-mods won't be any sort of conflict, it'll just leave it
> unversioned? I guess that's reasonable?

This is it. dlr and I discussed this issue on IRC a few days ago. We
ended up thinking that would be reasonable to leave the file
unversioned. After all, the file brings user modifications which we
wouldn't want to erase. I think that goes pretty much in the same
direction as sussman's recent work with copyfrom'ed update. dlr said
something that sounds relevant to me: "the user is trying to apply a
patch in a modified working copy, he can't expect it to be in the
exact same state as that of the patch creation".

Charles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Oct 17 00:48:19 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.