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/04/09 10:07:35 UTC

Re: Subversion reports status fault on substituted drive

[ cc -= dev@subversion.a.o ]

Hi APR devs,

The below report about a problem with Subversion working on 'subst'ed
drives (Windows), seemed to be related to a change in APR 1.7.0
(https://svn.apache.org/r1855950), and was cross-posted between
dev@subversion and dev@apr. Without any context this was probably not
the best way to draw attention of the apr devs (sorry), so I'll try
again :-).

The problem was actually nicely summarized already last November by
Stefan Küng and Evgeny Kotkov on dev@subversion (but unfortunately not
escalated upstream), see
https://svn.haxx.se/dev/archive-2019-11/0157.shtml:

On Tue, Nov 19, 2019 at 9:44 PM Evgeny Kotkov
<ev...@visualsvn.com> wrote:
>
> Stefan Kueng <to...@gmail.com> writes:
>
> > Using svn 1.13.0 on Windows:
> >
> > SUBST G:\ D:\Development\TestWC
> > G:
> > svn st .
> >
> > !       .
> > ?       Foo
> > ?       src\nonversioned
> >
> > As you can see, the root folder doesn't show the correct status.
> > This showed the correct status with 1.12.x.
>
> On my side, this issue *does not* reproduce with Subversion 1.13.0 built
> against APR 1.6.x, but reproduces with the TortoiseSVN 1.13.1 command-line
> binaries.
>
> I see that TortoiseSVN 1.13 has recently switched to APR 1.7.0, so that may
> be causing the issue:
>
>   https://osdn.net/projects/tortoisesvn/scm/svn/commits/28674
>
> Among the changes in APR 1.7.0, this one could be responsible for the faulty
> behavior, since it changes how apr_stat() works for reparse points:
>
>   https://svn.apache.org/r1855950
>
>
> Regards,
> Evgeny Kotkov

