On Mon, Feb 6, 2023 at 2:37 PM Ivan Zhakov <i...@visualsvn.com> wrote: > > On Thu, 16 Jun 2022 at 21:06, Ivan Zhakov <i...@visualsvn.com> wrote: >> >> On Wed, 5 Jan 2022 at 21:37, Ivan Zhakov <i...@visualsvn.com> wrote: >>> >>> On Mon, 13 Sept 2021 at 11:57, Johan Corveleyn <jcor...@gmail.com> wrote: >>> > >>> > On Tue, Sep 7, 2021 at 3:58 PM Johan Corveleyn <jcor...@gmail.com> wrote: >>> > > >>> > > On Tue, Feb 23, 2021 at 9:59 AM Mariusz W <mawa...@gmail.com> wrote: >>> > > > On 2021/01/26 14:54:04, Johan Corveleyn <jcor...@gmail.com> wrote: >>> > > > > On Thu, Dec 24, 2020 at 10:25 AM Johan Corveleyn >>> > > > > <jcor...@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
