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 2020/06/12 13:13:10 UTC

Re: Subversion reports status fault on substituted drive

On Sun, May 17, 2020 at 12:22 AM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> This information is basically all we needed to resolve. The additional call
> to obtain more fileinfo cannot resolve a root dir. It could, if we transform the
> path \\?\c:\ to \\.\c: (the device c). Looking at this further tomorrow after some
> family hangout time tonight.
>
> Thanks for this level of detail!
>
> Bill

Hi Bill,

Any chance this could get fixed in the short / medium term, and
perhaps included in a 1.7.x APR release? Just checking ... :-)

Apparently some TortoiseSVN users now miss the "Windows Deduplication
support" that APR 1.7.0 gave them ...
(TSVN 1.13.1 used apr 1.7.0, and TSVN 1.14.0 downgraded apr back to 1.6.5)

https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/tortoisesvn/IgmTDOmDNoM/ZesBeNXRBwAJ

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

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
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 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)

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
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..



On Fri, Dec 11, 2020 at 6:32 AM Johan Corveleyn <jc...@gmail.com> wrote:

> On Fri, Nov 27, 2020 at 10:30 AM Johan Corveleyn <jc...@gmail.com>
> wrote:
> >
> > On Fri, Nov 27, 2020 at 2:26 AM William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > >
> > > On Thu, Nov 26, 2020 at 3:35 PM Nick Kew <ni...@apache.org> wrote:
> > >>
> > >>
> > >> > On 24 Nov 2020, at 18:51, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > >> >
> > >> > Yes, I'm on it. 1.6.x and prior somewhat works. 1.7.0 does not,
> asking
> > >> > for apr_stat of a root drive. Seeing the request yesterday for a
> new release,
> > >> > I'd like the chance to fix that quirk of the Win32 API and populate
> the stat
> > >> > struct with something resembling correct info about the root mounts.
> > >>
> > >> Is the issue here that we have something that simply won't map onto
> > >> Windows expectations, so there is no solution that will work for Johan
> > >> without re-introducting PR#47630?
> > >
> > >
> > > Close, and no, we won't reintroduce the old defect.
> > >
> > > It's a combination of treating [C:\]"The device" specially because
> there is no
> > > concept on windows of getting the C:\ filesystem, even though it is an
> NTFS
> > > (root) directory object, with contents and context.
> >
> > Just to be sure: the same reasoning goes for subst'ed drives, yes?
> > Say, W:\ pointing to C:\working-copy. That's also a "device" then?
> >
> > >>
> > >> If so, perhaps what we need is an additional argument that'll switch
> > >> between the two behaviours (ignored on non-windows), and to
> > >> deprecate the existing problematic API.
> > >>
> > >> I'm reluctant to jump in here myself 'cos it's been many years since I
> > >> had access to a Windows machine, let alone a dev environment.
> > >> But it's simple enough, I guess I could hack up a "looks OK" patch
> > >> to do that in the absence of any alternative proposal?
> > >
> > >
> > > What we don't want to do is to put two different behaviors on the user.
> > > The fix I have on the bench is to react *when we see the anticipated
> error*
> > > and know that is should react as looking at the device not the
> directory,
> > > and "make stuff up" about the remaining fields to be filled in a
> consistent
> > > manner. What has my attention right now is authoring the tests to prove
> > > up success or failure in this attempt.
> >
> > Can you draw inspiration from the behavior of APR 1.6.5?
> > From a compatibility point of view it would make sense to look at what
> > values 1.6.5 returned in this case, and see if they are the way to go
> > (or even, if we want to go the "compat as much as possible" route,
> > emulate 1.6.5 as much as possible?).
> >
> > Also, in line with what I said above: it would probably be a good idea
> > to also include a "subst" case in the tests ("subst T: C:\test";
> > repeat test on T:\)
> >
> > Thanks for working on this!
>
> Hi,
>
> Any news on this issue?
>
> Just as a FYI: in SVN we're getting close to starting the release
> train on 1.14.1. Would be nice for our Windows users to be able to
> include a 1.7.x APR with this issue fixed ;-).
> (Of course I know I don't get to dictate any project's release
> schedule, not to mention anyone's time spent on an issue -- I know I'm
> not doing the work here ... just consider this some meta-data in case
> it matters :-))
>
> --
> Johan
>

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Nov 27, 2020 at 10:30 AM Johan Corveleyn <jc...@gmail.com> wrote:
>
> On Fri, Nov 27, 2020 at 2:26 AM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> >
> > On Thu, Nov 26, 2020 at 3:35 PM Nick Kew <ni...@apache.org> wrote:
> >>
> >>
> >> > On 24 Nov 2020, at 18:51, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> >> >
> >> > Yes, I'm on it. 1.6.x and prior somewhat works. 1.7.0 does not, asking
> >> > for apr_stat of a root drive. Seeing the request yesterday for a new release,
> >> > I'd like the chance to fix that quirk of the Win32 API and populate the stat
> >> > struct with something resembling correct info about the root mounts.
> >>
> >> Is the issue here that we have something that simply won't map onto
> >> Windows expectations, so there is no solution that will work for Johan
> >> without re-introducting PR#47630?
> >
> >
> > Close, and no, we won't reintroduce the old defect.
> >
> > It's a combination of treating [C:\]"The device" specially because there is no
> > concept on windows of getting the C:\ filesystem, even though it is an NTFS
> > (root) directory object, with contents and context.
>
> Just to be sure: the same reasoning goes for subst'ed drives, yes?
> Say, W:\ pointing to C:\working-copy. That's also a "device" then?
>
> >>
> >> If so, perhaps what we need is an additional argument that'll switch
> >> between the two behaviours (ignored on non-windows), and to
> >> deprecate the existing problematic API.
> >>
> >> I'm reluctant to jump in here myself 'cos it's been many years since I
> >> had access to a Windows machine, let alone a dev environment.
> >> But it's simple enough, I guess I could hack up a "looks OK" patch
> >> to do that in the absence of any alternative proposal?
> >
> >
> > What we don't want to do is to put two different behaviors on the user.
> > The fix I have on the bench is to react *when we see the anticipated error*
> > and know that is should react as looking at the device not the directory,
> > and "make stuff up" about the remaining fields to be filled in a consistent
> > manner. What has my attention right now is authoring the tests to prove
> > up success or failure in this attempt.
>
> Can you draw inspiration from the behavior of APR 1.6.5?
> From a compatibility point of view it would make sense to look at what
> values 1.6.5 returned in this case, and see if they are the way to go
> (or even, if we want to go the "compat as much as possible" route,
> emulate 1.6.5 as much as possible?).
>
> Also, in line with what I said above: it would probably be a good idea
> to also include a "subst" case in the tests ("subst T: C:\test";
> repeat test on T:\)
>
> Thanks for working on this!

