Apply audit fixes for pack-sequence (14 findings, 28 new tests)
Cross-validation audit by Codex + Opus identified 14 issues: CRITICAL: - Fix _find_subdir_ci() prefix match accepting wrong folders (e.g. Publish_old). Now uses exact case-insensitive match. HIGH: - Add pack-sequence CLI subcommand with unambiguous arg parsing. Deprecate --sequence on pack (nargs='+' consumed the target). - Fix Windows UNC/drive path normalization in bpathlib.make_absolute() - Fix ZIP failure losing traceback + progress bar stuck on error - Fix os.path.commonpath crash on cross-drive paths (new derive_common_project helper) - Fix progress bar saturating at 40% during trace (log scale) MEDIUM: - Add per-shot error isolation in sequence scan (collect all errors) - Add memory warning for >50 files in sequence pack - Improve pack-info.txt with dedup stats - Wrap single Path in list for old operators (type consistency) LOW/INFO: - Document VERSION_RE pattern - Add TASK_CHOICE_ITEMS empty guard 28 new tests in tests/test_sequence_pack.py, 0 regressions. Co-Authored-By: Mario Hawat <mario@autourdeminuit.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -170,6 +170,22 @@ class BlendPath(bytes):
|
||||
return BlendPath(os.path.join(root, my_relpath))
|
||||
|
||||
|
||||
def _is_windows_path(str_path: str) -> bool:
|
||||
"""Check if path looks like a Windows absolute path (drive letter or UNC)."""
|
||||
# Drive letter: C:/ or C:\
|
||||
if (
|
||||
len(str_path) >= 3
|
||||
and str_path[0].isalpha()
|
||||
and str_path[1] == ":"
|
||||
and str_path[2] in "/\\"
|
||||
):
|
||||
return True
|
||||
# UNC: //server/share or \\server\share
|
||||
if len(str_path) >= 2 and str_path[:2] in ("//", "\\\\"):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def make_absolute(path: pathlib.PurePath) -> pathlib.Path:
|
||||
"""Make the path absolute without resolving symlinks or drive letters.
|
||||
|
||||
@@ -180,17 +196,23 @@ def make_absolute(path: pathlib.PurePath) -> pathlib.Path:
|
||||
- Windows Network shares that are mapped to a drive letter are NOT resolved
|
||||
to their UNC notation.
|
||||
|
||||
On non-Windows platforms, Windows-style paths (drive letters and UNC) are
|
||||
normalized without prepending the CWD, preserving them for cross-platform use.
|
||||
|
||||
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.
|
||||
|
||||
if _is_windows_path(str_path) and platform.system() != "Windows":
|
||||
if len(str_path) >= 2 and str_path[1] == ":":
|
||||
# Drive letter path: normalize the part after X:
|
||||
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)
|
||||
else:
|
||||
# UNC path: normalize, preserving forward-slash style
|
||||
normalized = os.path.normpath(str_path.replace("\\", "/"))
|
||||
return pathlib.Path(normalized)
|
||||
|
||||
return pathlib.Path(os.path.abspath(str_path))
|
||||
|
||||
|
||||
@@ -79,6 +79,7 @@ def cli_main():
|
||||
|
||||
blocks.add_parser(subparsers)
|
||||
pack.add_parser(subparsers)
|
||||
pack.add_sequence_parser(subparsers)
|
||||
list_deps.add_parser(subparsers)
|
||||
version.add_parser(subparsers)
|
||||
|
||||
|
||||
@@ -104,12 +104,122 @@ def add_parser(subparsers):
|
||||
nargs="+",
|
||||
type=pathlib.Path,
|
||||
metavar="BLENDFILE",
|
||||
help="Pack multiple blend files together, deduplicating shared dependencies. "
|
||||
"All listed blend files and their dependencies are packed into the target.",
|
||||
help="(Deprecated: use 'pack-sequence' subcommand instead.) "
|
||||
"Pack multiple blend files together, deduplicating shared dependencies.",
|
||||
)
|
||||
|
||||
|
||||
def add_sequence_parser(subparsers):
|
||||
"""Add argparser for the pack-sequence subcommand."""
|
||||
|
||||
parser = subparsers.add_parser(
|
||||
"pack-sequence",
|
||||
help="Pack multiple blend files together, deduplicating shared dependencies.",
|
||||
)
|
||||
parser.set_defaults(func=cli_pack_sequence)
|
||||
parser.add_argument(
|
||||
"blendfiles",
|
||||
nargs="+",
|
||||
type=pathlib.Path,
|
||||
help="Blend files to pack.",
|
||||
)
|
||||
parser.add_argument(
|
||||
"-t",
|
||||
"--target",
|
||||
type=str,
|
||||
required=True,
|
||||
help="Target directory or ZIP file.",
|
||||
)
|
||||
parser.add_argument(
|
||||
"-p",
|
||||
"--project",
|
||||
type=pathlib.Path,
|
||||
help="Root directory of your project.",
|
||||
)
|
||||
parser.add_argument(
|
||||
"-n",
|
||||
"--noop",
|
||||
default=False,
|
||||
action="store_true",
|
||||
help="Don't copy files, just show what would be done.",
|
||||
)
|
||||
parser.add_argument(
|
||||
"-e",
|
||||
"--exclude",
|
||||
nargs="*",
|
||||
default="",
|
||||
help="Space-separated list of glob patterns to exclude.",
|
||||
)
|
||||
parser.add_argument(
|
||||
"-c",
|
||||
"--compress",
|
||||
default=False,
|
||||
action="store_true",
|
||||
help="Compress blend files while copying.",
|
||||
)
|
||||
parser.add_argument(
|
||||
"-r",
|
||||
"--relative-only",
|
||||
default=False,
|
||||
action="store_true",
|
||||
help="Only pack assets referred to with a relative path.",
|
||||
)
|
||||
|
||||
|
||||
def derive_common_project(bpaths: typing.List[pathlib.Path]) -> pathlib.Path:
|
||||
"""Derive common project directory from multiple blend file paths.
|
||||
|
||||
Raises ValueError if paths span multiple drives or if common root
|
||||
is the filesystem root.
|
||||
"""
|
||||
if len(bpaths) == 1:
|
||||
return bpaths[0].parent
|
||||
|
||||
try:
|
||||
ppath = pathlib.Path(os.path.commonpath([p.parent for p in bpaths]))
|
||||
except ValueError:
|
||||
raise ValueError(
|
||||
"Blend files span multiple drives or have no common path. "
|
||||
"Specify the project directory explicitly with -p/--project."
|
||||
)
|
||||
|
||||
if ppath == pathlib.Path(ppath.anchor):
|
||||
raise ValueError(
|
||||
"Computed project path is the filesystem root (%s). "
|
||||
"Specify the project directory explicitly." % ppath
|
||||
)
|
||||
|
||||
return ppath
|
||||
|
||||
|
||||
def cli_pack_sequence(args):
|
||||
"""CLI entry point for pack-sequence subcommand."""
|
||||
# Synthesize args to reuse paths_from_cli and create_packer
|
||||
args.blendfile = None
|
||||
args.sequence = args.blendfiles
|
||||
args.keep_hierarchy = True # always for sequence packing
|
||||
if not hasattr(args, 'target') or args.target is None:
|
||||
log.critical("No target specified. Use -t/--target.")
|
||||
sys.exit(3)
|
||||
|
||||
bpaths, ppath, tpath = paths_from_cli(args)
|
||||
|
||||
with create_packer(args, bpaths, ppath, tpath) as packer:
|
||||
packer.strategise()
|
||||
try:
|
||||
packer.execute()
|
||||
except blender_asset_tracer.pack.transfer.FileTransferError as ex:
|
||||
log.error(
|
||||
"%d files couldn't be copied, starting with %s",
|
||||
len(ex.files_remaining),
|
||||
ex.files_remaining[0],
|
||||
)
|
||||
raise SystemExit(1)
|
||||
|
||||
|
||||
def cli_pack(args):
|
||||
if args.sequence:
|
||||
log.warning("--sequence on 'pack' is deprecated. Use 'bat pack-sequence -t TARGET FILE...' instead.")
|
||||
bpaths, ppath, tpath = paths_from_cli(args)
|
||||
|
||||
with create_packer(args, bpaths, ppath, tpath) as packer:
|
||||
@@ -270,10 +380,11 @@ def paths_from_cli(args) -> typing.Tuple[typing.List[pathlib.Path], pathlib.Path
|
||||
tpath = args.target
|
||||
|
||||
if args.project is None:
|
||||
if len(bpaths) == 1:
|
||||
ppath = bpaths[0].parent
|
||||
else:
|
||||
ppath = pathlib.Path(os.path.commonpath([p.parent for p in bpaths]))
|
||||
try:
|
||||
ppath = derive_common_project(bpaths)
|
||||
except ValueError as ex:
|
||||
log.critical("%s", ex)
|
||||
sys.exit(5)
|
||||
log.warning("No project path given, using %s", ppath)
|
||||
else:
|
||||
ppath = bpathlib.make_absolute(args.project)
|
||||
|
||||
@@ -12,6 +12,8 @@ from bpy_extras.io_utils import ExportHelper
|
||||
|
||||
from blender_asset_tracer.pack import zipped, progress
|
||||
|
||||
# Matches filenames like scene_v02.blend (single integer version only).
|
||||
# Does NOT match: _v02.1.blend, _v2-1.blend, or files without _vNN suffix.
|
||||
VERSION_RE = re.compile(r'_v(\d+)\.blend$', re.IGNORECASE)
|
||||
|
||||
|
||||
@@ -34,11 +36,12 @@ class BlenderProgressCallback(progress.Callback):
|
||||
|
||||
def trace_asset(self, filename):
|
||||
self._assets_traced += 1
|
||||
# During tracing we don't know the total, so just increment slowly
|
||||
# Cap at 400 (40% of 1000) to leave room for the transfer phase
|
||||
val = min(self._assets_traced, 400)
|
||||
self._wm.progress_update(val)
|
||||
if self._assets_traced % 20 == 0:
|
||||
# Log scale so progress bar never saturates during trace phase
|
||||
# 100 assets → ~185, 1000 → ~320, 10000 → ~385
|
||||
import math
|
||||
val = int(400 * (1 - 1 / (1 + math.log1p(self._assets_traced) / 10)))
|
||||
self._wm.progress_update(min(val, 399))
|
||||
if self._assets_traced % 100 == 0:
|
||||
print("[BAT] Traced %d assets..." % self._assets_traced)
|
||||
|
||||
def transfer_file(self, src, dst):
|
||||
@@ -80,16 +83,29 @@ class ExportBatPack(Operator, ExportHelper):
|
||||
outfname = bpy.path.ensure_ext(self.filepath, ".zip")
|
||||
self.report({"INFO"}, "Executing ZipPacker ...")
|
||||
|
||||
progress_cb = BlenderProgressCallback(context.window_manager)
|
||||
wm = context.window_manager
|
||||
progress_cb = BlenderProgressCallback(wm)
|
||||
|
||||
try:
|
||||
with zipped.ZipPacker(
|
||||
[Path(bpy.data.filepath)],
|
||||
Path(bpy.data.filepath).parent,
|
||||
str(self.filepath),
|
||||
) as packer:
|
||||
packer.progress_cb = progress_cb
|
||||
packer.strategise()
|
||||
packer.execute()
|
||||
except Exception as ex:
|
||||
import traceback
|
||||
traceback.print_exc()
|
||||
self.report({"ERROR"}, "Packing failed: %s" % str(ex))
|
||||
return {"CANCELLED"}
|
||||
finally:
|
||||
try:
|
||||
wm.progress_end()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
with zipped.ZipPacker(
|
||||
Path(bpy.data.filepath),
|
||||
Path(bpy.data.filepath).parent,
|
||||
str(self.filepath),
|
||||
) as packer:
|
||||
packer.progress_cb = progress_cb
|
||||
packer.strategise()
|
||||
packer.execute()
|
||||
self.report({"INFO"}, "Packing successful!")
|
||||
|
||||
with zipfile.ZipFile(str(self.filepath)) as inzip:
|
||||
@@ -180,12 +196,24 @@ class BAT_OT_export_zip(Operator, ExportHelper):
|
||||
|
||||
self.report({"INFO"}, "Packing with hierarchy...")
|
||||
|
||||
progress_cb = BlenderProgressCallback(context.window_manager)
|
||||
wm = context.window_manager
|
||||
progress_cb = BlenderProgressCallback(wm)
|
||||
|
||||
with packer_cls(bfile, project, target, keep_hierarchy=True) as packer:
|
||||
packer.progress_cb = progress_cb
|
||||
packer.strategise()
|
||||
packer.execute()
|
||||
try:
|
||||
with packer_cls([bfile], project, target, keep_hierarchy=True) as packer:
|
||||
packer.progress_cb = progress_cb
|
||||
packer.strategise()
|
||||
packer.execute()
|
||||
except Exception as ex:
|
||||
import traceback
|
||||
traceback.print_exc()
|
||||
self.report({"ERROR"}, "Packing failed: %s" % str(ex))
|
||||
return {"CANCELLED"}
|
||||
finally:
|
||||
try:
|
||||
wm.progress_end()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
if self.use_zip:
|
||||
with zipfile.ZipFile(target) as inzip:
|
||||
@@ -243,60 +271,73 @@ for _tpl in STUDIO_TEMPLATES.values():
|
||||
if item[0] not in _seen:
|
||||
TASK_CHOICE_ITEMS.append(item)
|
||||
_seen.add(item[0])
|
||||
if not TASK_CHOICE_ITEMS:
|
||||
TASK_CHOICE_ITEMS.append(('NONE', "None", "No task filter"))
|
||||
|
||||
|
||||
def _find_subdir_ci(parent, name):
|
||||
"""Find a child directory matching `name` case-insensitively (prefix match)."""
|
||||
"""Find a child directory matching `name` exactly (case-insensitive)."""
|
||||
name_upper = name.upper()
|
||||
for child in parent.iterdir():
|
||||
if child.is_dir() and child.name.upper().startswith(name_upper):
|
||||
if child.is_dir() and child.name.upper() == name_upper:
|
||||
return child
|
||||
return None
|
||||
|
||||
|
||||
def find_latest_publishes(root_dir, template_key, task=''):
|
||||
"""Scan a sequence folder for the latest .blend in each shot using the given template."""
|
||||
"""Scan a sequence folder for the latest .blend in each shot using the given template.
|
||||
|
||||
Returns (results, errors) where results is a list of (shot_name, filepath) tuples
|
||||
and errors is a list of (shot_name, error_message) tuples for shots that could not
|
||||
be scanned.
|
||||
"""
|
||||
template = STUDIO_TEMPLATES.get(template_key)
|
||||
if not template:
|
||||
return []
|
||||
return [], []
|
||||
|
||||
results = []
|
||||
errors = []
|
||||
root = Path(root_dir)
|
||||
if not root.is_dir():
|
||||
return results
|
||||
return results, errors
|
||||
|
||||
path_segments = [seg.replace("{task}", task) if "{task}" in seg else seg
|
||||
for seg in template['path_segments']]
|
||||
path_segments = [
|
||||
seg.replace("{task}", task) if "{task}" in seg and task else seg
|
||||
for seg in template['path_segments']
|
||||
]
|
||||
|
||||
for shot_dir in sorted(root.iterdir()):
|
||||
if not shot_dir.is_dir():
|
||||
continue
|
||||
|
||||
# Walk the template path segments from the shot directory
|
||||
current = shot_dir
|
||||
for segment in path_segments:
|
||||
current = _find_subdir_ci(current, segment)
|
||||
try:
|
||||
# Walk the template path segments from the shot directory
|
||||
current = shot_dir
|
||||
for segment in path_segments:
|
||||
current = _find_subdir_ci(current, segment)
|
||||
if current is None:
|
||||
break
|
||||
if current is None:
|
||||
break
|
||||
if current is None:
|
||||
continue
|
||||
|
||||
# Find the .blend with the highest _vNNN version number
|
||||
best_version = -1
|
||||
best_file = None
|
||||
for f in current.iterdir():
|
||||
if f.suffix.lower() != '.blend':
|
||||
continue
|
||||
m = VERSION_RE.search(f.name)
|
||||
if m:
|
||||
ver = int(m.group(1))
|
||||
if ver > best_version:
|
||||
best_version = ver
|
||||
best_file = f
|
||||
if best_file:
|
||||
results.append((shot_dir.name, best_file.absolute()))
|
||||
|
||||
return results
|
||||
# Find the .blend with the highest _vNNN version number
|
||||
best_version = -1
|
||||
best_file = None
|
||||
for f in current.iterdir():
|
||||
if f.suffix.lower() != '.blend':
|
||||
continue
|
||||
m = VERSION_RE.search(f.name)
|
||||
if m:
|
||||
ver = int(m.group(1))
|
||||
if ver > best_version:
|
||||
best_version = ver
|
||||
best_file = f
|
||||
if best_file:
|
||||
results.append((shot_dir.name, best_file.absolute()))
|
||||
except OSError as ex:
|
||||
errors.append((shot_dir.name, str(ex)))
|
||||
|
||||
return results, errors
|
||||
|
||||
|
||||
class BAT_OT_scan_sequence(Operator):
|
||||
@@ -320,7 +361,12 @@ class BAT_OT_scan_sequence(Operator):
|
||||
|
||||
wm.bat_sequence_dir = seq_dir
|
||||
wm.bat_sequence_files.clear()
|
||||
publishes = find_latest_publishes(seq_dir, wm.bat_sequence_template, wm.bat_sequence_task)
|
||||
publishes, scan_errors = find_latest_publishes(seq_dir, wm.bat_sequence_template, wm.bat_sequence_task)
|
||||
|
||||
if scan_errors:
|
||||
error_summary = "; ".join("%s: %s" % (n, e) for n, e in scan_errors)
|
||||
self.report({"ERROR"}, "Failed to scan %d shot(s): %s" % (len(scan_errors), error_summary))
|
||||
return {"CANCELLED"}
|
||||
|
||||
if not publishes:
|
||||
self.report({"WARNING"}, "No published blend files found in %s" % seq_dir)
|
||||
@@ -344,7 +390,7 @@ class BAT_OT_scan_sequence(Operator):
|
||||
|
||||
|
||||
class BAT_OT_sequence_pack(Operator, ExportHelper):
|
||||
"""Pack a sequence of shots: scan a folder for latest published blend files and pack them into a ZIP"""
|
||||
"""Pack a sequence of shots with hierarchy preservation and shared asset deduplication"""
|
||||
|
||||
bl_idname = "bat.sequence_pack"
|
||||
bl_label = "BAT - Pack Sequence"
|
||||
@@ -403,7 +449,17 @@ class BAT_OT_sequence_pack(Operator, ExportHelper):
|
||||
return {"CANCELLED"}
|
||||
|
||||
target = bpy.path.ensure_ext(self.filepath, ".zip")
|
||||
project = Path(os.path.commonpath([p.parent for p in bpaths]))
|
||||
|
||||
try:
|
||||
project = Path(os.path.commonpath([p.parent for p in bpaths]))
|
||||
except ValueError:
|
||||
self.report({"ERROR"},
|
||||
"Blend files span multiple drives. Cannot determine project root.")
|
||||
return {"CANCELLED"}
|
||||
if project == Path(project.anchor):
|
||||
self.report({"WARNING"},
|
||||
"Project root is the filesystem root (%s). "
|
||||
"Consider setting a Root directory." % project)
|
||||
|
||||
self.report({"INFO"}, "Packing %d blend files..." % len(bpaths))
|
||||
|
||||
@@ -415,8 +471,15 @@ class BAT_OT_sequence_pack(Operator, ExportHelper):
|
||||
packer.strategise()
|
||||
packer.execute()
|
||||
except Exception as ex:
|
||||
import traceback
|
||||
traceback.print_exc()
|
||||
self.report({"ERROR"}, "Packing failed: %s" % str(ex))
|
||||
return {"CANCELLED"}
|
||||
finally:
|
||||
try:
|
||||
wm.progress_end()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
with zipfile.ZipFile(target) as inzip:
|
||||
inzip.testzip()
|
||||
|
||||
@@ -245,6 +245,13 @@ class Packer:
|
||||
in the execute() function.
|
||||
"""
|
||||
|
||||
if len(self.blendfiles) > 50:
|
||||
log.info(
|
||||
"Packing %d blend files. Peak memory may be high (~%.1f GB estimated).",
|
||||
len(self.blendfiles),
|
||||
len(self.blendfiles) * 0.005,
|
||||
)
|
||||
|
||||
self._progress_cb.pack_start()
|
||||
self._new_location_paths = set()
|
||||
self._output_paths = []
|
||||
@@ -656,12 +663,22 @@ class Packer:
|
||||
with infopath.open("wt", encoding="utf8") as infofile:
|
||||
print("This is a Blender Asset Tracer pack.", file=infofile)
|
||||
if len(self._output_paths) > 1:
|
||||
print("This pack contains the following blend files:", file=infofile)
|
||||
print(
|
||||
"This pack contains %d blend files:" % len(self._output_paths),
|
||||
file=infofile,
|
||||
)
|
||||
for op in self._output_paths:
|
||||
print(
|
||||
" %s" % op.relative_to(self._target_path).as_posix(),
|
||||
file=infofile,
|
||||
)
|
||||
print(
|
||||
"Total unique assets: %d" % len(self._actions), file=infofile
|
||||
)
|
||||
if self.missing_files:
|
||||
print(
|
||||
"Missing files: %d" % len(self.missing_files), file=infofile
|
||||
)
|
||||
else:
|
||||
print("Start by opening the following blend file:", file=infofile)
|
||||
print(
|
||||
|
||||
@@ -83,7 +83,7 @@ class ZipTransferrer(transfer.FileTransferer):
|
||||
except Exception:
|
||||
# We have to catch exceptions in a broad way, as this is running in
|
||||
# a separate thread, and exceptions won't otherwise be seen.
|
||||
log.exception("Error transferring %s to %s", src, dst)
|
||||
log.exception("Error writing %s to ZIP archive at %s", src, dst)
|
||||
# Put the files to copy back into the queue, and abort. This allows
|
||||
# the main thread to inspect the queue and see which files were not
|
||||
# copied. The one we just failed (due to this exception) should also
|
||||
|
||||
Reference in New Issue
Block a user