diff-based mod removal via --previous-manifest
Adds removal-detection: when --previous-manifest <path> is given, the converter diffs the previous publish against current packwiz state and emits modify[].type=remove entries for mods/resourcepacks/etc that disappeared, using simple-mod-sync's on-disk naming convention as the regex pattern. Reverse-engineered from upstream source: - simple-mod-sync writes <sanitized_name>-<sanitized_version>.<ext> - StringUtils.sanitize strips [^a-zA-Z0-9.\-_] - GetOlderVersion() finds files starting with <name>- and auto-deletes on version bumps. So version upgrades need no converter handling; only full removals do. 8 new tests including end-to-end CLI verification with a synthetic previous manifest. 23/23 pass.
This commit is contained in:
@@ -33,8 +33,26 @@ python3 packwiz_to_sms.py /path/to/packwiz/pack \
|
|||||||
-o manifest.json \
|
-o manifest.json \
|
||||||
--bundle-non-mods overrides.zip \
|
--bundle-non-mods overrides.zip \
|
||||||
--bundle-url https://packs.example.com/overrides.zip
|
--bundle-url https://packs.example.com/overrides.zip
|
||||||
|
|
||||||
|
# Generate removal entries for mods dropped since the previous publish
|
||||||
|
python3 packwiz_to_sms.py /path/to/packwiz/pack \
|
||||||
|
-o manifest.json \
|
||||||
|
--previous-manifest /path/to/last-published-manifest.json
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Mod removal
|
||||||
|
|
||||||
|
Two cases, handled differently:
|
||||||
|
|
||||||
|
| What changed | Who handles cleanup |
|
||||||
|
|---|---|
|
||||||
|
| **Mod version bump** (Sodium 0.5 → 0.6) | simple-mod-sync itself — it writes downloaded files as `<sanitized_name>-<sanitized_version>.<ext>` and on update looks for any prior file starting with `<name>-` and deletes it. No converter intervention needed. |
|
||||||
|
| **Mod removed entirely** (no longer in pack) | Converter emits an explicit `modify[].type=remove` entry with a regex matching simple-mod-sync's on-disk naming. Triggered by `--previous-manifest <path>` flag. |
|
||||||
|
|
||||||
|
Without `--previous-manifest`, removed mods stay on player disk forever. In CI, keep the last-published manifest and feed it in as the previous one on every run.
|
||||||
|
|
||||||
|
The regex follows simple-mod-sync's `StringUtils.sanitize()` rules: `[^a-zA-Z0-9.\-_]` characters are stripped. So `"Fabric API (Old)"` → pattern `^mods/FabricAPIOld-.*\.jar$`.
|
||||||
|
|
||||||
## What gets emitted
|
## What gets emitted
|
||||||
|
|
||||||
| Packwiz path | Becomes simple-mod-sync `type` |
|
| Packwiz path | Becomes simple-mod-sync `type` |
|
||||||
@@ -51,7 +69,6 @@ CurseForge mods that use `mode = "metadata:curseforge"` (no direct URL) are skip
|
|||||||
## What's not handled
|
## What's not handled
|
||||||
|
|
||||||
- **Optional mods** — simple-mod-sync has no per-client toggle UI. All non-server mods are emitted unconditionally. Ship two manifests (with/without optional) if you need this.
|
- **Optional mods** — simple-mod-sync has no per-client toggle UI. All non-server mods are emitted unconditionally. Ship two manifests (with/without optional) if you need this.
|
||||||
- **Mod removal** — simple-mod-sync's `modify`/`remove` is not auto-populated. Convert by hand if you're dropping mods.
|
|
||||||
- **Rename / regex transforms** — packwiz has no equivalent concept, so we don't generate `modify.rename` entries.
|
- **Rename / regex transforms** — packwiz has no equivalent concept, so we don't generate `modify.rename` entries.
|
||||||
|
|
||||||
## How it works
|
## How it works
|
||||||
|
|||||||
+98
-2
@@ -85,6 +85,40 @@ _DIR_TO_TYPE = {
|
|||||||
"shaderpacks": "shader",
|
"shaderpacks": "shader",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# simple-mod-sync writes downloaded files as <sanitized_name>-<sanitized_version>.<ext>
|
||||||
|
# under <type>'s target directory. See StringUtils.sanitize() in the mod source:
|
||||||
|
# replaceAll("[^a-zA-Z0-9.\\-_]", "")
|
||||||
|
_SANITIZE_RE = re.compile(r"[^a-zA-Z0-9.\-_]")
|
||||||
|
|
||||||
|
_TYPE_TO_DIR_AND_EXT = {
|
||||||
|
"mod": ("mods", "jar"),
|
||||||
|
"resourcepack": ("resourcepacks", "zip"),
|
||||||
|
"shader": ("shaderpacks", "zip"),
|
||||||
|
"datapack": ("datapacks", "zip"),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _sanitize(s: str) -> str:
|
||||||
|
return _SANITIZE_RE.sub("", s)
|
||||||
|
|
||||||
|
|
||||||
|
def _remove_pattern_for(entry: dict[str, Any]) -> str | None:
|
||||||
|
"""Build a regex that matches any on-disk file simple-mod-sync would have
|
||||||
|
written for this content entry (current or older versions).
|
||||||
|
|
||||||
|
Mirrors simple-mod-sync's `GetProjectName(): name + "-"` prefix search
|
||||||
|
in `DirUtils.DirContains` (which uses startswith)."""
|
||||||
|
ctype = entry.get("type")
|
||||||
|
spec = _TYPE_TO_DIR_AND_EXT.get(ctype)
|
||||||
|
if spec is None:
|
||||||
|
return None
|
||||||
|
target_dir, ext = spec
|
||||||
|
safe_name = _sanitize(entry.get("name", ""))
|
||||||
|
if not safe_name:
|
||||||
|
return None
|
||||||
|
# Anchor whole path, match any version suffix + the extension.
|
||||||
|
return rf"^{re.escape(target_dir)}/{re.escape(safe_name)}-.*\.{ext}$"
|
||||||
|
|
||||||
|
|
||||||
def _content_type_for(rel_path: Path) -> str | None:
|
def _content_type_for(rel_path: Path) -> str | None:
|
||||||
"""Map mods/foo.pw.toml -> "mod", resourcepacks/foo.pw.toml -> "resourcepack" etc.
|
"""Map mods/foo.pw.toml -> "mod", resourcepacks/foo.pw.toml -> "resourcepack" etc.
|
||||||
@@ -124,6 +158,39 @@ class ConvertResult:
|
|||||||
skipped_no_url: list[str]
|
skipped_no_url: list[str]
|
||||||
skipped_unknown_type: list[str]
|
skipped_unknown_type: list[str]
|
||||||
non_mod_files: list[Path]
|
non_mod_files: list[Path]
|
||||||
|
removed_entries: list[dict[str, Any]] = None # populated when --previous-manifest given
|
||||||
|
|
||||||
|
def __post_init__(self) -> None:
|
||||||
|
if self.removed_entries is None:
|
||||||
|
self.removed_entries = []
|
||||||
|
|
||||||
|
|
||||||
|
def diff_removals(
|
||||||
|
previous_sync: list[dict[str, Any]],
|
||||||
|
current_sync: list[dict[str, Any]],
|
||||||
|
) -> list[dict[str, Any]]:
|
||||||
|
"""Return simple-mod-sync `modify` entries that delete files for content
|
||||||
|
present in `previous_sync` but absent from `current_sync`.
|
||||||
|
|
||||||
|
Identity is `(name, type)` — covers the common case (rename a mod →
|
||||||
|
treated as remove+add, which is the right behavior because the old
|
||||||
|
file's prefix no longer matches anything in the sync list).
|
||||||
|
"""
|
||||||
|
cur_keys = {(e.get("name"), e.get("type")) for e in current_sync}
|
||||||
|
removals: list[dict[str, Any]] = []
|
||||||
|
for prev in previous_sync:
|
||||||
|
key = (prev.get("name"), prev.get("type"))
|
||||||
|
if key in cur_keys:
|
||||||
|
continue
|
||||||
|
pattern = _remove_pattern_for(prev)
|
||||||
|
if pattern is None:
|
||||||
|
continue
|
||||||
|
removals.append({
|
||||||
|
"type": "remove",
|
||||||
|
"pattern": pattern,
|
||||||
|
"path": ".",
|
||||||
|
})
|
||||||
|
return removals
|
||||||
|
|
||||||
|
|
||||||
def convert(
|
def convert(
|
||||||
@@ -131,6 +198,7 @@ def convert(
|
|||||||
*,
|
*,
|
||||||
include_client_only: bool = True,
|
include_client_only: bool = True,
|
||||||
include_both: bool = True,
|
include_both: bool = True,
|
||||||
|
previous_manifest: dict[str, Any] | None = None,
|
||||||
) -> ConvertResult:
|
) -> ConvertResult:
|
||||||
pack = _load_pack(pack_root)
|
pack = _load_pack(pack_root)
|
||||||
index = _load_index(pack_root)
|
index = _load_index(pack_root)
|
||||||
@@ -166,7 +234,7 @@ def convert(
|
|||||||
|
|
||||||
sync.append(_make_entry(meta, content_type))
|
sync.append(_make_entry(meta, content_type))
|
||||||
|
|
||||||
manifest = {
|
manifest: dict[str, Any] = {
|
||||||
"sync_version": 3,
|
"sync_version": 3,
|
||||||
"_generator": {
|
"_generator": {
|
||||||
"tool": "packwiz-to-sms",
|
"tool": "packwiz-to-sms",
|
||||||
@@ -176,12 +244,21 @@ def convert(
|
|||||||
"sync": sync,
|
"sync": sync,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
removed_entries: list[dict[str, Any]] = []
|
||||||
|
if previous_manifest is not None:
|
||||||
|
prev_sync = previous_manifest.get("sync", [])
|
||||||
|
modify = diff_removals(prev_sync, sync)
|
||||||
|
if modify:
|
||||||
|
manifest["modify"] = modify
|
||||||
|
removed_entries = modify
|
||||||
|
|
||||||
return ConvertResult(
|
return ConvertResult(
|
||||||
manifest=manifest,
|
manifest=manifest,
|
||||||
skipped_server_only=skipped_server_only,
|
skipped_server_only=skipped_server_only,
|
||||||
skipped_no_url=skipped_no_url,
|
skipped_no_url=skipped_no_url,
|
||||||
skipped_unknown_type=skipped_unknown_type,
|
skipped_unknown_type=skipped_unknown_type,
|
||||||
non_mod_files=non_mod_files,
|
non_mod_files=non_mod_files,
|
||||||
|
removed_entries=removed_entries,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -231,6 +308,12 @@ def _build_arg_parser() -> argparse.ArgumentParser:
|
|||||||
"-o", "--output", type=Path, default=None,
|
"-o", "--output", type=Path, default=None,
|
||||||
help="Write manifest to this file (default stdout).",
|
help="Write manifest to this file (default stdout).",
|
||||||
)
|
)
|
||||||
|
p.add_argument(
|
||||||
|
"--previous-manifest", type=Path, default=None, metavar="JSON_PATH",
|
||||||
|
help="Path to the previously-published manifest. Names that disappeared "
|
||||||
|
"between previous and current are emitted as modify[].type=remove entries "
|
||||||
|
"matching simple-mod-sync's on-disk naming, so clients auto-clean removed mods.",
|
||||||
|
)
|
||||||
p.add_argument(
|
p.add_argument(
|
||||||
"--bundle-non-mods", type=Path, default=None, metavar="ZIP_PATH",
|
"--bundle-non-mods", type=Path, default=None, metavar="ZIP_PATH",
|
||||||
help="Zip all non-mod files into this zip and add a 'packed' entry pointing at --bundle-url.",
|
help="Zip all non-mod files into this zip and add a 'packed' entry pointing at --bundle-url.",
|
||||||
@@ -254,7 +337,18 @@ def main(argv: list[str] | None = None) -> int:
|
|||||||
print(f"error: {args.pack_root} does not contain pack.toml", file=sys.stderr)
|
print(f"error: {args.pack_root} does not contain pack.toml", file=sys.stderr)
|
||||||
return 2
|
return 2
|
||||||
|
|
||||||
result = convert(args.pack_root)
|
previous_manifest = None
|
||||||
|
if args.previous_manifest:
|
||||||
|
if not args.previous_manifest.is_file():
|
||||||
|
print(
|
||||||
|
f"warning: --previous-manifest {args.previous_manifest} does not exist, "
|
||||||
|
f"skipping removal detection",
|
||||||
|
file=sys.stderr,
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
previous_manifest = json.loads(args.previous_manifest.read_text(encoding="utf-8"))
|
||||||
|
|
||||||
|
result = convert(args.pack_root, previous_manifest=previous_manifest)
|
||||||
|
|
||||||
if args.bundle_non_mods:
|
if args.bundle_non_mods:
|
||||||
if not args.bundle_url:
|
if not args.bundle_url:
|
||||||
@@ -277,6 +371,8 @@ def main(argv: list[str] | None = None) -> int:
|
|||||||
f"(config/, options.txt, ...) — pass --bundle-non-mods to ship them",
|
f"(config/, options.txt, ...) — pass --bundle-non-mods to ship them",
|
||||||
file=sys.stderr,
|
file=sys.stderr,
|
||||||
)
|
)
|
||||||
|
for r in result.removed_entries:
|
||||||
|
print(f"remove: {r['pattern']}", file=sys.stderr)
|
||||||
|
|
||||||
payload = json.dumps(result.manifest, indent=2) + "\n"
|
payload = json.dumps(result.manifest, indent=2) + "\n"
|
||||||
if args.output:
|
if args.output:
|
||||||
|
|||||||
@@ -168,6 +168,112 @@ def test_cli_missing_pack_toml(tmp_path: Path) -> None:
|
|||||||
assert "pack.toml" in proc.stderr
|
assert "pack.toml" in proc.stderr
|
||||||
|
|
||||||
|
|
||||||
|
def test_diff_removals_empty_when_no_changes() -> None:
|
||||||
|
"""No removals when current sync covers everything in previous."""
|
||||||
|
prev = [{"name": "Sodium", "type": "mod"}, {"name": "Fabric API", "type": "mod"}]
|
||||||
|
cur = [{"name": "Sodium", "type": "mod"}, {"name": "Fabric API", "type": "mod"}]
|
||||||
|
assert p2s.diff_removals(prev, cur) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_diff_removals_emits_remove_for_dropped_mod() -> None:
|
||||||
|
"""Mod present previously but not currently → modify[].type=remove entry."""
|
||||||
|
prev = [{"name": "Sodium", "type": "mod"}, {"name": "Iris", "type": "mod"}]
|
||||||
|
cur = [{"name": "Sodium", "type": "mod"}]
|
||||||
|
rm = p2s.diff_removals(prev, cur)
|
||||||
|
assert len(rm) == 1
|
||||||
|
assert rm[0]["type"] == "remove"
|
||||||
|
assert rm[0]["path"] == "."
|
||||||
|
# pattern targets simple-mod-sync's on-disk naming
|
||||||
|
assert rm[0]["pattern"] == r"^mods/Iris-.*\.jar$"
|
||||||
|
|
||||||
|
|
||||||
|
def test_diff_removals_sanitizes_name() -> None:
|
||||||
|
"""Name with spaces and symbols sanitized to match simple-mod-sync's filename."""
|
||||||
|
prev = [{"name": "Fabric API (Old)", "type": "mod"}]
|
||||||
|
cur: list[dict] = []
|
||||||
|
rm = p2s.diff_removals(prev, cur)
|
||||||
|
# sanitize() strips space, '(', ')'
|
||||||
|
assert rm[0]["pattern"] == r"^mods/FabricAPIOld-.*\.jar$"
|
||||||
|
|
||||||
|
|
||||||
|
def test_diff_removals_per_type_paths_and_extensions() -> None:
|
||||||
|
"""Each content type targets the right directory + extension."""
|
||||||
|
prev = [
|
||||||
|
{"name": "Branding", "type": "resourcepack"},
|
||||||
|
{"name": "Iris", "type": "shader"},
|
||||||
|
{"name": "Worldgen", "type": "datapack"},
|
||||||
|
]
|
||||||
|
rm = p2s.diff_removals(prev, [])
|
||||||
|
patterns = {r["pattern"] for r in rm}
|
||||||
|
assert patterns == {
|
||||||
|
r"^resourcepacks/Branding-.*\.zip$",
|
||||||
|
r"^shaderpacks/Iris-.*\.zip$",
|
||||||
|
r"^datapacks/Worldgen-.*\.zip$",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def test_convert_with_previous_manifest_adds_modify_section() -> None:
|
||||||
|
"""When --previous-manifest is supplied to convert(), modify[] appears
|
||||||
|
in output for any name that disappeared."""
|
||||||
|
previous = {
|
||||||
|
"sync_version": 3,
|
||||||
|
"sync": [
|
||||||
|
{"name": "Sodium", "type": "mod", "url": "x", "version": "1"},
|
||||||
|
{"name": "Iris", "type": "mod", "url": "y", "version": "1"},
|
||||||
|
{"name": "Some Removed Mod", "type": "mod", "url": "z", "version": "1"},
|
||||||
|
],
|
||||||
|
}
|
||||||
|
r = p2s.convert(FIXTURE, previous_manifest=previous)
|
||||||
|
assert "modify" in r.manifest
|
||||||
|
patterns = {e["pattern"] for e in r.manifest["modify"]}
|
||||||
|
# Fixture has Sodium + Fabric API (both kept under "mod"), so Iris + Some Removed Mod
|
||||||
|
# should both be removed. (Iris in fixture is type=shader under "Complementary Shaders",
|
||||||
|
# so the mod-keyed Iris from previous is genuinely gone.)
|
||||||
|
assert r"^mods/Iris-.*\.jar$" in patterns
|
||||||
|
assert r"^mods/SomeRemovedMod-.*\.jar$" in patterns
|
||||||
|
|
||||||
|
|
||||||
|
def test_convert_without_previous_manifest_omits_modify() -> None:
|
||||||
|
r = p2s.convert(FIXTURE)
|
||||||
|
assert "modify" not in r.manifest
|
||||||
|
assert r.removed_entries == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_cli_previous_manifest_flag(tmp_path: Path) -> None:
|
||||||
|
"""End-to-end CLI with --previous-manifest."""
|
||||||
|
prev = tmp_path / "old.json"
|
||||||
|
prev.write_text(json.dumps({
|
||||||
|
"sync_version": 3,
|
||||||
|
"sync": [
|
||||||
|
{"name": "Sodium", "type": "mod", "url": "x", "version": "1"},
|
||||||
|
{"name": "GoneAway", "type": "mod", "url": "y", "version": "1"},
|
||||||
|
],
|
||||||
|
}))
|
||||||
|
proc = subprocess.run(
|
||||||
|
[sys.executable, str(SCRIPT), str(FIXTURE), "--quiet",
|
||||||
|
"--previous-manifest", str(prev)],
|
||||||
|
capture_output=True, text=True, check=True,
|
||||||
|
)
|
||||||
|
payload = json.loads(proc.stdout)
|
||||||
|
assert "modify" in payload
|
||||||
|
patterns = {e["pattern"] for e in payload["modify"]}
|
||||||
|
assert r"^mods/GoneAway-.*\.jar$" in patterns
|
||||||
|
|
||||||
|
|
||||||
|
def test_cli_previous_manifest_missing_file_warns(tmp_path: Path) -> None:
|
||||||
|
"""Non-existent previous-manifest is a warning, not an error — first run case."""
|
||||||
|
nonexistent = tmp_path / "never-written.json"
|
||||||
|
proc = subprocess.run(
|
||||||
|
[sys.executable, str(SCRIPT), str(FIXTURE),
|
||||||
|
"--previous-manifest", str(nonexistent)],
|
||||||
|
capture_output=True, text=True, check=True,
|
||||||
|
)
|
||||||
|
# Should succeed, emit manifest with no modify section, and warn on stderr.
|
||||||
|
payload = json.loads(proc.stdout)
|
||||||
|
assert "modify" not in payload
|
||||||
|
assert "does not exist" in proc.stderr
|
||||||
|
|
||||||
|
|
||||||
def test_convert_against_real_packwiz_example_pack(tmp_path: Path) -> None:
|
def test_convert_against_real_packwiz_example_pack(tmp_path: Path) -> None:
|
||||||
"""Integration: clone the upstream packwiz-example-pack and convert it.
|
"""Integration: clone the upstream packwiz-example-pack and convert it.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user