On Tue, Sep 7, 2021 at 3:58 PM Johan Corveleyn <[email protected]> wrote:
>
> On Tue, Feb 23, 2021 at 9:59 AM Mariusz W <[email protected]> wrote:
> > On 2021/01/26 14:54:04, Johan Corveleyn <[email protected]> wrote:
> > > On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn <[email protected]>
> > > wrote:
> > > > On Wed, Dec 23, 2020 at 8:17 PM William A Rowe Jr <[email protected]>
> > > > 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