Can one of the APR developers please take a closer look? It's not
clear to me whether it's a new bug in APR 1.7.0, introduced by
r1855950 (causing problems for simple subst'ed drives), or whether
it's an intended behavior change, and the Subversion usage is doing
something wrong. To be clear: I haven't looked closely at the source
code myself (I'm just trying to build SVN on Windows, and trying to
determine whether I should go for APR 1.7.0 or 1.6.x).

For reference, below is the latest report on dev@subversion, which was
cross-posted to dev@apr.

Thanks.

On Thu, Jan 2, 2020 at 1:51 PM Mikael Rahbek <mr...@skov.dk> wrote:
>
> Hi
>
> Thank you for replying to my question!
>
> Here are 2 command line printouts.
>
> First is from R:\
>
> Microsoft Windows [Version 10.0.18363.535]
> (c) 2019 Microsoft Corporation. All rights reserved.
>
> R:\>svn info
> Path: .
> Working Copy Root Path: R:\
> URL: http://svn.skov.com/svn/ARM-ControlSW/ControlSW/branches/features/Production_work_7_1
> Relative URL: ^/ControlSW/branches/features/Production_work_7_1
> Repository Root: http://svn.skov.com/svn/ARM-ControlSW
> Repository UUID: 2a7e9ce4-0f0a-8847-ab6a-1601258db52d
> Revision: 84889
> Node Kind: directory
> Schedule: normal
> Last Changed Author: mr
> Last Changed Rev: 84879
> Last Changed Date: 2020-01-02 09:07:18 +0100 (to, 02 jan 2020)
>
>
> R:\>svn status
> !M      .
> X       Applications\BasicControl
> ?       Applications\FarmnetInterface\WinFarmNet\.vs
> ?       Applications\FarmnetInterface\WinFarmNet\Debug
> ?       Applications\FarmnetInterface\WinFarmNet\WinFarmNet\Debug
> X       Applications\GUI
> X       Applications\PluginInstaller\SimLauncherLib
>
> And from C:\Data\Production_work_7_1_Clean:
>
> Microsoft Windows [Version 10.0.18363.535]
> (c) 2019 Microsoft Corporation. All rights reserved.
>
> C:\Data\Production_work_7_1_Clean>svn info
> Path: .
> Working Copy Root Path: C:\Data\Production_work_7_1_Clean
> URL: http://svn.skov.com/svn/ARM-ControlSW/ControlSW/branches/features/Production_work_7_1
> Relative URL: ^/ControlSW/branches/features/Production_work_7_1
> Repository Root: http://svn.skov.com/svn/ARM-ControlSW
> Repository UUID: 2a7e9ce4-0f0a-8847-ab6a-1601258db52d
> Revision: 84889
> Node Kind: directory
> Schedule: normal
> Last Changed Author: mr
> Last Changed Rev: 84879
> Last Changed Date: 2020-01-02 09:07:18 +0100 (to, 02 jan 2020)
>
>
> C:\Data\Production_work_7_1_Clean>svn status
>  M      .
> X       Applications\BasicControl
> ?       Applications\FarmnetInterface\WinFarmNet\.vs
> ?       Applications\FarmnetInterface\WinFarmNet\Debug
> ?       Applications\FarmnetInterface\WinFarmNet\WinFarmNet\Debug
> X       Applications\GUI
> X       Applications\PluginInstaller\SimLauncherLib
>
> Svn info seems to be the same, but svn status indicate that in the root they are different:
>
> R:\>svn status
> !M      .
>
> And
>
> C:\Data\Production_work_7_1_Clean>svn status
>  M      .
>
> Regards
>
> Mikael
>
> -----Original Message-----
> From: Julian Foad <ju...@apache.org>
> Sent: 2. januar 2020 13:13
> To: Mikael Rahbek <mr...@skov.dk>
> Cc: dev@apr.apache.org; dev@subversion.apache.org
> Subject: Re: Subversion reports status fault on substituted drive
>
> Mikael Rahbek wrote:
> > As part of our development we use a substituted drive (Windows 10):
> >
> > E.g.:
> >
> > subst r: C:\Data\Production_work_7_1_Clean
> >
> > In the root of r: there is now always shown a change with status
> > missing
> >
> > If checking from C:\Data\Production_work_7_1_Clean it shows the status
> > correct.
>
> Thank you for reporting this problem.
>
> If you can reproduce the issue using 'svn' command-line commands like 'svn status', then please show the actual commands and their output.
>
> (If you cannot, then please report the problem to the TortoiseSVN project instead.)
>
> Also please show the output of 'svn info' in both places, like this:
>
> [[[
> C:
> cd \Data\Production_work_7_1_Clean
> svn info
> ]]]
>
> and
>
> [[[
> r:
> cd \
> svn info
> ]]]
>
> > The problem is new in 1.13.1, Build 28686 (TortoiseSvn)
> >
> > Before 13 this was not a problem.
> >
> > I have been told that the reason for the fault could be because
> > subversion APR is changed from 1.6.x to 1.7.0?
>
> I can't do Windows debugging myself but if you provide the above information there is a good chance that somebody here will be able to debug it.
>
> - Julian

--
Johan

Re: Subversion reports status fault on substituted drive

Posted by Nick Kew <ni...@webthing.com>.

> On 9 Apr 2020, at 12:19, Nick Kew <ni...@apache.org> wrote:
> 
> 
>> On 9 Apr 2020, at 11:07, Johan Corveleyn <jc...@gmail.com> wrote:
>> 
>> [ cc -= dev@subversion.a.o ]
>> 
>> Hi APR devs,
>> 
>> The below report about a problem with Subversion working on 'subst'ed
>> drives (Windows), seemed to be related to a change in APR 1.7.0
>> (https://svn.apache.org/r1855950),
> 
> That fixed a bug PR#47630 that had been extensively discussed in
> bugzilla and shows as having 22 watchers - that's a lot of interest!

Posting from another address to investigate possible harvesting of this
list for spam.  After my last post I got an automated message that looked
closely related to the svn subject!

-- 
Nick Kew

Re: Subversion reports status fault on substituted drive

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
I'm pursuing something like the attached patch. Only one test fails, which
I presume
devolves back to the /. root comparison (so the data directory comparison
is good.)
The stat/getfileinfo comparison doesn't alert if the info can't be
retrieved from one or
the other call, which isn't good (the availability bits should be
identical.)

testfileinfo        : -Line 105: apr_stat and apr_getfileinfo differ in
protection
FAILED 1 of 7
Failed Tests            Total   Fail    Failed %
===================================================
testfileinfo                7      1     14.29%

Out of cycles to dig deeper today, but just wanted to leave an update that
something
interesting is going to have to happen here to delve into the "root device"
vs a simple
"root directory"..


On Thu, Apr 9, 2020 at 11:31 AM William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Thu, Apr 9, 2020 at 8:01 AM Johan Corveleyn <jc...@gmail.com> wrote:
>
>> On Thu, Apr 9, 2020 at 1:20 PM Nick Kew <ni...@apache.org> wrote:
>> > >
>> > > The below report about a problem with Subversion working on 'subst'ed
>> > > drives (Windows), seemed to be related to a change in APR 1.7.0
>> > > (https://svn.apache.org/r1855950),
>> >
>> > That fixed a bug PR#47630 that had been extensively discussed in
>> > bugzilla and shows as having 22 watchers - that's a lot of interest!
>> >
>> > Does that discussion not throw any light on your issue?  For example,
>> > Michael Schlenker's comments there refer to that bug affecting SVN.
>>
>> Thanks, I've read through that issue in Bugzilla now.
>>
>> It definitely seems like a useful improvement for proper support of
>> (different types of) reparse points on Windows. I'm not saying this
>> bugfix needs to be reverted. And indeed, Michael Schlenker's comments
>> point to a specific problem affecting SVN (in combination with Windows
>> Data Deduplication, which creates reparse points with tag
>> IO_REPARSE_TAG_DEDUP). So in general this is a step forward, and will
>> probably benefit SVN for use on these specific types of Windows
>> systems.
>>
>> However, I think this also broke something (or at least inadvertently
>> changed something) related to the usage of "SUBST" (as in 'SUBST G:\
>> D:\Development\TestWC'). The usage of "subst" is not mentioned
>> anywhere in PR#47630, so perhaps the compatibility with / impact on
>> subst was overlooked? I'm not an expert at all, but from what I could
>> find with a quick internet search, "subst" is very old (from MSDOS
>> times), and it seems it's not the same as a junction (? not sure about
>> the relation between subst and reparse points in general). In any
>> case, it's not clear to me that the fix for PR#47630 intended to
>> change something for "subst".
>>
>
> That's very likely, and this can be researched further to dig up exactly
> how
> subst is handled. (As you mention, it hasn't been common since the
> introduction
> of proper junction and directory/file symlinks.)
>
> It appears that the root directory is always problematic for an svn
> checkout
> (actually, most checkouts were problematic on svn when junctions or
> symlinks
> were in play on windows.) Looks like some additional special handling is
> going
> to be required even with queries on c:\ (a real, not a substituted volume
> root.)
>
> Can you successfully `svn co {repos} /` on linux? Hoping to understand the
> scope of the issue.
>
>
>

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

Re: Subversion reports status fault on substituted drive

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


On Thu, Apr 23, 2020 at 7:00 PM Johan Corveleyn <jc...@gmail.com> wrote:

> On Wed, Apr 15, 2020 at 4:48 PM Johan Corveleyn <jc...@gmail.com> wrote:
> >
> > On Wed, Apr 15, 2020 at 3:51 PM William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > >
> > > Interesting.
> > >
> > > Not being familiar with the subversion code, it would be helpful to
> see what args
> > > are being passed to the offending (offended) apr_stat call, so we can
> investigate
> > > the behavior of APR 1.7.0 (or trunk) further.
> >
> > Understood. I'll try to dig into this a bit in the coming days.
>
> Okay, I finally got around to this.
>
> Subversion indeed performs an apr_stat call with the following flags [1]:
>
>     apr_int32_t wanted = APR_FINFO_TYPE | APR_FINFO_LINK
>                        | APR_FINFO_SIZE | APR_FINFO_MTIME;
>
> The call to apr_stat happens on line 4464 in libsvn_subr/io.c [2]:
>
>     status = apr_stat(finfo, fname_apr, wanted, pool);
>
> Apparently this call succeeds with apr 1.6.5 (on Windows), with
> fname_apr="C:/" and wanted as above (I added a screenshot from the VS
> Debugger to illustrate). The returned filetype is APR_DIR, etc ...
>
> But the call fails with apr 1.7.0, returning status == 720002 (see
> second screenshot, FWIW). Is Subversion doing something wrong here?
> Passing incorrect flags or something like that?
>
> [1]
> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l3149
> [2]
> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l4464
>
> --
> Johan
>

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, May 8, 2020 at 7:41 PM Johan Corveleyn <jc...@gmail.com> wrote:
>
> Op vr 8 mei 2020 16:25 schreef William A Rowe Jr <wr...@rowe-clan.net>:
>>
>> On Fri, May 8, 2020 at 4:50 AM Johan Corveleyn <jc...@gmail.com> wrote:
>>>
>>> On Fri, Apr 24, 2020 at 1:59 AM Johan Corveleyn <jc...@gmail.com> wrote:
>>> >
>>> > On Wed, Apr 15, 2020 at 4:48 PM Johan Corveleyn <jc...@gmail.com> wrote:
>>> > >
>>> > > On Wed, Apr 15, 2020 at 3:51 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>>> > > >
>>> > > > Interesting.
>>> > > >
>>> > > > Not being familiar with the subversion code, it would be helpful to see what args
>>> > > > are being passed to the offending (offended) apr_stat call, so we can investigate
>>> > > > the behavior of APR 1.7.0 (or trunk) further.
>>> > >
>>> > > Understood. I'll try to dig into this a bit in the coming days.
>>> >
>>> > Okay, I finally got around to this.
>>> >
>>> > Subversion indeed performs an apr_stat call with the following flags [1]:
>>> >
>>> >     apr_int32_t wanted = APR_FINFO_TYPE | APR_FINFO_LINK
>>> >                        | APR_FINFO_SIZE | APR_FINFO_MTIME;
>>> >
>>> > The call to apr_stat happens on line 4464 in libsvn_subr/io.c [2]:
>>> >
>>> >     status = apr_stat(finfo, fname_apr, wanted, pool);
>>> >
>>> > Apparently this call succeeds with apr 1.6.5 (on Windows), with
>>> > fname_apr="C:/" and wanted as above (I added a screenshot from the VS
>>> > Debugger to illustrate). The returned filetype is APR_DIR, etc ...
>>> >
>>> > But the call fails with apr 1.7.0, returning status == 720002 (see
>>> > second screenshot, FWIW). Is Subversion doing something wrong here?
>>> > Passing incorrect flags or something like that?
>>> >
>>> > [1] https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l3149
>>> > [2] https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l4464
>>>
>>> I can't debug further into APR itself at this time, I'm afraid. Is
>>> there anything else I can do? I'm not sure what the return code 720002
>>> means as a result of apr_stat().
>>>
>>> Is anyone able to reproduce this issue with APR 1.7.0 with a
>>> test-program merely calling apr_stat() on the root of a drive, with
>>> the flags
>>>
>>>     APR_FINFO_TYPE | APR_FINFO_LINK | APR_FINFO_SIZE | APR_FINFO_MTIME
>>>
>>> ?
>>>
>>> At this point it's not even clear to me whether this problem is
>>> Windows-specific, or can also be reproduced on Linux / Unix, if you
>>> call it on "/".
>>>
>>> If it helps I can file an issue in Bugzilla, if you guys want me to.
>>
>>
>> I'll pursue this further on Monday and see if we can offer some workaround for drive-root stat calls in apr itself.

Gentle nudge ... a Friday seems ideal for this kind of work, right? ;-)

