On Tue, 2010-08-03 at 20:08 +0300, Daniel Shahaf wrote:
> Julian Foad wrote on Tue, Aug 03, 2010 at 17:36:00 +0100:
> > On Tue, 2010-08-03 at 18:19 +0300, Daniel Shahaf wrote:
> > > Daniel Shahaf wrote on Tue, Aug 03, 2010 at 11:58:32 +0300:
> > > > There was yesterday on IRC [1] some discussion around whether revprop
> > > > editing can corrupt rev files.
> > > >
> > > > [[[
> > > > 0:% rm -rf r
> > > > 0:% ./subversion/svnadmin/svnadmin create r
> > > > 0:% ls -l r/db/revs/0/0
> > > > -rw-r--r-- 1 daniel daniel 115 2010-08-03 11:56 r/db/revs/0/0
> > > > ]]]
> > > >
> > > > Shouldn't we create rev files with the write bit disabled? (i.e., 0222 umask)
> > >
> > > r981921 + nominated for backport.
> >
> > The idea looks sane.
> >
> > There's perhaps a problem with revPROPS files: fs_fs.c copies the
> > permissions of a revprops file from the corresponding rev file, which
> > now means all revprops files will be read-only, which might cause a
> > problem when trying to edit a revprop.
> >
>
> Good catch.
>
> As far as I can tell, 'svnadmin setrevprop' and 'svn propset --revprop'
> still work and fs-test and fs-pack-test (which test svn_fs_change_rev_prop())
> still pass (even though the revprop files are mode 0444), so I'm tempted to
> leave the code as-is.
>
> Does anyone see a problem here, or see an error when trying to edit revprops
> with this patch applied?
I didn't encounter a problem on Ubuntu, in a simple manual test (not
trying this within the test suite, this time).
"svn propset --revprop -r <REV> ...": for a packed rev it modified
'revprops.db', leaving that file read-write, and for a not-yet-packed
revision (r30) it modified the file 'revprops/3/30', leaving that file
read-only.
"svnadmin pack" worked, and the pack files and manifests are read-only.
- Julian
Received on 2010-08-04 12:15:33 CEST