You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Näslund <da...@longitudo.com> on 2009/08/14 09:14:13 UTC
[PATCH v2] Use context_t in svn_wc_cleanup3
On Thu, Aug 13, 2009 at 10:03:11AM -0500, Hyrum K. Wright wrote:
> > Index: subversion/include/svn_wc.h
> ===================================================================
> svn_error_t *
> +svn_wc_cleanup3(const char *abspath,
>
> We've been calling these "local_abspath" in general, to distinguish
> from a repos_abspath.
I've changed the names.
> + * @deprecated Provided for backward compability with the 1.2 API.
>
> Keep the @since tag here.
Done
> You want to manually destroy the wc_ctx here, and use svn_error_return
> (see other examples in deprecated.c).
I wonder what I was thinking here? Something vague about the scrope of a
scratch_pool? Changed it anyway.
> Is cleanup_internal() fetching an absolute path internally? Can it be
> simplified now that we guarantee it's getting an absolute path?
There is no logic in update_internal for checking absolute paths.
> }
> Index: subversion/libsvn_client/cleanup.c
> ===================================================================
>
> Not a bit deal, but probably save this last bit for a separate patch.
I will do that.
A strange thing: After running make check I get 18 FAILS:s:
trunk> cat tests.log | grep ^FAIL
FAIL: depth_tests.py 32: make sure update handle svn_depth_exclude properly
FAIL: log_tests.py 18: test 'svn log -g' on a non-branching revision
FAIL: log_tests.py 19: test 'svn log -g' a path added before merge
FAIL: log_tests.py 20: test log -c for a single change
FAIL: log_tests.py 22: test log -c on comma-separated list of changes
FAIL: log_tests.py 23: svn log of two wc paths is disallowed
FAIL: log_tests.py 24: test revprop retrieval
FAIL: log_tests.py 25: log --xml escapes non-utf8 data (issue #2866)
FAIL: log_tests.py 26: 'svn log -g target_with_bogus_mergeinfo'
FAIL: log_tests.py 27: log -g and explicit mergeinfo replacing inherited
FAIL: log_tests.py 28: log -g and simple propmod to merge-inheriting path
FAIL: merge_tests.py 1: performing a merge, with mixed results
FAIL: merge_tests.py 2: merge and add new files/dirs with history
FAIL: merge_tests.py 3: merge that deletes items
FAIL: merge_tests.py 4: some simple property merges
FAIL: merge_tests.py 5: merging a file w/no explicit target path using -r
FAIL: switch_tests.py 18: switch shouldn't allow changing repos root
FAIL: switch_tests.py 21: forced switch detects tree conflicts
But when running ./log_tests.py and ./merge_tests.py I get no FAIL:s.
How can that be? Does this mean that those are failing or not?
But theese three tests still fail, both on trunk and with my patch
applied.
depth_tests.py 23: mkae sure update handle svn_depth_exclude properly
swithch_tests.py 18
switch_tests.py 21
Can someone confirm that depth_tests.py 23 is failing on trunk? I assume
that the two switch_tests haven't been fixed since yesterday.
Assuming that the patch is ok, what could be a good next step? Could any
of theese work or do you have better suggestions from the top of your
head?
Theese are called in svn_wc_cleanup3:
* subversion/libsvn_wc/log.c
(cleanup_internal): Uses adm_access_t. Should those be changed?
(run_existing_logs): Uses adm_access_t.
* subversion/libsvn_wc/lock.c
(svn_wc__adm_steal_write_lock): Uses adm_access_t
The next untouched function in the file:
* subversion/include/svn_wc.h
(svn_wc_revert3): This feels heavy. Uses adm_access_t
Mvh
Daniel
Re: [PATCH v2] Use context_t in svn_wc_cleanup3
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On Aug 14, 2009, at 4:14 AM, Daniel Näslund wrote:
> On Thu, Aug 13, 2009 at 10:03:11AM -0500, Hyrum K. Wright wrote:
>>> Index: subversion/include/svn_wc.h
>> ===================================================================
>> svn_error_t *
>> +svn_wc_cleanup3(const char *abspath,
>>
>> We've been calling these "local_abspath" in general, to distinguish
>> from a repos_abspath.
>
> I've changed the names.
>
>> + * @deprecated Provided for backward compability with the 1.2 API.
>>
>> Keep the @since tag here.
>
> Done
>
>> You want to manually destroy the wc_ctx here, and use
>> svn_error_return
>> (see other examples in deprecated.c).
>
> I wonder what I was thinking here? Something vague about the scrope
> of a
> scratch_pool? Changed it anyway.
>
>> Is cleanup_internal() fetching an absolute path internally? Can it
>> be
>> simplified now that we guarantee it's getting an absolute path?
>
> There is no logic in update_internal for checking absolute paths.
>
>> }
>> Index: subversion/libsvn_client/cleanup.c
>> ===================================================================
>>
>> Not a bit deal, but probably save this last bit for a separate patch.
>
> I will do that.
>
>
> A strange thing: After running make check I get 18 FAILS:s:
>
> trunk> cat tests.log | grep ^FAIL
> FAIL: depth_tests.py 32: make sure update handle svn_depth_exclude
> properly
> FAIL: log_tests.py 18: test 'svn log -g' on a non-branching revision
> FAIL: log_tests.py 19: test 'svn log -g' a path added before merge
> FAIL: log_tests.py 20: test log -c for a single change
> FAIL: log_tests.py 22: test log -c on comma-separated list of changes
> FAIL: log_tests.py 23: svn log of two wc paths is disallowed
> FAIL: log_tests.py 24: test revprop retrieval
> FAIL: log_tests.py 25: log --xml escapes non-utf8 data (issue #2866)
> FAIL: log_tests.py 26: 'svn log -g target_with_bogus_mergeinfo'
> FAIL: log_tests.py 27: log -g and explicit mergeinfo replacing
> inherited
> FAIL: log_tests.py 28: log -g and simple propmod to merge-
> inheriting path
> FAIL: merge_tests.py 1: performing a merge, with mixed results
> FAIL: merge_tests.py 2: merge and add new files/dirs with history
> FAIL: merge_tests.py 3: merge that deletes items
> FAIL: merge_tests.py 4: some simple property merges
> FAIL: merge_tests.py 5: merging a file w/no explicit target path
> using -r
> FAIL: switch_tests.py 18: switch shouldn't allow changing repos root
> FAIL: switch_tests.py 21: forced switch detects tree conflicts
Those are all consecutive tests. Where you doing something
(rebuilding the binary or a dependency) on the system at the time
which would case the svn binary to be unavailable?
In any case, I don't get these locally, except for depth 18, which
currently fails on a clean working copy.
> But when running ./log_tests.py and ./merge_tests.py I get no FAIL:s.
> How can that be? Does this mean that those are failing or not?
>
> But theese three tests still fail, both on trunk and with my patch
> applied.
> depth_tests.py 23: mkae sure update handle svn_depth_exclude properly
> swithch_tests.py 18
> switch_tests.py 21
>
> Can someone confirm that depth_tests.py 23 is failing on trunk? I
> assume
> that the two switch_tests haven't been fixed since yesterday.
Confirmed (see above).
> Assuming that the patch is ok, what could be a good next step? Could
> any
> of theese work or do you have better suggestions from the top of your
> head?
You can now update any callers to use the new API. On a more general
node, I'm busy removing calls to svn_wc_entry() in libsvn_client. The
pattern should be obvious enough, so pick a location, and just dive in.
> Theese are called in svn_wc_cleanup3:
> * subversion/libsvn_wc/log.c
> (cleanup_internal): Uses adm_access_t. Should those be changed?
> (run_existing_logs): Uses adm_access_t.
>
> * subversion/libsvn_wc/lock.c
> (svn_wc__adm_steal_write_lock): Uses adm_access_t
>
> The next untouched function in the file:
> * subversion/include/svn_wc.h
> (svn_wc_revert3): This feels heavy. Uses adm_access_t
I made a few changes in parameter ordering and the way you construct
the "dumby" working copy context in the deprecated wrapper. You may
want to take a look as hints for next time. Overall, the patch looks
good, and I committed it in r38752.
Thanks!
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2383662
New log message
Posted by Daniel Näslund <da...@longitudo.com>.
I forgot to say that I removed the changes in libsvn_client. Thus, the
new log message:
[[[
Use svn_wc_context_t and absolute paths in svn_wc_cleanup3. Remove
diff3_cmd parameter.
* subversion/include/svn_wc.h
(svn_wc_cleanup3): New.
(svn_wc_cleanup2): Deprecate.
* subversion/libsvn_wc/deprecated.c
(svn_wc_cleanup2): Reinplement as a wrapper.
* subversion/libsvn_wc/log.c
(svn_wc_cleanup3): New. Use absolute paths. No diff3_cmd. Add
svn_wc_context_t parameter
(svn_wc_cleanup2): Remove.
]]]
Mvh
Daniel
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2383577