On Thu, Nov 12, 2020 at 12:25 PM Branko Čibej <br...@apache.org> wrote:
>
> On 12.11.2020 17:38, Branko Čibej wrote:
> > On 26.06.2019 17:48, Branko Čibej wrote:
> >> On 26.06.2019 04:52, Quentin Smith wrote:
> >>> Hi,
> >>>
> >>> svn_io_file_flush_to_disk in subversion/libsvn_subr/io.c fails to
> >>> flush files on AFS filesystems on Darwin. The user-visible
> >>> experience is:
> >>>
> >>> $ svn commit
> >>> svn: E000025: Commit failed (details follow):
> >>> svn: E000025: Can't write '/afs/path/db/txn-current' atomically
> >>> svn: E000025: Can't flush file '/afs/path/db/svn-CNFY6N' to disk:
> >>> Inappropriate ioctl for device
> >>>
> >>> This is because Darwin defines F_FULLFSYNC, which on AFS (and maybe
> >>> other filesystems?) returns ENOTTY.
> >>>
> >>> The code ignores EINVAL with a comment about filesystems that don't
> >>> support it; evidently on Darwin this also needs to include ENOTTY.
> >>>
> >>> (Also, I suspect it should fall back from fcntl to fsync, since I
> >>> suspect fsync /does/ work on AFS filesystems.)
> >>>
> >>> I repro'd this with svn 1.10.3 (r1842928), but inspection of trunk
> >>> confirms the code has not changed.
> >> First of all, thanks for the repor.
> >>
> >> Please provide a standalone reproduction script, using our command-line
> >> client.
> >>
> >> Last but not least, I do wish filesystems were consistent in their
> >> implementation ... what on earth is ENOTTY doing here?
> >
> > Sometimes it's good to have ages old browser tabs lying around. This
> > is now fixed in apr_file_datasync on APR trunk and the 1.7.x branch.
> >
> > -- Brane
> >
> > https://svn.apache.org/viewvc?view=revision&revision=1883341
>
> And r1883355 makes Subversion use the right APR functions.
I just realized that this nomination:
[[[
* r1883355
Use the APR-1.4+ API for flushing file contents to disk.
Justification:
Reduce code duplication between APR and SVN.
Votes:
+1: brane
]]]
isn't a mere refactoring, as I thought previously, but rather is a fix
for the above flush-to-disk issue on AFS filesystems--assuming, of
course, that SVN is built with APR >= 1.7. (Perhaps this nomination
deserves a Notes section to point that out.)
(I will probably vote +1 for this, pending to check, since we support
building with APR >= 1.4, if there is a chance of some regression
being introduced when SVN is built with APR < 1.7.)
Nathan
