You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2008/11/11 15:08:56 UTC
Re: [Fwd: svn commit: r34133 - in branches/tc-resolve/subversion:
include libsvn_client libsvn_wc]
On Tue, 2008-11-11 at 05:26 +0100, Neels J. Hofmeyr wrote:
> Hey Julian,
>
> I've gotten as far as none of the tree_conflicts tests failing anymore. But
> could you please closely review these two things:
>
> 1
> [[[
> * subversion/libsvn_client/resolved.c
> (svn_client_resolve):
> Get the ADM_ACCESS on the target's parent directory to be able to resolve
> tree-conflicted directories. Extend the ADM_LOCK_LEVEL to still be able
> to lock the target node for text/prop conflicts.
Levels '0' and '1' both need extending, not just '0'. (And levels can go
higher than 1, so 1 can go to 2 and doesn't have to become infinite.)
+ /* In order to resolve tree-conflicts on the target PATH, we need an
+ * adm_access on its parent directory. The lock level then needs to
extend
+ * at least onto the immediate children. */
+ if (adm_lock_level == 0)
+ adm_lock_level = 1;
if (adm_lock_level >= 0)
adm_lock_level++;
+ SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL,
+ svn_path_dirname(path, pool),
(I needed to #include "svn_path.h" for svn_path_dirname().)
TRUE,
adm_lock_level,
ctx->cancel_func, ctx->cancel_baton,
pool));
Then I found that recursive resolves go wrong. After adjusting
commit_tests 60 to work with per-victim resolve, it successfully
resolves when it names the three victims as separate targets, but it
fails in recursive mode:
[[[
svn resolved .../A/D/G -R
svn: warning: Directory '.../A/D/G/G' is missing
]]]
I'll commit this much and then look further.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API design - walk_entries option for tree conflicts [was: svn
commit: r34133 - in branches/tc-resolve/...]
Posted by Julian Foad <ju...@btopenworld.com>.
On Sun, 2008-11-16 at 02:28 +0100, Neels J. Hofmeyr wrote:
> Julian Foad wrote:
> > On Wed, 2008-11-12 at 02:22 +0100, Neels J. Hofmeyr wrote:
> >> about making "info" use svn_wc__walk_entries_and_tc():
> >> svn_wc__walk_entries_and_tc() forces calling svn_wc_walk_entries3() with
> >> SHOW_HIDDEN = TRUE.
> >> However, "info" currently calls svn_wc_walk_entries3() with SHOW_HIDDEN = FALSE.
> >>
> >> My immediate thought would be to have the SHOW_HIDDEN flag in
> >> svn_wc__walk_entries_and_tc()'s signature as well and thus allow both TRUE
> >> and FALSE. However, do you already know it won't work due to design?
> >
> > Yes, that would be nice, and it could be made to work. Most callers of
> > any "walk_entries" function don't want to see hidden nodes. (Just 2 of
> > the approx. 13 callers of svn_wc_walk_entries3() set "show_hidden =
> > TRUE".)
> >
> > I had a go the other day at rewriting this "walk_entries" interface to
> > include all the options as bitwise flags in a single 'options' argument.
> > See the attached patch. Calls would look like:
> >
> > return svn_wc_walk_entries4(path, parent_access,
> > &walk_callbacks, &walk_baton, depth,
> > SVN_WC_WALK_ENTRIES_WITH_TREE_CONFLICTS,
> > cancel_func, cancel_baton, pool);
> >
> > or
> >
> > return svn_wc_walk_entries4(path, parent_access,
> > &walk_callbacks, &walk_baton, depth,
> > SVN_WC_WALK_ENTRIES_WITH_TREE_CONFLICTS
> > | SVN_WC_WALK_ENTRIES_WITH_HIDDEN,
> > cancel_func, cancel_baton, pool);
> >
> > Good?
> >
> > - Julian
>
>
> ...What, this mail is 4 days old?? Sorry for not replying sooner.
> This does look good. I wonder if you already committed it...
> Apparently not.
>
> +1
No, we're keeping it private for now. Might do something like this
if/when we make it public (for 1.7).
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API design - walk_entries option for tree conflicts [was: svn
commit: r34133 - in branches/tc-resolve/...]
Posted by "Neels J. Hofmeyr" <ne...@elego.de>.
Julian Foad wrote:
> On Wed, 2008-11-12 at 02:22 +0100, Neels J. Hofmeyr wrote:
>> about making "info" use svn_wc__walk_entries_and_tc():
>> svn_wc__walk_entries_and_tc() forces calling svn_wc_walk_entries3() with
>> SHOW_HIDDEN = TRUE.
>> However, "info" currently calls svn_wc_walk_entries3() with SHOW_HIDDEN = FALSE.
>>
>> My immediate thought would be to have the SHOW_HIDDEN flag in
>> svn_wc__walk_entries_and_tc()'s signature as well and thus allow both TRUE
>> and FALSE. However, do you already know it won't work due to design?
>
> Yes, that would be nice, and it could be made to work. Most callers of
> any "walk_entries" function don't want to see hidden nodes. (Just 2 of
> the approx. 13 callers of svn_wc_walk_entries3() set "show_hidden =
> TRUE".)
>
> I had a go the other day at rewriting this "walk_entries" interface to
> include all the options as bitwise flags in a single 'options' argument.
> See the attached patch. Calls would look like:
>
> return svn_wc_walk_entries4(path, parent_access,
> &walk_callbacks, &walk_baton, depth,
> SVN_WC_WALK_ENTRIES_WITH_TREE_CONFLICTS,
> cancel_func, cancel_baton, pool);
>
> or
>
> return svn_wc_walk_entries4(path, parent_access,
> &walk_callbacks, &walk_baton, depth,
> SVN_WC_WALK_ENTRIES_WITH_TREE_CONFLICTS
> | SVN_WC_WALK_ENTRIES_WITH_HIDDEN,
> cancel_func, cancel_baton, pool);
>
> Good?
>
> - Julian
...What, this mail is 4 days old?? Sorry for not replying sooner.
This does look good. I wonder if you already committed it...
Apparently not.
+1
~Neels
API design - walk_entries option for tree conflicts [was: svn
commit: r34133 - in branches/tc-resolve/...]
Posted by Julian Foad <ju...@btopenworld.com>.
On Wed, 2008-11-12 at 02:22 +0100, Neels J. Hofmeyr wrote:
> about making "info" use svn_wc__walk_entries_and_tc():
> svn_wc__walk_entries_and_tc() forces calling svn_wc_walk_entries3() with
> SHOW_HIDDEN = TRUE.
> However, "info" currently calls svn_wc_walk_entries3() with SHOW_HIDDEN = FALSE.
>
> My immediate thought would be to have the SHOW_HIDDEN flag in
> svn_wc__walk_entries_and_tc()'s signature as well and thus allow both TRUE
> and FALSE. However, do you already know it won't work due to design?
Yes, that would be nice, and it could be made to work. Most callers of
any "walk_entries" function don't want to see hidden nodes. (Just 2 of
the approx. 13 callers of svn_wc_walk_entries3() set "show_hidden =
TRUE".)
I had a go the other day at rewriting this "walk_entries" interface to
include all the options as bitwise flags in a single 'options' argument.
See the attached patch. Calls would look like:
return svn_wc_walk_entries4(path, parent_access,
&walk_callbacks, &walk_baton, depth,
SVN_WC_WALK_ENTRIES_WITH_TREE_CONFLICTS,
cancel_func, cancel_baton, pool);
or
return svn_wc_walk_entries4(path, parent_access,
&walk_callbacks, &walk_baton, depth,
SVN_WC_WALK_ENTRIES_WITH_TREE_CONFLICTS
| SVN_WC_WALK_ENTRIES_WITH_HIDDEN,
cancel_func, cancel_baton, pool);
Good?
- Julian
Re: [Fwd: svn commit: r34133 - in branches/tc-resolve/subversion:
include libsvn_client libsvn_wc]
Posted by "Neels J. Hofmeyr" <ne...@elego.de>.
Julian,
about making "info" use svn_wc__walk_entries_and_tc():
svn_wc__walk_entries_and_tc() forces calling svn_wc_walk_entries3() with
SHOW_HIDDEN = TRUE.
However, "info" currently calls svn_wc_walk_entries3() with SHOW_HIDDEN = FALSE.
My immediate thought would be to have the SHOW_HIDDEN flag in
svn_wc__walk_entries_and_tc()'s signature as well and thus allow both TRUE
and FALSE. However, do you already know it won't work due to design?
Thanks,
~Neels
Julian Foad wrote:
> On Tue, 2008-11-11 at 05:26 +0100, Neels J. Hofmeyr wrote:
>> Hey Julian,
>>
>> I've gotten as far as none of the tree_conflicts tests failing anymore. But
>> could you please closely review these two things:
>>
>> 1
>> [[[
>> * subversion/libsvn_client/resolved.c
>> (svn_client_resolve):
>> Get the ADM_ACCESS on the target's parent directory to be able to resolve
>> tree-conflicted directories. Extend the ADM_LOCK_LEVEL to still be able
>> to lock the target node for text/prop conflicts.
>
> Levels '0' and '1' both need extending, not just '0'. (And levels can go
> higher than 1, so 1 can go to 2 and doesn't have to become infinite.)
>
> + /* In order to resolve tree-conflicts on the target PATH, we need an
> + * adm_access on its parent directory. The lock level then needs to
> extend
> + * at least onto the immediate children. */
> + if (adm_lock_level == 0)
> + adm_lock_level = 1;
>
> if (adm_lock_level >= 0)
> adm_lock_level++;
>
> + SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL,
> + svn_path_dirname(path, pool),
>
> (I needed to #include "svn_path.h" for svn_path_dirname().)
>
> TRUE,
> adm_lock_level,
> ctx->cancel_func, ctx->cancel_baton,
> pool));
>
>
> Then I found that recursive resolves go wrong. After adjusting
> commit_tests 60 to work with per-victim resolve, it successfully
> resolves when it names the three victims as separate targets, but it
> fails in recursive mode:
>
> [[[
> svn resolved .../A/D/G -R
> svn: warning: Directory '.../A/D/G/G' is missing
> ]]]
>
> I'll commit this much and then look further.
>
> - Julian
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>