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
Received on 2015-12-21 15:58:09 CET