On Fri, Aug 7, 2015 at 1:25 AM, <b...@qqmail.nl> wrote: > [And now to the proper list] > > > > On the buildbots I see > > [[[ > > ..\..\..\subversion\tests\libsvn_fs_x\fs-x-pack-test.c:873, > > ..\..\..\subversion\libsvn_fs_x\batch_fsync.c:386, > > ..\..\..\subversion\libsvn_fs_x\batch_fsync.c:343, > > ..\..\..\subversion\libsvn_subr\io.c:3515: (apr_err=720005) > > svn_tests: E720005: Can't open file > 'E:\svn-local\tests\subversion\tests\libsvn_fs_x': Access is denied. > > FAIL: fs-x-pack-test 13: test batch fsync > > ]]] > > after this commit > > > > It looks like the batch fsync introduced in this patch is trying to open a > directory as a file? > > That is not going to work on Windows, and probably on more platforms. > Opening a directory requires other functions. > > Note that this code is called inside a 'SVN_ON_POSIX' block, which I would > assume shouldn't be active on Windows. > Yup, that's where the bug is / was. It must be "#if SVN_ON_POSIX" instead of "#ifdef SVN_ON_POSIX". Fixed in r1694669.
> Last time we discussed batch syncing you told me/us, that you would use > the original handles to perform the flush... This code appears to tell a > different story? > No, it doesn't. The svn_fs_x__batch_fsync_new_path call only informs the batch that this file is actually new, implying that the parent folder needs fsync-ing on POSIX. When used as intended, the file itself has been opened through / in the batch container. Only if it hadn't, the container would need to open it. > Reopening files may, and commonly will cause a huge performance hit on > Windows... because virusscanners are designed to delay those operations, > until they are sure the file doesn’t contain a virus; especially if the > file was just written. > The FSX code should not reopen files. The tests might - just to cover as many cases as they can. -- Stefan^2.