TIA,
-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
Op vr 8 mei 2020 16:25 schreef William A Rowe Jr <wr...@rowe-clan.net>:

> On Fri, May 8, 2020 at 4:50 AM Johan Corveleyn <jc...@gmail.com> wrote:
>
>> On Fri, Apr 24, 2020 at 1:59 AM Johan Corveleyn <jc...@gmail.com>
>> wrote:
>> >
>> > On Wed, Apr 15, 2020 at 4:48 PM Johan Corveleyn <jc...@gmail.com>
>> wrote:
>> > >
>> > > On Wed, Apr 15, 2020 at 3:51 PM William A Rowe Jr <
>> wrowe@rowe-clan.net> wrote:
>> > > >
>> > > > Interesting.
>> > > >
>> > > > Not being familiar with the subversion code, it would be helpful to
>> see what args
>> > > > are being passed to the offending (offended) apr_stat call, so we
>> can investigate
>> > > > the behavior of APR 1.7.0 (or trunk) further.
>> > >
>> > > Understood. I'll try to dig into this a bit in the coming days.
>> >
>> > Okay, I finally got around to this.
>> >
>> > Subversion indeed performs an apr_stat call with the following flags
>> [1]:
>> >
>> >     apr_int32_t wanted = APR_FINFO_TYPE | APR_FINFO_LINK
>> >                        | APR_FINFO_SIZE | APR_FINFO_MTIME;
>> >
>> > The call to apr_stat happens on line 4464 in libsvn_subr/io.c [2]:
>> >
>> >     status = apr_stat(finfo, fname_apr, wanted, pool);
>> >
>> > Apparently this call succeeds with apr 1.6.5 (on Windows), with
>> > fname_apr="C:/" and wanted as above (I added a screenshot from the VS
>> > Debugger to illustrate). The returned filetype is APR_DIR, etc ...
>> >
>> > But the call fails with apr 1.7.0, returning status == 720002 (see
>> > second screenshot, FWIW). Is Subversion doing something wrong here?
>> > Passing incorrect flags or something like that?
>> >
>> > [1]
>> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l3149
>> > [2]
>> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l4464
>>
>> I can't debug further into APR itself at this time, I'm afraid. Is
>> there anything else I can do? I'm not sure what the return code 720002
>> means as a result of apr_stat().
>>
>> Is anyone able to reproduce this issue with APR 1.7.0 with a
>> test-program merely calling apr_stat() on the root of a drive, with
>> the flags
>>
>>     APR_FINFO_TYPE | APR_FINFO_LINK | APR_FINFO_SIZE | APR_FINFO_MTIME
>>
>> ?
>>
>> At this point it's not even clear to me whether this problem is
>> Windows-specific, or can also be reproduced on Linux / Unix, if you
>> call it on "/".
>>
>> If it helps I can file an issue in Bugzilla, if you guys want me to.
>
>
> I'll pursue this further on Monday and see if we can offer some workaround
> for drive-root stat calls in apr itself.
>

