diff --git a/testing/baselines/tests.builtin-info/out b/testing/baselines/tests.builtin-info/out index efe9c5b0..23d2ad5c 100644 --- a/testing/baselines/tests.builtin-info/out +++ b/testing/baselines/tests.builtin-info/out @@ -8,7 +8,7 @@ is_loaded = True is_outdated = False is_pinned = True - tracking_method = builtin + tracking_method = None metadata (from version ""): diff --git a/testing/test_manager.py b/testing/test_manager.py index b6ce146b..fc6f9f7f 100644 --- a/testing/test_manager.py +++ b/testing/test_manager.py @@ -14,23 +14,18 @@ Manager, _info_from_snapshot, _is_directory_package, - _is_git_package, _prepare_snapshot, _resolve_git_version, _snapshot_from_directory, _snapshot_from_git_repo, ) from zeekpkg.package import ( - TRACKING_METHOD_BRANCH, - TRACKING_METHOD_BUILTIN, - TRACKING_METHOD_COMMIT, - TRACKING_METHOD_DIRECTORY, - TRACKING_METHOD_VERSION, InstalledPackage, Package, PackageInfo, PackageSnapshot, PackageStatus, + TrackingMethod, ) @@ -124,21 +119,21 @@ class TestResolveGitVersion: def test_defaults_to_branch_when_no_tags(self, repo: git.Repo) -> None: resolution = _resolve_git_version(repo, "") assert isinstance(resolution, GitResolution) - assert resolution.tracking_method == TRACKING_METHOD_BRANCH + assert resolution.tracking_method == TrackingMethod.BRANCH assert resolution.version == "main" def test_defaults_to_latest_tag(self, repo: git.Repo) -> None: repo.create_tag("v1.0.0") repo.create_tag("v2.0.0") resolution = _resolve_git_version(repo, "") - assert resolution.tracking_method == TRACKING_METHOD_VERSION + assert resolution.tracking_method == TrackingMethod.VERSION assert resolution.version == "v2.0.0" def test_explicit_version_tag(self, repo: git.Repo) -> None: repo.create_tag("v1.0.0") repo.create_tag("v2.0.0") resolution = _resolve_git_version(repo, "v1.0.0") - assert resolution.tracking_method == TRACKING_METHOD_VERSION + assert resolution.tracking_method == TrackingMethod.VERSION assert resolution.version == "v1.0.0" def test_explicit_branch(self, repo: git.Repo) -> None: @@ -147,13 +142,13 @@ def test_explicit_branch(self, repo: git.Repo) -> None: repo.remotes.origin.fetch() repo.git.checkout("feature") resolution = _resolve_git_version(repo, "feature") - assert resolution.tracking_method == TRACKING_METHOD_BRANCH + assert resolution.tracking_method == TrackingMethod.BRANCH assert resolution.version == "feature" def test_explicit_commit_hash(self, repo: git.Repo) -> None: hexsha = repo.head.object.hexsha resolution = _resolve_git_version(repo, hexsha) - assert resolution.tracking_method == TRACKING_METHOD_COMMIT + assert resolution.tracking_method == TrackingMethod.COMMIT assert resolution.current_hash == hexsha def test_unknown_version_raises(self, repo: git.Repo) -> None: @@ -208,22 +203,6 @@ def test_missing_version_raises(self, tmp_path: pathlib.Path) -> None: _snapshot_from_directory(str(tmp_path)) -class TestIsGitPackage: - @pytest.mark.parametrize( - "method,expected", - [ - (TRACKING_METHOD_VERSION, True), - (TRACKING_METHOD_BRANCH, True), - (TRACKING_METHOD_COMMIT, True), - (TRACKING_METHOD_BUILTIN, False), - (TRACKING_METHOD_DIRECTORY, False), - (None, False), - ], - ) - def test_is_git_package(self, method: str | None, expected: bool) -> None: - assert _is_git_package(PackageStatus(tracking_method=method)) is expected - - class TestIsDirectoryPackage: def test_plain_dir_is_directory_package( self, @@ -245,7 +224,7 @@ def test_directory( package = Package(git_url=str(pkg_dir), canonical=True) snapshot = _prepare_snapshot(package, None, str(tmp_path / "dest")) assert snapshot.version == "1.0.0" - assert snapshot.tracking_method == TRACKING_METHOD_DIRECTORY + assert snapshot.tracking_method == None assert snapshot.current_hash is None assert snapshot.is_outdated is False @@ -266,7 +245,7 @@ def test_git(self, repo: git.Repo, tmp_path: pathlib.Path) -> None: str(tmp_path / "dest"), existing_clone=repo, ) - assert snapshot.tracking_method == TRACKING_METHOD_BRANCH + assert snapshot.tracking_method == TrackingMethod.BRANCH assert snapshot.current_hash is not None @@ -294,7 +273,7 @@ def test_info_from_snapshot(self, repo: git.Repo) -> None: assert info.metadata["description"] == "hello" assert info.versions == ["v1.0.0"] assert info.default_branch == "main" - assert info.version_type == TRACKING_METHOD_VERSION + assert info.version_type == TrackingMethod.VERSION assert info.invalid_reason == "" def test_installed_missing_metadata(self, manager: Manager) -> None: @@ -311,7 +290,7 @@ def test_installed_missing_metadata(self, manager: Manager) -> None: _make_installed( manager, pkg_name, - tracking_method=TRACKING_METHOD_BRANCH, + tracking_method=TrackingMethod.BRANCH, current_version="main", ) info = manager.info(f"https://example.com/{pkg_name}", prefer_installed=True) @@ -326,7 +305,7 @@ def test_directory_backend( info = manager.info(str(pkg_dir)) assert info.invalid_reason == "" assert info.metadata_version == "1.0.0" - assert info.version_type == TRACKING_METHOD_DIRECTORY + assert info.version_type == None class TestManagerInstall: @@ -353,7 +332,7 @@ def test_directory_backend( assert result == "" ipkg = manager.find_installed_package("mypkg") assert ipkg is not None - assert ipkg.status.tracking_method == TRACKING_METHOD_DIRECTORY + assert ipkg.status.tracking_method == None assert ipkg.status.current_version == "1.0.0" def test_directory_missing_metadata( @@ -414,7 +393,7 @@ def test_skips_non_git_packages(self, manager: Manager) -> None: _make_installed( manager, "mypkg", - tracking_method=TRACKING_METHOD_DIRECTORY, + tracking_method=None, current_version="1.0.0", ) # Should complete without raising. @@ -456,7 +435,7 @@ def test_package_versions(self, manager: Manager) -> None: r.create_tag("v2.0.0") package = Package(git_url=str(pkg_dir), name="mypkg", canonical=True) - status = PackageStatus(tracking_method=TRACKING_METHOD_VERSION) + status = PackageStatus(tracking_method=TrackingMethod.VERSION) ipkg = InstalledPackage(package, status) assert manager.package_versions(ipkg) == ["v1.0.0", "v2.0.0"] @@ -526,7 +505,7 @@ def test_skips_non_git_existing_clone( _make_installed( manager, "mypkg", - tracking_method=TRACKING_METHOD_DIRECTORY, + tracking_method=None, current_version="1.0.0", ) bundle_file = str(tmp_path / "out.tar.gz") @@ -546,7 +525,7 @@ def test_directory_package( _make_installed( manager, "mypkg", - tracking_method=TRACKING_METHOD_DIRECTORY, + tracking_method=None, current_version="1.0.0", ) bundle_file = str(tmp_path / "out.tar.gz") diff --git a/testing/test_package.py b/testing/test_package.py new file mode 100644 index 00000000..7110460a --- /dev/null +++ b/testing/test_package.py @@ -0,0 +1,31 @@ +"""Unit tests for zeekpkg.package data structures.""" + +import pytest + +from zeekpkg.package import PackageStatus, TrackingMethod + + +@pytest.mark.parametrize("value", [m.value for m in TrackingMethod]) +def test_package_status_coerces_string_tracking_method(value: str) -> None: + # Manifest JSON deserialises tracking_method as a plain string; verify + # PackageStatus converts it to the enum. + status = PackageStatus(tracking_method=value) + assert status.tracking_method is TrackingMethod(value) + + +@pytest.mark.parametrize("value", ["builtin", "directory"]) +def test_package_status_maps_legacy_non_git_methods_to_none(value: str) -> None: + # Non-Git packages have no tracking method; older manifests use string + # values for them that must round-trip to None. + status = PackageStatus(tracking_method=value) + assert status.tracking_method is None + + +def test_package_status_accepts_enum_tracking_method() -> None: + status = PackageStatus(tracking_method=TrackingMethod.BRANCH) + assert status.tracking_method is TrackingMethod.BRANCH + + +def test_package_status_rejects_invalid_tracking_method() -> None: + with pytest.raises(ValueError): + PackageStatus(tracking_method="bogus") diff --git a/zeekpkg/manager.py b/zeekpkg/manager.py index 3538e4dc..1ca0dbb6 100644 --- a/zeekpkg/manager.py +++ b/zeekpkg/manager.py @@ -56,16 +56,13 @@ METADATA_FILENAME, PLUGIN_MAGIC_FILE, PLUGIN_MAGIC_FILE_DISABLED, - TRACKING_METHOD_BRANCH, - TRACKING_METHOD_COMMIT, - TRACKING_METHOD_DIRECTORY, - TRACKING_METHOD_VERSION, InstalledPackage, Package, PackageInfo, PackageSnapshot, PackageStatus, PackageVersion, + TrackingMethod, aliases, canonical_url, make_builtin_package, @@ -1324,7 +1321,7 @@ def refresh_installed_packages(self) -> None: ) continue - if not _is_git_package(ipkg.status): + if ipkg.status.tracking_method is None: continue # Deliberate git entry point: fetch requires network access and @@ -1386,21 +1383,21 @@ def upgrade(self, pkg_path: str) -> str: clone = self._open_package_clone(ipkg.package) - if ipkg.status.tracking_method == TRACKING_METHOD_VERSION: - version_tags = git_version_tags(clone) - return self._install(ipkg.package, version_tags[-1]) - - if ipkg.status.tracking_method == TRACKING_METHOD_BRANCH: - git_pull(clone) - assert ipkg.status.current_version - return self._install(ipkg.package, ipkg.status.current_version) - - if ipkg.status.tracking_method == TRACKING_METHOD_COMMIT: - # The above check for whether the installed package is outdated - # also should have already caught this situation. - return "package is not outdated" - - raise NotImplementedError + assert ipkg.status.tracking_method + match ipkg.status.tracking_method: + case TrackingMethod.VERSION: + version_tags = git_version_tags(clone) + return self._install(ipkg.package, version_tags[-1]) + case TrackingMethod.BRANCH: + git_pull(clone) + assert ipkg.status.current_version + return self._install(ipkg.package, ipkg.status.current_version) + case TrackingMethod.COMMIT: + # The above check for whether the installed package is outdated + # also should have already caught this situation. + return "package is not outdated" + case _: + raise NotImplementedError def remove(self, pkg_path: str) -> bool: """Remove an installed package. @@ -2089,7 +2086,7 @@ def package_versions(self, installed_package: InstalledPackage) -> list[str]: list of str: the version number tags. """ assert installed_package.package.name - assert _is_git_package(installed_package.status) + assert installed_package.status.tracking_method is not None return git_version_tags(self._open_package_clone(installed_package.package)) def validate_dependencies( @@ -2299,7 +2296,7 @@ def add_node(node: Node) -> str: if zeek_version: node = Node("zeek") node.installed_version = PackageVersion( - TRACKING_METHOD_VERSION, + TrackingMethod.VERSION, zeek_version, ) graph["zeek"] = node @@ -2308,7 +2305,7 @@ def add_node(node: Node) -> str: node = Node("zkg") node.installed_version = PackageVersion( - TRACKING_METHOD_VERSION, + TrackingMethod.VERSION, __version__, ) graph["zkg"] = node @@ -2604,10 +2601,14 @@ def match_package_url_and_version( ipkg = match_package_url_and_version(git_url, version) - if ipkg and not _is_git_package(ipkg.status): + if ipkg and ipkg.status.tracking_method is None: return f"cannot bundle directory package {git_url}: bundling requires a Git repository" - if prefer_existing_clones and ipkg and _is_git_package(ipkg.status): + if ( + prefer_existing_clones + and ipkg + and ipkg.status.tracking_method is not None + ): src = os.path.join(self.package_clonedir, ipkg.package.name) shutil.copytree(src, clonepath, symlinks=True) clone = git.Repo(clonepath) @@ -3291,7 +3292,7 @@ def _install( # `version` field, if present, matches the Git tag. meta_version = snapshot.meta.get("version") if ( - snapshot.tracking_method == TRACKING_METHOD_VERSION + snapshot.tracking_method == TrackingMethod.VERSION and meta_version and meta_version != snapshot.version ): @@ -3410,12 +3411,12 @@ class GitResolution: """The resolved Git state for a package after checkout.""" version: str - tracking_method: str + tracking_method: TrackingMethod current_hash: str is_outdated: bool -def _pick_version(clone: git.Repo, version: str | None) -> tuple[str, str]: +def _pick_version(clone: git.Repo, version: str | None) -> tuple[str, TrackingMethod]: """Return (resolved_version, tracking_method) for *version* in *clone*. When *version* is ``None`` or empty, the best available version is chosen @@ -3429,12 +3430,12 @@ def _pick_version(clone: git.Repo, version: str | None) -> tuple[str, str]: if version: if _is_commit_hash(clone, version): - return version, TRACKING_METHOD_COMMIT + return version, TrackingMethod.COMMIT if version in version_tags: - return version, TRACKING_METHOD_VERSION + return version, TrackingMethod.VERSION branches = _get_branch_names(clone) if version in branches: - return version, TRACKING_METHOD_BRANCH + return version, TrackingMethod.BRANCH LOG.info( 'branch "%s" not in available branches: %s', version, @@ -3443,8 +3444,8 @@ def _pick_version(clone: git.Repo, version: str | None) -> tuple[str, str]: raise ValueError(f'no such branch or version tag: "{version}"') if version_tags: - return version_tags[-1], TRACKING_METHOD_VERSION - return git_default_branch(clone), TRACKING_METHOD_BRANCH + return version_tags[-1], TrackingMethod.VERSION + return git_default_branch(clone), TrackingMethod.BRANCH def _resolve_git_version(clone: git.Repo, version: str | None) -> GitResolution: @@ -3552,7 +3553,7 @@ def _snapshot_from_directory(path: str) -> PackageSnapshot: working_dir=path, meta=meta, version=version, - tracking_method=TRACKING_METHOD_DIRECTORY, + tracking_method=None, ) @@ -3579,19 +3580,6 @@ def _is_directory_package(path: str) -> bool: return (path.startswith(".") or path.startswith("/")) and os.path.isdir(path) -def _is_git_package(status: PackageStatus) -> bool: - """Return True if *status* represents a git-backed package. - - Non-git package sources (e.g. local directories) will use a different - tracking method; this predicate guards operations that require a git clone. - """ - return status.tracking_method in ( - TRACKING_METHOD_VERSION, - TRACKING_METHOD_BRANCH, - TRACKING_METHOD_COMMIT, - ) - - def _is_version_outdated(clone: git.Repo, version: str) -> bool: version_tags = git_version_tags(clone) latest = normalize_version_tag(version_tags[-1]) @@ -3604,17 +3592,20 @@ def _is_branch_outdated(clone: git.Repo, branch: str) -> bool: return num_commits_behind > 0 -def _is_clone_outdated(clone: git.Repo, ref_name: str, tracking_method: str) -> bool: - if tracking_method == TRACKING_METHOD_VERSION: - return _is_version_outdated(clone, ref_name) - - if tracking_method == TRACKING_METHOD_BRANCH: - return _is_branch_outdated(clone, ref_name) - - if tracking_method == TRACKING_METHOD_COMMIT: - return False - - raise NotImplementedError +def _is_clone_outdated( + clone: git.Repo, + ref_name: str, + tracking_method: TrackingMethod, +) -> bool: + match tracking_method: + case TrackingMethod.VERSION: + return _is_version_outdated(clone, ref_name) + case TrackingMethod.BRANCH: + return _is_branch_outdated(clone, ref_name) + case TrackingMethod.COMMIT: + return False + case _: + raise NotImplementedError def _is_commit_hash(clone: git.Repo, text: str) -> bool: @@ -3773,8 +3764,6 @@ def _info_from_snapshot( All git-specific resolution (version tags, default branch, version type) must be performed by the caller before constructing the snapshot. """ - # Always set by `_snapshot_from_git_repo` and `_snapshot_from_directory`. - assert snapshot.tracking_method is not None metadata_file = _pick_metadata_file(snapshot.working_dir) if ( diff --git a/zeekpkg/package.py b/zeekpkg/package.py index 20e2f88f..e7fbfa73 100644 --- a/zeekpkg/package.py +++ b/zeekpkg/package.py @@ -6,6 +6,7 @@ import os import re from dataclasses import dataclass, field +from enum import Enum from functools import total_ordering import semantic_version as semver @@ -17,11 +18,17 @@ METADATA_FILENAME = "zkg.meta" LEGACY_METADATA_FILENAME = "bro-pkg.meta" -TRACKING_METHOD_VERSION = "version" -TRACKING_METHOD_BRANCH = "branch" -TRACKING_METHOD_COMMIT = "commit" -TRACKING_METHOD_BUILTIN = "builtin" -TRACKING_METHOD_DIRECTORY = "directory" + +class TrackingMethod(str, Enum): + """How a Git-backed package tracks upstream changes.""" + + VERSION = "version" + BRANCH = "branch" + COMMIT = "commit" + + def __str__(self) -> str: + return self.value + BUILTIN_SOURCE = "zeek-builtin" BUILTIN_SCHEME = "zeek-builtin://" @@ -172,7 +179,7 @@ class PackageSnapshot: meta: parsed contents of the ``zkg.meta`` file version: resolved version string (Git tag, branch, or metadata field) tracking_method: how the package version is tracked (e.g. - ``TRACKING_METHOD_VERSION``, ``TRACKING_METHOD_DIRECTORY``) + ``TrackingMethod.VERSION``, ``TRACKING_METHOD_DIRECTORY``) current_hash: Git commit hash at the time of snapshot, or ``None`` for non-Git sources is_outdated: whether a newer version is available; always ``False`` @@ -182,7 +189,7 @@ class PackageSnapshot: working_dir: str meta: dict[str, str] = field(default_factory=dict) version: str | None = None - tracking_method: str | None = None + tracking_method: "TrackingMethod | None" = None current_hash: str | None = None is_outdated: bool = False @@ -192,7 +199,7 @@ class PackageVersion: Helper class to compare package versions with version specs. """ - def __init__(self, method: str | None, version: str | None) -> None: + def __init__(self, method: "TrackingMethod | None", version: str | None) -> None: self.method = method self.version = version self.req_semver = None @@ -207,13 +214,13 @@ def fullfills(self, version_spec: str) -> tuple[str, bool]: if version_spec == "*": # anything goes return "", True - if self.method == TRACKING_METHOD_COMMIT: + if self.method == TrackingMethod.COMMIT: return f'tracking method commit not compatible with "{version_spec}"', False - if self.method == TRACKING_METHOD_BRANCH: + if self.method == TrackingMethod.BRANCH: return "tracking method branch and commit", False - # TRACKING_METHOD_BRANCH / TRACKING_METHOD_BUILTIN + # TrackingMethod.VERSION / None (builtin or directory) if version_spec.startswith("branch="): branch = version_spec[len("branch=") :] return ( @@ -293,8 +300,7 @@ class PackageStatus: is_outdated (bool): whether a newer version of the package exists. - tracking_method (str): either "branch", "version", "commit", or - "builtin" to indicate (respectively) whether package upgrades + tracking_method (TrackingMethod): indicates whether package upgrades should stick to a git branch, use git version tags, do nothing because the package is to always use a specific git commit hash, or do nothing because the package is built into Zeek. @@ -311,14 +317,23 @@ def __init__( is_loaded: bool = False, is_pinned: bool = False, is_outdated: bool = False, - tracking_method: str | None = None, + tracking_method: "TrackingMethod | str | None" = None, current_version: str | None = None, current_hash: str | None = None, ) -> None: self.is_loaded = is_loaded self.is_pinned = is_pinned self.is_outdated = is_outdated - self.tracking_method = tracking_method + # Non-Git sources have no tracking method; map their legacy string + # values to None for backward compatibility with older manifests. + _non_git = {"builtin", "directory"} + self.tracking_method = ( + None + if isinstance(tracking_method, str) and tracking_method in _non_git + else TrackingMethod(tracking_method) + if isinstance(tracking_method, str) + else tracking_method + ) self.current_version = current_version self.current_hash = current_hash @@ -346,9 +361,9 @@ class PackageInfo: metadata_version: the package version that the metadata is from - version_type: either 'version', 'branch', or 'commit' to - indicate whether the package info/metadata was taken from a release - version tag, a branch, or a specific commit hash. + version_type: a :class:`TrackingMethod` indicating whether the package + info/metadata was taken from a release version tag, a branch, or a + specific commit hash. invalid_reason (str): this attribute is set when there is a problem with gathering package information and explains what went wrong. @@ -367,7 +382,7 @@ def __init__( versions: list[str] | None = None, metadata_version: str | None = "", invalid_reason: str = "", - version_type: str = "", + version_type: "TrackingMethod | None" = None, metadata_file: str | None = None, default_branch: str | None = None, ) -> None: @@ -649,7 +664,7 @@ def make_builtin_package( is_loaded=True, # May not hold in the future? is_outdated=False, is_pinned=True, - tracking_method=TRACKING_METHOD_BUILTIN, + tracking_method=None, current_version=current_version, current_hash=current_hash, ) diff --git a/zkg b/zkg index 55a0ec41..581294f0 100755 --- a/zkg +++ b/zkg @@ -38,11 +38,10 @@ from zeekpkg._util import ( ) from zeekpkg.package import ( BUILTIN_SCHEME, - TRACKING_METHOD_DIRECTORY, - TRACKING_METHOD_VERSION, InstalledPackage, Package, PackageInfo, + TrackingMethod, ) from zeekpkg.template import ( InputError, @@ -917,7 +916,7 @@ def cmd_bundle( print_error(f'error: invalid package "{name}": {info.invalid_reason}') sys.exit(1) - if info.version_type == TRACKING_METHOD_DIRECTORY: + if info.version_type is None: print_error( f'error: cannot bundle directory package "{name}":' " bundling requires a Git repository", @@ -1394,7 +1393,7 @@ def version_change_string( new_version = old_version version_change = "" - if installed_package.status.tracking_method == TRACKING_METHOD_VERSION: + if installed_package.status.tracking_method == TrackingMethod.VERSION: versions = manager.package_versions(installed_package) if len(versions): @@ -1448,7 +1447,7 @@ def cmd_upgrade( next_version = ipkg.status.current_version - if ipkg.status.tracking_method == TRACKING_METHOD_VERSION and info.versions: + if ipkg.status.tracking_method == TrackingMethod.VERSION and info.versions: next_version = info.versions[-1] assert next_version