On May 26, 2009, at 3:21 PM, C. Michael Pilato wrote:
> Saw some "Aborted" message today trying to use a 1.6 client on a trunk
> working copy. When I explored the cause, I found a leaked error was
> to
> blame. If you read subversion/libsvn_wc/lock.c:do_open(), you see
> this
> chunk of code (minus the line I'm showing that I added to fix the
> problem).
>
> if (! under_construction)
> {
> err = svn_wc_check_wc(path, &wc_format, subpool);
>
> if (wc_format == 0 || (err && APR_STATUS_IS_ENOENT(err-
> >apr_err)))
> {
> return svn_error_createf(SVN_ERR_WC_NOT_DIRECTORY, err,
> _("'%s' is not a working copy"),
> svn_path_local_style(path, pool));
> }
> + SVN_ERR(err);
> }
>
> Of course, I was working backwards here, finding a bugfix on the
> branch and
> then looking for the same bug in trunk so I can commit the fix there
> and
> suggest backport. What I find is (surprise, surprise) that the WC
> code
> looks drastically different. I'm pretty sure that the matching
> chunk of
> code is this bit from open_single():
>
> SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, scratch_pool));
> err = svn_wc__internal_check_wc(&wc_format, db, local_abspath,
> scratch_pool);
> if (wc_format == 0 || (err && APR_STATUS_IS_ENOENT(err->apr_err)))
> {
> return svn_error_createf(SVN_ERR_WC_NOT_DIRECTORY, err,
> _("'%s' is not a working copy"),
> svn_path_local_style(path,
> scratch_pool));
> }
> svn_error_clear(err);
>
> This code doesn't leak the error like the 1.6.x branch code does,
> though.
> So that's a good thing. But I noticed that rather than raise 'err'
> as I'd
> expect, trunk just clears it with no nearby comment explaining why
> it's okay
> to do that.
>
> Does anyone know the reason why?
That sounds like the work of gstein (who is happily honeymooning right
now). He'd probably know the reason, and will be back sometime next
week, I think.
> What should be done for 1.6.x in terms of fixing this?
The fix seems pretty apparent for 1.6.x. I'd just create a backport
branch, apply the fix to the branch, and then nominate rev X from the
branch for backport.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2354227
Received on 2009-05-26 22:32:00 CEST