That would be great, thanks!

-- 
Johan

>

Re: Subversion reports status fault on substituted drive

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, May 8, 2020 at 4:50 AM Johan Corveleyn <jc...@gmail.com> wrote:

> On Fri, Apr 24, 2020 at 1:59 AM Johan Corveleyn <jc...@gmail.com> wrote:
> >
> > On Wed, Apr 15, 2020 at 4:48 PM Johan Corveleyn <jc...@gmail.com>
> wrote:
> > >
> > > On Wed, Apr 15, 2020 at 3:51 PM William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > > >
> > > > Interesting.
> > > >
> > > > Not being familiar with the subversion code, it would be helpful to
> see what args
> > > > are being passed to the offending (offended) apr_stat call, so we
> can investigate
> > > > the behavior of APR 1.7.0 (or trunk) further.
> > >
> > > Understood. I'll try to dig into this a bit in the coming days.
> >
> > Okay, I finally got around to this.
> >
> > Subversion indeed performs an apr_stat call with the following flags [1]:
> >
> >     apr_int32_t wanted = APR_FINFO_TYPE | APR_FINFO_LINK
> >                        | APR_FINFO_SIZE | APR_FINFO_MTIME;
> >
> > The call to apr_stat happens on line 4464 in libsvn_subr/io.c [2]:
> >
> >     status = apr_stat(finfo, fname_apr, wanted, pool);
> >
> > Apparently this call succeeds with apr 1.6.5 (on Windows), with
> > fname_apr="C:/" and wanted as above (I added a screenshot from the VS
> > Debugger to illustrate). The returned filetype is APR_DIR, etc ...
> >
> > But the call fails with apr 1.7.0, returning status == 720002 (see
> > second screenshot, FWIW). Is Subversion doing something wrong here?
> > Passing incorrect flags or something like that?
> >
> > [1]
> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l3149
> > [2]
> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l4464
>
> I can't debug further into APR itself at this time, I'm afraid. Is
> there anything else I can do? I'm not sure what the return code 720002
> means as a result of apr_stat().
>
> Is anyone able to reproduce this issue with APR 1.7.0 with a
> test-program merely calling apr_stat() on the root of a drive, with
> the flags
>
>     APR_FINFO_TYPE | APR_FINFO_LINK | APR_FINFO_SIZE | APR_FINFO_MTIME
>
> ?
>
> At this point it's not even clear to me whether this problem is
> Windows-specific, or can also be reproduced on Linux / Unix, if you
> call it on "/".
>
> If it helps I can file an issue in Bugzilla, if you guys want me to.


I'll pursue this further on Monday and see if we can offer some workaround
for drive-root stat calls in apr itself.

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Apr 24, 2020 at 1:59 AM Johan Corveleyn <jc...@gmail.com> wrote:
>
> On Wed, Apr 15, 2020 at 4:48 PM Johan Corveleyn <jc...@gmail.com> wrote:
> >
> > On Wed, Apr 15, 2020 at 3:51 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> > >
> > > Interesting.
> > >
> > > Not being familiar with the subversion code, it would be helpful to see what args
> > > are being passed to the offending (offended) apr_stat call, so we can investigate
> > > the behavior of APR 1.7.0 (or trunk) further.
> >
> > Understood. I'll try to dig into this a bit in the coming days.
>
> Okay, I finally got around to this.
>
> Subversion indeed performs an apr_stat call with the following flags [1]:
>
>     apr_int32_t wanted = APR_FINFO_TYPE | APR_FINFO_LINK
>                        | APR_FINFO_SIZE | APR_FINFO_MTIME;
>
> The call to apr_stat happens on line 4464 in libsvn_subr/io.c [2]:
>
>     status = apr_stat(finfo, fname_apr, wanted, pool);
>
> Apparently this call succeeds with apr 1.6.5 (on Windows), with
> fname_apr="C:/" and wanted as above (I added a screenshot from the VS
> Debugger to illustrate). The returned filetype is APR_DIR, etc ...
>
> But the call fails with apr 1.7.0, returning status == 720002 (see
> second screenshot, FWIW). Is Subversion doing something wrong here?
> Passing incorrect flags or something like that?
>
> [1] https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l3149
> [2] https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l4464

I can't debug further into APR itself at this time, I'm afraid. Is
there anything else I can do? I'm not sure what the return code 720002
means as a result of apr_stat().

Is anyone able to reproduce this issue with APR 1.7.0 with a
test-program merely calling apr_stat() on the root of a drive, with
the flags

    APR_FINFO_TYPE | APR_FINFO_LINK | APR_FINFO_SIZE | APR_FINFO_MTIME

?

At this point it's not even clear to me whether this problem is
Windows-specific, or can also be reproduced on Linux / Unix, if you
call it on "/".

If it helps I can file an issue in Bugzilla, if you guys want me to.

Thanks,
-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Wed, Apr 15, 2020 at 4:48 PM Johan Corveleyn <jc...@gmail.com> wrote:
>
> On Wed, Apr 15, 2020 at 3:51 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> >
> > Interesting.
> >
> > Not being familiar with the subversion code, it would be helpful to see what args
> > are being passed to the offending (offended) apr_stat call, so we can investigate
> > the behavior of APR 1.7.0 (or trunk) further.
>
> Understood. I'll try to dig into this a bit in the coming days.

Okay, I finally got around to this.

Subversion indeed performs an apr_stat call with the following flags [1]:

    apr_int32_t wanted = APR_FINFO_TYPE | APR_FINFO_LINK
                       | APR_FINFO_SIZE | APR_FINFO_MTIME;

The call to apr_stat happens on line 4464 in libsvn_subr/io.c [2]:

    status = apr_stat(finfo, fname_apr, wanted, pool);

Apparently this call succeeds with apr 1.6.5 (on Windows), with
fname_apr="C:/" and wanted as above (I added a screenshot from the VS
Debugger to illustrate). The returned filetype is APR_DIR, etc ...

But the call fails with apr 1.7.0, returning status == 720002 (see
second screenshot, FWIW). Is Subversion doing something wrong here?
Passing incorrect flags or something like that?

