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? > - Julian > > Thanks, Daniel
