Hi Brane,
Branko Čibej wrote:
> Wolfgang Stengel wrote:
>> Hello Subversion team,
>>
>> please consinder the attached patch. Some file systems (for example NFS)
>> produce a different error code on Windows than that which is handled by
>> APR natively. This results in a number of problems with Subversion, for
>> example the function svn_wc_check_wc() does not fail correctly on
>> inexistant files.
>>
>> The attached patch provides a simple workaround. It mappes the
>> error code ERROR_DIRECTORY to the error code ERROR_PATH_NOT_FOUND which
>> can be caught by the APR_STATUS_IS_ENOTDIR() macro. The patch also
>> checks if the constant ERROR_DIRECTORY even exists and if the problem
>> in APR has been fixed in the meantime (by checking if
>> APR_STATUS_IS_ENOENT() or APR_STATUS_IS_ENOTDIR() already catch
>> ERROR_DIRECTORY).
>>
>> Thank you for your opinions on this.
>>
>> Wolfgang
>
> This code belongs in APR, not in Subversion; in the apr_errno.h header.
> Turns out that it's a very, very old omission, since way back in 2002,
> see the tread starting with
> http://mail-archives.apache.org/mod_mbox/apr-dev/200204.mbox/%3C5.1.0.14.2.20020416163613.02327e58@pop3.rowe-clan.net%3E
I've seen this thread also, and because it's so old I assumed the
chances of APR getting fixed anytime soon are slim to none.
> However ...
>
>> subversion/libsvn_subr/error.c
>> =======================================
>> * subversion/libsvn_subr/error.c
>> (make_error_internal): Replace ERROR_DIRECTORY error code by
>> ERROR_PATH_NOT_FOUND so that it can be detected by APR_STATUS_IS_ENOTDIR.
>> ]]]
>> Index: subversion/libsvn_subr/error.c
>> ===================================================================
>>
>> --- subversion/libsvn_subr/error.c (revision 39738)
>> +++ subversion/libsvn_subr/error.c (working copy)
>> @@ -88,6 +88,12 @@
>> abort();
>> }
>> +#ifdef ERROR_DIRECTORY
>> + /* Workaround for missing ERROR_DIRECTORY equivalent in APR. */
>> + if (apr_err == (APR_OS_START_SYSERR + ERROR_DIRECTORY) && !
>> APR_STATUS_IS_ENOENT(apr_err) && ! APR_STATUS_IS_ENOTDIR(apr_err))
>> + apr_err = APR_OS_START_SYSERR + ERROR_PATH_NOT_FOUND;
>
> ... this last line is quite wrong, and not just because
> ERROR_PATH_NOT_FOUND is already part of APR_STATUS_IS_ENOENT.
That was the idea, to return ERROR_PATH_NOT_FOUND instead of
ERROR_DIRECTORY (which is not recognized by any of the macros).
The two other conditions are intended to ensure that this
workaround only takes action if ERROR_DIRECTORY is not already
part of APR_STATUS_IS_ENOENT or APR_STATUS_IS_ENOTDIR (in case APR
gets fixed).
Anyway, do you think there's any chance to get an improved version
of this patch (but with the same basic idea) applied or will I
have better luck talking to the APR team?
Wolfgang
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402627
Received on 2009-10-01 19:30:38 CEST