[1] https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l3149
[2] https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971&view=markup#l4464

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Wed, Apr 15, 2020 at 3:51 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> Interesting.
>
> Not being familiar with the subversion code, it would be helpful to see what args
> are being passed to the offending (offended) apr_stat call, so we can investigate
> the behavior of APR 1.7.0 (or trunk) further.

Understood. I'll try to dig into this a bit in the coming days.

Since yesterday I have a working subversion build on Windows (I wanted
to sign the svn 1.14.0-rc2 release after all). So I can probably
inject some printf's or try to reproduce it within the debugger to see
what calls we make to apr exactly.

> I had to tweak the logic of the test I presented above to ask a getfileinfo-based
> query. Typically, it isn't possible to open a root or directory with apr_file_open,
> but there is some hidden logic which could be extended to the public API for
> that purpose, if subversion is trying to rely on apr_file_info_get instead of
> apr_stat. I had to introduce that logic into the test in order to compare results
> between the two different code paths. Opening roots are particularly quirky
> because roots can be confusingly accessed as devices or directory entities.

Ack.

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Interesting.

Not being familiar with the subversion code, it would be helpful to see
what args
are being passed to the offending (offended) apr_stat call, so we can
investigate
the behavior of APR 1.7.0 (or trunk) further.

I had to tweak the logic of the test I presented above to ask a
getfileinfo-based
query. Typically, it isn't possible to open a root or directory with
apr_file_open,
but there is some hidden logic which could be extended to the public API for
that purpose, if subversion is trying to rely on apr_file_info_get instead
of
apr_stat. I had to introduce that logic into the test in order to compare
results
between the two different code paths. Opening roots are particularly quirky
because roots can be confusingly accessed as devices or directory entities.



On Wed, Apr 15, 2020 at 3:45 AM Johan Corveleyn <jc...@gmail.com> wrote:

> On Fri, Apr 10, 2020 at 4:19 PM Johan Corveleyn <jc...@gmail.com> wrote:
> >
> > On Fri, Apr 10, 2020 at 3:45 PM William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > > On Thu, Apr 9, 2020 at 2:48 PM Johan Corveleyn <jc...@gmail.com>
> wrote:
> > >> On Thu, Apr 9, 2020 at 6:31 PM William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > >> >
> > >> > That's very likely, and this can be researched further to dig up
> exactly how
> > >> > subst is handled. (As you mention, it hasn't been common since the
> introduction
> > >> > of proper junction and directory/file symlinks.)
> > >> >
> > >> > It appears that the root directory is always problematic for an svn
> checkout
> > >> > (actually, most checkouts were problematic on svn when junctions or
> symlinks
> > >> > were in play on windows.) Looks like some additional special
> handling is going
> > >> > to be required even with queries on c:\ (a real, not a substituted
> volume root.)
> > >>
> > >> Thank you for looking into this.
> > >>
> > >> You're absolutely right: with TSVN 1.13 (APR 1.7.0) I get the same
> > >> erroneous behavior when 'svn -st'-ing a checkout to C:\ (real drive)
> > >>
> > >> [[[
> > >> C:\>svn st -q
> > >> !       .
> > >> ]]]
> > >
> > >
> > > Here's another experiment that drives me batty...
> > >
> > > svn co https://svn.apache.org/repos/asf/httpd/httpd/trunk httpd-2.x
> > > svn co https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4
> httpd-2.4
> > > svn co https://svn.apache.org/repos/asf/httpd/docs/trunk httpd-docs
> > > ln -s httpd-docs httpd-2.4/docs/manual/build
> > > ln -s httpd-docs httpd-2.x/docs/manual/build
> > > (flip the args using mklink /d or mklink /j on windows)
> >
> > That's a bit special: you're creating embedded working copies, which
> > is already special to begin with (even if they were all normal,
> > separate checkouts instead of symlinks).
> > There is code that tries to crawl up the tree to find the working copy
> > root. Perhaps that is going wrong in this case, depending on your CWD.
> > Not sure.
> >
> > > You'll see svn is fluxored until you change directory. A similar
> ugliness
> > > occurs if you attempt to check out a number of versions of httpd-test,
> > > and attempt to point all of the X apache-test checkouts into a single
> > > group.
> > >
> > > What I've determined is that apr_stat and apr_getfileinfo differ in
> only
> > > one aspect, that protections on a root directory are 700 or 777 based
> > > on the way we dress up fprot bits from windows ACLs, or not.
> > >
> > >> > Can you successfully `svn co {repos} /` on linux? Hoping to
> understand the
> > >> > scope of the issue.
> > >>
> > >> Sorry, no Linux at my disposal where I'm root. But I suppose the test
> > >> above with the checkout to C:\ already confirms your hypothesis.
> > >
> > >
> > > I'm not sure what svn is doing with the '.' root entity, but it likely
> made
> > > some absurd assumptions about symlinks.
> >
> > Yes, that's very well possible. It's a difficult area I think (also:
> > there is the distinction between a symlink to a working copy root,
> > which just needs to be "read through" for svn commands to act upon,
> > and symlinks that are part your versioned data, which need to be
> > versioned and re-created as symlinks in other working copies (cf. the
> > "svn:special" property) -- something which currently is not supported
> > on Windows (junctions), they are just converted into a text file with
> > the link reference in it ... a long standing feature request for svn).
> >
> > I'm afraid I don't have much expertise in that area of the svn code.
> > If we need wider input on this from the svn devs, I suppose
> > dev@subversion should be added back to the cc list.
> >
> > However: with the issue reported here there is no symlink involved
> > (well, maybe sort of, with the subst case, but then it also fails on a
> > plain C:\ drive root). There seems to be a change in behavior between
> > APR 1.6.5 and 1.7.0 that makes svn see different information when the
> > working copy root is the root of a drive. Of course, this may very
> > well be svn's fault for jumping through strange hoops to work on drive
> > roots, and one of those hoops may have just changed form :-). I'm not
> > discounting that possibility at all.
>
> FYI, another data point:
> I just saw a new report on the TortoiseSVN mailinglist, where a user
> sees exactly the same problem with a "shared folder", where the
> wc-root is the share-root:
>
>
> https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/tortoisesvn/jgHE5_sHfFI/xOpP1Ce3AAAJ
>
> Quoting:
> [[[
> C:\Users\ta>svn status \\computer-a\shared-working-copy
> !       \\computer-a\shared-working-copy
> ]]]
>
> I'm guessing this is the same problem with "drive-roots", just as with
> subst or the root of real drives.
>
> --
> Johan
>

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Apr 10, 2020 at 4:19 PM Johan Corveleyn <jc...@gmail.com> wrote:
>
> On Fri, Apr 10, 2020 at 3:45 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> > On Thu, Apr 9, 2020 at 2:48 PM Johan Corveleyn <jc...@gmail.com> wrote:
> >> On Thu, Apr 9, 2020 at 6:31 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> >> >
> >> > That's very likely, and this can be researched further to dig up exactly how
> >> > subst is handled. (As you mention, it hasn't been common since the introduction
> >> > of proper junction and directory/file symlinks.)
> >> >
> >> > It appears that the root directory is always problematic for an svn checkout
> >> > (actually, most checkouts were problematic on svn when junctions or symlinks
> >> > were in play on windows.) Looks like some additional special handling is going
> >> > to be required even with queries on c:\ (a real, not a substituted volume root.)
> >>
> >> Thank you for looking into this.
> >>
> >> You're absolutely right: with TSVN 1.13 (APR 1.7.0) I get the same
> >> erroneous behavior when 'svn -st'-ing a checkout to C:\ (real drive)
> >>
> >> [[[
> >> C:\>svn st -q
> >> !       .
> >> ]]]
> >
> >
> > Here's another experiment that drives me batty...
> >
> > svn co https://svn.apache.org/repos/asf/httpd/httpd/trunk httpd-2.x
> > svn co https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4 httpd-2.4
> > svn co https://svn.apache.org/repos/asf/httpd/docs/trunk httpd-docs
> > ln -s httpd-docs httpd-2.4/docs/manual/build
> > ln -s httpd-docs httpd-2.x/docs/manual/build
> > (flip the args using mklink /d or mklink /j on windows)
>
> That's a bit special: you're creating embedded working copies, which
> is already special to begin with (even if they were all normal,
> separate checkouts instead of symlinks).
> There is code that tries to crawl up the tree to find the working copy
> root. Perhaps that is going wrong in this case, depending on your CWD.
> Not sure.
>
> > You'll see svn is fluxored until you change directory. A similar ugliness
> > occurs if you attempt to check out a number of versions of httpd-test,
> > and attempt to point all of the X apache-test checkouts into a single
> > group.
> >
> > What I've determined is that apr_stat and apr_getfileinfo differ in only
> > one aspect, that protections on a root directory are 700 or 777 based
> > on the way we dress up fprot bits from windows ACLs, or not.
> >
> >> > Can you successfully `svn co {repos} /` on linux? Hoping to understand the
> >> > scope of the issue.
> >>
> >> Sorry, no Linux at my disposal where I'm root. But I suppose the test
> >> above with the checkout to C:\ already confirms your hypothesis.
> >
> >
> > I'm not sure what svn is doing with the '.' root entity, but it likely made
> > some absurd assumptions about symlinks.
>
> Yes, that's very well possible. It's a difficult area I think (also:
> there is the distinction between a symlink to a working copy root,
> which just needs to be "read through" for svn commands to act upon,
> and symlinks that are part your versioned data, which need to be
> versioned and re-created as symlinks in other working copies (cf. the
> "svn:special" property) -- something which currently is not supported
> on Windows (junctions), they are just converted into a text file with
> the link reference in it ... a long standing feature request for svn).
>
> I'm afraid I don't have much expertise in that area of the svn code.
> If we need wider input on this from the svn devs, I suppose
> dev@subversion should be added back to the cc list.
>
> However: with the issue reported here there is no symlink involved
> (well, maybe sort of, with the subst case, but then it also fails on a
> plain C:\ drive root). There seems to be a change in behavior between
> APR 1.6.5 and 1.7.0 that makes svn see different information when the
> working copy root is the root of a drive. Of course, this may very
> well be svn's fault for jumping through strange hoops to work on drive
> roots, and one of those hoops may have just changed form :-). I'm not
> discounting that possibility at all.

