diff --git a/CHANGELOG.md b/CHANGELOG.md index 14c905a..c975940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ This file logs the changes that are actually interesting to users (new features, changed functionality, fixed bugs). +## Version 1.3 (in development) + +- When creating a BAT pack, symlinks are no longer followed. This allows BAT-packing a directory structure with symlinked files (such as a Shaman checkout). +- When creating a BAT pack, mapped network drives are no longer changed from a drive letter to UNC notation. For example, when mapping a share `\\SERVER\Share` to drive letter `S:\`, BAT will now keep referencing `S:\` instead of rewriting paths to `\\SERVER\Share`. +- Better handling of drive letters, and of paths that cross drive boundaries. +- Better testing of Windows-specific cases when running the tests on Windows, and of POSIX-specific cases on other platforms. + + ## Version 1.2 (2019-10-09) - Migrated from Pipenv to Poetry for managing Python package dependencies. diff --git a/blender_asset_tracer/blendfile/__init__.py b/blender_asset_tracer/blendfile/__init__.py index 308be8b..29afff4 100644 --- a/blender_asset_tracer/blendfile/__init__.py +++ b/blender_asset_tracer/blendfile/__init__.py @@ -49,7 +49,7 @@ def open_cached(path: pathlib.Path, mode='rb', assert_cached: typing.Optional[bool] = None) -> 'BlendFile': """Open a blend file, ensuring it is only opened once.""" my_log = log.getChild('open_cached') - bfile_path = path.absolute().resolve() + bfile_path = bpathlib.make_absolute(path) if assert_cached is not None: is_cached = bfile_path in _cached_bfiles @@ -84,13 +84,13 @@ def close_all_cached() -> None: def _cache(path: pathlib.Path, bfile: 'BlendFile'): """Add a BlendFile to the cache.""" - bfile_path = path.absolute().resolve() + bfile_path = bpathlib.make_absolute(path) _cached_bfiles[bfile_path] = bfile def _uncache(path: pathlib.Path): """Remove a BlendFile object from the cache.""" - bfile_path = path.absolute().resolve() + bfile_path = bpathlib.make_absolute(path) _cached_bfiles.pop(bfile_path, None) diff --git a/blender_asset_tracer/bpathlib.py b/blender_asset_tracer/bpathlib.py index 1a2fde6..5e878af 100644 --- a/blender_asset_tracer/bpathlib.py +++ b/blender_asset_tracer/bpathlib.py @@ -25,6 +25,7 @@ or vice versa. import os.path import pathlib +import platform import string import sys @@ -41,18 +42,29 @@ class BlendPath(bytes): return super().__new__(cls, path.replace(b'\\', b'/')) @classmethod - def mkrelative(cls, asset_path: pathlib.Path, bfile_path: pathlib.PurePath) -> 'BlendPath': + def mkrelative(cls, asset_path: pathlib.PurePath, bfile_path: pathlib.PurePath) -> 'BlendPath': """Construct a BlendPath to the asset relative to the blend file. Assumes that bfile_path is absolute. + + Note that this can return an absolute path on Windows when 'asset_path' + and 'bfile_path' are on different drives. """ from collections import deque + # Only compare absolute paths. assert bfile_path.is_absolute(), \ 'BlendPath().mkrelative(bfile_path=%r) should get absolute bfile_path' % bfile_path + assert asset_path.is_absolute(), \ + 'BlendPath().mkrelative(asset_path=%r) should get absolute asset_path' % asset_path + + # There is no way to construct a relative path between drives. + if bfile_path.drive != asset_path.drive: + return cls(asset_path) bdir_parts = deque(bfile_path.parent.parts) - asset_parts = deque(asset_path.absolute().parts) + asset_path = make_absolute(asset_path) + asset_parts = deque(asset_path.parts) # Remove matching initial parts. What is left in bdir_parts represents # the number of '..' we need. What is left in asset_parts represents @@ -101,6 +113,8 @@ class BlendPath(bytes): Interprets the path as UTF-8, and if that fails falls back to the local filesystem encoding. + + The exact type returned is determined by the current platform. """ # TODO(Sybren): once we target Python 3.6, implement __fspath__(). try: @@ -146,3 +160,53 @@ class BlendPath(bytes): else: my_relpath = self return BlendPath(os.path.join(root, my_relpath)) + + +def make_absolute(path: pathlib.PurePath) -> pathlib.Path: + """Make the path absolute without resolving symlinks or drive letters. + + This function is an alternative to `Path.resolve()`. It make the path absolute, + and resolves `../../`, but contrary to `Path.resolve()` does NOT perform these + changes: + - Symlinks are NOT followed. + - Windows Network shares that are mapped to a drive letter are NOT resolved + to their UNC notation. + + The type of the returned path is determined by the current platform. + """ + str_path = path.as_posix() + if len(str_path) >= 2 and str_path[0].isalpha() and str_path[1] == ':': + # This is an absolute Windows path. It must be handled with care on non-Windows platforms. + if platform.system() != 'Windows': + # Normalize the POSIX-like part of the path, but leave out the drive letter. + non_drive_path = str_path[2:] + normalized = os.path.normpath(non_drive_path) + # Stick the drive letter back on the normalized path. + return pathlib.Path(str_path[:2] + normalized) + + return pathlib.Path(os.path.abspath(str_path)) + + +def strip_root(path: pathlib.PurePath) -> pathlib.PurePosixPath: + """Turn the path into a relative path by stripping the root. + + This also turns any drive letter into a normal path component. + + This changes "C:/Program Files/Blender" to "C/Program Files/Blender", + and "/absolute/path.txt" to "absolute/path.txt", making it possible to + treat it as a relative path. + """ + + if path.drive: + return pathlib.PurePosixPath(path.drive[0], *path.parts[1:]) + if isinstance(path, pathlib.PurePosixPath): + # This happens when running on POSIX but still handling paths + # originating from a Windows machine. + parts = path.parts + if parts and len(parts[0]) == 2 and parts[0][0].isalpha() and parts[0][1] == ':': + # The first part is a drive letter. + return pathlib.PurePosixPath(parts[0][0], *path.parts[1:]) + + if path.is_absolute(): + return pathlib.PurePosixPath(*path.parts[1:]) + return pathlib.PurePosixPath(path) diff --git a/blender_asset_tracer/cli/list_deps.py b/blender_asset_tracer/cli/list_deps.py index 3912761..42c93ba 100644 --- a/blender_asset_tracer/cli/list_deps.py +++ b/blender_asset_tracer/cli/list_deps.py @@ -27,7 +27,7 @@ import sys import time import typing -from blender_asset_tracer import trace +from blender_asset_tracer import trace, bpathlib from . import common log = logging.getLogger(__name__) @@ -106,7 +106,7 @@ def report_text(bpath, *, include_sha256: bool, show_timing: bool): last_reported_bfile = filepath for assetpath in usage.files(): - assetpath = assetpath.resolve() + assetpath = bpathlib.make_absolute(assetpath) if assetpath in reported_assets: log.debug('Already reported %s', assetpath) continue diff --git a/blender_asset_tracer/cli/pack.py b/blender_asset_tracer/cli/pack.py index 8dcbe56..b7b400b 100644 --- a/blender_asset_tracer/cli/pack.py +++ b/blender_asset_tracer/cli/pack.py @@ -24,7 +24,7 @@ import sys import typing import blender_asset_tracer.pack.transfer -from blender_asset_tracer import pack +from blender_asset_tracer import pack, bpathlib log = logging.getLogger(__name__) @@ -168,15 +168,15 @@ def paths_from_cli(args) -> typing.Tuple[pathlib.Path, pathlib.Path, str]: if bpath.is_dir(): log.critical('%s is a directory, should be a blend file') sys.exit(3) - bpath = bpath.absolute().resolve() + bpath = bpathlib.make_absolute(bpath) tpath = args.target if args.project is None: - ppath = bpath.absolute().parent.resolve() + ppath = bpathlib.make_absolute(bpath).parent log.warning('No project path given, using %s', ppath) else: - ppath = args.project.absolute().resolve() + ppath = bpathlib.make_absolute(args.project) if not ppath.exists(): log.critical('Project directory %s does not exist', ppath) diff --git a/blender_asset_tracer/pack/__init__.py b/blender_asset_tracer/pack/__init__.py index ec1f56d..92b77b1 100644 --- a/blender_asset_tracer/pack/__init__.py +++ b/blender_asset_tracer/pack/__init__.py @@ -230,13 +230,13 @@ class Packer: # The blendfile that we pack is generally not its own dependency, so # we have to explicitly add it to the _packed_paths. - bfile_path = self.blendfile.absolute() + bfile_path = bpathlib.make_absolute(self.blendfile) # Both paths have to be resolved first, because this also translates # network shares mapped to Windows drive letters back to their UNC # notation. Only resolving one but not the other (which can happen # with the abosolute() call above) can cause errors. - bfile_pp = self._target_path / bfile_path.resolve().relative_to(self.project.resolve()) + bfile_pp = self._target_path / bfile_path.relative_to(bpathlib.make_absolute(self.project)) self._output_path = bfile_pp self._progress_cb.pack_start() @@ -330,15 +330,8 @@ class Packer: act = self._actions[path] assert isinstance(act, AssetAction) - # Remove the base of the path, effectively removing the 'absoluteness'. - # On POSIX this turns '/path/file.txt' into 'path/file.txt'. - # On Windows this turns 'X:/path/file.txt' into 'X/path/file.txt'. - if path.drive: - path_parts = (path.drive[0], *path.parts[1:]) - else: - path_parts = path.parts[1:] - - act.new_path = pathlib.Path(self._target_path, '_outside_project', *path_parts) + relpath = bpathlib.strip_root(path) + act.new_path = pathlib.Path(self._target_path, '_outside_project', relpath) def _group_rewrites(self) -> None: """For each blend file, collect which fields need rewriting. @@ -358,7 +351,7 @@ class Packer: continue for usage in action.usages: - bfile_path = usage.block.bfile.filepath.absolute().resolve() + bfile_path = bpathlib.make_absolute(usage.block.bfile.filepath) insert_new_action = bfile_path not in self._actions self._actions[bfile_path].rewrites.append(usage) @@ -367,10 +360,10 @@ class Packer: actions.add(self._actions[bfile_path]) def _path_in_project(self, path: pathlib.Path) -> bool: + abs_path = bpathlib.make_absolute(path) + abs_project = bpathlib.make_absolute(self.project) try: - # MUST use resolve(), otherwise /path/to/proj/../../asset.png - # will return True (relative_to will return ../../asset.png). - path.resolve().relative_to(self.project) + abs_path.relative_to(abs_project) except ValueError: return False return True diff --git a/blender_asset_tracer/pack/shaman/transfer.py b/blender_asset_tracer/pack/shaman/transfer.py index d24b937..20e6821 100644 --- a/blender_asset_tracer/pack/shaman/transfer.py +++ b/blender_asset_tracer/pack/shaman/transfer.py @@ -26,6 +26,7 @@ import typing import requests import blender_asset_tracer.pack.transfer as bat_transfer +from blender_asset_tracer import bpathlib MAX_DEFERRED_PATHS = 8 MAX_FAILED_PATHS = 8 @@ -83,6 +84,9 @@ class ShamanTransferrer(bat_transfer.FileTransferer): self.log.info('Created checkout definition file of %d KiB', len(definition_file) // 1024) self.log.info('Feeding %d files to the Shaman', len(self._file_info)) + if self.log.isEnabledFor(logging.INFO): + for path in self._file_info: + self.log.info(' - %s', path) # Try to upload all the files. failed_paths = set() # type: typing.Set[str] @@ -151,7 +155,7 @@ class ShamanTransferrer(bat_transfer.FileTransferer): checksum = cache.compute_cached_checksum(src) filesize = src.stat().st_size # relpath = dst.relative_to(self.project_root) - relpath = str(dst)[1:] + relpath = bpathlib.strip_root(dst).as_posix() self._file_info[relpath] = FileInfo( checksum=checksum, diff --git a/blender_asset_tracer/trace/file2blocks.py b/blender_asset_tracer/trace/file2blocks.py index 4537c7f..6fc58c6 100644 --- a/blender_asset_tracer/trace/file2blocks.py +++ b/blender_asset_tracer/trace/file2blocks.py @@ -82,7 +82,7 @@ class BlockIterator: yield from self._visit_linked_blocks(blocks_per_lib) def _visit_blocks(self, bfile, limit_to): - bpath = bfile.filepath.absolute().resolve() + bpath = bpathlib.make_absolute(bfile.filepath) root_dir = bpathlib.BlendPath(bpath.parent) # Mapping from library path to data blocks to expand. @@ -123,10 +123,10 @@ class BlockIterator: # We've gone through all the blocks in this file, now open the libraries # and iterate over the blocks referred there. for lib_bpath, idblocks in blocks_per_lib.items(): - lib_path = pathlib.Path(lib_bpath.to_path()) - try: - lib_path = lib_path.resolve() - except FileNotFoundError: + lib_path = bpathlib.make_absolute(lib_bpath.to_path()) + + assert lib_path.exists() + if not lib_path.exists(): log.warning('Library %s does not exist', lib_path) continue diff --git a/blender_asset_tracer/trace/result.py b/blender_asset_tracer/trace/result.py index e20599f..a769e8d 100644 --- a/blender_asset_tracer/trace/result.py +++ b/blender_asset_tracer/trace/result.py @@ -107,8 +107,8 @@ class BlockUsage: def __repr__(self): if self.path_full_field is None: field_name = self.path_dir_field.name.name_full.decode() + \ - '/' + \ - self.path_base_field.name.name_full.decode() + '/' + \ + self.path_base_field.name.name_full.decode() else: field_name = self.path_full_field.name.name_full.decode() return '' % ( @@ -141,20 +141,19 @@ class BlockUsage: log.warning('Path %s does not exist for %s', path, self) def __fspath__(self) -> pathlib.Path: - """Determine the absolute path of the asset on the filesystem. - - The path is resolved (see pathlib.Path.resolve()) if it exists on the - filesystem. - """ + """Determine the absolute path of the asset on the filesystem.""" if self._abspath is None: bpath = self.block.bfile.abspath(self.asset_path) + log.info('Resolved %s rel to %s -> %s', + self.asset_path, self.block.bfile.filepath, bpath) + as_path = pathlib.Path(bpath.to_path()) - # Windows cannot resolve() a path that has a glob pattern in it. + # Windows cannot make a path that has a glob pattern in it absolute. # Since globs are generally only on the filename part, we take that off, - # resolve() the parent directory, then put the filename back. + # make the parent directory absolute, then put the filename back. try: - abs_parent = as_path.parent.resolve() + abs_parent = bpathlib.make_absolute(as_path.parent) except FileNotFoundError: self._abspath = as_path else: diff --git a/tests/test_bpathlib.py b/tests/test_bpathlib.py index 09be835..ae9936c 100644 --- a/tests/test_bpathlib.py +++ b/tests/test_bpathlib.py @@ -1,8 +1,11 @@ -from pathlib import Path, PurePosixPath +import os +from pathlib import Path, PurePath, PurePosixPath, PureWindowsPath +import platform +import tempfile import unittest from unittest import mock -from blender_asset_tracer.bpathlib import BlendPath +from blender_asset_tracer.bpathlib import BlendPath, make_absolute, strip_root class BlendPathTest(unittest.TestCase): @@ -29,11 +32,11 @@ class BlendPathTest(unittest.TestCase): self.assertEqual("BlendPath(b'//some/file.blend')", repr(p)) def test_to_path(self): - self.assertEqual(PurePosixPath('/some/file.blend'), + self.assertEqual(PurePath('/some/file.blend'), BlendPath(b'/some/file.blend').to_path()) - self.assertEqual(PurePosixPath('C:/some/file.blend'), + self.assertEqual(PurePath('C:/some/file.blend'), BlendPath(b'C:/some/file.blend').to_path()) - self.assertEqual(PurePosixPath('C:/some/file.blend'), + self.assertEqual(PurePath('C:/some/file.blend'), BlendPath(br'C:\some\file.blend').to_path()) with mock.patch('sys.getfilesystemencoding') as mock_getfse: @@ -41,7 +44,7 @@ class BlendPathTest(unittest.TestCase): # \xe9 is Latin-1 for é, and BlendPath should revert to using the # (mocked) filesystem encoding when decoding as UTF-8 fails. - self.assertEqual(PurePosixPath('C:/some/filé.blend'), + self.assertEqual(PurePath('C:/some/filé.blend'), BlendPath(b'C:\\some\\fil\xe9.blend').to_path()) with self.assertRaises(ValueError): @@ -84,7 +87,8 @@ class BlendPathTest(unittest.TestCase): self.assertEqual(BlendPath(b'//root/parent.blend'), BlendPath(b'//root/') / b'parent.blend') - def test_mkrelative(self): + @unittest.skipIf(platform.system() == 'Windows', "POSIX paths cannot be used on Windows") + def test_mkrelative_posix(self): self.assertEqual(b'//asset.png', BlendPath.mkrelative( Path('/path/to/asset.png'), PurePosixPath('/path/to/bfile.blend'), @@ -109,3 +113,140 @@ class BlendPathTest(unittest.TestCase): Path('/shallow/asset.png'), PurePosixPath('/path/to/very/very/very/very/very/deep/bfile.blend'), )) + + @unittest.skipIf(platform.system() != 'Windows', "Windows paths cannot be used on POSIX") + def test_mkrelative_windows(self): + self.assertEqual(b'//asset.png', BlendPath.mkrelative( + Path('Q:/path/to/asset.png'), + PureWindowsPath('Q:/path/to/bfile.blend'), + )) + self.assertEqual(b'//to/asset.png', BlendPath.mkrelative( + Path('Q:/path/to/asset.png'), + PureWindowsPath('Q:/path/bfile.blend'), + )) + self.assertEqual(b'//../of/asset.png', BlendPath.mkrelative( + Path('Q:/path/of/asset.png'), + PureWindowsPath('Q:/path/to/bfile.blend'), + )) + self.assertEqual(b'//../../path/of/asset.png', BlendPath.mkrelative( + Path('Q:/path/of/asset.png'), + PureWindowsPath('Q:/some/weird/bfile.blend'), + )) + self.assertEqual(b'//very/very/very/very/very/deep/asset.png', BlendPath.mkrelative( + Path('Q:/path/to/very/very/very/very/very/deep/asset.png'), + PureWindowsPath('Q:/path/to/bfile.blend'), + )) + self.assertEqual(b'//../../../../../../../../shallow/asset.png', BlendPath.mkrelative( + Path('Q:/shallow/asset.png'), + PureWindowsPath('Q:/path/to/very/very/very/very/very/deep/bfile.blend'), + )) + self.assertEqual(b'D:/path/to/asset.png', BlendPath.mkrelative( + Path('D:/path/to/asset.png'), + PureWindowsPath('Q:/path/to/bfile.blend'), + )) + + +class MakeAbsoluteTest(unittest.TestCase): + def test_relative(self): + my_dir = Path(__file__).absolute().parent + cwd = os.getcwd() + try: + os.chdir(my_dir) + self.assertEqual(my_dir / 'blendfiles/Cube.btx', + make_absolute(Path('blendfiles/Cube.btx'))) + except Exception: + os.chdir(cwd) + raise + + @unittest.skipIf(platform.system() != 'Windows', "This test uses drive letters") + def test_relative_drive(self): + cwd = os.getcwd() + my_drive = Path(f'{Path(cwd).drive}/') + self.assertEqual(my_drive / 'blendfiles/Cube.btx', + make_absolute(Path('/blendfiles/Cube.btx'))) + + def test_drive_letters(self): + """PureWindowsPath should be accepted and work well on POSIX systems too.""" + in_path = PureWindowsPath('R:/wrongroot/oops/../../path/to/a/file') + expect_path = Path('R:/path/to/a/file') + self.assertNotEqual(expect_path, in_path, 'pathlib should not automatically resolve ../') + self.assertEqual(expect_path, make_absolute(in_path)) + + @unittest.skipIf(platform.system() == 'Windows', "This test ignores drive letters") + def test_dotdot_dotdot_posix(self): + in_path = Path('/wrongroot/oops/../../path/to/a/file') + expect_path = Path('/path/to/a/file') + self.assertNotEqual(expect_path, in_path, 'pathlib should not automatically resolve ../') + self.assertEqual(expect_path, make_absolute(in_path)) + + @unittest.skipIf(platform.system() != 'Windows', "This test uses drive letters") + def test_dotdot_dotdot_windows(self): + in_path = Path('Q:/wrongroot/oops/../../path/to/a/file') + expect_path = Path('Q:/path/to/a/file') + self.assertNotEqual(expect_path, in_path, 'pathlib should not automatically resolve ../') + self.assertEqual(expect_path, make_absolute(in_path)) + + @unittest.skipIf(platform.system() == 'Windows', "This test ignores drive letters") + def test_way_too_many_dotdot_posix(self): + in_path = Path('/webroot/../../../../../etc/passwd') + expect_path = Path('/etc/passwd') + self.assertEqual(expect_path, make_absolute(in_path)) + + @unittest.skipIf(platform.system() != 'Windows', "This test uses drive letters") + def test_way_too_many_dotdot_windows(self): + in_path = Path('G:/webroot/../../../../../etc/passwd') + expect_path = Path('G:/etc/passwd') + self.assertEqual(expect_path, make_absolute(in_path)) + + @unittest.skipIf(platform.system() == 'Windows', + "Symlinks on Windows require Administrator rights") + def test_symlinks(self): + with tempfile.TemporaryDirectory(suffix="-bat-symlink-test") as tmpdir_str: + tmpdir = Path(tmpdir_str) + + orig_path = tmpdir / 'some_file.txt' + with orig_path.open('w') as outfile: + outfile.write('this file exists now') + + symlink = tmpdir / 'subdir' / 'linked.txt' + symlink.parent.mkdir() + symlink.symlink_to(orig_path) + + self.assertEqual(symlink, make_absolute(symlink), 'Symlinks should not be resolved') + + @unittest.skipIf(platform.system() != 'Windows', + "Drive letters mapped to network share can only be tested on Windows") + @unittest.skip('Mapped drive letter testing should be mocked, but that is hard to do') + def test_mapped_drive_letters(self): + pass + + def test_path_types(self): + platorm_path = type(PurePath()) + self.assertIsInstance(make_absolute(PureWindowsPath('/some/path')), platorm_path) + self.assertIsInstance(make_absolute(PurePosixPath('/some/path')), platorm_path) + + +class StripRootTest(unittest.TestCase): + def test_windows_paths(self): + self.assertEqual(PurePosixPath(), strip_root(PureWindowsPath())) + self.assertEqual( + PurePosixPath('C/Program Files/Blender'), + strip_root(PureWindowsPath('C:/Program Files/Blender'))) + self.assertEqual( + PurePosixPath('C/Program Files/Blender'), + strip_root(PureWindowsPath('C:\\Program Files\\Blender'))) + self.assertEqual( + PurePosixPath('C/Program Files/Blender'), + strip_root(PureWindowsPath('C\\Program Files\\Blender'))) + + def test_posix_paths(self): + self.assertEqual(PurePosixPath(), strip_root(PurePosixPath())) + self.assertEqual( + PurePosixPath('C/path/to/blender'), + strip_root(PurePosixPath('C:/path/to/blender'))) + self.assertEqual( + PurePosixPath('C/path/to/blender'), + strip_root(PurePosixPath('C/path/to/blender'))) + self.assertEqual( + PurePosixPath('C/path/to/blender'), + strip_root(PurePosixPath('/C/path/to/blender'))) diff --git a/tests/test_pack.py b/tests/test_pack.py index 2322296..188110b 100644 --- a/tests/test_pack.py +++ b/tests/test_pack.py @@ -1,8 +1,11 @@ import logging -import pathlib -import typing - +import os +import platform +import shutil import tempfile +import typing +import unittest +from pathlib import Path, PurePosixPath from unittest import mock from blender_asset_tracer import blendfile, pack, bpathlib @@ -23,7 +26,7 @@ class AbstractPackTest(AbstractBlendFileTest): def setUp(self): super().setUp() self.tdir = tempfile.TemporaryDirectory(suffix='-packtest') - self.tpath = pathlib.Path(self.tdir.name) + self.tpath = Path(self.tdir.name) def tearDown(self): super().tearDown() @@ -35,10 +38,11 @@ class AbstractPackTest(AbstractBlendFileTest): for path, action in packer._actions.items() if action.rewrites} - def outside_project(self) -> pathlib.Path: + def outside_project(self) -> Path: """Return the '_outside_project' path for files in self.blendfiles.""" # /tmp/target + /workspace/bat/tests/blendfiles → /tmp/target/workspace/bat/tests/blendfiles - extpath = pathlib.Path(self.tpath, '_outside_project', *self.blendfiles.parts[1:]) + # /tmp/target + C:\workspace\bat\tests\blendfiles → /tmp/target/C/workspace/bat/tests/blendfiles + extpath = Path(self.tpath, '_outside_project', bpathlib.strip_root(self.blendfiles)) return extpath @@ -89,7 +93,8 @@ class PackTest(AbstractPackTest): path = self.blendfiles / fn act = packer._actions[path] self.assertEqual(pack.PathAction.FIND_NEW_LOCATION, act.path_action, 'for %s' % fn) - self.assertEqual(extpath / fn, act.new_path, 'for %s' % fn) + self.assertEqual(extpath / fn, act.new_path, + f'\nEXPECT: {extpath / fn}\nACTUAL: {act.new_path}\nfor {fn}') to_rewrite = ( 'linked_cube.blend', @@ -162,7 +167,12 @@ class PackTest(AbstractPackTest): def test_execute_rewrite(self): infile, _ = self._pack_with_rewrite() - extpath = pathlib.PurePosixPath('//_outside_project', *self.blendfiles.parts[1:]) + if platform.system() == 'Windows': + extpath = PurePosixPath('//_outside_project', + self.blendfiles.drive[0], + *self.blendfiles.parts[1:]) + else: + extpath = PurePosixPath('//_outside_project', *self.blendfiles.parts[1:]) extbpath = bpathlib.BlendPath(extpath) # Those libraries should be properly rewritten. @@ -185,6 +195,81 @@ class PackTest(AbstractPackTest): packer.close() self.assertFalse(packer._rewrite_in.exists()) + @unittest.skipIf(platform.system() == 'Windows', + "Symlinks on Windows require Administrator rights") + def test_symlinked_files(self): + """Test that symlinks are NOT resolved. + + When packing, an asset that is symlinked should be treated as if it + were really at that location. Symlinks should NOT be resolved. + + As a concrete example, a directory structure with only symlinked files + in it should still be BAT-packable and produce the same structure. + """ + orig_ppath = self.blendfiles / 'subdir' + + # This is the original structure when packing subdir/doubly_linked_up.blend: + # . + # ├── basic_file.blend + # ├── linked_cube.blend + # ├── material_textures.blend + # ├── subdir + # │   └── doubly_linked_up.blend + # └── textures + # └── Bricks + # ├── brick_dotted_04-bump.jpg + # └── brick_dotted_04-color.jpg + + # This test copies the files to a temporary location and renames them, + # then recreates the above structure with symlinks. Packing the symlinks + # should be no different than packing the originals. + + orig_paths = [ + Path('basic_file.blend'), + Path('linked_cube.blend'), + Path('material_textures.blend'), + Path('subdir/doubly_linked_up.blend'), + Path('textures/Bricks/brick_dotted_04-bump.jpg'), + Path('textures/Bricks/brick_dotted_04-color.jpg'), + ] + + import hashlib + + with tempfile.TemporaryDirectory(suffix="-bat-symlink") as tmpdir_str: + tmpdir = Path(tmpdir_str) + + real_file_dir = tmpdir / 'real' + symlinked_dir = tmpdir / 'symlinked' + + real_file_dir.mkdir() + symlinked_dir.mkdir() + + for orig_path in orig_paths: + hashed_name = hashlib.new('md5', bytes(orig_path)).hexdigest() + # Copy the file to the temporary project, under a hashed name. + # This will break Blendfile linking. + real_file_path = real_file_dir / hashed_name + shutil.copy(self.blendfiles / orig_path, real_file_path) + + # Create a symlink to the above file, in such a way that it + # restores the original directory structure, and thus repairs + # the Blendfile linking. + symlink = symlinked_dir / orig_path + symlink.parent.mkdir(parents=True, exist_ok=True) + symlink.symlink_to(real_file_path) + + # Pack the symlinked directory structure. + pack_dir = tmpdir / 'packed' + packer = pack.Packer(self.blendfiles / 'subdir/doubly_linked_up.blend', + self.blendfiles, + pack_dir) + packer.strategise() + packer.execute() + + for orig_path in orig_paths: + packed_path = pack_dir / orig_path + self.assertTrue(packed_path.exists(), f'{packed_path} should exist') + def _pack_with_rewrite(self): ppath = self.blendfiles / 'subdir' infile = ppath / 'doubly_linked_up.blend' @@ -209,9 +294,13 @@ class PackTest(AbstractPackTest): seq = ed.get_pointer((b'seqbase', b'first')) seq_strip = seq.get_pointer(b'strip') - imgseq_path = (self.blendfiles / 'imgseq').absolute() - as_bytes = str(imgseq_path.relative_to(imgseq_path.anchor)).encode() + imgseq_path = bpathlib.make_absolute(self.blendfiles / 'imgseq') + print(f'imgseq_path: {imgseq_path!r}') + print(f' anchor: {imgseq_path.anchor!r}') + as_bytes = bpathlib.strip_root(imgseq_path).as_posix().encode() + print(f'as_bytes: {as_bytes!r}') relpath = bpathlib.BlendPath(b'//_outside_project') / as_bytes + print(f'relpath: {relpath!r}') # The image sequence base path should be rewritten. self.assertEqual(b'SQ000210.png', seq[b'name']) @@ -313,7 +402,7 @@ class PackTest(AbstractPackTest): 'Expected %s to be compressed' % bpath) break else: - self.fail('Expected to have Blend files in the BAT pack.') + self.fail(f'Expected to have Blend files in the BAT pack at {self.tpath}.') for imgpath in self.tpath.rglob('*.jpg'): with imgpath.open('rb') as imgfile: @@ -322,7 +411,7 @@ class PackTest(AbstractPackTest): 'Expected %s to NOT be compressed' % imgpath) break else: - self.fail('Expected to have JPEG files in the BAT pack.') + self.fail(f'Expected to have JPEG files in the BAT pack at {self.tpath}.') class ProgressTest(AbstractPackTest): @@ -485,7 +574,7 @@ class AbortTest(AbstractPackTest): packer = pack.Packer(infile, self.blendfiles, self.tpath) class AbortingCallback(progress.Callback): - def trace_blendfile(self, filename: pathlib.Path): + def trace_blendfile(self, filename: Path): # Call abort() somewhere during the strategise() call. if filename.name == 'linked_cube.blend': packer.abort() diff --git a/tests/test_shaman_transfer.py b/tests/test_shaman_transfer.py index 8d5454a..6e17f60 100644 --- a/tests/test_shaman_transfer.py +++ b/tests/test_shaman_transfer.py @@ -18,6 +18,7 @@ # # (c) 2019, Blender Foundation - Sybren A. Stüvel import pathlib +import platform import responses @@ -27,7 +28,7 @@ from blender_asset_tracer.pack.shaman import transfer httpmock = responses.RequestsMock() -class AbstractChecksumTest(AbstractBlendFileTest): +class ShamanTransferTest(AbstractBlendFileTest): @classmethod def setUpClass(cls): super().setUpClass() @@ -42,8 +43,8 @@ class AbstractChecksumTest(AbstractBlendFileTest): cls.test_file2: cls.test_file2.stat().st_size, } cls.packed_names = { - cls.test_file1: pathlib.Path('path/in/pack/test1.blend'), - cls.test_file2: pathlib.Path('path/in/pack/test2.blend'), + cls.test_file1: pathlib.PurePosixPath('path/in/pack/test1.blend'), + cls.test_file2: pathlib.PurePosixPath('path/in/pack/test2.blend'), } def assertValidCheckoutDef(self, definition_file: bytes): @@ -82,9 +83,10 @@ class AbstractChecksumTest(AbstractBlendFileTest): trans = transfer.ShamanTransferrer('auth-token', self.blendfiles, 'http://unittest.local:1234/', 'DA-JOB-ID') + trans.start() - trans.queue_copy(self.test_file1, pathlib.Path('/') / self.packed_names[self.test_file1]) - trans.queue_copy(self.test_file2, pathlib.Path('/') / self.packed_names[self.test_file2]) + trans.queue_copy(self.test_file1, self.packed_names[self.test_file1]) + trans.queue_copy(self.test_file2, self.packed_names[self.test_file2]) trans.done_and_join() self.assertFalse(trans.has_error, trans.error_message())