To begin with, I edited the patch to implement Daniel's (a)-proposal.
Cheers,
Roberto
On Wed, Dec 23, 2015 at 10:11 AM, Daniel Shahaf <d...@daniel.shahaf.name> wrote:
> Roberto Reale wrote on Mon, Dec 21, 2015 at 16:25:10 +0100:
>> Of course I would be glad to modify the patch, but... shall we content
>> ourselves with the somewhat elusive fs-type? :)
>
> Using fs-type would break backwards compatibility with BDB repositories
> created by svnadmin 1.0.x (or earlier) that have never been dump/load-ed.
>
> Now, since hot-backup.py was in tools/ in 1.0, it's formally covered by
> the same backwards compatibility promise as everything else.
>
> However, I don't think there are many people who use a repository
> created by 1.0 that has never been dump/load-ed by any later version of
> svnadmin; that is, I think few if any people would be affected by
> breaking compatibility in this specific case. (And those who *would* be
> affected would receive an error message, as opposed to silent misbehaviour.)
>
> So, how about we (a) make the patch use fs-type, (b) issue an erratumน
> stating the breakage and providing the workaround?
>
> Alternatively, we could just find some file that's guaranteed to exist
> in all 1.0 BDB repositories and use that file if fs-type is missing.
>
> Cheers,
>
> Daniel
>
> น See notes/api-errata/.
[[[
* tools/backup/hot-backup.py.in: Always save correct file
permissions in tar archives.
]]]
Index: tools/backup/hot-backup.py.in
===================================================================
--- tools/backup/hot-backup.py.in (revision 1706408)
+++ tools/backup/hot-backup.py.in (working copy)
@@ -292,7 +292,61 @@ if archive_type:
try:
import tarfile
tar = tarfile.open(archive_path, 'w:' + archive_type)
- tar.add(backup_subdir, os.path.basename(backup_subdir))
+
+ # Build up the archive file by file, paying special care in replicating
+ # the permissions for each file from the source repository itself rather
+ # than from the hotcopy dump.
+ #
+ # This can be useful in such cases when the source repository is on a
+ # POSIX filesystem and the hotcopy dump is written e.g. on a FAT or a
+ # CIFS filesystem, thus leading to a possible "permission leakage" were
+ # the archive built directly from hotcopy dump via tar.add().
+ #
+ # Since, however, nothing guarantees that all the files in the backup are
+ # still present in the source repository (due e.g. to revision files
+ # being removed by pack on a FSFS repository, or to a log file being
+ # deleted by a commit to a BDB repository), we are forced to resort to a
+ # heuristic approach.
+ #
+ # First and foremost, we try and assign permissions based on those of the
+ # corresponding file in the source repository (case marked S below).
+ # Should this fail, we infer them from the context, i.e. we choose one
+ # "typical file" from the same folder (case C). Should the folder itself
+ # be empty, we assign permissions based on those of a file which is
+ # guaranteed to exist (case F).
+
+ for dirpath, dirnames, filenames in os.walk(backup_subdir,
followlinks=True):
+ for leafname in dirnames + filenames:
+ backup_based_path = os.path.join(dirpath, leafname)
+ relpath = os.path.relpath(backup_based_path, backup_subdir)
+ repo_based_path = os.path.join(repo_dir, relpath)
+ if not os.path.exists(repo_based_path):
+ # (C) Infer permissions from some "typical file" to be found
+ # underneath the same folder.
+ def choose_one_file_in_path(path):
+ files_in_path = [f for f in os.listdir(path)
+ if os.path.isfile(os.path.join(path, f))]
+ if files_in_path:
+ return files_in_path[0]
+ repo_based_p_path = os.path.dirname(repo_based_path) # "parent
path"
+ typical_file_in_path = choose_one_file_in_path(repo_based_p_path)
+ if not typical_file_in_path:
+ repo_based_path = None
+ repo_based_path = os.path.join(repo_based_p_path,
typical_file_in_path)
+ tarinfo = tar.gettarinfo(backup_based_path, os.path.join(repo,
relpath))
+ if not repo_based_path:
+ # (S) Assign permissions based on those of the corresponding file
+ # in the source repository.
+ tarinfo.mode = os.stat(repo_based_path).st_mode
+ else:
+ # (T) Assign permissions based on those of the file db/fs-type in
+ # the source repository.
+ tarinfo.mode = os.stat(os.path.join(path, "db", "fs-type"))
+ if tarinfo.isreg():
+ with open(backup_based_path, "rb") as f:
+ tar.addfile(tarinfo, f)
+ else:
+ tar.addfile(tarinfo)
tar.close()
except ImportError, e:
err_msg = "Import failed: " + str(e)