FYI, another data point:
I just saw a new report on the TortoiseSVN mailinglist, where a user
sees exactly the same problem with a "shared folder", where the
wc-root is the share-root:

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

Quoting:
[[[
C:\Users\ta>svn status \\computer-a\shared-working-copy
!       \\computer-a\shared-working-copy
]]]

I'm guessing this is the same problem with "drive-roots", just as with
subst or the root of real drives.

--
Johan

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Apr 10, 2020 at 3:45 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Thu, Apr 9, 2020 at 2:48 PM Johan Corveleyn <jc...@gmail.com> wrote:
>> On Thu, Apr 9, 2020 at 6:31 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>> >
>> > That's very likely, and this can be researched further to dig up exactly how
>> > subst is handled. (As you mention, it hasn't been common since the introduction
>> > of proper junction and directory/file symlinks.)
>> >
>> > It appears that the root directory is always problematic for an svn checkout
>> > (actually, most checkouts were problematic on svn when junctions or symlinks
>> > were in play on windows.) Looks like some additional special handling is going
>> > to be required even with queries on c:\ (a real, not a substituted volume root.)
>>
>> Thank you for looking into this.
>>
>> You're absolutely right: with TSVN 1.13 (APR 1.7.0) I get the same
>> erroneous behavior when 'svn -st'-ing a checkout to C:\ (real drive)
>>
>> [[[
>> C:\>svn st -q
>> !       .
>> ]]]
>
>
> Here's another experiment that drives me batty...
>
> svn co https://svn.apache.org/repos/asf/httpd/httpd/trunk httpd-2.x
> svn co https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4 httpd-2.4
> svn co https://svn.apache.org/repos/asf/httpd/docs/trunk httpd-docs
> ln -s httpd-docs httpd-2.4/docs/manual/build
> ln -s httpd-docs httpd-2.x/docs/manual/build
> (flip the args using mklink /d or mklink /j on windows)

That's a bit special: you're creating embedded working copies, which
is already special to begin with (even if they were all normal,
separate checkouts instead of symlinks).
There is code that tries to crawl up the tree to find the working copy
root. Perhaps that is going wrong in this case, depending on your CWD.
Not sure.

