You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Wolfgang Stengel <wo...@efactory.de> on 2009/10/01 14:35:53 UTC
[PATCH] Workaround for missing ERROR_DIRECTORY constant in APR
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
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402548
Re: [PATCH] Workaround for missing ERROR_DIRECTORY constant in APR
Posted by Branko Cibej <br...@xbc.nu>.
Bert Huijben wrote:
>> -----Original Message-----
>> From: Wolfgang Stengel [mailto:wolfgang.stengel@efactory.de]
>> Sent: donderdag 1 oktober 2009 16:36
>> To: dev@subversion.tigris.org
>> Subject: [PATCH] Workaround for missing ERROR_DIRECTORY constant in APR
>>
>> 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).
>>
>
> Did you check if this is not just an issue in your NFS driver?
>
> If the issue is fixed there, all application issues disappear at the same
> time.
>
> It would be strange if all applications have to apply a fix, because a
> specific network driver causes an error.
>
> What would happen if your video or soundcard driver did the same thing?
> Should Subversion change to support your hardware or should the driver
> properly support the OS?
>
In fact, APR never maps the ERROR_DIRECTORY code; so even if it's a
driver problem, the issue in APR is real.
-- Brane
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2403261
RE: [PATCH] Workaround for missing ERROR_DIRECTORY constant in APR
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Wolfgang Stengel [mailto:wolfgang.stengel@efactory.de]
> Sent: donderdag 1 oktober 2009 16:36
> To: dev@subversion.tigris.org
> Subject: [PATCH] Workaround for missing ERROR_DIRECTORY constant in APR
>
> 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).
Did you check if this is not just an issue in your NFS driver?
If the issue is fixed there, all application issues disappear at the same
time.
It would be strange if all applications have to apply a fix, because a
specific network driver causes an error.
What would happen if your video or soundcard driver did the same thing?
Should Subversion change to support your hardware or should the driver
properly support the OS?
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2403253
Re: [PATCH] Workaround for missing ERROR_DIRECTORY constant in APR
Posted by Branko Cibej <br...@xbc.nu>.
If you're building everything from source, you could try this patch for
APR -- it should apply unchanged to trunk, 1.3 and 1.4; possibly older
versions too, but I haven't checked.
http://svn.apache.org/viewvc/apr/apr/trunk/include/apr_errno.h?r1=659285&r2=821306&pathrev=821306
-- Brane
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
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2403203
Re: [PATCH] Workaround for missing ERROR_DIRECTORY constant in APR
Posted by Branko Cibej <br...@xbc.nu>.
Wolfgang Stengel wrote:
> Hi Brane,
[...]
> 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?
I quite frankly can't remember why I dropped this issue way back in
2002; thank google for reminding me that I'd run across it before.
You know what ... I'll just fix APR, this is certainly a bug.
-- Brane
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402649
Re: [PATCH] Workaround for missing ERROR_DIRECTORY constant in APR
Posted by Wolfgang Stengel <wo...@efactory.de>.
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
Re: [PATCH] Workaround for missing ERROR_DIRECTORY constant in APR
Posted by Branko Cibej <br...@xbc.nu>.
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
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.
> +#endif
> +
> /* Create the new error structure */
> new_error = apr_pcalloc(pool, sizeof(*new_error));
-- Brane
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402611