Hi,

Any news on this issue?

Just as a FYI: in SVN we're getting close to starting the release
train on 1.14.1. Would be nice for our Windows users to be able to
include a 1.7.x APR with this issue fixed ;-).
(Of course I know I don't get to dictate any project's release
schedule, not to mention anyone's time spent on an issue -- I know I'm
not doing the work here ... just consider this some meta-data in case
it matters :-))

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Nov 27, 2020 at 2:26 AM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> On Thu, Nov 26, 2020 at 3:35 PM Nick Kew <ni...@apache.org> wrote:
>>
>>
>> > On 24 Nov 2020, at 18:51, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>> >
>> > Yes, I'm on it. 1.6.x and prior somewhat works. 1.7.0 does not, asking
>> > for apr_stat of a root drive. Seeing the request yesterday for a new release,
>> > I'd like the chance to fix that quirk of the Win32 API and populate the stat
>> > struct with something resembling correct info about the root mounts.
>>
>> Is the issue here that we have something that simply won't map onto
>> Windows expectations, so there is no solution that will work for Johan
>> without re-introducting PR#47630?
>
>
> Close, and no, we won't reintroduce the old defect.
>
> It's a combination of treating [C:\]"The device" specially because there is no
> concept on windows of getting the C:\ filesystem, even though it is an NTFS
> (root) directory object, with contents and context.

Just to be sure: the same reasoning goes for subst'ed drives, yes?
Say, W:\ pointing to C:\working-copy. That's also a "device" then?