> You'll see svn is fluxored until you change directory. A similar ugliness
> occurs if you attempt to check out a number of versions of httpd-test,
> and attempt to point all of the X apache-test checkouts into a single
> group.
>
> What I've determined is that apr_stat and apr_getfileinfo differ in only
> one aspect, that protections on a root directory are 700 or 777 based
> on the way we dress up fprot bits from windows ACLs, or not.
>
>> > Can you successfully `svn co {repos} /` on linux? Hoping to understand the
>> > scope of the issue.
>>
>> Sorry, no Linux at my disposal where I'm root. But I suppose the test
>> above with the checkout to C:\ already confirms your hypothesis.
>
>
> I'm not sure what svn is doing with the '.' root entity, but it likely made
> some absurd assumptions about symlinks.

Yes, that's very well possible. It's a difficult area I think (also:
there is the distinction between a symlink to a working copy root,
which just needs to be "read through" for svn commands to act upon,
and symlinks that are part your versioned data, which need to be
versioned and re-created as symlinks in other working copies (cf. the
"svn:special" property) -- something which currently is not supported
on Windows (junctions), they are just converted into a text file with
the link reference in it ... a long standing feature request for svn).

I'm afraid I don't have much expertise in that area of the svn code.
If we need wider input on this from the svn devs, I suppose
dev@subversion should be added back to the cc list.

However: with the issue reported here there is no symlink involved
(well, maybe sort of, with the subst case, but then it also fails on a
plain C:\ drive root). There seems to be a change in behavior between
APR 1.6.5 and 1.7.0 that makes svn see different information when the
working copy root is the root of a drive. Of course, this may very
well be svn's fault for jumping through strange hoops to work on drive
roots, and one of those hoops may have just changed form :-). I'm not
discounting that possibility at all.

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Thu, Apr 9, 2020 at 2:48 PM Johan Corveleyn <jc...@gmail.com> wrote:

> On Thu, Apr 9, 2020 at 6:31 PM William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> >
> > That's very likely, and this can be researched further to dig up exactly
> how
> > subst is handled. (As you mention, it hasn't been common since the
> introduction
> > of proper junction and directory/file symlinks.)
> >
> > It appears that the root directory is always problematic for an svn
> checkout
> > (actually, most checkouts were problematic on svn when junctions or
> symlinks
> > were in play on windows.) Looks like some additional special handling is
> going
> > to be required even with queries on c:\ (a real, not a substituted
> volume root.)
>
> Thank you for looking into this.
>
> You're absolutely right: with TSVN 1.13 (APR 1.7.0) I get the same
> erroneous behavior when 'svn -st'-ing a checkout to C:\ (real drive)
>
> [[[
> C:\>svn st -q
> !       .
> ]]]
>

Here's another experiment that drives me batty...

svn co https://svn.apache.org/repos/asf/httpd/httpd/trunk httpd-2.x
svn co https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4 httpd-2.4
svn co https://svn.apache.org/repos/asf/httpd/docs/trunk httpd-docs
ln -s httpd-docs httpd-2.4/docs/manual/build
ln -s httpd-docs httpd-2.x/docs/manual/build
(flip the args using mklink /d or mklink /j on windows)

You'll see svn is fluxored until you change directory. A similar ugliness
occurs if you attempt to check out a number of versions of httpd-test,
and attempt to point all of the X apache-test checkouts into a single
group.

What I've determined is that apr_stat and apr_getfileinfo differ in only
one aspect, that protections on a root directory are 700 or 777 based
on the way we dress up fprot bits from windows ACLs, or not.

> Can you successfully `svn co {repos} /` on linux? Hoping to understand the
> > scope of the issue.
>
> Sorry, no Linux at my disposal where I'm root. But I suppose the test
> above with the checkout to C:\ already confirms your hypothesis.
>

I'm not sure what svn is doing with the '.' root entity, but it likely made
some absurd assumptions about symlinks.

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Thu, Apr 9, 2020 at 6:31 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> On Thu, Apr 9, 2020 at 8:01 AM Johan Corveleyn <jc...@gmail.com> wrote:
>>
>> On Thu, Apr 9, 2020 at 1:20 PM Nick Kew <ni...@apache.org> wrote:
>> > >
>> > > The below report about a problem with Subversion working on 'subst'ed
>> > > drives (Windows), seemed to be related to a change in APR 1.7.0
>> > > (https://svn.apache.org/r1855950),
>> >
>> > That fixed a bug PR#47630 that had been extensively discussed in
>> > bugzilla and shows as having 22 watchers - that's a lot of interest!
>> >
>> > Does that discussion not throw any light on your issue?  For example,
>> > Michael Schlenker's comments there refer to that bug affecting SVN.
>>
>> Thanks, I've read through that issue in Bugzilla now.
>>
>> It definitely seems like a useful improvement for proper support of
>> (different types of) reparse points on Windows. I'm not saying this
>> bugfix needs to be reverted. And indeed, Michael Schlenker's comments
>> point to a specific problem affecting SVN (in combination with Windows
>> Data Deduplication, which creates reparse points with tag
>> IO_REPARSE_TAG_DEDUP). So in general this is a step forward, and will
>> probably benefit SVN for use on these specific types of Windows
>> systems.
>>
>> However, I think this also broke something (or at least inadvertently
>> changed something) related to the usage of "SUBST" (as in 'SUBST G:\
>> D:\Development\TestWC'). The usage of "subst" is not mentioned
>> anywhere in PR#47630, so perhaps the compatibility with / impact on
>> subst was overlooked? I'm not an expert at all, but from what I could
>> find with a quick internet search, "subst" is very old (from MSDOS
>> times), and it seems it's not the same as a junction (? not sure about
>> the relation between subst and reparse points in general). In any
>> case, it's not clear to me that the fix for PR#47630 intended to
>> change something for "subst".
>
>
> That's very likely, and this can be researched further to dig up exactly how
> subst is handled. (As you mention, it hasn't been common since the introduction
> of proper junction and directory/file symlinks.)
>
> It appears that the root directory is always problematic for an svn checkout
> (actually, most checkouts were problematic on svn when junctions or symlinks
> were in play on windows.) Looks like some additional special handling is going
> to be required even with queries on c:\ (a real, not a substituted volume root.)

Thank you for looking into this.

