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
Received on 2009-08-14 17:54:47 CEST