You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Johan Corveleyn <jc...@gmail.com> on 2021/01/26 14:54:04 UTC

Re: Subversion reports status fault on substituted drive

On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn <jc...@gmail.com> wrote:
> On Wed, Dec 23, 2020 at 8:17 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> >
> > Sorry, I'd been pounding on a beta deliverable, it's my bad. This coming week I have cycles to give
> > back to APR, CMake and other projects neglected during this crunch period. I hope an end-of-year
> > (or very early January) release will meet your goals..
>
> Yes, thanks, and no problem. I think that might be just in time, so
> I'm crossing my fingers :-).
>
> (as you might know, Subversion development is going quite slowly these
> days, so it's hard to say when the cycles of the different volunteers
> will align -- it might be end of this year still, but I'm guessing
> it's more likely to be early January too I think ... we're progressing
> with small steps here and there)

Hi again,

Is there any news on this issue? We're getting pretty close to rolling
SVN 1.14.1 (probably end of this week, then we need a week of testing
and signing, during which I might be able to test with a new version
of APR 1.7, if there would be one). Of course, it's not the end of the
world if this wouldn't be included, but it would be nice to have.

Thanks,
-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Mon, Feb 6, 2023 at 2:37 PM Ivan Zhakov <iv...@visualsvn.com> wrote:
>
> On Thu, 16 Jun 2022 at 21:06, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>
>> On Wed, 5 Jan 2022 at 21:37, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>
>>> On Mon, 13 Sept 2021 at 11:57, Johan Corveleyn <jc...@gmail.com> wrote:
>>> >
>>> > On Tue, Sep 7, 2021 at 3:58 PM Johan Corveleyn <jc...@gmail.com> wrote:
>>> > >
>>> > > On Tue, Feb 23, 2021 at 9:59 AM Mariusz W <ma...@gmail.com> wrote:
>>> > > > On 2021/01/26 14:54:04, Johan Corveleyn <jc...@gmail.com> wrote:
>>> > > > > On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn <jc...@gmail.com> wrote:
>>> > > > > > On Wed, Dec 23, 2020 at 8:17 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>>> > > > > > >
>>> > > > > > > Sorry, I'd been pounding on a beta deliverable, it's my bad. This coming week I have cycles to give
>>> > > > > > > back to APR, CMake and other projects neglected during this crunch period. I hope an end-of-year
>>> > > > > > > (or very early January) release will meet your goals..
>>> > > > > >
>>> > > > > > Yes, thanks, and no problem. I think that might be just in time, so
>>> > > > > > I'm crossing my fingers :-).
>>> > > > > >
>>> > > > > > (as you might know, Subversion development is going quite slowly these
>>> > > > > > days, so it's hard to say when the cycles of the different volunteers
>>> > > > > > will align -- it might be end of this year still, but I'm guessing
>>> > > > > > it's more likely to be early January too I think ... we're progressing
>>> > > > > > with small steps here and there)
>>> > > > Hi,
>>> > > > Maybe problem is in this line 627 in filestat.c [1] (changed in revision [2]) mainly by adding APR_FINFO_LINK in line 627
>>> > > >
>>> > > > Diff to prev: [3]
>>> > > > I think that this change is causing that "if" branch is not executed (svn is using APR_FINFO_LINK in call) and in "else" branch we get error from FindFirstFileW [6] because root (e.g. after subst) is ending with "/" or "\" as in example from [4])
>>> > > > The error code 720002 from apr_stat means ERROR_FILE_NOT_FOUND (code 2 (0x2) [5]) from FindFirstFileW plus some adding and multiplication operations in apr layer (last digit in 720002 is code 2).
>>> > > >
>>> > > > [1] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l627
>>> > > > [2] - https://svn.apache.org/r1855950
>>> > > > [3] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?r1=1795930&r2=1855950&pathrev=1855950&diff_format=h
>>> > > > [4] - https://lists.apache.org/thread.html/rd890d13b826e8cd7acaa96769e10e7143b7e35e11c99bcaa1a75d481%40%3Cdev.apr.apache.org%3E
>>> > > > [5] - https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
>>> > > > [6] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l642
>>> > > >
>>> > > > Regards,
>>> > > > Mariusz
>>> > >
>>> > > Thanks for looking into this, Mariusz.
>>> > >
>>> > > I think you might be onto something. I intended to look closer into
>>> > > your diagnosis, and perhaps try to test if changing those lines back
>>> > > to the original would fix it. But I didn't get around to it, sorry.
>>> > >
>>> > > Now that there was talk of an APR 1.7.1 release on this list, it
>>> > > popped back into my memory :-).
>>> > >
>>> > > @apr devs: what do you think about Mariusz pointer towards the
>>> > > possible cause of what we're seeing with SVN on Windows with apr
>>> > > 1.7.0? And is there an easy fix (for instance, just reverting that
>>> > > single hunk), which has a chance of being accepted into 1.7.1 (if so,
>>> > > I'll try to test it on my svn-dev machine, where I could reproduce the
>>> > > issue)? IIUC, right now apr_stat does seem to error out on paths
>>> > > ending with "/" or "\" on Windows, if APR_FINFO_LINK is given (while
>>> > > APR 1.6 did return a sensible result in that case).
>>> > >
>>> > > To be clear, IIUC Mariusz is referring to this hunk in filestat.c in r1855950:
>>> > >
>>> > > [[[
>>> > > @@ -590,7 +664,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
>>> > >          rv = apr_filepath_root(&root, &test, APR_FILEPATH_NATIVE, pool);
>>> > >          isroot = (root && *root && !(*test));
>>> > >
>>> > > -        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
>>> > > APR_FINFO_NAME) || isroot))
>>> > > +        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
>>> > > (APR_FINFO_NAME | APR_FINFO_LINK)) || isroot))
>>> > >          {
>>> > >              /* cannot use FindFile on a Win98 root, it returns \*
>>> > >               * GetFileAttributesExA is not available on Win95
>>> > > ]]]
>>> >
>>> > Oops, I meant (or Mariusz meant) this hunk in filestat.c r1855950:
>>> >
>>> > [[[
>>> >
>>> > @@ -555,7 +624,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
>>> >          if ((rv = utf8_to_unicode_path(wfname, sizeof(wfname)
>>> >                                              / sizeof(apr_wchar_t), fname)))
>>> >              return rv;
>>> > -        if (!(wanted & APR_FINFO_NAME)) {
>>> > +        if (!(wanted & (APR_FINFO_NAME | APR_FINFO_LINK))) {
>>> >              if (!GetFileAttributesExW(wfname, GetFileExInfoStandard,
>>> >                                        &FileInfo.i))
>>> >                  return apr_get_os_error();
>>> >
>>> > ]]]
>>> >
>>> > And yes, if I revert that single hunk, the Subversion problem with
>>> > subst'ed drives on Windows (or working copies on drive roots) is gone!
>>> >
>>> > Of course, I have no idea what other effects this has, but just
>>> > confirming that taking another turn in the above conditional (like it
>>> > was before) makes apr_stat return the same (AFAICS) as in 1.6.5, for
>>> > substed drives or drive roots on Windows.
>>> >
>>>
>>> Hi,
>>>
>>> The problem with apr_stat(APR_FINFO_LINK | APR_FINFO_MIN) should be
>>> fixed by r1896717 [1] in trunk.
>>>
>>> This fix also should resolve performance regression in apr_stat()
>>> in most common cases.
>>>
>>> I plan to backport this fix to APR 1.7.x at some point later.
>>>
>> I've backported the r1896717 fix to 1.8.x branch. Please test and vote.
>>
>
> The change backported to the 1.7.x branch in r1904030 and released as part of APR 1.7.1.
>
> --
> Ivan Zhakov

Yay! Thanks all for your efforts in fixing and backporting this!

I can't test this anytime soon (but will get around to it eventually).
In the meantime I've left a short message on the tortoisesvn-users and
the svn-dev mailinglists where this issue was brought up.

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by Ivan Zhakov via dev <de...@apr.apache.org>.
On Thu, 16 Jun 2022 at 21:06, Ivan Zhakov <iv...@visualsvn.com> wrote:

> On Wed, 5 Jan 2022 at 21:37, Ivan Zhakov <iv...@visualsvn.com> wrote:
>
>> On Mon, 13 Sept 2021 at 11:57, Johan Corveleyn <jc...@gmail.com> wrote:
>> >
>> > On Tue, Sep 7, 2021 at 3:58 PM Johan Corveleyn <jc...@gmail.com>
>> wrote:
>> > >
>> > > On Tue, Feb 23, 2021 at 9:59 AM Mariusz W <ma...@gmail.com> wrote:
>> > > > On 2021/01/26 14:54:04, Johan Corveleyn <jc...@gmail.com> wrote:
>> > > > > On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn <
>> jcorvel@gmail.com> wrote:
>> > > > > > On Wed, Dec 23, 2020 at 8:17 PM William A Rowe Jr <
>> wrowe@rowe-clan.net> wrote:
>> > > > > > >
>> > > > > > > Sorry, I'd been pounding on a beta deliverable, it's my bad.
>> This coming week I have cycles to give
>> > > > > > > back to APR, CMake and other projects neglected during this
>> crunch period. I hope an end-of-year
>> > > > > > > (or very early January) release will meet your goals..
>> > > > > >
>> > > > > > Yes, thanks, and no problem. I think that might be just in
>> time, so
>> > > > > > I'm crossing my fingers :-).
>> > > > > >
>> > > > > > (as you might know, Subversion development is going quite
>> slowly these
>> > > > > > days, so it's hard to say when the cycles of the different
>> volunteers
>> > > > > > will align -- it might be end of this year still, but I'm
>> guessing
>> > > > > > it's more likely to be early January too I think ... we're
>> progressing
>> > > > > > with small steps here and there)
>> > > > Hi,
>> > > > Maybe problem is in this line 627 in filestat.c [1] (changed in
>> revision [2]) mainly by adding APR_FINFO_LINK in line 627
>> > > >
>> > > > Diff to prev: [3]
>> > > > I think that this change is causing that "if" branch is not
>> executed (svn is using APR_FINFO_LINK in call) and in "else" branch we get
>> error from FindFirstFileW [6] because root (e.g. after subst) is ending
>> with "/" or "\" as in example from [4])
>> > > > The error code 720002 from apr_stat means ERROR_FILE_NOT_FOUND
>> (code 2 (0x2) [5]) from FindFirstFileW plus some adding and multiplication
>> operations in apr layer (last digit in 720002 is code 2).
>> > > >
>> > > > [1] -
>> https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l627
>> > > > [2] - https://svn.apache.org/r1855950
>> > > > [3] -
>> https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?r1=1795930&r2=1855950&pathrev=1855950&diff_format=h
>> > > > [4] -
>> https://lists.apache.org/thread.html/rd890d13b826e8cd7acaa96769e10e7143b7e35e11c99bcaa1a75d481%40%3Cdev.apr.apache.org%3E
>> > > > [5] -
>> https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
>> > > > [6] -
>> https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l642
>> > > >
>> > > > Regards,
>> > > > Mariusz
>> > >
>> > > Thanks for looking into this, Mariusz.
>> > >
>> > > I think you might be onto something. I intended to look closer into
>> > > your diagnosis, and perhaps try to test if changing those lines back
>> > > to the original would fix it. But I didn't get around to it, sorry.
>> > >
>> > > Now that there was talk of an APR 1.7.1 release on this list, it
>> > > popped back into my memory :-).
>> > >
>> > > @apr devs: what do you think about Mariusz pointer towards the
>> > > possible cause of what we're seeing with SVN on Windows with apr
>> > > 1.7.0? And is there an easy fix (for instance, just reverting that
>> > > single hunk), which has a chance of being accepted into 1.7.1 (if so,
>> > > I'll try to test it on my svn-dev machine, where I could reproduce the
>> > > issue)? IIUC, right now apr_stat does seem to error out on paths
>> > > ending with "/" or "\" on Windows, if APR_FINFO_LINK is given (while
>> > > APR 1.6 did return a sensible result in that case).
>> > >
>> > > To be clear, IIUC Mariusz is referring to this hunk in filestat.c in
>> r1855950:
>> > >
>> > > [[[
>> > > @@ -590,7 +664,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
>> > >          rv = apr_filepath_root(&root, &test, APR_FILEPATH_NATIVE,
>> pool);
>> > >          isroot = (root && *root && !(*test));
>> > >
>> > > -        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
>> > > APR_FINFO_NAME) || isroot))
>> > > +        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
>> > > (APR_FINFO_NAME | APR_FINFO_LINK)) || isroot))
>> > >          {
>> > >              /* cannot use FindFile on a Win98 root, it returns \*
>> > >               * GetFileAttributesExA is not available on Win95
>> > > ]]]
>> >
>> > Oops, I meant (or Mariusz meant) this hunk in filestat.c r1855950:
>> >
>> > [[[
>> >
>> > @@ -555,7 +624,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
>> >          if ((rv = utf8_to_unicode_path(wfname, sizeof(wfname)
>> >                                              / sizeof(apr_wchar_t),
>> fname)))
>> >              return rv;
>> > -        if (!(wanted & APR_FINFO_NAME)) {
>> > +        if (!(wanted & (APR_FINFO_NAME | APR_FINFO_LINK))) {
>> >              if (!GetFileAttributesExW(wfname, GetFileExInfoStandard,
>> >                                        &FileInfo.i))
>> >                  return apr_get_os_error();
>> >
>> > ]]]
>> >
>> > And yes, if I revert that single hunk, the Subversion problem with
>> > subst'ed drives on Windows (or working copies on drive roots) is gone!
>> >
>> > Of course, I have no idea what other effects this has, but just
>> > confirming that taking another turn in the above conditional (like it
>> > was before) makes apr_stat return the same (AFAICS) as in 1.6.5, for
>> > substed drives or drive roots on Windows.
>> >
>>
>> Hi,
>>
>> The problem with apr_stat(APR_FINFO_LINK | APR_FINFO_MIN) should be
>> fixed by r1896717 [1] in trunk.
>>
>> This fix also should resolve performance regression in apr_stat()
>> in most common cases.
>>
>> I plan to backport this fix to APR 1.7.x at some point later.
>>
>> I've backported the r1896717 fix to 1.8.x branch. Please test and vote.
>
>
The change backported to the 1.7.x branch in r1904030 and released as part
of APR 1.7.1.

-- 
Ivan Zhakov

Re: Subversion reports status fault on substituted drive

Posted by Ivan Zhakov via dev <de...@apr.apache.org>.
On Wed, 5 Jan 2022 at 21:37, Ivan Zhakov <iv...@visualsvn.com> wrote:

> On Mon, 13 Sept 2021 at 11:57, Johan Corveleyn <jc...@gmail.com> wrote:
> >
> > On Tue, Sep 7, 2021 at 3:58 PM Johan Corveleyn <jc...@gmail.com>
> wrote:
> > >
> > > On Tue, Feb 23, 2021 at 9:59 AM Mariusz W <ma...@gmail.com> wrote:
> > > > On 2021/01/26 14:54:04, Johan Corveleyn <jc...@gmail.com> wrote:
> > > > > On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn <
> jcorvel@gmail.com> wrote:
> > > > > > On Wed, Dec 23, 2020 at 8:17 PM William A Rowe Jr <
> wrowe@rowe-clan.net> wrote:
> > > > > > >
> > > > > > > Sorry, I'd been pounding on a beta deliverable, it's my bad.
> This coming week I have cycles to give
> > > > > > > back to APR, CMake and other projects neglected during this
> crunch period. I hope an end-of-year
> > > > > > > (or very early January) release will meet your goals..
> > > > > >
> > > > > > Yes, thanks, and no problem. I think that might be just in time,
> so
> > > > > > I'm crossing my fingers :-).
> > > > > >
> > > > > > (as you might know, Subversion development is going quite slowly
> these
> > > > > > days, so it's hard to say when the cycles of the different
> volunteers
> > > > > > will align -- it might be end of this year still, but I'm
> guessing
> > > > > > it's more likely to be early January too I think ... we're
> progressing
> > > > > > with small steps here and there)
> > > > Hi,
> > > > Maybe problem is in this line 627 in filestat.c [1] (changed in
> revision [2]) mainly by adding APR_FINFO_LINK in line 627
> > > >
> > > > Diff to prev: [3]
> > > > I think that this change is causing that "if" branch is not executed
> (svn is using APR_FINFO_LINK in call) and in "else" branch we get error
> from FindFirstFileW [6] because root (e.g. after subst) is ending with "/"
> or "\" as in example from [4])
> > > > The error code 720002 from apr_stat means ERROR_FILE_NOT_FOUND (code
> 2 (0x2) [5]) from FindFirstFileW plus some adding and multiplication
> operations in apr layer (last digit in 720002 is code 2).
> > > >
> > > > [1] -
> https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l627
> > > > [2] - https://svn.apache.org/r1855950
> > > > [3] -
> https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?r1=1795930&r2=1855950&pathrev=1855950&diff_format=h
> > > > [4] -
> https://lists.apache.org/thread.html/rd890d13b826e8cd7acaa96769e10e7143b7e35e11c99bcaa1a75d481%40%3Cdev.apr.apache.org%3E
> > > > [5] -
> https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
> > > > [6] -
> https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l642
> > > >
> > > > Regards,
> > > > Mariusz
> > >
> > > Thanks for looking into this, Mariusz.
> > >
> > > I think you might be onto something. I intended to look closer into
> > > your diagnosis, and perhaps try to test if changing those lines back
> > > to the original would fix it. But I didn't get around to it, sorry.
> > >
> > > Now that there was talk of an APR 1.7.1 release on this list, it
> > > popped back into my memory :-).
> > >
> > > @apr devs: what do you think about Mariusz pointer towards the
> > > possible cause of what we're seeing with SVN on Windows with apr
> > > 1.7.0? And is there an easy fix (for instance, just reverting that
> > > single hunk), which has a chance of being accepted into 1.7.1 (if so,
> > > I'll try to test it on my svn-dev machine, where I could reproduce the
> > > issue)? IIUC, right now apr_stat does seem to error out on paths
> > > ending with "/" or "\" on Windows, if APR_FINFO_LINK is given (while
> > > APR 1.6 did return a sensible result in that case).
> > >
> > > To be clear, IIUC Mariusz is referring to this hunk in filestat.c in
> r1855950:
> > >
> > > [[[
> > > @@ -590,7 +664,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
> > >          rv = apr_filepath_root(&root, &test, APR_FILEPATH_NATIVE,
> pool);
> > >          isroot = (root && *root && !(*test));
> > >
> > > -        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
> > > APR_FINFO_NAME) || isroot))
> > > +        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
> > > (APR_FINFO_NAME | APR_FINFO_LINK)) || isroot))
> > >          {
> > >              /* cannot use FindFile on a Win98 root, it returns \*
> > >               * GetFileAttributesExA is not available on Win95
> > > ]]]
> >
> > Oops, I meant (or Mariusz meant) this hunk in filestat.c r1855950:
> >
> > [[[
> >
> > @@ -555,7 +624,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
> >          if ((rv = utf8_to_unicode_path(wfname, sizeof(wfname)
> >                                              / sizeof(apr_wchar_t),
> fname)))
> >              return rv;
> > -        if (!(wanted & APR_FINFO_NAME)) {
> > +        if (!(wanted & (APR_FINFO_NAME | APR_FINFO_LINK))) {
> >              if (!GetFileAttributesExW(wfname, GetFileExInfoStandard,
> >                                        &FileInfo.i))
> >                  return apr_get_os_error();
> >
> > ]]]
> >
> > And yes, if I revert that single hunk, the Subversion problem with
> > subst'ed drives on Windows (or working copies on drive roots) is gone!
> >
> > Of course, I have no idea what other effects this has, but just
> > confirming that taking another turn in the above conditional (like it
> > was before) makes apr_stat return the same (AFAICS) as in 1.6.5, for
> > substed drives or drive roots on Windows.
> >
>
> Hi,
>
> The problem with apr_stat(APR_FINFO_LINK | APR_FINFO_MIN) should be
> fixed by r1896717 [1] in trunk.
>
> This fix also should resolve performance regression in apr_stat()
> in most common cases.
>
> I plan to backport this fix to APR 1.7.x at some point later.
>
> I've backported the r1896717 fix to 1.8.x branch. Please test and vote.

-- 
Ivan Zhakov

Re: Subversion reports status fault on substituted drive

Posted by Nathan Hartman <ha...@gmail.com>.
Bringing this from APR's list back to dev@s.a.o... For those affected
by issues with working copies on 'subst' drives on Windows (as
reported in mails such as [1], [2]) the following should come as
welcome news:

On Wed, Jan 5, 2022 at 1:37 PM Ivan Zhakov <iv...@visualsvn.com> wrote:
> Hi,
>
> The problem with apr_stat(APR_FINFO_LINK | APR_FINFO_MIN) should be
> fixed by r1896717 [1] in trunk.
>
> This fix also should resolve performance regression in apr_stat()
> in most common cases.
>
> I plan to backport this fix to APR 1.7.x at some point later.
>
> Testing and review will be much appreciated! :)
>
> [1] https://svn.apache.org/r1896717
>
> --
> Ivan Zhakov

If possible I encourage to help with test and review. With any luck
hopefully SVN binary providers on Windows will be able to have this
issue fixed with the next release.

Cheers,
Nathan

[1] https://lists.apache.org/thread/yxhgoslnyqs9wn5ln2yq7kc24xhqrzym

[2] https://lists.apache.org/thread/gbdpwfcr0h9gohdooy993yq1szqkwo9l

Re: Subversion reports status fault on substituted drive

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Mon, 13 Sept 2021 at 11:57, Johan Corveleyn <jc...@gmail.com> wrote:
>
> On Tue, Sep 7, 2021 at 3:58 PM Johan Corveleyn <jc...@gmail.com> wrote:
> >
> > On Tue, Feb 23, 2021 at 9:59 AM Mariusz W <ma...@gmail.com> wrote:
> > > On 2021/01/26 14:54:04, Johan Corveleyn <jc...@gmail.com> wrote:
> > > > On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn <jc...@gmail.com> wrote:
> > > > > On Wed, Dec 23, 2020 at 8:17 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> > > > > >
> > > > > > Sorry, I'd been pounding on a beta deliverable, it's my bad. This coming week I have cycles to give
> > > > > > back to APR, CMake and other projects neglected during this crunch period. I hope an end-of-year
> > > > > > (or very early January) release will meet your goals..
> > > > >
> > > > > Yes, thanks, and no problem. I think that might be just in time, so
> > > > > I'm crossing my fingers :-).
> > > > >
> > > > > (as you might know, Subversion development is going quite slowly these
> > > > > days, so it's hard to say when the cycles of the different volunteers
> > > > > will align -- it might be end of this year still, but I'm guessing
> > > > > it's more likely to be early January too I think ... we're progressing
> > > > > with small steps here and there)
> > > Hi,
> > > Maybe problem is in this line 627 in filestat.c [1] (changed in revision [2]) mainly by adding APR_FINFO_LINK in line 627
> > >
> > > Diff to prev: [3]
> > > I think that this change is causing that "if" branch is not executed (svn is using APR_FINFO_LINK in call) and in "else" branch we get error from FindFirstFileW [6] because root (e.g. after subst) is ending with "/" or "\" as in example from [4])
> > > The error code 720002 from apr_stat means ERROR_FILE_NOT_FOUND (code 2 (0x2) [5]) from FindFirstFileW plus some adding and multiplication operations in apr layer (last digit in 720002 is code 2).
> > >
> > > [1] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l627
> > > [2] - https://svn.apache.org/r1855950
> > > [3] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?r1=1795930&r2=1855950&pathrev=1855950&diff_format=h
> > > [4] - https://lists.apache.org/thread.html/rd890d13b826e8cd7acaa96769e10e7143b7e35e11c99bcaa1a75d481%40%3Cdev.apr.apache.org%3E
> > > [5] - https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
> > > [6] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l642
> > >
> > > Regards,
> > > Mariusz
> >
> > Thanks for looking into this, Mariusz.
> >
> > I think you might be onto something. I intended to look closer into
> > your diagnosis, and perhaps try to test if changing those lines back
> > to the original would fix it. But I didn't get around to it, sorry.
> >
> > Now that there was talk of an APR 1.7.1 release on this list, it
> > popped back into my memory :-).
> >
> > @apr devs: what do you think about Mariusz pointer towards the
> > possible cause of what we're seeing with SVN on Windows with apr
> > 1.7.0? And is there an easy fix (for instance, just reverting that
> > single hunk), which has a chance of being accepted into 1.7.1 (if so,
> > I'll try to test it on my svn-dev machine, where I could reproduce the
> > issue)? IIUC, right now apr_stat does seem to error out on paths
> > ending with "/" or "\" on Windows, if APR_FINFO_LINK is given (while
> > APR 1.6 did return a sensible result in that case).
> >
> > To be clear, IIUC Mariusz is referring to this hunk in filestat.c in r1855950:
> >
> > [[[
> > @@ -590,7 +664,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
> >          rv = apr_filepath_root(&root, &test, APR_FILEPATH_NATIVE, pool);
> >          isroot = (root && *root && !(*test));
> >
> > -        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
> > APR_FINFO_NAME) || isroot))
> > +        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
> > (APR_FINFO_NAME | APR_FINFO_LINK)) || isroot))
> >          {
> >              /* cannot use FindFile on a Win98 root, it returns \*
> >               * GetFileAttributesExA is not available on Win95
> > ]]]
>
> Oops, I meant (or Mariusz meant) this hunk in filestat.c r1855950:
>
> [[[
>
> @@ -555,7 +624,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
>          if ((rv = utf8_to_unicode_path(wfname, sizeof(wfname)
>                                              / sizeof(apr_wchar_t), fname)))
>              return rv;
> -        if (!(wanted & APR_FINFO_NAME)) {
> +        if (!(wanted & (APR_FINFO_NAME | APR_FINFO_LINK))) {
>              if (!GetFileAttributesExW(wfname, GetFileExInfoStandard,
>                                        &FileInfo.i))
>                  return apr_get_os_error();
>
> ]]]
>
> And yes, if I revert that single hunk, the Subversion problem with
> subst'ed drives on Windows (or working copies on drive roots) is gone!
>
> Of course, I have no idea what other effects this has, but just
> confirming that taking another turn in the above conditional (like it
> was before) makes apr_stat return the same (AFAICS) as in 1.6.5, for
> substed drives or drive roots on Windows.
>

Hi,

The problem with apr_stat(APR_FINFO_LINK | APR_FINFO_MIN) should be
fixed by r1896717 [1] in trunk.

This fix also should resolve performance regression in apr_stat()
in most common cases.

I plan to backport this fix to APR 1.7.x at some point later.

Testing and review will be much appreciated! :)

[1] https://svn.apache.org/r1896717

--
Ivan Zhakov

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, Sep 7, 2021 at 3:58 PM Johan Corveleyn <jc...@gmail.com> wrote:
>
> On Tue, Feb 23, 2021 at 9:59 AM Mariusz W <ma...@gmail.com> wrote:
> > On 2021/01/26 14:54:04, Johan Corveleyn <jc...@gmail.com> wrote:
> > > On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn <jc...@gmail.com> wrote:
> > > > On Wed, Dec 23, 2020 at 8:17 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> > > > >
> > > > > Sorry, I'd been pounding on a beta deliverable, it's my bad. This coming week I have cycles to give
> > > > > back to APR, CMake and other projects neglected during this crunch period. I hope an end-of-year
> > > > > (or very early January) release will meet your goals..
> > > >
> > > > Yes, thanks, and no problem. I think that might be just in time, so
> > > > I'm crossing my fingers :-).
> > > >
> > > > (as you might know, Subversion development is going quite slowly these
> > > > days, so it's hard to say when the cycles of the different volunteers
> > > > will align -- it might be end of this year still, but I'm guessing
> > > > it's more likely to be early January too I think ... we're progressing
> > > > with small steps here and there)
> > Hi,
> > Maybe problem is in this line 627 in filestat.c [1] (changed in revision [2]) mainly by adding APR_FINFO_LINK in line 627
> >
> > Diff to prev: [3]
> > I think that this change is causing that "if" branch is not executed (svn is using APR_FINFO_LINK in call) and in "else" branch we get error from FindFirstFileW [6] because root (e.g. after subst) is ending with "/" or "\" as in example from [4])
> > The error code 720002 from apr_stat means ERROR_FILE_NOT_FOUND (code 2 (0x2) [5]) from FindFirstFileW plus some adding and multiplication operations in apr layer (last digit in 720002 is code 2).
> >
> > [1] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l627
> > [2] - https://svn.apache.org/r1855950
> > [3] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?r1=1795930&r2=1855950&pathrev=1855950&diff_format=h
> > [4] - https://lists.apache.org/thread.html/rd890d13b826e8cd7acaa96769e10e7143b7e35e11c99bcaa1a75d481%40%3Cdev.apr.apache.org%3E
> > [5] - https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
> > [6] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l642
> >
> > Regards,
> > Mariusz
>
> Thanks for looking into this, Mariusz.
>
> I think you might be onto something. I intended to look closer into
> your diagnosis, and perhaps try to test if changing those lines back
> to the original would fix it. But I didn't get around to it, sorry.
>
> Now that there was talk of an APR 1.7.1 release on this list, it
> popped back into my memory :-).
>
> @apr devs: what do you think about Mariusz pointer towards the
> possible cause of what we're seeing with SVN on Windows with apr
> 1.7.0? And is there an easy fix (for instance, just reverting that
> single hunk), which has a chance of being accepted into 1.7.1 (if so,
> I'll try to test it on my svn-dev machine, where I could reproduce the
> issue)? IIUC, right now apr_stat does seem to error out on paths
> ending with "/" or "\" on Windows, if APR_FINFO_LINK is given (while
> APR 1.6 did return a sensible result in that case).
>
> To be clear, IIUC Mariusz is referring to this hunk in filestat.c in r1855950:
>
> [[[
> @@ -590,7 +664,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
>          rv = apr_filepath_root(&root, &test, APR_FILEPATH_NATIVE, pool);
>          isroot = (root && *root && !(*test));
>
> -        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
> APR_FINFO_NAME) || isroot))
> +        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
> (APR_FINFO_NAME | APR_FINFO_LINK)) || isroot))
>          {
>              /* cannot use FindFile on a Win98 root, it returns \*
>               * GetFileAttributesExA is not available on Win95
> ]]]

Oops, I meant (or Mariusz meant) this hunk in filestat.c r1855950:

[[[

@@ -555,7 +624,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
         if ((rv = utf8_to_unicode_path(wfname, sizeof(wfname)
                                             / sizeof(apr_wchar_t), fname)))
             return rv;
-        if (!(wanted & APR_FINFO_NAME)) {
+        if (!(wanted & (APR_FINFO_NAME | APR_FINFO_LINK))) {
             if (!GetFileAttributesExW(wfname, GetFileExInfoStandard,
                                       &FileInfo.i))
                 return apr_get_os_error();

]]]

And yes, if I revert that single hunk, the Subversion problem with
subst'ed drives on Windows (or working copies on drive roots) is gone!

Of course, I have no idea what other effects this has, but just
confirming that taking another turn in the above conditional (like it
was before) makes apr_stat return the same (AFAICS) as in 1.6.5, for
substed drives or drive roots on Windows.

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, Feb 23, 2021 at 9:59 AM Mariusz W <ma...@gmail.com> wrote:
> On 2021/01/26 14:54:04, Johan Corveleyn <jc...@gmail.com> wrote:
> > On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn <jc...@gmail.com> wrote:
> > > On Wed, Dec 23, 2020 at 8:17 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> > > >
> > > > Sorry, I'd been pounding on a beta deliverable, it's my bad. This coming week I have cycles to give
> > > > back to APR, CMake and other projects neglected during this crunch period. I hope an end-of-year
> > > > (or very early January) release will meet your goals..
> > >
> > > Yes, thanks, and no problem. I think that might be just in time, so
> > > I'm crossing my fingers :-).
> > >
> > > (as you might know, Subversion development is going quite slowly these
> > > days, so it's hard to say when the cycles of the different volunteers
> > > will align -- it might be end of this year still, but I'm guessing
> > > it's more likely to be early January too I think ... we're progressing
> > > with small steps here and there)
> Hi,
> Maybe problem is in this line 627 in filestat.c [1] (changed in revision [2]) mainly by adding APR_FINFO_LINK in line 627
>
> Diff to prev: [3]
> I think that this change is causing that "if" branch is not executed (svn is using APR_FINFO_LINK in call) and in "else" branch we get error from FindFirstFileW [6] because root (e.g. after subst) is ending with "/" or "\" as in example from [4])
> The error code 720002 from apr_stat means ERROR_FILE_NOT_FOUND (code 2 (0x2) [5]) from FindFirstFileW plus some adding and multiplication operations in apr layer (last digit in 720002 is code 2).
>
> [1] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l627
> [2] - https://svn.apache.org/r1855950
> [3] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?r1=1795930&r2=1855950&pathrev=1855950&diff_format=h
> [4] - https://lists.apache.org/thread.html/rd890d13b826e8cd7acaa96769e10e7143b7e35e11c99bcaa1a75d481%40%3Cdev.apr.apache.org%3E
> [5] - https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
> [6] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l642
>
> Regards,
> Mariusz

Thanks for looking into this, Mariusz.

I think you might be onto something. I intended to look closer into
your diagnosis, and perhaps try to test if changing those lines back
to the original would fix it. But I didn't get around to it, sorry.

Now that there was talk of an APR 1.7.1 release on this list, it
popped back into my memory :-).

@apr devs: what do you think about Mariusz pointer towards the
possible cause of what we're seeing with SVN on Windows with apr
1.7.0? And is there an easy fix (for instance, just reverting that
single hunk), which has a chance of being accepted into 1.7.1 (if so,
I'll try to test it on my svn-dev machine, where I could reproduce the
issue)? IIUC, right now apr_stat does seem to error out on paths
ending with "/" or "\" on Windows, if APR_FINFO_LINK is given (while
APR 1.6 did return a sensible result in that case).

To be clear, IIUC Mariusz is referring to this hunk in filestat.c in r1855950:

[[[
@@ -590,7 +664,7 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
         rv = apr_filepath_root(&root, &test, APR_FILEPATH_NATIVE, pool);
         isroot = (root && *root && !(*test));

-        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
APR_FINFO_NAME) || isroot))
+        if ((apr_os_level >= APR_WIN_98) && (!(wanted &
(APR_FINFO_NAME | APR_FINFO_LINK)) || isroot))
         {
             /* cannot use FindFile on a Win98 root, it returns \*
              * GetFileAttributesExA is not available on Win95
]]]

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by Mariusz W <ma...@gmail.com>.

On 2021/01/26 14:54:04, Johan Corveleyn <jc...@gmail.com> wrote: 
> On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn <jc...@gmail.com> wrote:
> > On Wed, Dec 23, 2020 at 8:17 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> > >
> > > Sorry, I'd been pounding on a beta deliverable, it's my bad. This coming week I have cycles to give
> > > back to APR, CMake and other projects neglected during this crunch period. I hope an end-of-year
> > > (or very early January) release will meet your goals..
> >
> > Yes, thanks, and no problem. I think that might be just in time, so
> > I'm crossing my fingers :-).
> >
> > (as you might know, Subversion development is going quite slowly these
> > days, so it's hard to say when the cycles of the different volunteers
> > will align -- it might be end of this year still, but I'm guessing
> > it's more likely to be early January too I think ... we're progressing
> > with small steps here and there)
Hi,
Maybe problem is in this line 627 in filestat.c [1] (changed in revision [2]) mainly by adding APR_FINFO_LINK in line 627 

Diff to prev: [3]
I think that this change is causing that "if" branch is not executed (svn is using APR_FINFO_LINK in call) and in "else" branch we get error from FindFirstFileW [6] because root (e.g. after subst) is ending with "/" or "\" as in example from [4]) 
The error code 720002 from apr_stat means ERROR_FILE_NOT_FOUND (code 2 (0x2) [5]) from FindFirstFileW plus some adding and multiplication operations in apr layer (last digit in 720002 is code 2).

[1] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l627
[2] - https://svn.apache.org/r1855950
[3] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?r1=1795930&r2=1855950&pathrev=1855950&diff_format=h
[4] - https://lists.apache.org/thread.html/rd890d13b826e8cd7acaa96769e10e7143b7e35e11c99bcaa1a75d481%40%3Cdev.apr.apache.org%3E
[5] - https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
[6] - https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/filestat.c?view=markup#l642

Regards,
Mariusz