You're absolutely right: with TSVN 1.13 (APR 1.7.0) I get the same
erroneous behavior when 'svn -st'-ing a checkout to C:\ (real drive)

[[[
C:\>svn st -q
!       .
]]]

Doing the same with SlikSVN 1.12 (APR 1.6.5), there is no problem:

[[[
C:\>"Program Files\SlikSvn\bin\svn.exe" st -q

]]]

(no output, as it should be -- I've made no modifications)

> Can you successfully `svn co {repos} /` on linux? Hoping to understand the
> scope of the issue.

Sorry, no Linux at my disposal where I'm root. But I suppose the test
above with the checkout to C:\ already confirms your hypothesis.

-- 
Johan

Re: Subversion reports status fault on substituted drive

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Thu, Apr 9, 2020 at 8:01 AM Johan Corveleyn <jc...@gmail.com> wrote:

> On Thu, Apr 9, 2020 at 1:20 PM Nick Kew <ni...@apache.org> wrote:
> > >
> > > The below report about a problem with Subversion working on 'subst'ed
> > > drives (Windows), seemed to be related to a change in APR 1.7.0
> > > (https://svn.apache.org/r1855950),
> >
> > That fixed a bug PR#47630 that had been extensively discussed in
> > bugzilla and shows as having 22 watchers - that's a lot of interest!
> >
> > Does that discussion not throw any light on your issue?  For example,
> > Michael Schlenker's comments there refer to that bug affecting SVN.
>
> Thanks, I've read through that issue in Bugzilla now.
>
> It definitely seems like a useful improvement for proper support of
> (different types of) reparse points on Windows. I'm not saying this
> bugfix needs to be reverted. And indeed, Michael Schlenker's comments
> point to a specific problem affecting SVN (in combination with Windows
> Data Deduplication, which creates reparse points with tag
> IO_REPARSE_TAG_DEDUP). So in general this is a step forward, and will
> probably benefit SVN for use on these specific types of Windows
> systems.
>
> However, I think this also broke something (or at least inadvertently
> changed something) related to the usage of "SUBST" (as in 'SUBST G:\
> D:\Development\TestWC'). The usage of "subst" is not mentioned
> anywhere in PR#47630, so perhaps the compatibility with / impact on
> subst was overlooked? I'm not an expert at all, but from what I could
> find with a quick internet search, "subst" is very old (from MSDOS
> times), and it seems it's not the same as a junction (? not sure about
> the relation between subst and reparse points in general). In any
> case, it's not clear to me that the fix for PR#47630 intended to
> change something for "subst".
>

That's very likely, and this can be researched further to dig up exactly how
subst is handled. (As you mention, it hasn't been common since the
introduction
of proper junction and directory/file symlinks.)

It appears that the root directory is always problematic for an svn checkout
(actually, most checkouts were problematic on svn when junctions or symlinks
were in play on windows.) Looks like some additional special handling is
going
to be required even with queries on c:\ (a real, not a substituted volume
root.)

Can you successfully `svn co {repos} /` on linux? Hoping to understand the
scope of the issue.

Re: Subversion reports status fault on substituted drive

Posted by Johan Corveleyn <jc...@gmail.com>.
On Thu, Apr 9, 2020 at 1:20 PM Nick Kew <ni...@apache.org> wrote:
> > On 9 Apr 2020, at 11:07, Johan Corveleyn <jc...@gmail.com> wrote:
> >
> > [ cc -= dev@subversion.a.o ]
> >
> > Hi APR devs,
> >
> > The below report about a problem with Subversion working on 'subst'ed
> > drives (Windows), seemed to be related to a change in APR 1.7.0
> > (https://svn.apache.org/r1855950),
>
> That fixed a bug PR#47630 that had been extensively discussed in
> bugzilla and shows as having 22 watchers - that's a lot of interest!
>
> Does that discussion not throw any light on your issue?  For example,
> Michael Schlenker's comments there refer to that bug affecting SVN.

Thanks, I've read through that issue in Bugzilla now.

It definitely seems like a useful improvement for proper support of
(different types of) reparse points on Windows. I'm not saying this
bugfix needs to be reverted. And indeed, Michael Schlenker's comments
point to a specific problem affecting SVN (in combination with Windows
Data Deduplication, which creates reparse points with tag
IO_REPARSE_TAG_DEDUP). So in general this is a step forward, and will
probably benefit SVN for use on these specific types of Windows
systems.

However, I think this also broke something (or at least inadvertently
changed something) related to the usage of "SUBST" (as in 'SUBST G:\
D:\Development\TestWC'). The usage of "subst" is not mentioned
anywhere in PR#47630, so perhaps the compatibility with / impact on
subst was overlooked? I'm not an expert at all, but from what I could
find with a quick internet search, "subst" is very old (from MSDOS
times), and it seems it's not the same as a junction (? not sure about
the relation between subst and reparse points in general). In any
case, it's not clear to me that the fix for PR#47630 intended to
change something for "subst".

As a result of this problem, Subversion on Windows cannot switch to
APR 1.7.0 (it is used in TortoiseSVN 1.13.1, which is currently TSVN's
latest release, but it has been reverted in
https://osdn.net/projects/tortoisesvn/scm/svn/commits/28700 and
backported to TortoiseSVN's 1.13.x branch, so the next release will be
back on APR 1.6.5).

(To be clear: I have no personal stake in this, I don't even use subst
-- I just want to unblock this seemingly blocked situation for the
greater good :-), and also to get on with building svn's next release
candidate on Windows, for which I need to pick an APR version.)

-- 
Johan

Re: Subversion reports status fault on substituted drive

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

> On 9 Apr 2020, at 11:07, Johan Corveleyn <jc...@gmail.com> wrote:
> 
> [ cc -= dev@subversion.a.o ]
> 
> Hi APR devs,
> 
> The below report about a problem with Subversion working on 'subst'ed
> drives (Windows), seemed to be related to a change in APR 1.7.0
> (https://svn.apache.org/r1855950),

That fixed a bug PR#47630 that had been extensively discussed in
bugzilla and shows as having 22 watchers - that's a lot of interest!

Does that discussion not throw any light on your issue?  For example,
Michael Schlenker's comments there refer to that bug affecting SVN.

-- 
Nick Kew