Re: handling of reparse points
On 08.06.2018 00:30, Branko Čibej wrote: Just FYI: in that case svn would have a requirement for NTFS. Because both hard links (for files) and junctions (for directories) only work on NTFS. So it wouldn't be possible anymore to use svn on e.g. a usb stick formatted with FAT32. Yes ... we'd _also_ have to detect the capabilities of the underlying filesystem. And not just that: you'd also have to check whether a reparse point is in fact a junction and not something else (like the sparse files of the OneDrive folder). On Windows, the MoveFileEx() API which is used by win32_file_rename which is used by svn_io_file_rename2 can work that way: the flag MOVEFILE_COPY_ALLOWED must be passed as well. Then the Move works across volumes because Windows then does the move as a copy/delete instead. We do use that flag, but — just like cross-device copies on Unix — the move is no longer atomic, which could break the working copy quite badly. there's also the MoveFileTransacted(), but MS recommends to not use that if there's another way to do it. Also: the current implementation on Windows handles links wrong: it assumes that a reparse point is always a junction, but that's not the case. A junction is just a special form of a reparse point. Well, the more patches we get to fix these issues, the better. In the mean time, can we apply my patch to make things work with OneDrive folders? Stefan
Re: handling of reparse points
Branko Čibej writes: >> We do use that flag, but — just like cross-device copies on Unix — the >> move is no longer atomic, which could break the working copy quite badly. > > I mean cross-device moves, of course, not copies. svn_io_file_move() is our wrapper that is intended to make it work: it does a cross-device copy to a temporary name and then an atomic rename. That looks like it should be safe and workqueue.c:run_file_move() has a comment claiming this works. However there appears to be a different bug in svn_io_file_move: it doesn't delete the original after a successful cross-device copy/rename. It looks like r1685793 accidentally removed the delete. The cross-device code is old, it predates wc-ng, so it is possible that it is no longer exercised. For wc-ng it is not currently possible to put .svn or .svn/tmp on a separate device. -- Philip
Re: handling of reparse points
On 08.06.2018 00:30, Branko Čibej wrote: > On 07.06.2018 18:50, Stefan Kueng wrote: >> >> On 05.06.2018 23:35, Philip Martin wrote: >>> Stefan Küng writes: >>> Index: subversion/libsvn_subr/io.c === --- subversion/libsvn_subr/io.c (revision 1831874) +++ subversion/libsvn_subr/io.c (working copy) @@ -342,8 +342,11 @@ /* Not using svn_io_stat() here because we want to check the apr_err return explicitly. */ SVN_ERR(cstring_from_utf8(&path_apr, path, pool)); - +#ifdef WIN32 + flags = APR_FINFO_MIN; +#else flags = resolve_symlinks ? APR_FINFO_MIN : (APR_FINFO_MIN | APR_FINFO_LINK); +#endif apr_err = apr_stat(&finfo, path_apr, flags, pool); if (APR_STATUS_IS_ENOENT(apr_err)) >>> This needs a comment to explain why Windows needs to do something >>> different. >> patch with comment attached. >> >>> I think we would have to back this change out if we ever attempted to >>> add svn:special support on Windows, but I suppose any such support is >>> unlikely in the near future; as Branko pointed out CreateSymbolicLink >>> doesn't have the semantics we want. >> Just FYI: in that case svn would have a requirement for NTFS. Because >> both hard links (for files) and junctions (for directories) only work >> on NTFS. So it wouldn't be possible anymore to use svn on e.g. a usb >> stick formatted with FAT32. > Yes ... we'd _also_ have to detect the capabilities of the underlying > filesystem. > >>> Making .svn a symbolic link on Linux came up in another thread recently: >>> >>> https://lists.apache.org/thread.html/d08f3a0b71600e2b67cd39817cd99e4de25a7e4f12817fe451f162e5@%3Cdev.subversion.apache.org%3E >>> >>> >>> At present there is an assumption that .svn lives on the same filesystem >>> as the working copy and that files can be atomically moved from .svn/tmp >>> into the working copy (see workqueue.c calling svn_io_file_rename2). If >>> .svn becomes a symlink that points to a different filesystem then these >>> moves fail and Subversion doesn't work. We might be able to work around >>> this by replacing svn_io_file_rename2 with svn_io_file_move. >>> >>> I don't know if Windows has the same problem. >> On Windows, the MoveFileEx() API which is used by win32_file_rename >> which is used by svn_io_file_rename2 can work that way: the flag >> MOVEFILE_COPY_ALLOWED must be passed as well. Then the Move works >> across volumes because Windows then does the move as a copy/delete >> instead. > We do use that flag, but — just like cross-device copies on Unix — the > move is no longer atomic, which could break the working copy quite badly. I mean cross-device moves, of course, not copies. >> Also: the current implementation on Windows handles links wrong: it >> assumes that a reparse point is always a junction, but that's not the >> case. A junction is just a special form of a reparse point. > Well, the more patches we get to fix these issues, the better. > > -- Brane >
Re: handling of reparse points
On 07.06.2018 18:50, Stefan Kueng wrote: > > > On 05.06.2018 23:35, Philip Martin wrote: >> Stefan Küng writes: >> >>> Index: subversion/libsvn_subr/io.c >>> === >>> --- subversion/libsvn_subr/io.c (revision 1831874) >>> +++ subversion/libsvn_subr/io.c (working copy) >>> @@ -342,8 +342,11 @@ >>> /* Not using svn_io_stat() here because we want to check the >>> apr_err return explicitly. */ >>> SVN_ERR(cstring_from_utf8(&path_apr, path, pool)); >>> - >>> +#ifdef WIN32 >>> + flags = APR_FINFO_MIN; >>> +#else >>> flags = resolve_symlinks ? APR_FINFO_MIN : (APR_FINFO_MIN | >>> APR_FINFO_LINK); >>> +#endif >>> apr_err = apr_stat(&finfo, path_apr, flags, pool); >>> if (APR_STATUS_IS_ENOENT(apr_err)) >>> >> >> This needs a comment to explain why Windows needs to do something >> different. > > patch with comment attached. > >> I think we would have to back this change out if we ever attempted to >> add svn:special support on Windows, but I suppose any such support is >> unlikely in the near future; as Branko pointed out CreateSymbolicLink >> doesn't have the semantics we want. > > Just FYI: in that case svn would have a requirement for NTFS. Because > both hard links (for files) and junctions (for directories) only work > on NTFS. So it wouldn't be possible anymore to use svn on e.g. a usb > stick formatted with FAT32. Yes ... we'd _also_ have to detect the capabilities of the underlying filesystem. >> Making .svn a symbolic link on Linux came up in another thread recently: >> >> https://lists.apache.org/thread.html/d08f3a0b71600e2b67cd39817cd99e4de25a7e4f12817fe451f162e5@%3Cdev.subversion.apache.org%3E >> >> >> At present there is an assumption that .svn lives on the same filesystem >> as the working copy and that files can be atomically moved from .svn/tmp >> into the working copy (see workqueue.c calling svn_io_file_rename2). If >> .svn becomes a symlink that points to a different filesystem then these >> moves fail and Subversion doesn't work. We might be able to work around >> this by replacing svn_io_file_rename2 with svn_io_file_move. >> >> I don't know if Windows has the same problem. > > On Windows, the MoveFileEx() API which is used by win32_file_rename > which is used by svn_io_file_rename2 can work that way: the flag > MOVEFILE_COPY_ALLOWED must be passed as well. Then the Move works > across volumes because Windows then does the move as a copy/delete > instead. We do use that flag, but — just like cross-device copies on Unix — the move is no longer atomic, which could break the working copy quite badly. > Also: the current implementation on Windows handles links wrong: it > assumes that a reparse point is always a junction, but that's not the > case. A junction is just a special form of a reparse point. Well, the more patches we get to fix these issues, the better. -- Brane
Re: handling of reparse points
On 05.06.2018 23:35, Philip Martin wrote: Stefan Küng writes: Index: subversion/libsvn_subr/io.c === --- subversion/libsvn_subr/io.c (revision 1831874) +++ subversion/libsvn_subr/io.c (working copy) @@ -342,8 +342,11 @@ /* Not using svn_io_stat() here because we want to check the apr_err return explicitly. */ SVN_ERR(cstring_from_utf8(&path_apr, path, pool)); - +#ifdef WIN32 + flags = APR_FINFO_MIN; +#else flags = resolve_symlinks ? APR_FINFO_MIN : (APR_FINFO_MIN | APR_FINFO_LINK); +#endif apr_err = apr_stat(&finfo, path_apr, flags, pool); if (APR_STATUS_IS_ENOENT(apr_err)) This needs a comment to explain why Windows needs to do something different. patch with comment attached. I think we would have to back this change out if we ever attempted to add svn:special support on Windows, but I suppose any such support is unlikely in the near future; as Branko pointed out CreateSymbolicLink doesn't have the semantics we want. Just FYI: in that case svn would have a requirement for NTFS. Because both hard links (for files) and junctions (for directories) only work on NTFS. So it wouldn't be possible anymore to use svn on e.g. a usb stick formatted with FAT32. Making .svn a symbolic link on Linux came up in another thread recently: https://lists.apache.org/thread.html/d08f3a0b71600e2b67cd39817cd99e4de25a7e4f12817fe451f162e5@%3Cdev.subversion.apache.org%3E At present there is an assumption that .svn lives on the same filesystem as the working copy and that files can be atomically moved from .svn/tmp into the working copy (see workqueue.c calling svn_io_file_rename2). If .svn becomes a symlink that points to a different filesystem then these moves fail and Subversion doesn't work. We might be able to work around this by replacing svn_io_file_rename2 with svn_io_file_move. I don't know if Windows has the same problem. On Windows, the MoveFileEx() API which is used by win32_file_rename which is used by svn_io_file_rename2 can work that way: the flag MOVEFILE_COPY_ALLOWED must be passed as well. Then the Move works across volumes because Windows then does the move as a copy/delete instead. Also: the current implementation on Windows handles links wrong: it assumes that a reparse point is always a junction, but that's not the case. A junction is just a special form of a reparse point. Stefan Index: libsvn_subr/io.c === --- libsvn_subr/io.c(revision 1833061) +++ libsvn_subr/io.c(working copy) @@ -342,8 +342,12 @@ /* Not using svn_io_stat() here because we want to check the apr_err return explicitly. */ SVN_ERR(cstring_from_utf8(&path_apr, path, pool)); - +#ifdef WIN32 + /* on Windows, svn does not handle reparse points or hard links */ + flags = APR_FINFO_MIN; +#else flags = resolve_symlinks ? APR_FINFO_MIN : (APR_FINFO_MIN | APR_FINFO_LINK); +#endif apr_err = apr_stat(&finfo, path_apr, flags, pool); if (APR_STATUS_IS_ENOENT(apr_err))
Re: handling of reparse points
Stefan Küng writes: > Index: subversion/libsvn_subr/io.c > === > --- subversion/libsvn_subr/io.c (revision 1831874) > +++ subversion/libsvn_subr/io.c (working copy) > @@ -342,8 +342,11 @@ >/* Not using svn_io_stat() here because we want to check the > apr_err return explicitly. */ >SVN_ERR(cstring_from_utf8(&path_apr, path, pool)); > - > +#ifdef WIN32 > + flags = APR_FINFO_MIN; > +#else >flags = resolve_symlinks ? APR_FINFO_MIN : (APR_FINFO_MIN | > APR_FINFO_LINK); > +#endif >apr_err = apr_stat(&finfo, path_apr, flags, pool); > >if (APR_STATUS_IS_ENOENT(apr_err)) > This needs a comment to explain why Windows needs to do something different. I think we would have to back this change out if we ever attempted to add svn:special support on Windows, but I suppose any such support is unlikely in the near future; as Branko pointed out CreateSymbolicLink doesn't have the semantics we want. Making .svn a symbolic link on Linux came up in another thread recently: https://lists.apache.org/thread.html/d08f3a0b71600e2b67cd39817cd99e4de25a7e4f12817fe451f162e5@%3Cdev.subversion.apache.org%3E At present there is an assumption that .svn lives on the same filesystem as the working copy and that files can be atomically moved from .svn/tmp into the working copy (see workqueue.c calling svn_io_file_rename2). If .svn becomes a symlink that points to a different filesystem then these moves fail and Subversion doesn't work. We might be able to work around this by replacing svn_io_file_rename2 with svn_io_file_move. I don't know if Windows has the same problem. -- Philip
Re: handling of reparse points
On Sat, May 26, 2018 at 7:35 AM Stefan Küng wrote: > Next try, patch attached. > Basically what this does is to never check for soft/hard links on Windows > ever. Since Subversion doesn't really handle those on Windows anyway, > checking for them doesn't make sense and causes problems. > > I've done some testing with this patch and it seems to work fine. > > Any comments on my patch? If there are no objections, I'd like to have this committed. Stefan
Re: handling of reparse points
Johan Corveleyn-3 wrote > On Sat, May 26, 2018 at 9:42 AM, Branko Čibej < > brane@ > > wrote: >> On 25.05.2018 20:30, Stefan Kueng wrote: >>> >>> >>> On 25.05.2018 18:07, Daniel Shahaf wrote: Stefan Küng wrote on Fri, 25 May 2018 17:37 +0200: > Can anyone comment on this please? I'm not familiar with the Windows side of things, but I gave the patch a spin. >>> >>> And I'm not familiar with soft/hardlinks on Linux. :) >>> If I do 'mv .svn x; ln -s x .svn', things seem to work. However, if the target of the '.svn' symlink isn't in the same directory as the symlink, 'status' just shows everything with '!' status. I didn't check, but if I'd done 'ln -s ../../.svn subversion/tests/.svn' and run 'status' in subversion/tests/, I assume it would confuse trunk/README and trunk/subversion/tests/README (due to having the same basename). >>> >>> ok, so this breaks on non-Windows systems. >>> I'll try another patch soon. >>> >>> question: symlink handling in svn does not seem to actually work on >>> Windows. At least the docs state that they're not handled on Windows. >>> So why do we even check for 'special' nodes on a Windows build? >>> >>> If e.g. svn_io_check_special_path() on Windows would always return >>> false and svn_io_check_path() always call svn_io_check_resolved_path() >>> instead, this would work fine with the reparse points on Windows. >>> >>> So is there a reason why this is not done so on Windows? Is there >>> maybe a reason for handling reparse points specially on Windows that I >>> don't know of? >> >> Subversion can't create symbolic links on Windows the way it can on any >> normal OS, for several reasons: >> >> * The Windows CreateSymbolicLink function needs to know in advance if >> the target of a symlink (or any other kind of reparse point, really) >> is a directory or not. That's a non-starter; Unix symlinks can point >> to a non-existent target. >> * It requires elevated permissions on most versions of Windows. >> * Windows reparse points have subtly different semantics than symlinks >> on Unix. > > I don't think that's what StefanK is after (versioning symlinks / > handling versioned symlinks on Windows). AFAIU Stefan is trying to > make SVN work properly on Windows when the entire working copy itself > (or maybe only its .svn folder?) is "tucked away" behind a reparse > point. > > "It seems that svn doesn't handle reparse points properly in all > situations. Which causes big problems for TSVN when a working copy is > on a Onedrive folder. [...] I've attached a patch for handling reparse > points better when detecting and reading the working copy database." > > Or maybe you were replying specifically to Stefan's points / questions > about svn_io_check_[special_|resolved_]path ? Does he actually need to > use another API (or write his own helper functions) for what he's > trying to do here? > > (Just trying to clear up a possible source of confusion) > -- > Johan Thank you for responce - Mobile App Development Company -- Sent from: http://subversion.1072662.n5.nabble.com/Subversion-Dev-f4725.html
Re: handling of reparse points
On Sat, May 26, 2018 at 9:42 AM, Branko Čibej wrote: > On 25.05.2018 20:30, Stefan Kueng wrote: >> >> >> On 25.05.2018 18:07, Daniel Shahaf wrote: >>> Stefan Küng wrote on Fri, 25 May 2018 17:37 +0200: Can anyone comment on this please? >>> >>> I'm not familiar with the Windows side of things, but I gave the >>> patch a spin. >> >> And I'm not familiar with soft/hardlinks on Linux. :) >> >>> If I do 'mv .svn x; ln -s x .svn', things seem to work. However, if the >>> target of the '.svn' symlink isn't in the same directory as the symlink, >>> 'status' just shows everything with '!' status. I didn't check, but >>> if I'd >>> done 'ln -s ../../.svn subversion/tests/.svn' and run 'status' in >>> subversion/tests/, I assume it would confuse trunk/README and >>> trunk/subversion/tests/README (due to having the same basename). >> >> ok, so this breaks on non-Windows systems. >> I'll try another patch soon. >> >> question: symlink handling in svn does not seem to actually work on >> Windows. At least the docs state that they're not handled on Windows. >> So why do we even check for 'special' nodes on a Windows build? >> >> If e.g. svn_io_check_special_path() on Windows would always return >> false and svn_io_check_path() always call svn_io_check_resolved_path() >> instead, this would work fine with the reparse points on Windows. >> >> So is there a reason why this is not done so on Windows? Is there >> maybe a reason for handling reparse points specially on Windows that I >> don't know of? > > Subversion can't create symbolic links on Windows the way it can on any > normal OS, for several reasons: > > * The Windows CreateSymbolicLink function needs to know in advance if > the target of a symlink (or any other kind of reparse point, really) > is a directory or not. That's a non-starter; Unix symlinks can point > to a non-existent target. > * It requires elevated permissions on most versions of Windows. > * Windows reparse points have subtly different semantics than symlinks > on Unix. I don't think that's what StefanK is after (versioning symlinks / handling versioned symlinks on Windows). AFAIU Stefan is trying to make SVN work properly on Windows when the entire working copy itself (or maybe only its .svn folder?) is "tucked away" behind a reparse point. "It seems that svn doesn't handle reparse points properly in all situations. Which causes big problems for TSVN when a working copy is on a Onedrive folder. [...] I've attached a patch for handling reparse points better when detecting and reading the working copy database." Or maybe you were replying specifically to Stefan's points / questions about svn_io_check_[special_|resolved_]path ? Does he actually need to use another API (or write his own helper functions) for what he's trying to do here? (Just trying to clear up a possible source of confusion) -- Johan
Re: handling of reparse points
On 25.05.2018 20:30, Stefan Kueng wrote:
>
>
> On 25.05.2018 18:07, Daniel Shahaf wrote:
>> Stefan Küng wrote on Fri, 25 May 2018 17:37 +0200:
>>> Can anyone comment on this please?
>>
>> I'm not familiar with the Windows side of things, but I gave the
>> patch a spin.
>
> And I'm not familiar with soft/hardlinks on Linux. :)
>
>> If I do 'mv .svn x; ln -s x .svn', things seem to work. However, if the
>> target of the '.svn' symlink isn't in the same directory as the symlink,
>> 'status' just shows everything with '!' status. I didn't check, but
>> if I'd
>> done 'ln -s ../../.svn subversion/tests/.svn' and run 'status' in
>> subversion/tests/, I assume it would confuse trunk/README and
>> trunk/subversion/tests/README (due to having the same basename).
>
> ok, so this breaks on non-Windows systems.
> I'll try another patch soon.
>
> question: symlink handling in svn does not seem to actually work on
> Windows. At least the docs state that they're not handled on Windows.
> So why do we even check for 'special' nodes on a Windows build?
>
> If e.g. svn_io_check_special_path() on Windows would always return
> false and svn_io_check_path() always call svn_io_check_resolved_path()
> instead, this would work fine with the reparse points on Windows.
>
> So is there a reason why this is not done so on Windows? Is there
> maybe a reason for handling reparse points specially on Windows that I
> don't know of?
Subversion can't create symbolic links on Windows the way it can on any
normal OS, for several reasons:
* The Windows CreateSymbolicLink function needs to know in advance if
the target of a symlink (or any other kind of reparse point, really)
is a directory or not. That's a non-starter; Unix symlinks can point
to a non-existent target.
* It requires elevated permissions on most versions of Windows.
* Windows reparse points have subtly different semantics than symlinks
on Unix.
IMHO handling links/reparse-points specially is only required in very
specific situations, like adding them. In most cases, resolving the
links/reparse points is what should be done. Or am I missing something
big here?
>>
>> This is a very general statement ("most code should use stat() rather
>> than lstat()"). What specifically are you proposing to do?
>
> Seems on Linux this really needs special handling.
> But as far as I can see, on Windows this special handling actually
> breaks things.
>
> Stefan
>
Re: handling of reparse points
Next try, patch attached.
Basically what this does is to never check for soft/hard links on Windows
ever. Since Subversion doesn't really handle those on Windows anyway,
checking for them doesn't make sense and causes problems.
I've done some testing with this patch and it seems to work fine.
Stefan
On Fri, May 25, 2018 at 8:30 PM Stefan Kueng wrote:
>
>
> On 25.05.2018 18:07, Daniel Shahaf wrote:
> > Stefan Küng wrote on Fri, 25 May 2018 17:37 +0200:
> >> Can anyone comment on this please?
> >
> > I'm not familiar with the Windows side of things, but I gave the
> > patch a spin.
>
> And I'm not familiar with soft/hardlinks on Linux. :)
>
> > If I do 'mv .svn x; ln -s x .svn', things seem to work. However, if the
> > target of the '.svn' symlink isn't in the same directory as the symlink,
> > 'status' just shows everything with '!' status. I didn't check, but if
> I'd
> > done 'ln -s ../../.svn subversion/tests/.svn' and run 'status' in
> > subversion/tests/, I assume it would confuse trunk/README and
> > trunk/subversion/tests/README (due to having the same basename).
>
> ok, so this breaks on non-Windows systems.
> I'll try another patch soon.
>
> question: symlink handling in svn does not seem to actually work on
> Windows. At least the docs state that they're not handled on Windows.
> So why do we even check for 'special' nodes on a Windows build?
>
> If e.g. svn_io_check_special_path() on Windows would always return false
> and svn_io_check_path() always call svn_io_check_resolved_path()
> instead, this would work fine with the reparse points on Windows.
>
> So is there a reason why this is not done so on Windows? Is there maybe
> a reason for handling reparse points specially on Windows that I don't
> know of?
>
> >>> IMHO handling links/reparse-points specially is only required in very
> >>> specific situations, like adding them. In most cases, resolving the
> >>> links/reparse points is what should be done. Or am I missing something
> >>> big here?
> >
> > This is a very general statement ("most code should use stat() rather
> > than lstat()"). What specifically are you proposing to do?
>
> Seems on Linux this really needs special handling.
> But as far as I can see, on Windows this special handling actually
> breaks things.
>
> Stefan
>
>
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/>The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
reparse.patch
Description: Binary data
Re: handling of reparse points
On 25.05.2018 18:07, Daniel Shahaf wrote:
Stefan Küng wrote on Fri, 25 May 2018 17:37 +0200:
Can anyone comment on this please?
I'm not familiar with the Windows side of things, but I gave the
patch a spin.
And I'm not familiar with soft/hardlinks on Linux. :)
If I do 'mv .svn x; ln -s x .svn', things seem to work. However, if the
target of the '.svn' symlink isn't in the same directory as the symlink,
'status' just shows everything with '!' status. I didn't check, but if I'd
done 'ln -s ../../.svn subversion/tests/.svn' and run 'status' in
subversion/tests/, I assume it would confuse trunk/README and
trunk/subversion/tests/README (due to having the same basename).
ok, so this breaks on non-Windows systems.
I'll try another patch soon.
question: symlink handling in svn does not seem to actually work on
Windows. At least the docs state that they're not handled on Windows.
So why do we even check for 'special' nodes on a Windows build?
If e.g. svn_io_check_special_path() on Windows would always return false
and svn_io_check_path() always call svn_io_check_resolved_path()
instead, this would work fine with the reparse points on Windows.
So is there a reason why this is not done so on Windows? Is there maybe
a reason for handling reparse points specially on Windows that I don't
know of?
IMHO handling links/reparse-points specially is only required in very
specific situations, like adding them. In most cases, resolving the
links/reparse points is what should be done. Or am I missing something
big here?
This is a very general statement ("most code should use stat() rather
than lstat()"). What specifically are you proposing to do?
Seems on Linux this really needs special handling.
But as far as I can see, on Windows this special handling actually
breaks things.
Stefan
Re: handling of reparse points
Stefan Küng wrote on Fri, 25 May 2018 17:37 +0200:
> Can anyone comment on this please?
I'm not familiar with the Windows side of things, but I gave the
patch a spin.
If I do 'mv .svn x; ln -s x .svn', things seem to work. However, if the
target of the '.svn' symlink isn't in the same directory as the symlink,
'status' just shows everything with '!' status. I didn't check, but if I'd
done 'ln -s ../../.svn subversion/tests/.svn' and run 'status' in
subversion/tests/, I assume it would confuse trunk/README and
trunk/subversion/tests/README (due to having the same basename).
It also has a test failure, attached.
> On Fri, May 18, 2018 at 10:29 PM Stefan Kueng wrote:
>
> > Hi,
> >
> > It seems that svn doesn't handle reparse points properly in all
> > situations. Which causes big problems for TSVN when a working copy is on
> > a Onedrive folder.
> >
> > See here for more details:
> > https://groups.google.com/d/msg/tortoisesvn/hlQj5OifhBg/RX_cw_sQCgAJ
> >
> > While in this particular situation, other svn clients are not affected,
> > it still means that there's a problem.
> >
> > I've attached a patch for handling reparse points better when detecting
> > and reading the working copy database. With this change, the TSVN
> > context menu shows up properly.
> >
> > But while I was trying to figure out this particular problem I noticed
> > that svn_io_check_path() never tries to resolve the reparse points. Why
> > is that? I mean if we need to check for links/reparse-points there's
> > svn_io_check_special_path(). Also, svn_io_check_path() always returns
> > node_file for reparse points, even if they point to directories.
I, too, would expect stat() to return 'directory' for a symlink to a directory.
> > IMHO handling links/reparse-points specially is only required in very
> > specific situations, like adding them. In most cases, resolving the
> > links/reparse points is what should be done. Or am I missing something
> > big here?
This is a very general statement ("most code should use stat() rather
than lstat()"). What specifically are you proposing to do?
Cheers,
Daniel
special-tests-23.out
Description: Binary data
Re: handling of reparse points
Can anyone comment on this please? On Fri, May 18, 2018 at 10:29 PM Stefan Kueng wrote: > Hi, > > It seems that svn doesn't handle reparse points properly in all > situations. Which causes big problems for TSVN when a working copy is on > a Onedrive folder. > > See here for more details: > https://groups.google.com/d/msg/tortoisesvn/hlQj5OifhBg/RX_cw_sQCgAJ > > While in this particular situation, other svn clients are not affected, > it still means that there's a problem. > > I've attached a patch for handling reparse points better when detecting > and reading the working copy database. With this change, the TSVN > context menu shows up properly. > > But while I was trying to figure out this particular problem I noticed > that svn_io_check_path() never tries to resolve the reparse points. Why > is that? I mean if we need to check for links/reparse-points there's > svn_io_check_special_path(). Also, svn_io_check_path() always returns > node_file for reparse points, even if they point to directories. > IMHO handling links/reparse-points specially is only required in very > specific situations, like adding them. In most cases, resolving the > links/reparse points is what should be done. Or am I missing something > big here? > > Stefan > -- ___ oo // \\ "De Chelonian Mobile" (_,\/ \_/ \ TortoiseSVN \ \_/_\_/>The coolest Interface to (Sub)Version Control /_/ \_\ http://tortoisesvn.net
handling of reparse points
Hi,
It seems that svn doesn't handle reparse points properly in all
situations. Which causes big problems for TSVN when a working copy is on
a Onedrive folder.
See here for more details:
https://groups.google.com/d/msg/tortoisesvn/hlQj5OifhBg/RX_cw_sQCgAJ
While in this particular situation, other svn clients are not affected,
it still means that there's a problem.
I've attached a patch for handling reparse points better when detecting
and reading the working copy database. With this change, the TSVN
context menu shows up properly.
But while I was trying to figure out this particular problem I noticed
that svn_io_check_path() never tries to resolve the reparse points. Why
is that? I mean if we need to check for links/reparse-points there's
svn_io_check_special_path(). Also, svn_io_check_path() always returns
node_file for reparse points, even if they point to directories.
IMHO handling links/reparse-points specially is only required in very
specific situations, like adding them. In most cases, resolving the
links/reparse points is what should be done. Or am I missing something
big here?
Stefan
Index: subversion/libsvn_wc/wc_db_wcroot.c
===
--- subversion/libsvn_wc/wc_db_wcroot.c (revision 1831872)
+++ subversion/libsvn_wc/wc_db_wcroot.c (working copy)
@@ -554,6 +554,11 @@
### into wc_db which references a file. calls for directories could
### get an early-exit in the hash lookup just above. */
SVN_ERR(get_path_kind(&kind, db, local_abspath, scratch_pool));
+ if (kind == svn_node_symlink)
+{
+ /* if this is a symlink, try to get the real node kind */
+ SVN_ERR(svn_io_check_resolved_path(local_abspath, &kind, scratch_pool));
+}
if (kind != svn_node_dir)
{
/* If the node specified by the path is NOT present, then it cannot
@@ -622,7 +627,7 @@
const char *adm_subdir = svn_dirent_join(local_abspath, adm_relpath,
scratch_pool);
- SVN_ERR(svn_io_check_path(adm_subdir, &adm_subdir_kind, scratch_pool));
+ SVN_ERR(svn_io_check_resolved_path(adm_subdir, &adm_subdir_kind,
scratch_pool));
if (adm_subdir_kind == svn_node_dir)
{
