Roberto Reale wrote on Mon, Oct 26, 2015 at 20:56:30 +0100:
> I've endeavoured to improve on my previous patch, by adopting a
> heuristic approach as follows:
>
> 1) First and foremost, for each file in the backup copy I try and
> assign permissions based on those of the corresponding file in the
> source repository, if the latter does indeed exist.
> 2) Should this fail, I infer the permissions from the context, i.e. I
> choose one "typical file" from the same folder.
> 3) Should the folder itself be empty, I assign permissions based on
> the current umask.
>
> Do you deem such an approach to be correct?
A bit late, but:
I think (3) should fall back to $REPOS_DIR/db/fs-type (which is
guaranteed to exist) rather than to umask. (The umask has the right
dimension — permission bits — but using it might surprise users.)
Of academic interest [if you accept the change in the above paragraph]:
the os.listdir() call had an O(N²) cost where N is the number of files
in a directory, aka, was inefficient for unsharded FSFS repositories.
I don't have any other comments, but I haven't read the patch closely
enough to be comfortable committing it, either. Please feel free to
file an issue in our bug tracker about this patch, so we don't forget
it: https://issues.apache.org/jira/browse/SVN
Cheers,
Daniel
Received on 2015-12-21 15:58:09 CET