You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Danil Shopyrin <da...@visualsvn.com> on 2008/09/10 10:37:21 UTC
[PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Hi!
Here is the patch that is trying to fix on of the problem discussed at
http://svn.haxx.se/dev/archive-2008-08/0793.shtml
[[
2) Mergeinfo produced during WC-to-WC copy differs from equivalent
REPO-to-REPO copy. If you copy file in working copy and there is no
mergeinfo in the working copy then the new file will get an empty
mergeinfo record. Similar REPO-to-REPO copy operation doesn't produce
empty mergeinfo record.
]]
The general idea of the patch is as follows: we don't produce any
mergeinfo record if we [definitely] know that it's unnecessary. The
patch should ease the pain in the most cases. For example, it's not
needed to worry about mergeinfo in the cases when:
a. copy is performed within a working copy (it's just a 'local file
rename because of refactoring')
b. there is no merges except ones on the trunk/branches level (or
there is no merges at all)
The log messages is as follows:
[[[
Don't create any explicit (including empty) mergeinfo record during WC-to-WC
copy if it's a 'mergeinfo NON-affecting' copy. WC-to-WC copy can be
considered as 'megeinfo NON-affecting' if and only if the following
conditions are satisfied:
a) src and dst are located below the same URL
b) mergeinfo (if any) for src and dst is inherited from this URL
c) inherited mergeinfos (if any) for src and dst are equal.
* subversion/libsvn_client/copy.c
(calculate_target_mergeinfo): svn_client__get_wc_mergeinfo() signature is
changed.
(check_mergeinfo_affecting): Heuristic algorihthm that checks is the copy
operation 'mergeinfo affectiong' or not.
(propagate_mergeinfo_within_wc): Mergeinfo isn't propagated if
check_mergeinfo_affecting() returns FALSE.
* subversion/libsvn_client/merge.c
(update_wc_mergeinfo,
get_mergeinfo_paths,
process_children_with_new_mergeinfo,
do_directory_merge): svn_client__get_wc_mergeinfo() signature is changed.
* subversion/libsvn_client/mergeinfo.c
(svn_client__get_wc_mergeinfo): Signature is changed. Now returns
effective_mergeinfo and effective_url and accepts additional
allow_mixed_revisions flag.
(svn_client__get_wc_or_repos_mergeinfo,
svn_client__elide_mergeinfo): svn_client__get_wc_mergeinfo() signature is
changed.
* subversion/libsvn_client/mergeinfo.h
(svn_client__get_wc_mergeinfo): Signature is changed. Now returns
effective_mergeinfo and effective_url and accepts additional
allow_mixed_revisions flag.
* subversion/tests/cmdline/copy_tests.py
(copy_replace_with_props,
copy_added_paths_with_props,
copy_peg_rev_local_files,
copy_peg_rev_local_dirs): New copy behavior is considered: empty
mergeinfo isn't generated during mergeinfo non-affecting copy.
(no_empty_mergeinfo_on_local_copy_file,
no_empty_mergeinfo_on_local_copy_dir,
local_copy_with_explicit_mergeinfo,
local_mergeinfo_non_affecting_copy,
non_local_copy_between_wc,
non_local_copy_into_switched,
non_local_copy_into_nested,
no_empty_mergeinfo_in_mixed_revision_wc): New tests.
(branch_change_and_merge_back): Helper for the new tests.
(test_list): Run the new tests.
]]]
The patch itself is in attachment.
--
With best regards,
Danil Shopyrin
VisualSVN Team
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Mark Phippard <ma...@gmail.com>.
On Wed, Sep 10, 2008 at 9:13 AM, Vlad Georgescu <vg...@gmail.com> wrote:
> 2008/9/10 Danil Shopyrin <da...@visualsvn.com>:
>> Hi!
>>
>> Here is the patch that is trying to fix on of the problem discussed at
>> http://svn.haxx.se/dev/archive-2008-08/0793.shtml
>> [[
>> 2) Mergeinfo produced during WC-to-WC copy differs from equivalent
>> REPO-to-REPO copy. If you copy file in working copy and there is no
>> mergeinfo in the working copy then the new file will get an empty
>> mergeinfo record. Similar REPO-to-REPO copy operation doesn't produce
>> empty mergeinfo record.
>> ]]
>>
>> The general idea of the patch is as follows: we don't produce any
>> mergeinfo record if we [definitely] know that it's unnecessary. The
>> patch should ease the pain in the most cases. For example, it's not
>> needed to worry about mergeinfo in the cases when:
>> a. copy is performed within a working copy (it's just a 'local file
>> rename because of refactoring')
>> b. there is no merges except ones on the trunk/branches level (or
>> there is no merges at all)
>>
>
> The patch looks correct, but I think it's more complicated than it
> needs to be. I'd rather not put the extra checks in a different
> function, and I don't think you need to extend
> svn_client__get_wc_mergeinfo().
>
> In propagate_mergeinfo_within_wc(), we need to do the following:
>
> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
> we're done, there's nothing to propagate, because the destination
> already has the same explicit mergeinfo as the source, as a result of
> the copy. Currently, propagate_mergeinfo_within_wc() calls
> calculate_target_mergeinfo(), we can replace this with a call to
> svn_client__get_wc_mergeinfo().
> 2. Figure out which path the destination inherits mergeinfo from, if any.
> 3. If the source and destination inherit from the same parent path (or
> no path at all), we're done.
> 4. Add the mergeinfo to the destination.
>
> If we want to be really smart, we can add a check between steps 1 and
> 2 to see if the source and destination are in the same directory, and
> bail out if they are.
+1. I think Vlad described the desired "logic" to use here perfectly.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by "C. Michael Pilato" <cm...@collab.net>.
Vlad Georgescu wrote:
> On Wed, Sep 10, 2008 at 5:57 PM, Paul Burba <pt...@gmail.com> wrote:
[...]
>> How do we account for mixed-revision working copies? Do we give up
>> and set empty mergeinfo in that case or do you think it can be worked
>> around?
>
> I ... don't know. I'll have to think about this.
I spent some time thinking about this earlier today, actually. My gut
instinct tells me that if we don't consider mixed revisions, we're doing
something wrong. But after twenty minutes, I couldn't find a solid recipe
that demonstrated a real problem. I remain pretty confident that there are
snares in this jungle -- I just haven't yet stepped into one.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Vlad Georgescu <vg...@gmail.com>.
On Wed, Sep 10, 2008 at 5:57 PM, Paul Burba <pt...@gmail.com> wrote:
> On Wed, Sep 10, 2008 at 9:13 AM, Vlad Georgescu <vg...@gmail.com> wrote:
[...]
>> In propagate_mergeinfo_within_wc(), we need to do the following:
>>
>> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
>> we're done, there's nothing to propagate, because the destination
>> already has the same explicit mergeinfo as the source, as a result of
>> the copy.
>
> Hi Vlad,
>
> Agreed, but doesn't this already work this way?
As far as I can tell from looking at the code, we simply read the
source's mergeinfo and add it to the destination's (thus causing the
destination's mergeinfo to be needlessly rewritten with the same value
if the source had explicit mergeinfo).
>
>> Currently, propagate_mergeinfo_within_wc() calls
>> calculate_target_mergeinfo(), we can replace this with a call to
>> svn_client__get_wc_mergeinfo().
>> 2. Figure out which path the destination inherits mergeinfo from, if any.
>
> How do we account for mixed-revision working copies? Do we give up
> and set empty mergeinfo in that case or do you think it can be worked
> around?
I ... don't know. I'll have to think about this.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Danil Shopyrin <da...@visualsvn.com>.
> But I wonder how it's really important to have absolutely correct
> mergeinfo on the copied file or folder (except such cases when we make
> local branch etc). What is the use case when things become wrong
> because of the fact that we rely on a mixed-revision working copy
> (with the original patch) or on a not-up-to-date working copy (in the
> original trunk)? And how empty mergeinfo is better than null-mergeinfo
> in that case?
Ping?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Danil Shopyrin <da...@visualsvn.com>.
> While on that topic: Danil - I noticed in your original patch
> svn_client__get_wc_mergeinfo was tweaked to support inheritance of
> mergeinfo across mixed-revision boundaries. It seems to me we might
> easily inherit incorrect mergeinfo if doing this, am I missing
> something?
First of all, it surprising how easy we can get mixed-revision working
copy. It appears just after plain commit. User needs to update after
every commit to have his copy always in the single-revision state.
Maybe it's a good practice but I don't think that majority of users
follow this way. So we can treat the most of real working copies as a
mixed-revision ones.
Other question is how we can rely on any mergeinfo stored in the
working copy. For example, we copy file1.txt to the dir1\file1.txt.
There is no of any mergeinfo in the working copy and we don't record
anything for the new dir1\file1.txt. But dir1 have some explicit
mergeinfo in the repository (as a result of cherry picking, for
example). And we will get incorrect mergeinfo for the new
dir1\file1.txt when the working copy will be updated.
But I wonder how it's really important to have absolutely correct
mergeinfo on the copied file or folder (except such cases when we make
local branch etc). What is the use case when things become wrong
because of the fact that we rely on a mixed-revision working copy
(with the original patch) or on a not-up-to-date working copy (in the
original trunk)? And how empty mergeinfo is better than null-mergeinfo
in that case?
--
Danil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Paul Burba <pt...@gmail.com>.
On Wed, Sep 10, 2008 at 9:13 AM, Vlad Georgescu <vg...@gmail.com> wrote:
> Hi,
>
> 2008/9/10 Danil Shopyrin <da...@visualsvn.com>:
>> Hi!
>>
>> Here is the patch that is trying to fix on of the problem discussed at
>> http://svn.haxx.se/dev/archive-2008-08/0793.shtml
>> [[
>> 2) Mergeinfo produced during WC-to-WC copy differs from equivalent
>> REPO-to-REPO copy. If you copy file in working copy and there is no
>> mergeinfo in the working copy then the new file will get an empty
>> mergeinfo record. Similar REPO-to-REPO copy operation doesn't produce
>> empty mergeinfo record.
>> ]]
>>
>> The general idea of the patch is as follows: we don't produce any
>> mergeinfo record if we [definitely] know that it's unnecessary. The
>> patch should ease the pain in the most cases. For example, it's not
>> needed to worry about mergeinfo in the cases when:
>> a. copy is performed within a working copy (it's just a 'local file
>> rename because of refactoring')
>> b. there is no merges except ones on the trunk/branches level (or
>> there is no merges at all)
>>
>
> The patch looks correct, but I think it's more complicated than it
> needs to be. I'd rather not put the extra checks in a different
> function, and I don't think you need to extend
> svn_client__get_wc_mergeinfo().
>
> In propagate_mergeinfo_within_wc(), we need to do the following:
>
> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
> we're done, there's nothing to propagate, because the destination
> already has the same explicit mergeinfo as the source, as a result of
> the copy.
Hi Vlad,
Agreed, but doesn't this already work this way?
> Currently, propagate_mergeinfo_within_wc() calls
> calculate_target_mergeinfo(), we can replace this with a call to
> svn_client__get_wc_mergeinfo().
> 2. Figure out which path the destination inherits mergeinfo from, if any.
How do we account for mixed-revision working copies? Do we give up
and set empty mergeinfo in that case or do you think it can be worked
around?
While on that topic: Danil - I noticed in your original patch
svn_client__get_wc_mergeinfo was tweaked to support inheritance of
mergeinfo across mixed-revision boundaries. It seems to me we might
easily inherit incorrect mergeinfo if doing this, am I missing
something?
Paul
> 3. If the source and destination inherit from the same parent path (or
> no path at all), we're done.
> 4. Add the mergeinfo to the destination.
>
> If we want to be really smart, we can add a check between steps 1 and
> 2 to see if the source and destination are in the same directory, and
> bail out if they are.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Paul Burba <pt...@gmail.com>.
2008/9/30 Danil Shopyrin <da...@visualsvn.com>:
> Ping?
>
> Here is the up-to-date version of the patch (there are no changes
> except conflicts resolving with the current trunk).
This topic of this thread was taken up elsewhere, see
http://svn.haxx.se/dev/archive-2008-11/0432.shtml. In a nutshell,
WC-to-WC copies/moves never set mergeinfo on the destination unless
the source had explicit mergeinfo.
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Danil Shopyrin <da...@visualsvn.com>.
Ping?
Here is the up-to-date version of the patch (there are no changes
except conflicts resolving with the current trunk).
Log message is:
[[[
Don't create any explicit (including empty) mergeinfo record during WC-to-WC
copy if it's a 'mergeinfo NON-affecting' copy. WC-to-WC copy can be
considered as 'megeinfo NON-affecting' if and only if the following
conditions are satisfied:
a) src and dst are located below the same URL
b) mergeinfo (if any) for src and dst is inherited from this URL
c) inherited mergeinfos (if any) for src and dst are equal.
* subversion/libsvn_client/copy.c
(calculate_target_mergeinfo): svn_client__get_wc_mergeinfo() signature is
changed.
(propagate_mergeinfo_within_wc): a) Heuristic algorihthm that checks is the
copy operation 'mergeinfo affectiong' or not is added. Mergeinfo isn't
propagated if it's 'mergeinfo NON-affecting' copy. b) Now this function makes
no difference between copying of added and copied files. The generic
algorithm handles both cases universally.
* subversion/libsvn_client/merge.c
(update_wc_mergeinfo,
get_mergeinfo_paths,
process_children_with_new_mergeinfo,
do_directory_merge): svn_client__get_wc_mergeinfo() signature is changed.
* subversion/libsvn_client/mergeinfo.c
(svn_client__get_wc_mergeinfo): Signature is changed. Now returns
original_mergeinfo and original_mergeinfo_url and accepts additional
allow_mixed_revisions flag.
(svn_client__get_wc_or_repos_mergeinfo,
svn_client__elide_mergeinfo): svn_client__get_wc_mergeinfo() signature is
changed.
* subversion/libsvn_client/mergeinfo.h
(svn_client__get_wc_mergeinfo): Signature is changed. Now returns
original_mergeinfo and original_mergeinfo_url and accepts additional
allow_mixed_revisions flag.
* subversion/tests/cmdline/copy_tests.py
(copy_replace_with_props,
copy_added_paths_with_props,
copy_peg_rev_local_files,
copy_peg_rev_local_dirs): New copy behavior is considered: empty
mergeinfo isn't generated during mergeinfo non-affecting copy.
(no_empty_mergeinfo_on_local_copy_file,
no_empty_mergeinfo_on_local_copy_dir,
local_copy_with_explicit_mergeinfo,
local_mergeinfo_non_affecting_copy,
non_local_copy_between_wc,
non_local_copy_into_switched,
non_local_copy_into_nested,
no_empty_mergeinfo_in_mixed_revision_wc,
copy_src_explicit_mergeinfo,
copy_added_file_with_empty_explicit_mergeinfo): New tests.
(branch_change_and_merge_back): Helper for the new tests.
(test_list): Run the new tests.
]]]
The patch itself is in the attachment.
--
Danil
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Danil Shopyrin <da...@visualsvn.com>.
Here is the updated patch.
There are only the following changes:
1)
>> The word "effective" does not convey the meaning of "original (not
>> adjusted)". "original" might be better.
>
> Agreed. I'll update the patch.
I've renamed effective_mergeinfo to original_mergeinfo and
effective_url to original_mergeinfo_url. All dependent entities are
renamed too.
2)
>>> + Set *EFFECTIVE_URL to the last url scanned for mergeinfo. It can be the
>>> + url of WCPATH (in the case of explicit mergeinfo) or one of its ancestors
>>> + (in the case of inherited mergeinfo).
>>> +
>>> + Don't climb through mixed revision working copy if ALLOW_MIXED_REVISIONS
>>> + is FALSE.
>>
>> If it does climb through a mixed-revision working copy, that contradicts
>> a paragraph higher up in the doc string (not visible in this diff). If
>> it doesn't climb through a mixed-revision working copy, what does it do?
>
> I agreed that the updated docstring is inconsistent. I'll fix that.
This docstring is fixed.
3)
The logic of propagate_mergeinfo_within_wc() is changed. Now this
function makes no difference between copying of added and copied
files. The generic algorithm handles both cases universally. The new
test is added.
The new version of the patch works like the originally posted version.
The logic was broken in the second version of the patch.
4)
Some minor formatting changes.
The log message is as follows:
[[[
Don't create any explicit (including empty) mergeinfo record during WC-to-WC
copy if it's a 'mergeinfo NON-affecting' copy. WC-to-WC copy can be
considered as 'megeinfo NON-affecting' if and only if the following
conditions are satisfied:
a) src and dst are located below the same URL
b) mergeinfo (if any) for src and dst is inherited from this URL
c) inherited mergeinfos (if any) for src and dst are equal.
* subversion/libsvn_client/copy.c
(calculate_target_mergeinfo): svn_client__get_wc_mergeinfo() signature is
changed.
(propagate_mergeinfo_within_wc): a) Heuristic algorihthm that checks is the
copy operation 'mergeinfo affectiong' or not is added. Mergeinfo isn't
propagated if it's 'mergeinfo NON-affecting' copy. b) Now this function makes
no difference between copying of added and copied files. The generic
algorithm handles both cases universally.
* subversion/libsvn_client/merge.c
(update_wc_mergeinfo,
get_mergeinfo_paths,
process_children_with_new_mergeinfo,
do_directory_merge): svn_client__get_wc_mergeinfo() signature is changed.
* subversion/libsvn_client/mergeinfo.c
(svn_client__get_wc_mergeinfo): Signature is changed. Now returns
original_mergeinfo and original_mergeinfo_url and accepts additional
allow_mixed_revisions flag.
(svn_client__get_wc_or_repos_mergeinfo,
svn_client__elide_mergeinfo): svn_client__get_wc_mergeinfo() signature is
changed.
* subversion/libsvn_client/mergeinfo.h
(svn_client__get_wc_mergeinfo): Signature is changed. Now returns
original_mergeinfo and original_mergeinfo_url and accepts additional
allow_mixed_revisions flag.
* subversion/tests/cmdline/copy_tests.py
(copy_replace_with_props,
copy_added_paths_with_props,
copy_peg_rev_local_files,
copy_peg_rev_local_dirs): New copy behavior is considered: empty
mergeinfo isn't generated during mergeinfo non-affecting copy.
(no_empty_mergeinfo_on_local_copy_file,
no_empty_mergeinfo_on_local_copy_dir,
local_copy_with_explicit_mergeinfo,
local_mergeinfo_non_affecting_copy,
non_local_copy_between_wc,
non_local_copy_into_switched,
non_local_copy_into_nested,
no_empty_mergeinfo_in_mixed_revision_wc,
copy_src_explicit_mergeinfo,
copy_added_file_with_empty_explicit_mergeinfo): New tests.
(branch_change_and_merge_back): Helper for the new tests.
(test_list): Run the new tests.
]]]
The new version of the patch is attached.
--
Danil
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Danil Shopyrin <da...@visualsvn.com>.
On Tue, Sep 16, 2008 at 5:47 PM, Julian Foad <ju...@btopenworld.com> wrote:
> I'm not following this thread, but here are some observations.
>
> On Tue, 2008-09-16 at 15:12 +0400, Danil Shopyrin wrote:
>> Here is the improved version of the original patch. Generally spoken,
>> the song remains the same. There are only following changes:
>> 1) things become more optimized:
>> a) we don't event call svn_client__get_wc_mergeinfo() for dst if src has
>> explicit mergeinfo set on it
>> b) we don't call svn_client__get_wc_mergeinfo() second time for src if it's
>> a 'mergeinfo affecting' copy
>> 2) there is one more test
>> 3) propagate_mergeinfo_within_wc()'s logic modified more carefully. We
>> don't check for 'mergeinfo NON-affecting' copy for a locally
>> added/replaced PAIR->src without history.
>>
>> The new log message is as follows:
>> [[[
>> Don't create any explicit (including empty) mergeinfo record during WC-to-WC
>> copy if it's a 'mergeinfo NON-affecting' copy. WC-to-WC copy can be
>> considered as 'megeinfo NON-affecting' if and only if the following
>> conditions are satisfied:
>> a) src and dst are located below the same URL
>> b) mergeinfo (if any) for src and dst is inherited from this URL
>> c) inherited mergeinfos (if any) for src and dst are equal.
>
> I noticed that those conditions sound a bit redundant and not
> crystal-clear. As I understand it, mergeinfo can only be inherited
> as-is, not with modifications. Therefore doesn't (c) follow necessarily
> from (b)? (And (a) is true for all nodes in a repository, and only
> relevant as the referent of condition (b).)
The c) condition can be untrue if WC-to-WC copy is performed between
two different working copies. For example, the destination working
copy has some merged (but uncommited) revisions.
> Would the conditions be better said as:
>
> - The destination node must inherit its mergeinfo from the same node
> as the source node did.
> - To know this, it is sufficient to determine that both chains of
> inheritance pass through a common node.
See comments above.
>> Index: subversion/libsvn_client/mergeinfo.h
>> ===================================================================
>> --- subversion/libsvn_client/mergeinfo.h (revision 33072)
>> +++ subversion/libsvn_client/mergeinfo.h (working copy)
>> @@ -83,7 +83,19 @@
>> (ignored if NULL) or beyond any switched path.
>>
>> Set *WALKED_PATH to the path climbed from WCPATH to find inherited
>> - mergeinfo, or "" if none was found. (ignored if NULL). */
>> + mergeinfo, or "" if none was found. (ignored if NULL).
>
> (I know you didn't change that sentence, but I'll note in passing that
> it's not clear to me how a "path climbed" would be represented in a
> string.)
Actually, WALKED_PATH isn't used anywhere. I plan to remove this
parameter after this patch will be committed (if it will be).
>> + Set *EFFECTIVE_MERGEINFO to the original (not adjusted) mergeinfo found on
>> + WCPATH or one of its ancestors. Set *EFFECTIVE_MERGEINFO to NULL if there is
>> + no mergeinfo found. (Ignore if EFFECTIVE_MERGEINFO is NULL).
>
> The word "effective" does not convey the meaning of "original (not
> adjusted)". "original" might be better.
Agreed. I'll update the patch.
>> + Set *EFFECTIVE_URL to the last url scanned for mergeinfo. It can be the
>> + url of WCPATH (in the case of explicit mergeinfo) or one of its ancestors
>> + (in the case of inherited mergeinfo).
>> +
>> + Don't climb through mixed revision working copy if ALLOW_MIXED_REVISIONS
>> + is FALSE.
>
> If it does climb through a mixed-revision working copy, that contradicts
> a paragraph higher up in the doc string (not visible in this diff). If
> it doesn't climb through a mixed-revision working copy, what does it do?
I agreed that the updated docstring is inconsistent. I'll fix that.
>> + */
>> svn_error_t *
>> svn_client__get_wc_mergeinfo(svn_mergeinfo_t *mergeinfo,
>> svn_boolean_t *inherited,
>> @@ -93,7 +105,10 @@
>> const char *wcpath,
>> const char *limit_path,
>> const char **walked_path,
>> + svn_mergeinfo_t *effective_mergeinfo,
>> + const char **effective_url,
>> svn_wc_adm_access_t *adm_access,
>> + svn_boolean_t allow_mixed_revisions,
>> svn_client_ctx_t *ctx,
>> apr_pool_t *pool);
>
> I wonder if this API is being made more complex than is necessary. Might
> it be better to split it, because the other callers don't want these new
> facilities? (Do you really need to know the original mergeinfo? If not,
> that would simplify the patch a bit too.)
See my paragraph above for why we need to compare original (effective)
mergeinfos. The API wasn't split to keep WC climbing logic in one
function (and not duplicate it).
--
Danil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Julian Foad <ju...@btopenworld.com>.
I'm not following this thread, but here are some observations.
On Tue, 2008-09-16 at 15:12 +0400, Danil Shopyrin wrote:
> Here is the improved version of the original patch. Generally spoken,
> the song remains the same. There are only following changes:
> 1) things become more optimized:
> a) we don't event call svn_client__get_wc_mergeinfo() for dst if src has
> explicit mergeinfo set on it
> b) we don't call svn_client__get_wc_mergeinfo() second time for src if it's
> a 'mergeinfo affecting' copy
> 2) there is one more test
> 3) propagate_mergeinfo_within_wc()'s logic modified more carefully. We
> don't check for 'mergeinfo NON-affecting' copy for a locally
> added/replaced PAIR->src without history.
>
> The new log message is as follows:
> [[[
> Don't create any explicit (including empty) mergeinfo record during WC-to-WC
> copy if it's a 'mergeinfo NON-affecting' copy. WC-to-WC copy can be
> considered as 'megeinfo NON-affecting' if and only if the following
> conditions are satisfied:
> a) src and dst are located below the same URL
> b) mergeinfo (if any) for src and dst is inherited from this URL
> c) inherited mergeinfos (if any) for src and dst are equal.
I noticed that those conditions sound a bit redundant and not
crystal-clear. As I understand it, mergeinfo can only be inherited
as-is, not with modifications. Therefore doesn't (c) follow necessarily
from (b)? (And (a) is true for all nodes in a repository, and only
relevant as the referent of condition (b).)
Would the conditions be better said as:
- The destination node must inherit its mergeinfo from the same node
as the source node did.
- To know this, it is sufficient to determine that both chains of
inheritance pass through a common node.
> Index: subversion/libsvn_client/mergeinfo.h
> ===================================================================
> --- subversion/libsvn_client/mergeinfo.h (revision 33072)
> +++ subversion/libsvn_client/mergeinfo.h (working copy)
> @@ -83,7 +83,19 @@
> (ignored if NULL) or beyond any switched path.
>
> Set *WALKED_PATH to the path climbed from WCPATH to find inherited
> - mergeinfo, or "" if none was found. (ignored if NULL). */
> + mergeinfo, or "" if none was found. (ignored if NULL).
(I know you didn't change that sentence, but I'll note in passing that
it's not clear to me how a "path climbed" would be represented in a
string.)
> + Set *EFFECTIVE_MERGEINFO to the original (not adjusted) mergeinfo found on
> + WCPATH or one of its ancestors. Set *EFFECTIVE_MERGEINFO to NULL if there is
> + no mergeinfo found. (Ignore if EFFECTIVE_MERGEINFO is NULL).
The word "effective" does not convey the meaning of "original (not
adjusted)". "original" might be better.
> + Set *EFFECTIVE_URL to the last url scanned for mergeinfo. It can be the
> + url of WCPATH (in the case of explicit mergeinfo) or one of its ancestors
> + (in the case of inherited mergeinfo).
> +
> + Don't climb through mixed revision working copy if ALLOW_MIXED_REVISIONS
> + is FALSE.
If it does climb through a mixed-revision working copy, that contradicts
a paragraph higher up in the doc string (not visible in this diff). If
it doesn't climb through a mixed-revision working copy, what does it do?
> + */
> svn_error_t *
> svn_client__get_wc_mergeinfo(svn_mergeinfo_t *mergeinfo,
> svn_boolean_t *inherited,
> @@ -93,7 +105,10 @@
> const char *wcpath,
> const char *limit_path,
> const char **walked_path,
> + svn_mergeinfo_t *effective_mergeinfo,
> + const char **effective_url,
> svn_wc_adm_access_t *adm_access,
> + svn_boolean_t allow_mixed_revisions,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool);
I wonder if this API is being made more complex than is necessary. Might
it be better to split it, because the other callers don't want these new
facilities? (Do you really need to know the original mergeinfo? If not,
that would simplify the patch a bit too.)
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Danil Shopyrin <da...@visualsvn.com>.
Here is the improved version of the original patch. Generally spoken,
the song remains the same. There are only following changes:
1) things become more optimized:
a) we don't event call svn_client__get_wc_mergeinfo() for dst if src has
explicit mergeinfo set on it
b) we don't call svn_client__get_wc_mergeinfo() second time for src if it's
a 'mergeinfo affecting' copy
2) there is one more test
3) propagate_mergeinfo_within_wc()'s logic modified more carefully. We
don't check for 'mergeinfo NON-affecting' copy for a locally
added/replaced PAIR->src without history.
The new log message is as follows:
[[[
Don't create any explicit (including empty) mergeinfo record during WC-to-WC
copy if it's a 'mergeinfo NON-affecting' copy. WC-to-WC copy can be
considered as 'megeinfo NON-affecting' if and only if the following
conditions are satisfied:
a) src and dst are located below the same URL
b) mergeinfo (if any) for src and dst is inherited from this URL
c) inherited mergeinfos (if any) for src and dst are equal.
* subversion/libsvn_client/copy.c
(calculate_target_mergeinfo): svn_client__get_wc_mergeinfo() signature is
changed.
(propagate_mergeinfo_within_wc): Heuristic algorihthm that checks is the copy
operation 'mergeinfo affectiong' or not is added. Mergeinfo isn't propagated
if it's 'mergeinfo NON-affecting' copy.
* subversion/libsvn_client/merge.c
(update_wc_mergeinfo,
get_mergeinfo_paths,
process_children_with_new_mergeinfo,
do_directory_merge): svn_client__get_wc_mergeinfo() signature is changed.
* subversion/libsvn_client/mergeinfo.c
(svn_client__get_wc_mergeinfo): Signature is changed. Now returns
effective_mergeinfo and effective_url and accepts additional
allow_mixed_revisions flag.
(svn_client__get_wc_or_repos_mergeinfo,
svn_client__elide_mergeinfo): svn_client__get_wc_mergeinfo() signature is
changed.
* subversion/libsvn_client/mergeinfo.h
(svn_client__get_wc_mergeinfo): Signature is changed. Now returns
effective_mergeinfo and effective_url and accepts additional
allow_mixed_revisions flag.
* subversion/tests/cmdline/copy_tests.py
(copy_replace_with_props,
copy_added_paths_with_props,
copy_peg_rev_local_files,
copy_peg_rev_local_dirs): New copy behavior is considered: empty
mergeinfo isn't generated during mergeinfo non-affecting copy.
(no_empty_mergeinfo_on_local_copy_file,
no_empty_mergeinfo_on_local_copy_dir,
local_copy_with_explicit_mergeinfo,
local_mergeinfo_non_affecting_copy,
non_local_copy_between_wc,
non_local_copy_into_switched,
non_local_copy_into_nested,
no_empty_mergeinfo_in_mixed_revision_wc,
copy_src_explicit_mergeinfo): New tests.
(branch_change_and_merge_back): Helper for the new tests.
(test_list): Run the new tests.
]]]
The new patch is in the attachment.
--
Danil
On Wed, Sep 10, 2008 at 8:44 PM, Danil Shopyrin <da...@visualsvn.com> wrote:
>> Sure. My point was that if check_mergeinfo_affecting() returns FALSE,
>> then we later redo some of the work that it does (like getting
>> mergeinfo for the source). I'd like to see the
>> avoid-setting-empty-mergeinfo logic merged into
>> propagate_mergeinfo_within_wc(), not kept separately.
>
> I can provide improved patch.
>
>>>> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
>>>> we're done, there's nothing to propagate, because the destination
>>>> already has the same explicit mergeinfo as the source, as a result of
>>>> the copy. Currently, propagate_mergeinfo_within_wc() calls
>>>> calculate_target_mergeinfo(), we can replace this with a call to
>>>> svn_client__get_wc_mergeinfo().
>>>
>>> We don't need this step. This check is performed 'automatically'.
>>> Source and destination receive mergeinfo from different urls if the
>>> source have explicit mergeinfo.
>>
>> You read both the source and the target's mergeinfo unconditionally.
>> If the source has explicit mergeinfo, you don't have to read the
>> destination's mergeinfo.
>
> I agree that logic can be optimized (it haven't done to keep the patch
> as simple as possible). I can improve the patch (especially if the
> whole concept will be approved :).
>
> --
> Danil
>
--
With best regards,
Danil Shopyrin
VisualSVN Team
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Danil Shopyrin <da...@visualsvn.com>.
> Sure. My point was that if check_mergeinfo_affecting() returns FALSE,
> then we later redo some of the work that it does (like getting
> mergeinfo for the source). I'd like to see the
> avoid-setting-empty-mergeinfo logic merged into
> propagate_mergeinfo_within_wc(), not kept separately.
I can provide improved patch.
>>> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
>>> we're done, there's nothing to propagate, because the destination
>>> already has the same explicit mergeinfo as the source, as a result of
>>> the copy. Currently, propagate_mergeinfo_within_wc() calls
>>> calculate_target_mergeinfo(), we can replace this with a call to
>>> svn_client__get_wc_mergeinfo().
>>
>> We don't need this step. This check is performed 'automatically'.
>> Source and destination receive mergeinfo from different urls if the
>> source have explicit mergeinfo.
>
> You read both the source and the target's mergeinfo unconditionally.
> If the source has explicit mergeinfo, you don't have to read the
> destination's mergeinfo.
I agree that logic can be optimized (it haven't done to keep the patch
as simple as possible). I can improve the patch (especially if the
whole concept will be approved :).
--
Danil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Vlad Georgescu <vg...@gmail.com>.
On Wed, Sep 10, 2008 at 7:18 PM, Danil Shopyrin <da...@visualsvn.com> wrote:
>> The patch looks correct, but I think it's more complicated than it
>> needs to be. I'd rather not put the extra checks in a different
>> function, and I don't think you need to extend
>> svn_client__get_wc_mergeinfo().
>
>> In propagate_mergeinfo_within_wc(), we need to do the following:
>
> Looks like that the proposed patch does exactly the same things. But
> with a little bit more sophisticated logic. For example, copy between
> different working copies is also handled.
Sure. My point was that if check_mergeinfo_affecting() returns FALSE,
then we later redo some of the work that it does (like getting
mergeinfo for the source). I'd like to see the
avoid-setting-empty-mergeinfo logic merged into
propagate_mergeinfo_within_wc(), not kept separately.
>
>> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
>> we're done, there's nothing to propagate, because the destination
>> already has the same explicit mergeinfo as the source, as a result of
>> the copy. Currently, propagate_mergeinfo_within_wc() calls
>> calculate_target_mergeinfo(), we can replace this with a call to
>> svn_client__get_wc_mergeinfo().
>
> We don't need this step. This check is performed 'automatically'.
> Source and destination receive mergeinfo from different urls if the
> source have explicit mergeinfo.
You read both the source and the target's mergeinfo unconditionally.
If the source has explicit mergeinfo, you don't have to read the
destination's mergeinfo.
>
>> 2. Figure out which path the destination inherits mergeinfo from, if any.
>
> The proposed patch relies on urls, not on paths. There are some
> problems with absolute and relative paths etc. And I don't want to
> duplicate svn_client__get_wc_mergeinfo()'s sophisticated working copy
> climbing logic. That's why svn_client__get_wc_mergeinfo() is tweaked.
I agree that using URL's is better than paths. I hadn't thought of
inter-WC copies and other weird situations when I wrote that. So +1
on extending svn_client__get_wc_mergeinfo().
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Danil Shopyrin <da...@visualsvn.com>.
> The patch looks correct, but I think it's more complicated than it
> needs to be. I'd rather not put the extra checks in a different
> function, and I don't think you need to extend
> svn_client__get_wc_mergeinfo().
> In propagate_mergeinfo_within_wc(), we need to do the following:
Looks like that the proposed patch does exactly the same things. But
with a little bit more sophisticated logic. For example, copy between
different working copies is also handled.
> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
> we're done, there's nothing to propagate, because the destination
> already has the same explicit mergeinfo as the source, as a result of
> the copy. Currently, propagate_mergeinfo_within_wc() calls
> calculate_target_mergeinfo(), we can replace this with a call to
> svn_client__get_wc_mergeinfo().
We don't need this step. This check is performed 'automatically'.
Source and destination receive mergeinfo from different urls if the
source have explicit mergeinfo.
> 2. Figure out which path the destination inherits mergeinfo from, if any.
The proposed patch relies on urls, not on paths. There are some
problems with absolute and relative paths etc. And I don't want to
duplicate svn_client__get_wc_mergeinfo()'s sophisticated working copy
climbing logic. That's why svn_client__get_wc_mergeinfo() is tweaked.
> 3. If the source and destination inherit from the same parent path (or
> no path at all), we're done.
> 4. Add the mergeinfo to the destination.
--
Danil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Vlad Georgescu <vg...@gmail.com>.
On Wed, Sep 10, 2008 at 6:11 PM, Fyodor Sheremetyev
<fy...@visualsvn.com> wrote:
> On Wed, Sep 10, 2008 at 6:35 PM, Vlad Georgescu <vg...@gmail.com> wrote:
>> On Wed, Sep 10, 2008 at 5:27 PM, C. Michael Pilato <cm...@collab.net> wrote:
>>> Vlad Georgescu wrote:
>>>> 2. Figure out which path the destination inherits mergeinfo from, if any.
>>>
>>> ...making sure to honor the wc-to-wc copy's promise not to talk to the
>>> repository. If, in this scenario, you don't find any inheritable
>>> mergeinfo in a working copy parent, you must not assume that *no*
>>> parent has mergeinfo (since maybe you only checked out /trunk/project
>>> but it's /trunk that carries the mergeinfo). Unless you can otherwise
>>> determine that's safe not to do so in this case, you must continue to
>>> set the empty mergeinfo property on the copy destination.
>>
>> We don't need to know exactly which path the destination or the source
>> inherit from. If neither inherits from a path in the WC, it means that
>> either:
>> a) there's really no inheritable mergeinfo at all, so we shouldn't set
>> empty mergeinfo
>> b) there's mergeinfo in a path above the wc root, but in this case
>> both the source and the destination inherit it, so there's no need to
>> set empty mergeinfo.
>
> c) source and destination are from different working copies.
>
Yes. In this case svn_client__get_wc_mergeinfo() can return the last
path that was checked for mergeinfo. If the paths differ and are not
in the same directory, then we give up and set empty mergeinfo.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Fyodor Sheremetyev <fy...@visualsvn.com>.
On Wed, Sep 10, 2008 at 6:35 PM, Vlad Georgescu <vg...@gmail.com> wrote:
> On Wed, Sep 10, 2008 at 5:27 PM, C. Michael Pilato <cm...@collab.net> wrote:
>> Vlad Georgescu wrote:
>>> 2. Figure out which path the destination inherits mergeinfo from, if any.
>>
>> ...making sure to honor the wc-to-wc copy's promise not to talk to the
>> repository. If, in this scenario, you don't find any inheritable
>> mergeinfo in a working copy parent, you must not assume that *no*
>> parent has mergeinfo (since maybe you only checked out /trunk/project
>> but it's /trunk that carries the mergeinfo). Unless you can otherwise
>> determine that's safe not to do so in this case, you must continue to
>> set the empty mergeinfo property on the copy destination.
>
> We don't need to know exactly which path the destination or the source
> inherit from. If neither inherits from a path in the WC, it means that
> either:
> a) there's really no inheritable mergeinfo at all, so we shouldn't set
> empty mergeinfo
> b) there's mergeinfo in a path above the wc root, but in this case
> both the source and the destination inherit it, so there's no need to
> set empty mergeinfo.
c) source and destination are from different working copies.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Vlad Georgescu <vg...@gmail.com>.
On Wed, Sep 10, 2008 at 5:27 PM, C. Michael Pilato <cm...@collab.net> wrote:
> Vlad Georgescu wrote:
>> Hi,
>>
>> 2008/9/10 Danil Shopyrin <da...@visualsvn.com>:
>>> Hi!
>>>
>>> Here is the patch that is trying to fix on of the problem discussed at
>>> http://svn.haxx.se/dev/archive-2008-08/0793.shtml
>>> [[
>>> 2) Mergeinfo produced during WC-to-WC copy differs from equivalent
>>> REPO-to-REPO copy. If you copy file in working copy and there is no
>>> mergeinfo in the working copy then the new file will get an empty
>>> mergeinfo record. Similar REPO-to-REPO copy operation doesn't produce
>>> empty mergeinfo record.
>>> ]]
>>>
>>> The general idea of the patch is as follows: we don't produce any
>>> mergeinfo record if we [definitely] know that it's unnecessary. The
>>> patch should ease the pain in the most cases. For example, it's not
>>> needed to worry about mergeinfo in the cases when:
>>> a. copy is performed within a working copy (it's just a 'local file
>>> rename because of refactoring')
>>> b. there is no merges except ones on the trunk/branches level (or
>>> there is no merges at all)
>>>
>>
>> The patch looks correct, but I think it's more complicated than it
>> needs to be. I'd rather not put the extra checks in a different
>> function, and I don't think you need to extend
>> svn_client__get_wc_mergeinfo().
>>
>> In propagate_mergeinfo_within_wc(), we need to do the following:
>>
>> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
>> we're done, there's nothing to propagate, because the destination
>> already has the same explicit mergeinfo as the source, as a result of
>> the copy. Currently, propagate_mergeinfo_within_wc() calls
>> calculate_target_mergeinfo(), we can replace this with a call to
>> svn_client__get_wc_mergeinfo().
>> 2. Figure out which path the destination inherits mergeinfo from, if any.
>
> ...making sure to honor the wc-to-wc copy's promise not to talk to the
> repository. If, in this scenario, you don't find any inheritable
> mergeinfo in a working copy parent, you must not assume that *no*
> parent has mergeinfo (since maybe you only checked out /trunk/project
> but it's /trunk that carries the mergeinfo). Unless you can otherwise
> determine that's safe not to do so in this case, you must continue to
> set the empty mergeinfo property on the copy destination.
We don't need to know exactly which path the destination or the source
inherit from. If neither inherits from a path in the WC, it means that
either:
a) there's really no inheritable mergeinfo at all, so we shouldn't set
empty mergeinfo
b) there's mergeinfo in a path above the wc root, but in this case
both the source and the destination inherit it, so there's no need to
set empty mergeinfo.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Danil Shopyrin <da...@visualsvn.com>.
> Is it possible to do the logic at commit time and discard the property
> if it is not needed?
Mergeinfo is stored in the plain user-visible property. So I think
it's not a good idea to commit changes that somehow differ from
examined ones in the working copy. It's non-deterministic and I don't
expect such behavior from systems like VCS. But it's my own opinion :)
--
Danil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Vlad Georgescu <vg...@gmail.com>.
On Wed, Sep 10, 2008 at 6:54 PM, Vlad Georgescu <vg...@gmail.com> wrote:
> On Wed, Sep 10, 2008 at 5:44 PM, Mark Phippard <ma...@gmail.com> wrote:
>> On Wed, Sep 10, 2008 at 10:37 AM, C. Michael Pilato <cm...@collab.net> wrote:
> [...]
>>>> Is it possible to do the logic at commit time and discard the property
>>>> if it is not needed?
>>>
>>> I trust you mean, "to *also* do the logic at commit time"? We already
>>> discussed how delaying the logic altogether could mean corrupting merges
>>> made into a copied-but-uncommitted directory. And I would suspect that
>>> introducing the commit-time mergeinfo calculation has its own problems (at
>>> the very least, WC schema changes to persist the previously calculated
>>> value, complexity to ensure that we aren't blowing away a user's 'svn merge
>>> --record-only' results, etc.)
>>>
>>> It suspect it can be done. I suspect it comes with a great price of
>>> complexity and special-cased-ness.
>>
>> Vlad seems to have a valid point that this does not even need to be
>> done. But what I was suggesting is that the way I think this works
>> today is that we currently create empty mergeinfo and at commit time
>> some work is done to fill in the mergeinfo with the right values.
>> Maybe I am wrong about that? But what does not happen is any logic to
>> determine that the property was not needed and discard it.
>>
>> The commit time part is just about waiting until we have a repository
>> connection which is why I thought we created the empty mergeinfo in
>> the first place.
>
> Yeah, I was thinking that at commit time, if we detect empty mergeinfo
> on an added-with-history entry we can get the inherited mergeinfo for
> the source and destination and either remove the mergeinfo if they're
> equal or set it to the correct value otherwise.
I realize after rereading cmpilato's mail that it's not as simple as
that. All the more reason to avoid setting empty mergeinfo, then.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Vlad Georgescu <vg...@gmail.com>.
On Wed, Sep 10, 2008 at 5:44 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Wed, Sep 10, 2008 at 10:37 AM, C. Michael Pilato <cm...@collab.net> wrote:
[...]
>>> Is it possible to do the logic at commit time and discard the property
>>> if it is not needed?
>>
>> I trust you mean, "to *also* do the logic at commit time"? We already
>> discussed how delaying the logic altogether could mean corrupting merges
>> made into a copied-but-uncommitted directory. And I would suspect that
>> introducing the commit-time mergeinfo calculation has its own problems (at
>> the very least, WC schema changes to persist the previously calculated
>> value, complexity to ensure that we aren't blowing away a user's 'svn merge
>> --record-only' results, etc.)
>>
>> It suspect it can be done. I suspect it comes with a great price of
>> complexity and special-cased-ness.
>
> Vlad seems to have a valid point that this does not even need to be
> done. But what I was suggesting is that the way I think this works
> today is that we currently create empty mergeinfo and at commit time
> some work is done to fill in the mergeinfo with the right values.
> Maybe I am wrong about that? But what does not happen is any logic to
> determine that the property was not needed and discard it.
>
> The commit time part is just about waiting until we have a repository
> connection which is why I thought we created the empty mergeinfo in
> the first place.
Yeah, I was thinking that at commit time, if we detect empty mergeinfo
on an added-with-history entry we can get the inherited mergeinfo for
the source and destination and either remove the mergeinfo if they're
equal or set it to the correct value otherwise.
It's still worth it to try to avoid setting empty mergeinfo in the
first place, to make commits faster and for people with pre-1.5
servers.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Mark Phippard <ma...@gmail.com>.
On Wed, Sep 10, 2008 at 10:37 AM, C. Michael Pilato <cm...@collab.net> wrote:
> Mark Phippard wrote:
>> On Wed, Sep 10, 2008 at 10:27 AM, C. Michael Pilato <cm...@collab.net> wrote:
>>
>>>> 2008/9/10 Danil Shopyrin <da...@visualsvn.com>:
>>>>> Hi!
>>>>>
>>>>> Here is the patch that is trying to fix on of the problem discussed at
>>>>> http://svn.haxx.se/dev/archive-2008-08/0793.shtml
>>>>> [[
>>>>> 2) Mergeinfo produced during WC-to-WC copy differs from equivalent
>>>>> REPO-to-REPO copy. If you copy file in working copy and there is no
>>>>> mergeinfo in the working copy then the new file will get an empty
>>>>> mergeinfo record. Similar REPO-to-REPO copy operation doesn't produce
>>>>> empty mergeinfo record.
>>>>> ]]
>>>>>
>>>>> The general idea of the patch is as follows: we don't produce any
>>>>> mergeinfo record if we [definitely] know that it's unnecessary. The
>>>>> patch should ease the pain in the most cases. For example, it's not
>>>>> needed to worry about mergeinfo in the cases when:
>>>>> a. copy is performed within a working copy (it's just a 'local file
>>>>> rename because of refactoring')
>>>>> b. there is no merges except ones on the trunk/branches level (or
>>>>> there is no merges at all)
>>>>>
>>>> The patch looks correct, but I think it's more complicated than it
>>>> needs to be. I'd rather not put the extra checks in a different
>>>> function, and I don't think you need to extend
>>>> svn_client__get_wc_mergeinfo().
>>>>
>>>> In propagate_mergeinfo_within_wc(), we need to do the following:
>>>>
>>>> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
>>>> we're done, there's nothing to propagate, because the destination
>>>> already has the same explicit mergeinfo as the source, as a result of
>>>> the copy. Currently, propagate_mergeinfo_within_wc() calls
>>>> calculate_target_mergeinfo(), we can replace this with a call to
>>>> svn_client__get_wc_mergeinfo().
>>>> 2. Figure out which path the destination inherits mergeinfo from, if any.
>>> ...making sure to honor the wc-to-wc copy's promise not to talk to the
>>> repository. If, in this scenario, you don't find any inheritable
>>> mergeinfo in a working copy parent, you must not assume that *no*
>>> parent has mergeinfo (since maybe you only checked out /trunk/project
>>> but it's /trunk that carries the mergeinfo). Unless you can otherwise
>>> determine that's safe not to do so in this case, you must continue to
>>> set the empty mergeinfo property on the copy destination.
>>
>> Is it possible to do the logic at commit time and discard the property
>> if it is not needed?
>
> I trust you mean, "to *also* do the logic at commit time"? We already
> discussed how delaying the logic altogether could mean corrupting merges
> made into a copied-but-uncommitted directory. And I would suspect that
> introducing the commit-time mergeinfo calculation has its own problems (at
> the very least, WC schema changes to persist the previously calculated
> value, complexity to ensure that we aren't blowing away a user's 'svn merge
> --record-only' results, etc.)
>
> It suspect it can be done. I suspect it comes with a great price of
> complexity and special-cased-ness.
Vlad seems to have a valid point that this does not even need to be
done. But what I was suggesting is that the way I think this works
today is that we currently create empty mergeinfo and at commit time
some work is done to fill in the mergeinfo with the right values.
Maybe I am wrong about that? But what does not happen is any logic to
determine that the property was not needed and discard it.
The commit time part is just about waiting until we have a repository
connection which is why I thought we created the empty mergeinfo in
the first place.
Anyway, I prefer Vlad's solution anyway. As I have said many times, I
think we could just chuck out this whole check and never create
mergeinfo and it would be better than what we have today. The logic
Vlad suggested would be the ideal and seems doable.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by "C. Michael Pilato" <cm...@collab.net>.
Mark Phippard wrote:
> On Wed, Sep 10, 2008 at 10:27 AM, C. Michael Pilato <cm...@collab.net> wrote:
>
>>> 2008/9/10 Danil Shopyrin <da...@visualsvn.com>:
>>>> Hi!
>>>>
>>>> Here is the patch that is trying to fix on of the problem discussed at
>>>> http://svn.haxx.se/dev/archive-2008-08/0793.shtml
>>>> [[
>>>> 2) Mergeinfo produced during WC-to-WC copy differs from equivalent
>>>> REPO-to-REPO copy. If you copy file in working copy and there is no
>>>> mergeinfo in the working copy then the new file will get an empty
>>>> mergeinfo record. Similar REPO-to-REPO copy operation doesn't produce
>>>> empty mergeinfo record.
>>>> ]]
>>>>
>>>> The general idea of the patch is as follows: we don't produce any
>>>> mergeinfo record if we [definitely] know that it's unnecessary. The
>>>> patch should ease the pain in the most cases. For example, it's not
>>>> needed to worry about mergeinfo in the cases when:
>>>> a. copy is performed within a working copy (it's just a 'local file
>>>> rename because of refactoring')
>>>> b. there is no merges except ones on the trunk/branches level (or
>>>> there is no merges at all)
>>>>
>>> The patch looks correct, but I think it's more complicated than it
>>> needs to be. I'd rather not put the extra checks in a different
>>> function, and I don't think you need to extend
>>> svn_client__get_wc_mergeinfo().
>>>
>>> In propagate_mergeinfo_within_wc(), we need to do the following:
>>>
>>> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
>>> we're done, there's nothing to propagate, because the destination
>>> already has the same explicit mergeinfo as the source, as a result of
>>> the copy. Currently, propagate_mergeinfo_within_wc() calls
>>> calculate_target_mergeinfo(), we can replace this with a call to
>>> svn_client__get_wc_mergeinfo().
>>> 2. Figure out which path the destination inherits mergeinfo from, if any.
>> ...making sure to honor the wc-to-wc copy's promise not to talk to the
>> repository. If, in this scenario, you don't find any inheritable
>> mergeinfo in a working copy parent, you must not assume that *no*
>> parent has mergeinfo (since maybe you only checked out /trunk/project
>> but it's /trunk that carries the mergeinfo). Unless you can otherwise
>> determine that's safe not to do so in this case, you must continue to
>> set the empty mergeinfo property on the copy destination.
>
> Is it possible to do the logic at commit time and discard the property
> if it is not needed?
I trust you mean, "to *also* do the logic at commit time"? We already
discussed how delaying the logic altogether could mean corrupting merges
made into a copied-but-uncommitted directory. And I would suspect that
introducing the commit-time mergeinfo calculation has its own problems (at
the very least, WC schema changes to persist the previously calculated
value, complexity to ensure that we aren't blowing away a user's 'svn merge
--record-only' results, etc.)
It suspect it can be done. I suspect it comes with a great price of
complexity and special-cased-ness.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Mark Phippard <ma...@gmail.com>.
On Wed, Sep 10, 2008 at 10:27 AM, C. Michael Pilato <cm...@collab.net> wrote:
>> 2008/9/10 Danil Shopyrin <da...@visualsvn.com>:
>>> Hi!
>>>
>>> Here is the patch that is trying to fix on of the problem discussed at
>>> http://svn.haxx.se/dev/archive-2008-08/0793.shtml
>>> [[
>>> 2) Mergeinfo produced during WC-to-WC copy differs from equivalent
>>> REPO-to-REPO copy. If you copy file in working copy and there is no
>>> mergeinfo in the working copy then the new file will get an empty
>>> mergeinfo record. Similar REPO-to-REPO copy operation doesn't produce
>>> empty mergeinfo record.
>>> ]]
>>>
>>> The general idea of the patch is as follows: we don't produce any
>>> mergeinfo record if we [definitely] know that it's unnecessary. The
>>> patch should ease the pain in the most cases. For example, it's not
>>> needed to worry about mergeinfo in the cases when:
>>> a. copy is performed within a working copy (it's just a 'local file
>>> rename because of refactoring')
>>> b. there is no merges except ones on the trunk/branches level (or
>>> there is no merges at all)
>>>
>>
>> The patch looks correct, but I think it's more complicated than it
>> needs to be. I'd rather not put the extra checks in a different
>> function, and I don't think you need to extend
>> svn_client__get_wc_mergeinfo().
>>
>> In propagate_mergeinfo_within_wc(), we need to do the following:
>>
>> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
>> we're done, there's nothing to propagate, because the destination
>> already has the same explicit mergeinfo as the source, as a result of
>> the copy. Currently, propagate_mergeinfo_within_wc() calls
>> calculate_target_mergeinfo(), we can replace this with a call to
>> svn_client__get_wc_mergeinfo().
>> 2. Figure out which path the destination inherits mergeinfo from, if any.
>
> ...making sure to honor the wc-to-wc copy's promise not to talk to the
> repository. If, in this scenario, you don't find any inheritable
> mergeinfo in a working copy parent, you must not assume that *no*
> parent has mergeinfo (since maybe you only checked out /trunk/project
> but it's /trunk that carries the mergeinfo). Unless you can otherwise
> determine that's safe not to do so in this case, you must continue to
> set the empty mergeinfo property on the copy destination.
Is it possible to do the logic at commit time and discard the property
if it is not needed?
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by "C. Michael Pilato" <cm...@collab.net>.
Vlad Georgescu wrote:
> Hi,
>
> 2008/9/10 Danil Shopyrin <da...@visualsvn.com>:
>> Hi!
>>
>> Here is the patch that is trying to fix on of the problem discussed at
>> http://svn.haxx.se/dev/archive-2008-08/0793.shtml
>> [[
>> 2) Mergeinfo produced during WC-to-WC copy differs from equivalent
>> REPO-to-REPO copy. If you copy file in working copy and there is no
>> mergeinfo in the working copy then the new file will get an empty
>> mergeinfo record. Similar REPO-to-REPO copy operation doesn't produce
>> empty mergeinfo record.
>> ]]
>>
>> The general idea of the patch is as follows: we don't produce any
>> mergeinfo record if we [definitely] know that it's unnecessary. The
>> patch should ease the pain in the most cases. For example, it's not
>> needed to worry about mergeinfo in the cases when:
>> a. copy is performed within a working copy (it's just a 'local file
>> rename because of refactoring')
>> b. there is no merges except ones on the trunk/branches level (or
>> there is no merges at all)
>>
>
> The patch looks correct, but I think it's more complicated than it
> needs to be. I'd rather not put the extra checks in a different
> function, and I don't think you need to extend
> svn_client__get_wc_mergeinfo().
>
> In propagate_mergeinfo_within_wc(), we need to do the following:
>
> 1. Get the mergeinfo for the source. If it's explicit, not inherited,
> we're done, there's nothing to propagate, because the destination
> already has the same explicit mergeinfo as the source, as a result of
> the copy. Currently, propagate_mergeinfo_within_wc() calls
> calculate_target_mergeinfo(), we can replace this with a call to
> svn_client__get_wc_mergeinfo().
> 2. Figure out which path the destination inherits mergeinfo from, if any.
...making sure to honor the wc-to-wc copy's promise not to talk to the
repository. If, in this scenario, you don't find any inheritable
mergeinfo in a working copy parent, you must not assume that *no*
parent has mergeinfo (since maybe you only checked out /trunk/project
but it's /trunk that carries the mergeinfo). Unless you can otherwise
determine that's safe not to do so in this case, you must continue to
set the empty mergeinfo property on the copy destination.
> 3. If the source and destination inherit from the same parent path (or
> no path at all), we're done.
> 4. Add the mergeinfo to the destination.
>
> If we want to be really smart, we can add a check between steps 1 and
> 2 to see if the source and destination are in the same directory, and
> bail out if they are.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] no mergeinfo on 'mergeinfo NON-affecting' wc-to-wc copy
Posted by Vlad Georgescu <vg...@gmail.com>.
Hi,
2008/9/10 Danil Shopyrin <da...@visualsvn.com>:
> Hi!
>
> Here is the patch that is trying to fix on of the problem discussed at
> http://svn.haxx.se/dev/archive-2008-08/0793.shtml
> [[
> 2) Mergeinfo produced during WC-to-WC copy differs from equivalent
> REPO-to-REPO copy. If you copy file in working copy and there is no
> mergeinfo in the working copy then the new file will get an empty
> mergeinfo record. Similar REPO-to-REPO copy operation doesn't produce
> empty mergeinfo record.
> ]]
>
> The general idea of the patch is as follows: we don't produce any
> mergeinfo record if we [definitely] know that it's unnecessary. The
> patch should ease the pain in the most cases. For example, it's not
> needed to worry about mergeinfo in the cases when:
> a. copy is performed within a working copy (it's just a 'local file
> rename because of refactoring')
> b. there is no merges except ones on the trunk/branches level (or
> there is no merges at all)
>
The patch looks correct, but I think it's more complicated than it
needs to be. I'd rather not put the extra checks in a different
function, and I don't think you need to extend
svn_client__get_wc_mergeinfo().
In propagate_mergeinfo_within_wc(), we need to do the following:
1. Get the mergeinfo for the source. If it's explicit, not inherited,
we're done, there's nothing to propagate, because the destination
already has the same explicit mergeinfo as the source, as a result of
the copy. Currently, propagate_mergeinfo_within_wc() calls
calculate_target_mergeinfo(), we can replace this with a call to
svn_client__get_wc_mergeinfo().
2. Figure out which path the destination inherits mergeinfo from, if any.
3. If the source and destination inherit from the same parent path (or
no path at all), we're done.
4. Add the mergeinfo to the destination.
If we want to be really smart, we can add a check between steps 1 and
2 to see if the source and destination are in the same directory, and
bail out if they are.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org