On Wed, Nov 28, 2012 at 8:27 PM, C. Michael Pilato <cmpil...@collab.net> wrote:
> On 11/28/2012 03:39 PM, Julian Foad wrote:
>> @@ -2425,10 +2441,8 @@ merge_dir_added(svn_wc_notify_state_t *s
>>> }
>>> else
>>> {
>>> - const char *added_path = apr_pstrdup(merge_b->pool,
>>> - local_abspath);
>>> - apr_hash_set(merge_b->dry_run_added, added_path,
>>> - APR_HASH_KEY_STRING, added_path);
>>> + merge_b->dry_run_last_added_dir =
>>> + apr_pstrdup(merge_b->pool, local_abspath);
>>
>> Oops -- no longer setting the hash here. Maybe use a macro or function to
>> encapsulate both parts of the "registration".
>
> Hrm... the original logic didn't add it to the hash here, either. I seem to
> have made that additional apr_hash_set() accidentally in r1414810. You can
> examine the sum of my changes to this file like so:
>
> svn diff -r1414809:1414880 subversion/libsvn_client/merge.c
>
> Doing so reveals that, in the end, I didn't actually changed the behavior in
> this code section. And yet ... it does make me wonder if this is just some
> latent bug waiting to be revealed. Certainly seems suspect.
>
> Paul: have you any opinions here? To summarize, I'm wondering why, in
> merge_dir_added(), in the switch that begins like so:
>
> /* Switch on the on-disk state of this path */
> switch (kind)
>
> ... the merged addition of a brand new directory earns that directory a
> registration in both the dry_run_last_added_dir slot *and* the dry_run_added
> hash, but a merged directory addition atop an unversioned or previously
> deleted directory only winds up in dry_run_last_added_dir (and *not* the
> dry_run_added hash). Any ideas?
Julian is correct, that's a bug in the original code. We can see the
problem in merge_tests.py 2 if we add some unversioned directories
which obstruct incoming directory adds:
>mkdir A\C\Q A\C\Q2
>svn st
? A\C\Q
? A\C\Q2
>svn merge ^^/A/B/F A\C -r1:2
--- Merging r2 into 'A\C':
A A\C\Q
A A\C\Q2
A A\C\Q\bar
A A\C\Q\bar2
A A\C\foo
A A\C\foo2
--- Recording mergeinfo for merge of r2 into 'A\C':
U A\C
Do the same with a --dry-run and the output is skipped (as per Mike's
earlier problems http://svn.haxx.se/dev/archive-2012-11/0681.shtml):
>svn revert -Rq . & mkdir A\C\Q A\C\Q2
>svn merge ^^/A/B/F A\C -r1:2 --dry-run
--- Merging r2 into 'A\C':
A A\C\Q
A A\C\Q2
Skipped 'A\C\Q\bar'
Skipped 'A\C\Q\bar2'
A A\C\foo
A A\C\foo2
Summary of conflicts:
Skipped paths: 2
Fixed and added test in r1415214.
>>> }
>>> if (state)
>>> *state = svn_wc_notify_state_changed;
>>> @@ -2467,6 +2481,9 @@ merge_dir_added(svn_wc_notify_state_t *s
>>> }
>>> break;
>>> case svn_node_file:
>>> + if (merge_b->dry_run)
>>> + merge_b->dry_run_last_added_dir = NULL;
>>
>> Clearing that path is just an optimization, right? (Also below.)
>
> As far as I can tell, yes. As above, I was merely restoring the behavior to
> what it was before I started mucking with this code at all.
Hmmm, that code goes *way* back. I 'm not sure it's even needed
anymore or if it ever was.
> --
> C. Michael Pilato <cmpil...@collab.net>
> CollabNet <> www.collab.net <> Enterprise Cloud Development
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