>>
>> If so, perhaps what we need is an additional argument that'll switch
>> between the two behaviours (ignored on non-windows), and to
>> deprecate the existing problematic API.
>>
>> I'm reluctant to jump in here myself 'cos it's been many years since I
>> had access to a Windows machine, let alone a dev environment.
>> But it's simple enough, I guess I could hack up a "looks OK" patch
>> to do that in the absence of any alternative proposal?
>
>
> What we don't want to do is to put two different behaviors on the user.
> The fix I have on the bench is to react *when we see the anticipated error*
> and know that is should react as looking at the device not the directory,
> and "make stuff up" about the remaining fields to be filled in a consistent
> manner. What has my attention right now is authoring the tests to prove
> up success or failure in this attempt.

Can you draw inspiration from the behavior of APR 1.6.5?
From a compatibility point of view it would make sense to look at what
values 1.6.5 returned in this case, and see if they are the way to go
(or even, if we want to go the "compat as much as possible" route,
emulate 1.6.5 as much as possible?).

Also, in line with what I said above: it would probably be a good idea
to also include a "subst" case in the tests ("subst T: C:\test";
repeat test on T:\)

Thanks for working on this!

>>
>> BTW, please don't Cc: me replies to this thread.  I get it on the APR
>> dev list, and I don't need a second copy!
>
>
> :) Anyone is welcome to propose switching to httpd-dev like semantics for
> reply-to handling. What I'm curious is whether the svn-dev side of contributors
> have a strong feeling one way or the other, because I know httpd-dev related
> contributors all do.

Heh, sorry Nick, understood. I'm just used to hitting "reply all",
which is how it works best in svn-dev land (AFAIK). I don't know about
the other svn devs, it's just what I'm used to -- I don't have any
strong feelings about list reply behavior :-).

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Thu, Nov 26, 2020 at 3:35 PM Nick Kew <ni...@apache.org> wrote:

>
> > On 24 Nov 2020, at 18:51, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> >
> > Yes, I'm on it. 1.6.x and prior somewhat works. 1.7.0 does not, asking
> > for apr_stat of a root drive. Seeing the request yesterday for a new
> release,
> > I'd like the chance to fix that quirk of the Win32 API and populate the
> stat
> > struct with something resembling correct info about the root mounts.
>
> Is the issue here that we have something that simply won't map onto
> Windows expectations, so there is no solution that will work for Johan
> without re-introducting PR#47630?
>

Close, and no, we won't reintroduce the old defect.

It's a combination of treating [C:\]"The device" specially because there is
no
concept on windows of getting the C:\ filesystem, even though it is an NTFS
(root) directory object, with contents and context.




> If so, perhaps what we need is an additional argument that'll switch
> between the two behaviours (ignored on non-windows), and to
> deprecate the existing problematic API.
>
> I'm reluctant to jump in here myself 'cos it's been many years since I
> had access to a Windows machine, let alone a dev environment.
> But it's simple enough, I guess I could hack up a "looks OK" patch
> to do that in the absence of any alternative proposal?
>

What we don't want to do is to put two different behaviors on the user.
The fix I have on the bench is to react *when we see the anticipated error*
and know that is should react as looking at the device not the directory,
and "make stuff up" about the remaining fields to be filled in a consistent
manner. What has my attention right now is authoring the tests to prove
up success or failure in this attempt.


> BTW, please don't Cc: me replies to this thread.  I get it on the APR
> dev list, and I don't need a second copy!
>

:) Anyone is welcome to propose switching to httpd-dev like semantics for
reply-to handling. What I'm curious is whether the svn-dev side of
contributors
have a strong feeling one way or the other, because I know httpd-dev related
contributors all do.

Re: Subversion reports status fault on substituted drive

Posted by Nick Kew <ni...@apache.org>.

> On 24 Nov 2020, at 18:51, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> 
> Yes, I'm on it. 1.6.x and prior somewhat works. 1.7.0 does not, asking
> for apr_stat of a root drive. Seeing the request yesterday for a new release,
> I'd like the chance to fix that quirk of the Win32 API and populate the stat
> struct with something resembling correct info about the root mounts.

Is the issue here that we have something that simply won't map onto
Windows expectations, so there is no solution that will work for Johan
without re-introducting PR#47630?

If so, perhaps what we need is an additional argument that'll switch
between the two behaviours (ignored on non-windows), and to
deprecate the existing problematic API.

I'm reluctant to jump in here myself 'cos it's been many years since I
had access to a Windows machine, let alone a dev environment.
But it's simple enough, I guess I could hack up a "looks OK" patch
to do that in the absence of any alternative proposal?

BTW, please don't Cc: me replies to this thread.  I get it on the APR
dev list, and I don't need a second copy!

-- 
Nick Kew


Re: Subversion reports status fault on substituted drive

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Yes, I'm on it. 1.6.x and prior somewhat works. 1.7.0 does not, asking
for apr_stat of a root drive. Seeing the request yesterday for a new
release,
I'd like the chance to fix that quirk of the Win32 API and populate the stat
struct with something resembling correct info about the root mounts.

That's particularly important to the subversion developers.



On Tue, Nov 24, 2020 at 4:37 AM Johan Corveleyn <jc...@gmail.com> wrote:

> On Fri, Jun 12, 2020 at 3:13 PM Johan Corveleyn <jc...@gmail.com> wrote:
> >
> > On Sun, May 17, 2020 at 12:22 AM William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > >
> > > This information is basically all we needed to resolve. The additional
> call
> > > to obtain more fileinfo cannot resolve a root dir. It could, if we
> transform the
> > > path \\?\c:\ to \\.\c: (the device c). Looking at this further
> tomorrow after some
> > > family hangout time tonight.
> > >
> > > Thanks for this level of detail!
> > >
> > > Bill
> >
> > Hi Bill,
> >
> > Any chance this could get fixed in the short / medium term, and
> > perhaps included in a 1.7.x APR release? Just checking ... :-)
> >
> > Apparently some TortoiseSVN users now miss the "Windows Deduplication
> > support" that APR 1.7.0 gave them ...
> > (TSVN 1.13.1 used apr 1.7.0, and TSVN 1.14.0 downgraded apr back to
> 1.6.5)
> >
> >
> https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/tortoisesvn/IgmTDOmDNoM/ZesBeNXRBwAJ
> >
> > Thanks,
> > --
> > Johan
>
> Hi,
>
> I wanted to check again if there has been any progress on this issue.
> It would be nice to get this fixed in a 1.7.x APR release, so
> Subversion on Windows could get (the benefits of) APR 1.7.
>
> To recap, the following call to apr_stat:
>
>     status = apr_stat(finfo, "C:/",
>         APR_FINFO_TYPE | APR_FINFO_LINK | APR_FINFO_SIZE | APR_FINFO_MTIME,
>         pool);
>
> succeeds with APR 1.6.5 on Windows, but fails with ARP 1.7.0 (returns
> 720002). Same goes for any other drive root (for instance with
> subst'ed drives, which was the original report -- sometimes people
> point a subst'ed drive to an SVN working copy root).
>
> Thanks,
> --
> Johan
>

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Jun 12, 2020 at 3:13 PM Johan Corveleyn <jc...@gmail.com> wrote:
>
> On Sun, May 17, 2020 at 12:22 AM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> >
> > This information is basically all we needed to resolve. The additional call
> > to obtain more fileinfo cannot resolve a root dir. It could, if we transform the
> > path \\?\c:\ to \\.\c: (the device c). Looking at this further tomorrow after some
> > family hangout time tonight.
> >
> > Thanks for this level of detail!
> >
> > Bill
>
> Hi Bill,
>
> Any chance this could get fixed in the short / medium term, and
> perhaps included in a 1.7.x APR release? Just checking ... :-)
>
> Apparently some TortoiseSVN users now miss the "Windows Deduplication
> support" that APR 1.7.0 gave them ...
> (TSVN 1.13.1 used apr 1.7.0, and TSVN 1.14.0 downgraded apr back to 1.6.5)
>
> https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/tortoisesvn/IgmTDOmDNoM/ZesBeNXRBwAJ
>
> Thanks,
> --
> Johan

Hi,

I wanted to check again if there has been any progress on this issue.
It would be nice to get this fixed in a 1.7.x APR release, so
Subversion on Windows could get (the benefits of) APR 1.7.

To recap, the following call to apr_stat:

    status = apr_stat(finfo, "C:/",
        APR_FINFO_TYPE | APR_FINFO_LINK | APR_FINFO_SIZE | APR_FINFO_MTIME,
        pool);

succeeds with APR 1.6.5 on Windows, but fails with ARP 1.7.0 (returns
720002). Same goes for any other drive root (for instance with
subst'ed drives, which was the original report -- sometimes people
point a subst'ed drive to an SVN working copy root).

Thanks,
-- 
Johan