diff --git a/ARTIFACT.md b/ARTIFACT.md index 5f01d182e..66010744f 100644 --- a/ARTIFACT.md +++ b/ARTIFACT.md @@ -51,3 +51,71 @@ OMMX Artifact is a collection of `config`, `layers`, and annotations. Note that other annotations listed above are also allowed. The key may not start with `org.ommx.v1.`, but must be a valid reverse domain name as specified by OCI specification. + +Storage Formats +--------------- + +OMMX Local Registry supports two storage formats for artifacts: + +### OCI Directory Format (oci-dir) + +The traditional format where artifacts are stored as directory structures following the OCI Image Layout specification. Each artifact is stored as a directory containing: +- `oci-layout` file indicating OCI compliance +- `blobs/` directory containing content-addressable blobs +- `index.json` file with manifest references + +This format is suitable for: +- Local development and testing +- File system-based storage +- Cases where individual blobs need direct access + +### OCI Archive Format (oci-archive) + +A single-file format where the entire artifact is packaged as a tar archive with `.ommx` extension. This format contains the same OCI structure but packaged for easier distribution. + +This format is suitable for: +- Cloud object storage systems (AWS S3, Google Cloud Storage, etc.) +- Artifact distribution and sharing +- Backup and archiving scenarios +- Network transfer optimization + +### Format Selection and Compatibility + +- **New artifacts default to oci-archive format** for better cloud storage compatibility +- **Both formats are fully supported** for loading and manipulation +- **Backward compatibility is maintained** - existing oci-dir artifacts continue to work +- **Format detection is automatic** - the system transparently handles both formats +- **Cross-format operations are supported** - you can load from one format and save to another + +### Local Registry Directory Structure + +Artifacts are stored in the local registry with the following path structure. Image names are converted to file system paths with `:` (colon) in tags escaped to `__` (double underscore): + +**Example: `ghcr.io/jij-inc/ommx/qplib:3734`** + +``` +/ +└── ghcr.io/ + └── jij-inc/ + └── ommx/ + └── qplib/ + ├── __3734/ # oci-dir format (: converted to __) + │ ├── oci-layout + │ ├── index.json + │ └── blobs/ + │ └── sha256/... + │ + └── __3734.ommx # oci-archive format (: converted to __) +``` + +**Path Conversion Rules:** +- Tags with `:` are escaped to `__` (e.g., `my-app:v1.0` → `my-app/__v1.0`) +- oci-dir format: `/` +- oci-archive format: `.ommx` + +**Format Priority:** +When both formats exist for the same image name, **oci-archive format takes precedence**. The system checks for the `.ommx` file first, then falls back to the directory format. + +### Migration Between Formats + +The OMMX artifact system provides transparent format conversion. Artifacts can be loaded from either format and saved to either format without manual intervention. The system automatically detects the source format based on file system structure (directory vs. archive file) and handles the conversion internally. diff --git a/CLAUDE.md b/CLAUDE.md index a6dc577e5..e9d2537d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,6 +43,61 @@ The project has completed its migration from Protocol Buffers auto-generated Pyt **Implementation Details**: See actual code in `rust/ommx/src/` for current type definitions and API. +### Artifact API (Unified Format Handling) ✅ + +The project uses a unified `Artifact` enum API that handles both oci-dir and oci-archive formats transparently. + +**Rust API:** +```rust +use ommx::artifact::{Artifact, Builder}; + +// Load from either format (automatic detection) +let mut artifact = Artifact::from_oci_archive(path)?; +let mut artifact = Artifact::from_oci_dir(path)?; +let mut artifact = Artifact::from_remote(image_name)?; +let artifact = Artifact::load(&image_name)?; // Auto-detect local or pull from remote + +// Save to specific format +artifact.save()?; // Default: oci-archive to local registry +artifact.save_as_archive(path)?; +artifact.save_as_dir(path)?; + +// Pull from remote +artifact.pull()?; // Saves as oci-archive to local registry + +// Build new artifacts +let mut builder = Builder::new_archive(path, image_name)?; +let mut builder = Builder::for_github("org", "repo", "name", "tag")?; +builder.add_instance(instance, annotations)?; +builder.add_annotation("key".to_string(), "value".to_string()); +let artifact = builder.build()?; +``` + +**Python API:** +```python +from ommx.artifact import Artifact, ArtifactBuilder + +# Load from either format (automatic detection) +artifact = Artifact.load("image-name:tag") +artifact = Artifact.load_archive("path.ommx") +artifact = Artifact.load_dir("path/to/dir") + +# Build new artifacts (defaults to oci-archive) +builder = ArtifactBuilder.new("image-name:tag") +builder.add_instance(instance) +builder.add_annotation("key", "value") +artifact = builder.build() + +# For legacy oci-dir format +builder = ArtifactBuilder.new_dir(path, "image-name:tag") +``` + +**Key Points:** +- The API automatically detects and handles both oci-dir and oci-archive formats +- New artifacts default to oci-archive format for better cloud storage compatibility +- Format conversion happens transparently during load/save operations +- Python API maintains backward compatibility - no changes needed in user code + ## Development Commands This project uses [Taskfile](https://taskfile.dev/) for task management. **⚠️ All commands must be run from the project root directory.** diff --git a/python/ommx-tests/tests/conftest.py b/python/ommx-tests/tests/conftest.py new file mode 100644 index 000000000..bd562ea7e --- /dev/null +++ b/python/ommx-tests/tests/conftest.py @@ -0,0 +1,30 @@ +""" +Global pytest configuration for ommx-tests. + +This module sets up fixtures that are shared across all test modules. +""" + +import tempfile +import shutil +from pathlib import Path + +import pytest + +from ommx.artifact import set_local_registry_root + + +@pytest.fixture(scope="session", autouse=True) +def setup_local_registry(): + """ + Set up a temporary local registry for all tests. + + This fixture is automatically used for all tests (autouse=True) and runs once + per test session (scope="session"). The local registry root can only be set + once per process due to OnceLock constraints in the Rust implementation. + """ + temp_dir = tempfile.mkdtemp(prefix="ommx-test-registry-") + try: + set_local_registry_root(temp_dir) + yield Path(temp_dir) + finally: + shutil.rmtree(temp_dir, ignore_errors=True) diff --git a/python/ommx-tests/tests/test_artifact_dual_format.py b/python/ommx-tests/tests/test_artifact_dual_format.py new file mode 100644 index 000000000..f77d17edd --- /dev/null +++ b/python/ommx-tests/tests/test_artifact_dual_format.py @@ -0,0 +1,105 @@ +""" +Test dual format support for OMMX artifacts (oci-dir and oci-archive). + +This test validates that: +1. New artifacts default to oci-archive format +2. Both oci-dir and oci-archive formats can be loaded +3. Backward compatibility is maintained +4. get_artifact_path correctly identifies both formats +""" + +import uuid + +import pytest + +from ommx.artifact import ( + Artifact, + ArtifactBuilder, + get_local_registry_path, +) +from ommx.testing import SingleFeasibleLPGenerator, DataType + + +@pytest.fixture +def test_instance(): + """Create a test instance for artifact tests.""" + generator = SingleFeasibleLPGenerator(3, DataType.INT) + return generator.get_v1_instance() + + +def test_new_artifacts_default_to_archive_format(test_instance): + """Test that new artifacts created with ArtifactBuilder.new() use oci-archive format.""" + image_name = f"test.local/archive-default:{uuid.uuid4()}" + + # Build artifact using the default new() method + builder = ArtifactBuilder.new(image_name) + builder.add_instance(test_instance) + artifact = builder.build() + + # Verify artifact is stored as oci-archive format (.ommx file) + base_path = get_local_registry_path(image_name) + archive_path = base_path.with_suffix(".ommx") + assert archive_path.exists() + assert archive_path.is_file() + assert archive_path.suffix == ".ommx" + + # Verify we can load it back + loaded = Artifact.load(image_name) + assert loaded.image_name == image_name + assert len(loaded.layers) == len(artifact.layers) + + +def test_legacy_oci_dir_format_still_works(test_instance): + """Test that the legacy oci-dir format can still be created and loaded.""" + image_name = f"test.local/dir-legacy:{uuid.uuid4()}" + + # Build artifact using the dir format via new_dir() + dir_path = get_local_registry_path(image_name) + builder = ArtifactBuilder.new_dir(dir_path, image_name) + builder.add_instance(test_instance) + artifact = builder.build() + + # Verify artifact is stored as oci-dir format (directory) + assert dir_path.exists() + assert dir_path.is_dir() + assert (dir_path / "oci-layout").exists() + + # Verify we can load it back + loaded = Artifact.load(image_name) + assert loaded.image_name == image_name + assert len(loaded.layers) == len(artifact.layers) + + +def test_dual_format_interoperability(test_instance): + """Test that both formats work seamlessly together.""" + archive_image = f"test.local/interop-archive:{uuid.uuid4()}" + dir_image = f"test.local/interop-dir:{uuid.uuid4()}" + + # Create artifacts in both formats + archive_builder = ArtifactBuilder.new(archive_image) + archive_builder.add_instance(test_instance) + archive_builder.build() + + dir_path = get_local_registry_path(dir_image) + dir_builder = ArtifactBuilder.new_dir(dir_path, dir_image) + dir_builder.add_instance(test_instance) + dir_builder.build() + + # Both can be loaded with the same API + loaded_archive = Artifact.load(archive_image) + loaded_dir = Artifact.load(dir_image) + assert loaded_archive.image_name == archive_image + assert loaded_dir.image_name == dir_image + + # get_local_registry_path returns base path, format checked via path suffix/directory + archive_base_path = get_local_registry_path(archive_image) + archive_file = archive_base_path.with_suffix(".ommx") + assert archive_file.exists() and archive_file.is_file() + + dir_base_path = get_local_registry_path(dir_image) + assert dir_base_path.exists() and dir_base_path.is_dir() + + # Non-existent artifacts don't have files/directories + nonexistent_base = get_local_registry_path(f"test.local/nonexistent:{uuid.uuid4()}") + assert not nonexistent_base.exists() + assert not nonexistent_base.with_suffix(".ommx").exists() diff --git a/python/ommx/ommx/_ommx_rust.pyi b/python/ommx/ommx/_ommx_rust.pyi index b8bd53c08..28d4b6b88 100644 --- a/python/ommx/ommx/_ommx_rust.pyi +++ b/python/ommx/ommx/_ommx_rust.pyi @@ -7,66 +7,6 @@ import pathlib import typing from enum import Enum -class ArtifactArchive: - image_name: typing.Optional[builtins.str] - annotations: builtins.dict[builtins.str, builtins.str] - layers: builtins.list[Descriptor] - @staticmethod - def from_oci_archive( - path: builtins.str | os.PathLike | pathlib.Path, - ) -> ArtifactArchive: ... - def get_blob(self, digest: builtins.str) -> bytes: ... - def push(self) -> None: ... - -class ArtifactArchiveBuilder: - @staticmethod - def new_unnamed( - path: builtins.str | os.PathLike | pathlib.Path, - ) -> ArtifactArchiveBuilder: ... - @staticmethod - def new( - path: builtins.str | os.PathLike | pathlib.Path, image_name: builtins.str - ) -> ArtifactArchiveBuilder: ... - @staticmethod - def temp() -> ArtifactArchiveBuilder: ... - def add_layer( - self, - media_type: builtins.str, - blob: bytes, - annotations: typing.Mapping[builtins.str, builtins.str], - ) -> Descriptor: ... - def add_annotation(self, key: builtins.str, value: builtins.str) -> None: ... - def build(self) -> ArtifactArchive: ... - -class ArtifactDir: - image_name: typing.Optional[builtins.str] - annotations: builtins.dict[builtins.str, builtins.str] - layers: builtins.list[Descriptor] - @staticmethod - def from_image_name(image_name: builtins.str) -> ArtifactDir: ... - @staticmethod - def from_oci_dir( - path: builtins.str | os.PathLike | pathlib.Path, - ) -> ArtifactDir: ... - def get_blob(self, digest: builtins.str) -> bytes: ... - def push(self) -> None: ... - -class ArtifactDirBuilder: - @staticmethod - def new(image_name: builtins.str) -> ArtifactDirBuilder: ... - @staticmethod - def for_github( - org: builtins.str, repo: builtins.str, name: builtins.str, tag: builtins.str - ) -> ArtifactDirBuilder: ... - def add_layer( - self, - media_type: builtins.str, - blob: bytes, - annotations: typing.Mapping[builtins.str, builtins.str], - ) -> Descriptor: ... - def add_annotation(self, key: builtins.str, value: builtins.str) -> None: ... - def build(self) -> ArtifactDir: ... - class Bound: r""" Variable bound wrapper for Python @@ -700,6 +640,53 @@ class Polynomial: def __copy__(self) -> Polynomial: ... def __deepcopy__(self, _memo: typing.Any) -> Polynomial: ... +class PyArtifact: + image_name: typing.Optional[builtins.str] + annotations: builtins.dict[builtins.str, builtins.str] + layers: builtins.list[Descriptor] + @staticmethod + def from_oci_archive( + path: builtins.str | os.PathLike | pathlib.Path, + ) -> PyArtifact: ... + @staticmethod + def from_oci_dir(path: builtins.str | os.PathLike | pathlib.Path) -> PyArtifact: ... + @staticmethod + def from_remote(image_name: builtins.str) -> PyArtifact: ... + @staticmethod + def load(image_name: builtins.str) -> PyArtifact: ... + def get_blob(self, digest: builtins.str) -> bytes: ... + def save(self) -> None: ... + def save_as_archive( + self, path: builtins.str | os.PathLike | pathlib.Path + ) -> None: ... + def save_as_dir(self, path: builtins.str | os.PathLike | pathlib.Path) -> None: ... + def pull(self) -> None: ... + def push(self) -> None: ... + +class PyArtifactBuilder: + @staticmethod + def new_archive( + path: builtins.str | os.PathLike | pathlib.Path, image_name: builtins.str + ) -> PyArtifactBuilder: ... + @staticmethod + def new_archive_unnamed( + path: builtins.str | os.PathLike | pathlib.Path, + ) -> PyArtifactBuilder: ... + @staticmethod + def temp_archive() -> PyArtifactBuilder: ... + @staticmethod + def new_dir( + path: builtins.str | os.PathLike | pathlib.Path, image_name: builtins.str + ) -> PyArtifactBuilder: ... + def add_annotation(self, key: builtins.str, value: builtins.str) -> None: ... + def add_layer( + self, + media_type: builtins.str, + blob: bytes, + annotations: typing.Mapping[builtins.str, builtins.str], + ) -> Descriptor: ... + def build(self) -> PyArtifact: ... + class Quadratic: linear_terms: builtins.dict[builtins.int, builtins.float] constant_term: builtins.float @@ -1183,6 +1170,16 @@ def get_image_dir(image_name: builtins.str) -> builtins.str: - The directory may not exist if the image is not in the local registry. """ +def get_local_registry_path(image_name: builtins.str) -> builtins.str: + r""" + Get the base path for the given image name in the local registry + + This returns the path where the artifact should be stored, without format-specific extensions. + The caller should check: + - If this path is a directory with oci-layout -> oci-dir format + - If "{path}.ommx" exists as a file -> oci-archive format + """ + def get_local_registry_root() -> builtins.str: r""" Get the current OMMX Local Registry root path. diff --git a/python/ommx/ommx/artifact.py b/python/ommx/ommx/artifact.py index 8f994b848..de50cd750 100644 --- a/python/ommx/ommx/artifact.py +++ b/python/ommx/ommx/artifact.py @@ -4,127 +4,56 @@ import json import pandas import numpy -from dataclasses import dataclass from pathlib import Path -from abc import ABC, abstractmethod from ._ommx_rust import ( - ArtifactArchive as _ArtifactArchive, - ArtifactDir as _ArtifactDir, Descriptor, - ArtifactArchiveBuilder as _ArtifactArchiveBuilder, - ArtifactDirBuilder as _ArtifactDirBuilder, get_local_registry_root, set_local_registry_root, get_image_dir, + get_local_registry_path as _get_local_registry_path, + PyArtifact as _PyArtifact, # Experimental Artifact API + PyArtifactBuilder as _PyArtifactBuilder, # Experimental Builder API ) from .v1 import Instance, Solution, ParametricInstance, SampleSet +def get_local_registry_path(image_name: str) -> Path: + """Get the base path for the given image name in the local registry. + + This returns the path where the artifact should be stored, without format-specific extensions. + The caller should check: + - If this path is a directory with oci-layout -> oci-dir format + - If "{path}.ommx" exists as a file -> oci-archive format + """ + return Path(_get_local_registry_path(image_name)) + + __all__ = [ "Artifact", "ArtifactBuilder", - "ArtifactBase", - "ArtifactDir", - "ArtifactDirBuilder", - "ArtifactArchive", - "ArtifactArchiveBuilder", "Descriptor", "get_local_registry_root", "set_local_registry_root", "get_image_dir", + "get_local_registry_path", ] -class ArtifactBase(ABC): - @property - @abstractmethod - def image_name(self) -> str | None: ... - - @property - @abstractmethod - def annotations(self) -> dict[str, str]: ... - - @property - @abstractmethod - def layers(self) -> list[Descriptor]: ... - - @abstractmethod - def get_blob(self, digest: str) -> bytes: ... - - @abstractmethod - def push(self): ... - - -# FIXME: This wrapper class should be defined in Rust binding directly, -# but PyO3 does not support inheriting Python class https://github.com/PyO3/pyo3/issues/991 -@dataclass -class ArtifactArchive(ArtifactBase): - _base: _ArtifactArchive - - @staticmethod - def from_oci_archive(path: str) -> ArtifactArchive: - return ArtifactArchive(_ArtifactArchive.from_oci_archive(path)) - - @property - def image_name(self) -> str | None: - return self._base.image_name - - @property - def annotations(self) -> dict[str, str]: - return self._base.annotations - - @property - def layers(self) -> list[Descriptor]: - return self._base.layers - - def get_blob(self, digest: str) -> bytes: - return self._base.get_blob(digest) - - def push(self): - self._base.push() - - -# FIXME: This wrapper class should be defined in Rust binding directly, -# but PyO3 does not support inheriting Python class https://github.com/PyO3/pyo3/issues/991 -@dataclass -class ArtifactDir(ArtifactBase): - _base: _ArtifactDir - - @staticmethod - def from_oci_dir(path: str) -> ArtifactDir: - return ArtifactDir(_ArtifactDir.from_oci_dir(path)) - - @staticmethod - def from_image_name(image_name: str) -> ArtifactDir: - return ArtifactDir(_ArtifactDir.from_image_name(image_name)) - - @property - def image_name(self) -> str | None: - return self._base.image_name - - @property - def annotations(self) -> dict[str, str]: - return self._base.annotations - - @property - def layers(self) -> list[Descriptor]: - return self._base.layers - - def get_blob(self, digest: str) -> bytes: - return self._base.get_blob(digest) - - def push(self): - self._base.push() +# Legacy classes removed - now using experimental::artifact internally +# ArtifactBase, ArtifactArchive, ArtifactDir are no longer needed -@dataclass class Artifact: """ Reader for OMMX Artifacts. + + Now uses experimental::artifact internally for improved performance and format handling. """ - _base: ArtifactBase + def __init__(self, rust_artifact: _PyArtifact): + """Internal constructor - use static methods instead""" + self._rust = rust_artifact @staticmethod def load_archive(path: str | Path) -> Artifact: @@ -143,13 +72,13 @@ def load_archive(path: str | Path) -> Artifact: path = Path(path) if path.is_file(): - base = ArtifactArchive.from_oci_archive(str(path)) + rust_artifact = _PyArtifact.from_oci_archive(str(path)) elif path.is_dir(): - base = ArtifactDir.from_oci_dir(str(path)) + rust_artifact = _PyArtifact.from_oci_dir(str(path)) else: raise ValueError("Path must be a file or a directory") - return Artifact(base) + return Artifact(rust_artifact) @staticmethod def load(image_name: str) -> Artifact: @@ -157,6 +86,7 @@ def load(image_name: str) -> Artifact: Load an artifact stored as container image in local or remote registry If the image is not found in local registry, it will try to pull from remote registry. + Supports both oci-dir and oci-archive formats in local registry. >>> artifact = Artifact.load("ghcr.io/jij-inc/ommx/random_lp_instance:4303c7f") >>> print(artifact.image_name) @@ -166,18 +96,19 @@ def load(image_name: str) -> Artifact: sha256:93fdc9fcb8e21b34e3517809a348938d9455e9b9e579548bbf018a514c082df2 """ - base = ArtifactDir.from_image_name(image_name) - return Artifact(base) + # Use experimental Artifact.load which handles format detection and remote pull + rust_artifact = _PyArtifact.load(image_name) + return Artifact(rust_artifact) def push(self): """ Push the artifact to remote registry """ - self._base.push() + self._rust.push() @property def image_name(self) -> str | None: - return self._base.image_name + return self._rust.image_name @property def annotations(self) -> dict[str, str]: @@ -191,11 +122,11 @@ def annotations(self) -> dict[str, str]: Test artifact created by examples/artifact_archive.rs """ - return self._base.annotations + return self._rust.annotations @property def layers(self) -> list[Descriptor]: - return self._base.layers + return self._rust.layers def get_layer_descriptor(self, digest: str) -> Descriptor: """ @@ -215,7 +146,7 @@ def get_layer_descriptor(self, digest: str) -> Descriptor: def get_blob(self, digest: str | Descriptor) -> bytes: if isinstance(digest, Descriptor): digest = digest.digest - return self._base.get_blob(digest) + return self._rust.get_blob(digest) def get_layer(self, descriptor: Descriptor) -> Instance | Solution | numpy.ndarray: """ @@ -350,82 +281,20 @@ def get_json(self, descriptor: Descriptor): return json.loads(blob) -class ArtifactBuilderBase(ABC): - @abstractmethod - def add_layer( - self, media_type: str, blob: bytes, annotations: dict[str, str] - ) -> Descriptor: ... - - @abstractmethod - def add_annotation(self, key: str, value: str): ... - - @abstractmethod - def build(self) -> ArtifactBase: ... - - -# FIXME: This wrapper class should be defined in Rust binding directly, -# but PyO3 does not support inheriting Python class https://github.com/PyO3/pyo3/issues/991 -@dataclass -class ArtifactArchiveBuilder(ArtifactBuilderBase): - _base: _ArtifactArchiveBuilder - - @staticmethod - def new(path: str, image_name: str) -> ArtifactArchiveBuilder: - return ArtifactArchiveBuilder(_ArtifactArchiveBuilder.new(path, image_name)) - - @staticmethod - def new_unnamed(path: str) -> ArtifactArchiveBuilder: - return ArtifactArchiveBuilder(_ArtifactArchiveBuilder.new_unnamed(path)) - - @staticmethod - def temp() -> ArtifactArchiveBuilder: - return ArtifactArchiveBuilder(_ArtifactArchiveBuilder.temp()) - - def add_layer( - self, media_type: str, blob: bytes, annotations: dict[str, str] = {} - ) -> Descriptor: - return self._base.add_layer(media_type, blob, annotations) - - def add_annotation(self, key: str, value: str): - self._base.add_annotation(key, value) - - def build(self) -> ArtifactArchive: - return ArtifactArchive(self._base.build()) - +# Legacy builder classes removed - now using experimental::artifact::Builder internally +# ArtifactBuilderBase, ArtifactArchiveBuilder, ArtifactDirBuilder are no longer needed -# FIXME: This wrapper class should be defined in Rust binding directly, -# but PyO3 does not support inheriting Python class https://github.com/PyO3/pyo3/issues/991 -@dataclass -class ArtifactDirBuilder(ArtifactBuilderBase): - _base: _ArtifactDirBuilder - @staticmethod - def new(image_name: str) -> ArtifactDirBuilder: - return ArtifactDirBuilder(_ArtifactDirBuilder.new(image_name)) - - @staticmethod - def for_github(org: str, repo: str, name: str, tag: str) -> ArtifactDirBuilder: - return ArtifactDirBuilder(_ArtifactDirBuilder.for_github(org, repo, name, tag)) - - def add_layer( - self, media_type: str, blob: bytes, annotations: dict[str, str] = {} - ) -> Descriptor: - return self._base.add_layer(media_type, blob, annotations) - - def add_annotation(self, key: str, value: str): - self._base.add_annotation(key, value) - - def build(self) -> ArtifactDir: - return ArtifactDir(self._base.build()) - - -@dataclass(frozen=True) class ArtifactBuilder: """ Builder for OMMX Artifacts. + + Now uses experimental::artifact::Builder internally. """ - _base: ArtifactBuilderBase + def __init__(self, rust_builder: _PyArtifactBuilder): + """Internal constructor - use static methods instead""" + self._rust = rust_builder @staticmethod def new_archive_unnamed(path: str | Path) -> ArtifactBuilder: @@ -460,7 +329,8 @@ def new_archive_unnamed(path: str | Path) -> ArtifactBuilder: """ if isinstance(path, str): path = Path(path) - return ArtifactBuilder(ArtifactArchiveBuilder.new_unnamed(str(path))) + rust_builder = _PyArtifactBuilder.new_archive_unnamed(path) + return ArtifactBuilder(rust_builder) @staticmethod def new_archive(path: str | Path, image_name: str) -> ArtifactBuilder: @@ -495,7 +365,8 @@ def new_archive(path: str | Path, image_name: str) -> ArtifactBuilder: """ if isinstance(path, str): path = Path(path) - return ArtifactBuilder(ArtifactArchiveBuilder.new(str(path), image_name)) + rust_builder = _PyArtifactBuilder.new_archive(path, image_name) + return ArtifactBuilder(rust_builder) @staticmethod def new(image_name: str) -> ArtifactBuilder: @@ -526,7 +397,26 @@ def new(image_name: str) -> ArtifactBuilder: ghcr.io/jij-inc/ommx/single_feasible_lp:... """ - return ArtifactBuilder(ArtifactDirBuilder.new(image_name)) + # Get base path and create oci-archive format path + base_path = get_local_registry_path(image_name) + archive_path = base_path.with_suffix(".ommx") + # Ensure parent directory exists + archive_path.parent.mkdir(parents=True, exist_ok=True) + rust_builder = _PyArtifactBuilder.new_archive(archive_path, image_name) + return ArtifactBuilder(rust_builder) + + @staticmethod + def new_dir(path: str | Path, image_name: str) -> ArtifactBuilder: + """ + Create a new artifact in oci-dir format at the specified path. + + This is mainly for backward compatibility with the old dir-based format. + For new code, prefer using new() which creates oci-archive format. + """ + if isinstance(path, str): + path = Path(path) + rust_builder = _PyArtifactBuilder.new_dir(path, image_name) + return ArtifactBuilder(rust_builder) @staticmethod def temp() -> ArtifactBuilder: @@ -543,7 +433,8 @@ def temp() -> ArtifactBuilder: >>> artifact.push() """ - return ArtifactBuilder(ArtifactArchiveBuilder.temp()) + rust_builder = _PyArtifactBuilder.temp_archive() + return ArtifactBuilder(rust_builder) @staticmethod def for_github(org: str, repo: str, name: str, tag: str) -> ArtifactBuilder: @@ -574,7 +465,12 @@ def for_github(org: str, repo: str, name: str, tag: str) -> ArtifactBuilder: ghcr.io/jij-inc/ommx/single_feasible_lp:... """ - return ArtifactBuilder(ArtifactDirBuilder.for_github(org, repo, name, tag)) + image_name = f"ghcr.io/{org.lower()}/{repo.lower()}/{name}:{tag}" + builder = ArtifactBuilder.new(image_name) + builder.add_annotation( + "org.opencontainers.image.source", f"https://github.com/{org}/{repo}" + ) + return builder def add_instance(self, instance: Instance) -> Descriptor: """ @@ -809,16 +705,17 @@ def add_layer( """ Low-level API to add any type of layer to the artifact with annotations. Use :meth:`add_instance` or other high-level methods if possible. """ - return self._base.add_layer(media_type, blob, annotations) + return self._rust.add_layer(media_type, blob, annotations) def add_annotation(self, key: str, value: str): """ Add annotation to the artifact itself. """ - self._base.add_annotation(key, value) + self._rust.add_annotation(key, value) def build(self) -> Artifact: """ Build the artifact. """ - return Artifact(self._base.build()) + rust_artifact = self._rust.build() + return Artifact(rust_artifact) diff --git a/python/ommx/src/artifact.rs b/python/ommx/src/artifact.rs index 777ca34ab..e39bf77c4 100644 --- a/python/ommx/src/artifact.rs +++ b/python/ommx/src/artifact.rs @@ -1,60 +1,115 @@ use crate::PyDescriptor; use anyhow::Result; -use derive_more::{Deref, From}; -use ocipkg::{ - image::{Image, OciArchive, OciDir}, - Digest, ImageName, -}; -use ommx::artifact::Artifact; +use ocipkg::{Digest, ImageName}; use pyo3::{prelude::*, types::PyBytes}; use std::{collections::HashMap, path::PathBuf, sync::Mutex}; +// Import artifact +use ommx::artifact::Artifact as RustArtifact; + +/// Get the current OMMX Local Registry root path. +#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyfunction)] +#[pyfunction] +pub fn get_local_registry_root() -> PathBuf { + ommx::artifact::get_local_registry_root().to_path_buf() +} + +/// Set the OMMX Local Registry root path. +/// +/// - The local registry root can be set only once per process, +/// and this function will return an error if it is already set. +/// - The root path is automatically set when used for creating artifacts without calling this function. +/// - Default path is following: +/// - If `OMMX_LOCAL_REGISTRY_ROOT` environment variable is set, its value is used. +/// - Otherwise, OS-specific path by [directories](https://docs.rs/directories/latest/directories/struct.ProjectDirs.html#method.data_dir) is used: +/// - `$XDG_DATA_HOME/ommx/` on Linux +/// - `$HOME/Library/Application Support/org.ommx.ommx/` on macOS +/// +#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyfunction)] +#[pyfunction] +pub fn set_local_registry_root(path: PathBuf) -> Result<()> { + ommx::artifact::set_local_registry_root(path)?; + Ok(()) +} + +/// Get the path where given image is stored in the local registry. +/// +/// - The directory may not exist if the image is not in the local registry. +/// +#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyfunction)] +#[pyfunction] +pub fn get_image_dir(image_name: &str) -> Result { + let image_name = ImageName::parse(image_name)?; + #[allow(deprecated)] + Ok(ommx::artifact::get_image_dir(&image_name)) +} + +/// Get the base path for the given image name in the local registry +/// +/// This returns the path where the artifact should be stored, without format-specific extensions. +/// The caller should check: +/// - If this path is a directory with oci-layout -> oci-dir format +/// - If "{path}.ommx" exists as a file -> oci-archive format +/// +#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyfunction)] +#[pyfunction] +pub fn get_local_registry_path(image_name: &str) -> Result { + let image_name = ImageName::parse(image_name)?; + Ok(ommx::artifact::get_local_registry_path(&image_name)) +} + +// ============================================================================ +// Artifact API - Using ommx::artifact +// ============================================================================ + #[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyclass)] #[pyclass] #[pyo3(module = "ommx._ommx_rust")] -#[derive(From, Deref)] -pub struct ArtifactArchive(Mutex>); - -impl From> for ArtifactArchive { - fn from(artifact: Artifact) -> Self { - Self(Mutex::new(artifact)) - } -} +pub struct PyArtifact(Mutex); #[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pymethods)] #[pymethods] -impl ArtifactArchive { +impl PyArtifact { #[staticmethod] pub fn from_oci_archive(path: PathBuf) -> Result { - let artifact = Artifact::from_oci_archive(&path)?; - Ok(Self(artifact.into())) + let artifact = RustArtifact::from_oci_archive(&path)?; + Ok(Self(Mutex::new(artifact))) + } + + #[staticmethod] + pub fn from_oci_dir(path: PathBuf) -> Result { + let artifact = RustArtifact::from_oci_dir(&path)?; + Ok(Self(Mutex::new(artifact))) + } + + #[staticmethod] + pub fn from_remote(image_name: &str) -> Result { + let image_name = ImageName::parse(image_name)?; + let artifact = RustArtifact::from_remote(image_name)?; + Ok(Self(Mutex::new(artifact))) + } + + #[staticmethod] + pub fn load(image_name: &str) -> Result { + let image_name = ImageName::parse(image_name)?; + let artifact = RustArtifact::load(&image_name)?; + Ok(Self(Mutex::new(artifact))) } #[getter] pub fn image_name(&mut self) -> Option { - self.0 - .lock() - .unwrap() - .get_name() - .map(|name| name.to_string()) - .ok() + self.0.lock().unwrap().image_name() } #[getter] pub fn annotations(&mut self) -> Result> { - let manifest = self.0.lock().unwrap().get_manifest()?; - Ok(manifest.annotations().as_ref().cloned().unwrap_or_default()) + self.0.lock().unwrap().annotations() } #[getter] pub fn layers(&mut self) -> Result> { - let manifest = self.0.lock().unwrap().get_manifest()?; - Ok(manifest - .layers() - .iter() - .cloned() - .map(PyDescriptor::from) - .collect()) + let layers = self.0.lock().unwrap().layers()?; + Ok(layers.into_iter().map(PyDescriptor::from).collect()) } pub fn get_blob<'py>(&mut self, py: Python<'py>, digest: &str) -> Result> { @@ -63,108 +118,103 @@ impl ArtifactArchive { Ok(PyBytes::new(py, blob.as_ref())) } + pub fn save(&mut self) -> Result<()> { + self.0.lock().unwrap().save() + } + + pub fn save_as_archive(&mut self, path: PathBuf) -> Result<()> { + self.0.lock().unwrap().save_as_archive(&path) + } + + pub fn save_as_dir(&mut self, path: PathBuf) -> Result<()> { + self.0.lock().unwrap().save_as_dir(&path) + } + + pub fn pull(&mut self) -> Result<()> { + self.0.lock().unwrap().pull() + } + pub fn push(&mut self) -> Result<()> { - // Do not expose Artifact to Python API for simplicity. - // In Python API, the `Artifact` class always refers to the local artifact, which may be either an OCI archive or an OCI directory. - let _remote = self.0.lock().unwrap().push()?; - Ok(()) + self.0.lock().unwrap().push() } } +// ============================================================================ +// Builder API +// ============================================================================ + +use ommx::artifact::Builder as RustBuilder; + #[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyclass)] #[pyclass] #[pyo3(module = "ommx._ommx_rust")] -#[derive(From, Deref)] -pub struct ArtifactDir(Artifact); +pub struct PyArtifactBuilder(Option); #[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pymethods)] #[pymethods] -impl ArtifactDir { +impl PyArtifactBuilder { #[staticmethod] - pub fn from_image_name(image_name: &str) -> Result { + pub fn new_archive(path: PathBuf, image_name: &str) -> Result { let image_name = ImageName::parse(image_name)?; - let local_path = ommx::artifact::get_image_dir(&image_name); - if local_path.exists() { - return Ok(Self(Artifact::from_oci_dir(&local_path)?)); - } - let mut remote = Artifact::from_remote(image_name)?; - Ok(Self(remote.pull()?)) + let builder = RustBuilder::new_archive(path, image_name)?; + Ok(Self(Some(builder))) } #[staticmethod] - pub fn from_oci_dir(path: PathBuf) -> Result { - let artifact = Artifact::from_oci_dir(&path)?; - Ok(Self(artifact)) + pub fn new_archive_unnamed(path: PathBuf) -> Result { + let builder = RustBuilder::new_archive_unnamed(path)?; + Ok(Self(Some(builder))) } - #[getter] - pub fn image_name(&mut self) -> Option { - self.0.get_name().map(|name| name.to_string()).ok() + #[staticmethod] + pub fn temp_archive() -> Result { + let builder = RustBuilder::temp_archive()?; + Ok(Self(Some(builder))) } - #[getter] - pub fn annotations(&mut self) -> Result> { - let manifest = self.0.get_manifest()?; - Ok(manifest.annotations().as_ref().cloned().unwrap_or_default()) + #[staticmethod] + pub fn new_dir(path: PathBuf, image_name: &str) -> Result { + let image_name = ImageName::parse(image_name)?; + let builder = RustBuilder::new_dir(path, image_name)?; + Ok(Self(Some(builder))) } - #[getter] - pub fn layers(&mut self) -> Result> { - let manifest = self.0.get_manifest()?; - Ok(manifest - .layers() - .iter() - .cloned() - .map(PyDescriptor::from) - .collect()) + pub fn add_annotation(&mut self, key: String, value: String) { + if let Some(builder) = &mut self.0 { + builder.add_annotation(key, value); + } } - pub fn get_blob<'py>(&mut self, py: Python<'py>, digest: &str) -> Result> { - let digest = Digest::new(digest)?; - let blob = self.0.get_blob(&digest)?; - Ok(PyBytes::new(py, blob.as_ref())) - } + pub fn add_layer( + &mut self, + media_type: &str, + blob: &Bound, + annotations: HashMap, + ) -> Result { + use ocipkg::distribution::MediaType; - pub fn push(&mut self) -> Result<()> { - // Do not expose Artifact to Python API for simplicity. - // In Python API, the `Artifact` class always refers to the local artifact, which may be either an OCI archive or an OCI directory. - let _remote = self.0.push()?; - Ok(()) - } -} + let media_type = MediaType::Other(media_type.to_string()); -/// Get the current OMMX Local Registry root path. -#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyfunction)] -#[pyfunction] -pub fn get_local_registry_root() -> PathBuf { - ommx::artifact::get_local_registry_root().to_path_buf() -} + let builder = self + .0 + .as_mut() + .ok_or_else(|| anyhow::anyhow!("Builder already consumed"))?; -/// Set the OMMX Local Registry root path. -/// -/// - The local registry root can be set only once per process, -/// and this function will return an error if it is already set. -/// - The root path is automatically set when used for creating artifacts without calling this function. -/// - Default path is following: -/// - If `OMMX_LOCAL_REGISTRY_ROOT` environment variable is set, its value is used. -/// - Otherwise, OS-specific path by [directories](https://docs.rs/directories/latest/directories/struct.ProjectDirs.html#method.data_dir) is used: -/// - `$XDG_DATA_HOME/ommx/` on Linux -/// - `$HOME/Library/Application Support/org.ommx.ommx/` on macOS -/// -#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyfunction)] -#[pyfunction] -pub fn set_local_registry_root(path: PathBuf) -> Result<()> { - ommx::artifact::set_local_registry_root(path)?; - Ok(()) -} + let blob_bytes = blob.as_bytes(); -/// Get the path where given image is stored in the local registry. -/// -/// - The directory may not exist if the image is not in the local registry. -/// -#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyfunction)] -#[pyfunction] -pub fn get_image_dir(image_name: &str) -> Result { - let image_name = ImageName::parse(image_name)?; - Ok(ommx::artifact::get_image_dir(&image_name)) + let desc = match builder { + RustBuilder::Archive(b) => b.add_layer(media_type, blob_bytes, annotations)?, + RustBuilder::Dir(b) => b.add_layer(media_type, blob_bytes, annotations)?, + }; + Ok(PyDescriptor::from(desc)) + } + + pub fn build(&mut self) -> Result { + let builder = self + .0 + .take() + .ok_or_else(|| anyhow::anyhow!("Builder already consumed"))?; + let artifact = builder.build()?; + Ok(PyArtifact(Mutex::new(artifact))) + } } diff --git a/python/ommx/src/builder.rs b/python/ommx/src/builder.rs deleted file mode 100644 index 013d6d9f3..000000000 --- a/python/ommx/src/builder.rs +++ /dev/null @@ -1,124 +0,0 @@ -use anyhow::{bail, Result}; -use ocipkg::{ - image::{OciArchiveBuilder, OciDirBuilder}, - ImageName, -}; -use ommx::artifact::Builder; -use pyo3::{prelude::*, types::PyBytes}; -use std::{collections::HashMap, path::PathBuf}; - -use crate::{ArtifactArchive, ArtifactDir, PyDescriptor}; - -#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyclass)] -#[pyclass] -#[pyo3(module = "ommx._ommx_rust")] -pub struct ArtifactArchiveBuilder(Option>); - -#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pymethods)] -#[pymethods] -impl ArtifactArchiveBuilder { - #[staticmethod] - pub fn new_unnamed(path: PathBuf) -> Result { - let builder = Builder::new_archive_unnamed(path)?; - Ok(Self(Some(builder))) - } - - #[staticmethod] - pub fn new(path: PathBuf, image_name: &str) -> Result { - let image_name = ImageName::parse(image_name)?; - let builder = Builder::new_archive(path, image_name)?; - Ok(Self(Some(builder))) - } - - #[staticmethod] - pub fn temp() -> Result { - let builder = Builder::temp_archive()?; - Ok(Self(Some(builder))) - } - - pub fn add_layer( - &mut self, - media_type: &str, - blob: Bound, - annotations: HashMap, - ) -> Result { - if let Some(builder) = self.0.as_mut() { - let desc = builder.add_layer(media_type.into(), blob.as_bytes(), annotations)?; - Ok(PyDescriptor::from(desc)) - } else { - bail!("Already built artifact") - } - } - - pub fn add_annotation(&mut self, key: &str, value: &str) -> Result<()> { - if let Some(builder) = self.0.as_mut() { - builder.add_annotation(key.into(), value.into()); - Ok(()) - } else { - bail!("Already built artifact") - } - } - - pub fn build(&mut self) -> Result { - if let Some(builder) = self.0.take() { - let artifact = builder.build()?; - Ok(ArtifactArchive::from(artifact)) - } else { - bail!("Already built artifact") - } - } -} - -#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pyclass)] -#[pyclass] -#[pyo3(module = "ommx._ommx_rust")] -pub struct ArtifactDirBuilder(Option>); - -#[cfg_attr(feature = "stub_gen", pyo3_stub_gen::derive::gen_stub_pymethods)] -#[pymethods] -impl ArtifactDirBuilder { - #[staticmethod] - pub fn new(image_name: &str) -> Result { - let image_name = ImageName::parse(image_name)?; - let builder = Builder::new(image_name)?; - Ok(Self(Some(builder))) - } - - #[staticmethod] - pub fn for_github(org: &str, repo: &str, name: &str, tag: &str) -> Result { - let builder = Builder::for_github(org, repo, name, tag)?; - Ok(Self(Some(builder))) - } - - pub fn add_layer( - &mut self, - media_type: &str, - blob: Bound, - annotations: HashMap, - ) -> Result { - if let Some(builder) = self.0.as_mut() { - let desc = builder.add_layer(media_type.into(), blob.as_bytes(), annotations)?; - Ok(PyDescriptor::from(desc)) - } else { - bail!("Already built artifact") - } - } - - pub fn add_annotation(&mut self, key: &str, value: &str) -> Result<()> { - if let Some(builder) = self.0.as_mut() { - builder.add_annotation(key.into(), value.into()); - Ok(()) - } else { - bail!("Already built artifact") - } - } - - pub fn build(&mut self) -> Result { - if let Some(builder) = self.0.take() { - let artifact = builder.build()?; - Ok(ArtifactDir::from(artifact)) - } else { - bail!("Already built artifact") - } - } -} diff --git a/python/ommx/src/lib.rs b/python/ommx/src/lib.rs index 22027b9a9..bee75d496 100644 --- a/python/ommx/src/lib.rs +++ b/python/ommx/src/lib.rs @@ -1,6 +1,5 @@ mod artifact; mod bound; -mod builder; mod constraint; mod constraint_hints; mod dataset; @@ -26,7 +25,6 @@ mod state; pub use artifact::*; pub use bound::*; -pub use builder::*; pub use constraint::*; pub use constraint_hints::*; pub use dataset::*; @@ -70,15 +68,14 @@ pub fn get_default_atol() -> f64 { fn _ommx_rust(_py: Python, m: &Bound) -> PyResult<()> { pyo3_log::init(); - // OMMX Artifact - m.add_class::()?; - m.add_class::()?; - m.add_class::()?; - m.add_class::()?; + // OMMX Artifact (Experimental API) + m.add_class::()?; + m.add_class::()?; m.add_class::()?; m.add_function(wrap_pyfunction!(set_local_registry_root, m)?)?; m.add_function(wrap_pyfunction!(get_local_registry_root, m)?)?; m.add_function(wrap_pyfunction!(get_image_dir, m)?)?; + m.add_function(wrap_pyfunction!(get_local_registry_path, m)?)?; // OMMX Message m.add_class::()?; diff --git a/rust/ommx/examples/pull_artifact.rs b/rust/ommx/examples/pull_artifact.rs index 2742ec491..6876f13c9 100644 --- a/rust/ommx/examples/pull_artifact.rs +++ b/rust/ommx/examples/pull_artifact.rs @@ -11,12 +11,15 @@ fn main() -> Result<()> { let image_name = ImageName::parse("ghcr.io/jij-inc/ommx/random_lp_instance:4303c7f")?; // Pull the artifact from remote registry - let mut remote = Artifact::from_remote(image_name)?; - let mut local = remote.pull()?; + let mut artifact = Artifact::from_remote(image_name)?; + artifact.pull()?; // Load the instance message from the artifact - for desc in local.get_layer_descriptors(&media_types::v1_instance())? { - println!("{}", desc.digest()); + let layers = artifact.layers()?; + for desc in layers.iter() { + if desc.media_type() == &media_types::v1_instance() { + println!("{}", desc.digest()); + } } Ok(()) } diff --git a/rust/ommx/src/artifact.rs b/rust/ommx/src/artifact.rs index f22015b66..8adc01050 100644 --- a/rust/ommx/src/artifact.rs +++ b/rust/ommx/src/artifact.rs @@ -1,31 +1,31 @@ //! Manage messages as container //! +//! This module provides a unified Artifact API that dynamically manages different storage formats: +//! - OCI Archive format (`.ommx` files, default for new artifacts) +//! - OCI Directory format (legacy support) +//! - Remote registry references mod annotations; mod builder; mod config; +mod io; +mod layers; pub mod media_types; +#[cfg(test)] +mod tests; + pub use annotations::*; -pub use builder::*; +pub use builder::Builder; pub use config::*; -use crate::v1; -use anyhow::{bail, ensure, Context, Result}; +use anyhow::{ensure, Context, Result}; use ocipkg::{ - distribution::MediaType, - image::{ - Image, OciArchive, OciArchiveBuilder, OciArtifact, OciDir, OciDirBuilder, Remote, - RemoteBuilder, - }, - oci_spec::image::{Descriptor, ImageManifest}, - Digest, ImageName, + image::{Image, OciArchive, OciArtifact, OciDir, Remote}, + oci_spec::image::Descriptor, + ImageName, }; -use prost::Message; +use std::path::Path; use std::{env, path::PathBuf, sync::OnceLock}; -use std::{ - ops::{Deref, DerefMut}, - path::Path, -}; /// Global storage for the local registry root path static LOCAL_REGISTRY_ROOT: OnceLock = OnceLock::new(); @@ -91,10 +91,27 @@ pub fn data_dir() -> Result { } /// Get the directory for the given image name in the local registry +/// +/// # Deprecated +/// Use [`get_local_registry_path`] instead, which is format-agnostic and works with both oci-dir and oci-archive formats. +#[deprecated( + since = "2.1.0", + note = "Use get_local_registry_path instead for dual format support" +)] pub fn get_image_dir(image_name: &ImageName) -> PathBuf { get_local_registry_root().join(image_name.as_path()) } +/// Get the base path for the given image name in the local registry +/// +/// This returns the path where the artifact should be stored, without format-specific extensions. +/// The caller should check: +/// - If this path is a directory with oci-layout -> oci-dir format +/// - If "{path}.ommx" exists as a file -> oci-archive format +pub fn get_local_registry_path(image_name: &ImageName) -> PathBuf { + get_local_registry_root().join(image_name.as_path()) +} + #[deprecated(note = "Use get_image_dir instead")] pub fn image_dir(image_name: &ImageName) -> Result { #[allow(deprecated)] @@ -112,156 +129,107 @@ pub fn ghcr(org: &str, repo: &str, name: &str, tag: &str) -> Result { } fn gather_oci_dirs(dir: &Path) -> Result> { - let mut images = Vec::new(); + gather_artifacts(dir) +} + +fn gather_artifacts(dir: &Path) -> Result> { + let mut artifacts = Vec::new(); for entry in std::fs::read_dir(dir)? { let entry = entry?; let path = entry.path(); if path.is_dir() { if path.join("oci-layout").exists() { - images.push(path); + // OCI directory format + artifacts.push(path); } else { - let mut sub_images = gather_oci_dirs(&path)?; - images.append(&mut sub_images) + let mut sub_artifacts = gather_artifacts(&path)?; + artifacts.append(&mut sub_artifacts) + } + } else if path.is_file() { + // Check if it's an OCI archive by trying to parse it + // OCI archives typically have .ommx extension or can be detected by content + if is_oci_archive(&path) { + artifacts.push(path); } } } - Ok(images) + Ok(artifacts) } -fn auth_from_env() -> Result<(String, String, String)> { - if let (Ok(domain), Ok(username), Ok(password)) = ( - env::var("OMMX_BASIC_AUTH_DOMAIN"), - env::var("OMMX_BASIC_AUTH_USERNAME"), - env::var("OMMX_BASIC_AUTH_PASSWORD"), - ) { - log::info!( - "Detect OMMX_BASIC_AUTH_DOMAIN, OMMX_BASIC_AUTH_USERNAME, OMMX_BASIC_AUTH_PASSWORD for authentication." - ); - return Ok((domain, username, password)); +fn is_oci_archive(path: &Path) -> bool { + // Check file extension first (most common case) + if let Some(ext) = path.extension() { + if ext == "ommx" || ext == "tar" { + return true; + } } - bail!("No authentication information found in environment variables"); -} - -/// Get all images stored in the local registry -pub fn get_images() -> Result> { - let root = get_local_registry_root(); - let dirs = gather_oci_dirs(root)?; - dirs.into_iter() - .map(|dir| { - let relative = dir - .strip_prefix(root) - .context("Failed to get relative path")?; - ImageName::from_path(relative) - }) - .collect() -} - -/// OMMX Artifact, an OCI Artifact of type [`application/org.ommx.v1.artifact`][media_types::v1_artifact] -pub struct Artifact(OciArtifact); -impl Deref for Artifact { - type Target = OciArtifact; - fn deref(&self) -> &Self::Target { - &self.0 - } + // For files without recognized extensions, don't try to parse them + // as this could be expensive and most files won't be OCI archives + false } -impl DerefMut for Artifact { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 - } +/// OMMX Artifact with dynamic format handling +/// +/// This enum replaces the parametric `Artifact` with a simpler API that +/// automatically manages different storage formats. +/// +/// # Variants +/// +/// - `Archive`: OCI archive format (`.ommx` file, default for new artifacts) +/// - `Dir`: OCI directory format (legacy support) +/// - `Remote`: Remote registry reference (transitions to Archive/Dir after pull) +pub enum Artifact { + Archive(OciArtifact), + Dir(OciArtifact), + Remote(Box>), } -impl Artifact { +impl Artifact { + /// Create an Artifact from an OCI archive file (`.ommx`) pub fn from_oci_archive(path: &Path) -> Result { - let artifact = OciArtifact::from_oci_archive(path)?; - Self::new(artifact) - } - - pub fn push(&mut self) -> Result> { - let name = self.get_name()?; - log::info!("Pushing: {name}"); - let mut remote = RemoteBuilder::new(name)?; - if let Ok((domain, username, password)) = auth_from_env() { - remote.add_basic_auth(&domain, &username, &password); - } - let out = ocipkg::image::copy(self.0.deref_mut(), remote)?; - Ok(Artifact(OciArtifact::new(out))) + let mut artifact = OciArtifact::from_oci_archive(path)?; + Self::validate_artifact_type(&mut artifact)?; + Ok(Self::Archive(artifact)) } - pub fn load(&mut self) -> Result<()> { - let image_name = self.get_name()?; - let path = get_image_dir(&image_name); - if path.exists() { - log::trace!("Already exists in local registry: {}", path.display()); - return Ok(()); - } - log::info!("Loading to local registry: {image_name}"); - ocipkg::image::copy(self.0.deref_mut(), OciDirBuilder::new(path, image_name)?)?; - Ok(()) - } -} - -impl Artifact { + /// Create an Artifact from an OCI directory pub fn from_oci_dir(path: &Path) -> Result { - let artifact = OciArtifact::from_oci_dir(path)?; - Self::new(artifact) - } - - pub fn push(&mut self) -> Result> { - let name = self.get_name()?; - log::info!("Pushing: {name}"); - let mut remote = RemoteBuilder::new(name)?; - if let Ok((domain, username, password)) = auth_from_env() { - remote.add_basic_auth(&domain, &username, &password); - } - let out = ocipkg::image::copy(self.0.deref_mut(), remote)?; - Ok(Artifact(OciArtifact::new(out))) + let mut artifact = OciArtifact::from_oci_dir(path)?; + Self::validate_artifact_type(&mut artifact)?; + Ok(Self::Dir(artifact)) } - pub fn save(&mut self, output: &Path) -> Result<()> { - if output.exists() { - bail!("Output file already exists: {}", output.display()); - } - let builder = if let Ok(name) = self.get_name() { - OciArchiveBuilder::new(output.to_path_buf(), name)? - } else { - OciArchiveBuilder::new_unnamed(output.to_path_buf())? - }; - ocipkg::image::copy(self.0.deref_mut(), builder)?; - Ok(()) - } -} - -impl Artifact { + /// Create an Artifact from a remote registry pub fn from_remote(image_name: ImageName) -> Result { let artifact = OciArtifact::from_remote(image_name)?; - Self::new(artifact) + Ok(Self::Remote(Box::new(artifact))) } - pub fn pull(&mut self) -> Result> { - let image_name = self.get_name()?; - let path = get_image_dir(&image_name); - if path.exists() { - log::trace!("Already exists in local registry: {}", path.display()); - return Ok(Artifact(OciArtifact::from_oci_dir(&path)?)); + /// Get the image name if available + pub fn image_name(&mut self) -> Option { + match self { + Self::Archive(a) => a.get_name().ok().map(|n| n.to_string()), + Self::Dir(a) => a.get_name().ok().map(|n| n.to_string()), + Self::Remote(a) => a.as_mut().get_name().ok().map(|n| n.to_string()), } - log::info!("Pulling to local registry: {image_name}"); - if let Ok((domain, username, password)) = auth_from_env() { - self.0.add_basic_auth(&domain, &username, &password); - } - let out = ocipkg::image::copy(self.0.deref_mut(), OciDirBuilder::new(path, image_name)?)?; - Ok(Artifact(OciArtifact::new(out))) } -} -impl Artifact { - pub fn new(artifact: OciArtifact) -> Result { - Ok(Self(artifact)) + /// Get manifest annotations + pub fn annotations(&mut self) -> Result> { + let manifest = self.get_manifest()?; + Ok(manifest.annotations().clone().unwrap_or_default()) + } + + /// Get layer descriptors + pub fn layers(&mut self) -> Result> { + let manifest = self.get_manifest()?; + Ok(manifest.layers().to_vec()) } - pub fn get_manifest(&mut self) -> Result { - let manifest = self.0.get_manifest()?; + /// Validate that the artifact has the correct OMMX artifact type + pub(crate) fn validate_artifact_type(artifact: &mut OciArtifact) -> Result<()> { + let manifest = artifact.get_manifest()?; let ty = manifest .artifact_type() .as_ref() @@ -271,113 +239,40 @@ impl Artifact { "Not an OMMX Artifact: {}", ty ); - Ok(manifest) - } - - pub fn get_config(&mut self) -> Result { - let (_desc, blob) = self.0.get_config()?; - let config = serde_json::from_slice(&blob)?; - Ok(config) - } - - pub fn get_layer_descriptors(&mut self, media_type: &MediaType) -> Result> { - let manifest = self.get_manifest()?; - Ok(manifest - .layers() - .iter() - .filter(|desc| desc.media_type() == media_type) - .cloned() - .collect()) - } - - pub fn get_layer(&mut self, digest: &Digest) -> Result<(Descriptor, Vec)> { - for (desc, blob) in self.0.get_layers()? { - if desc.digest() == &digest.to_string() { - return Ok((desc, blob)); - } - } - bail!("Layer of digest {} not found", digest) - } - - pub fn get_solution(&mut self, digest: &Digest) -> Result<(v1::State, SolutionAnnotations)> { - let (desc, blob) = self.get_layer(digest)?; - ensure!( - desc.media_type() == &media_types::v1_solution(), - "Layer {digest} is not an ommx.v1.Solution: {}", - desc.media_type() - ); - Ok(( - v1::State::decode(blob.as_slice())?, - SolutionAnnotations::from_descriptor(&desc), - )) - } - - pub fn get_sample_set( - &mut self, - digest: &Digest, - ) -> Result<(v1::SampleSet, SampleSetAnnotations)> { - let (desc, blob) = self.get_layer(digest)?; - ensure!( - desc.media_type() == &media_types::v1_sample_set(), - "Layer {digest} is not an ommx.v1.SampleSet: {}", - desc.media_type() - ); - Ok(( - v1::SampleSet::decode(blob.as_slice())?, - SampleSetAnnotations::from_descriptor(&desc), - )) - } - - pub fn get_instance(&mut self, digest: &Digest) -> Result<(v1::Instance, InstanceAnnotations)> { - let (desc, blob) = self.get_layer(digest)?; - ensure!( - desc.media_type() == &media_types::v1_instance(), - "Layer {digest} is not an ommx.v1.Instance: {}", - desc.media_type() - ); - Ok(( - v1::Instance::decode(blob.as_slice())?, - InstanceAnnotations::from_descriptor(&desc), - )) - } - - pub fn get_parametric_instance( - &mut self, - digest: &Digest, - ) -> Result<(v1::ParametricInstance, ParametricInstanceAnnotations)> { - let (desc, blob) = self.get_layer(digest)?; - ensure!( - desc.media_type() == &media_types::v1_parametric_instance(), - "Layer {digest} is not an ommx.v1.ParametricInstance: {}", - desc.media_type() - ); - Ok(( - v1::ParametricInstance::decode(blob.as_slice())?, - ParametricInstanceAnnotations::from_descriptor(&desc), - )) + Ok(()) } +} - pub fn get_solutions(&mut self) -> Result> { - let mut out = Vec::new(); - for (desc, blob) in self.0.get_layers()? { - if desc.media_type() != &media_types::v1_solution() { - continue; - } - let solution = v1::State::decode(blob.as_slice())?; - out.push((desc, solution)); - } - Ok(out) - } +/// Get all images stored in the local registry +pub fn get_images() -> Result> { + let root = get_local_registry_root(); + let artifacts = gather_oci_dirs(root)?; + artifacts + .into_iter() + .map(|artifact_path| { + let relative = artifact_path + .strip_prefix(root) + .context("Failed to get relative path")?; - pub fn get_instances(&mut self) -> Result> { - let mut out = Vec::new(); - for (desc, blob) in self.0.get_layers()? { - if desc.media_type() != &media_types::v1_instance() { - continue; + // For archive files, we need to extract the image name differently + if artifact_path.is_file() { + // Try to extract image name from the archive metadata + if let Ok(mut artifact) = Artifact::from_oci_archive(&artifact_path) { + if let Some(name) = artifact.image_name() { + return ImageName::parse(&name); + } + } + // Fallback: use the file path without extension as image name + let path_without_ext = if let Some(stem) = relative.file_stem() { + relative.with_file_name(stem) + } else { + relative.to_path_buf() + }; + ImageName::from_path(&path_without_ext) + } else { + // Directory format - use path as before + ImageName::from_path(relative) } - let instance = v1::Instance::decode(blob.as_slice())?; - out.push((desc, instance)); - } - Ok(out) - } + }) + .collect() } diff --git a/rust/ommx/src/artifact/builder.rs b/rust/ommx/src/artifact/builder.rs index d50949427..96645b5e5 100644 --- a/rust/ommx/src/artifact/builder.rs +++ b/rust/ommx/src/artifact/builder.rs @@ -1,60 +1,49 @@ -use crate::{ - artifact::{ - get_local_registry_root, ghcr, media_types, Artifact, Config, InstanceAnnotations, - SolutionAnnotations, - }, - v1, +use super::Artifact; +use crate::artifact::{ + get_local_registry_root, media_types, Config, InstanceAnnotations, + ParametricInstanceAnnotations, SampleSetAnnotations, SolutionAnnotations, }; -use anyhow::Result; +use crate::v1; +use anyhow::{bail, Context, Result}; use ocipkg::{ - image::{ImageBuilder, OciArchiveBuilder, OciArtifactBuilder, OciDirBuilder}, + image::{OciArchiveBuilder, OciArtifactBuilder, OciDirBuilder}, ImageName, }; use prost::Message; -use std::{ - collections::HashMap, - ops::{Deref, DerefMut}, - path::PathBuf, -}; -use url::Url; +use std::{collections::HashMap, path::PathBuf}; use uuid::Uuid; -use super::{ParametricInstanceAnnotations, SampleSetAnnotations}; - -/// Build [Artifact] -pub struct Builder(OciArtifactBuilder); - -impl Deref for Builder { - type Target = OciArtifactBuilder; - fn deref(&self) -> &Self::Target { - &self.0 - } +/// Builder for experimental [`Artifact`] values with runtime-selected backend. +/// +/// The builder mirrors the legacy generic builders but stores the backend choice +/// inside the enum variants, allowing callers to construct archives or +/// directories without specifying the type parameter in the signature. +pub enum Builder { + Archive(OciArtifactBuilder), + Dir(OciArtifactBuilder), } -impl DerefMut for Builder { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 +impl Builder { + /// Create a builder that writes into an OCI archive file at `path`. + pub fn new_archive(path: PathBuf, image_name: ImageName) -> Result { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).with_context(|| { + format!("Failed to create parent directory: {}", parent.display()) + })?; + } + let archive = OciArchiveBuilder::new(path, image_name)?; + let builder = OciArtifactBuilder::new(archive, media_types::v1_artifact())?; + Ok(Self::Archive(builder)) } -} -impl Builder { + /// Create an unnamed archive builder (primarily for tests). pub fn new_archive_unnamed(path: PathBuf) -> Result { let archive = OciArchiveBuilder::new_unnamed(path)?; - Ok(Self(OciArtifactBuilder::new( - archive, - media_types::v1_artifact(), - )?)) - } - - pub fn new_archive(path: PathBuf, image_name: ImageName) -> Result { - let archive = OciArchiveBuilder::new(path, image_name)?; - Ok(Self(OciArtifactBuilder::new( - archive, - media_types::v1_artifact(), - )?)) + let builder = OciArtifactBuilder::new(archive, media_types::v1_artifact())?; + Ok(Self::Archive(builder)) } - /// Create a new artifact builder for a temporary file. This is insecure and should only be used in tests. + /// Convenience helper that stores the archive in a temporary location. pub fn temp_archive() -> Result { let id = Uuid::new_v4(); Self::new_archive( @@ -62,39 +51,70 @@ impl Builder { ImageName::parse(&format!("ttl.sh/{id}:1h"))?, ) } -} -impl Builder { - pub fn new(image_name: ImageName) -> Result { - let dir = get_local_registry_root().join(image_name.as_path()); - let layout = OciDirBuilder::new(dir, image_name)?; - Ok(Self(OciArtifactBuilder::new( - layout, - media_types::v1_artifact(), - )?)) + /// Create a builder that writes to an OCI directory at the provided path. + pub fn new_dir(path: PathBuf, image_name: ImageName) -> Result { + if path.exists() { + bail!("Output directory already exists: {}", path.display()); + } + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).with_context(|| { + format!("Failed to create parent directory: {}", parent.display()) + })?; + } + let layout = OciDirBuilder::new(path, image_name)?; + let builder = OciArtifactBuilder::new(layout, media_types::v1_artifact())?; + Ok(Self::Dir(builder)) } - /// Create a new artifact builder for a GitHub container registry image + /// Create a builder targeting the local registry directory for the image name. + pub fn new_registry_dir(image_name: ImageName) -> Result { + let path = get_local_registry_root().join(image_name.as_path()); + Self::new_dir(path, image_name) + } + + /// Create a new artifact builder for a GitHub container registry image. + /// + /// This creates an oci-archive artifact in the local registry. pub fn for_github(org: &str, repo: &str, name: &str, tag: &str) -> Result { + use crate::artifact::ghcr; + use url::Url; + let image_name = ghcr(org, repo, name, tag)?; let source = Url::parse(&format!("https://github.com/{org}/{repo}"))?; - let mut builder = Self::new(image_name)?; - builder.add_source(&source); + let base_path = get_local_registry_root().join(image_name.as_path()); + let archive_path = base_path.with_extension("ommx"); + + if archive_path.exists() { + bail!("Artifact already exists: {}", archive_path.display()); + } + + let mut builder = Self::new_archive(archive_path, image_name)?; + match &mut builder { + Self::Archive(b) => { + b.add_source(&source); + } + Self::Dir(_) => unreachable!("new_archive always returns Archive variant"), + } Ok(builder) } -} -impl Builder { pub fn add_instance( &mut self, instance: v1::Instance, annotations: InstanceAnnotations, ) -> Result<()> { let blob = instance.encode_to_vec(); - self.0 - .add_layer(media_types::v1_instance(), &blob, annotations.into())?; + match self { + Self::Archive(builder) => { + builder.add_layer(media_types::v1_instance(), &blob, annotations.into())?; + } + Self::Dir(builder) => { + builder.add_layer(media_types::v1_instance(), &blob, annotations.into())?; + } + } Ok(()) } @@ -104,8 +124,14 @@ impl Builder { annotations: SolutionAnnotations, ) -> Result<()> { let blob = solution.encode_to_vec(); - self.0 - .add_layer(media_types::v1_solution(), &blob, annotations.into())?; + match self { + Self::Archive(builder) => { + builder.add_layer(media_types::v1_solution(), &blob, annotations.into())?; + } + Self::Dir(builder) => { + builder.add_layer(media_types::v1_solution(), &blob, annotations.into())?; + } + } Ok(()) } @@ -115,11 +141,22 @@ impl Builder { annotations: ParametricInstanceAnnotations, ) -> Result<()> { let blob = instance.encode_to_vec(); - self.0.add_layer( - media_types::v1_parametric_instance(), - &blob, - annotations.into(), - )?; + match self { + Self::Archive(builder) => { + builder.add_layer( + media_types::v1_parametric_instance(), + &blob, + annotations.into(), + )?; + } + Self::Dir(builder) => { + builder.add_layer( + media_types::v1_parametric_instance(), + &blob, + annotations.into(), + )?; + } + } Ok(()) } @@ -129,19 +166,69 @@ impl Builder { annotations: SampleSetAnnotations, ) -> Result<()> { let blob = sample_set.encode_to_vec(); - self.0 - .add_layer(media_types::v1_sample_set(), &blob, annotations.into())?; + match self { + Self::Archive(builder) => { + builder.add_layer(media_types::v1_sample_set(), &blob, annotations.into())?; + } + Self::Dir(builder) => { + builder.add_layer(media_types::v1_sample_set(), &blob, annotations.into())?; + } + } Ok(()) } pub fn add_config(&mut self, config: Config) -> Result<()> { let blob = serde_json::to_string_pretty(&config)?; - self.0 - .add_config(media_types::v1_config(), blob.as_bytes(), HashMap::new())?; + match self { + Self::Archive(builder) => { + builder.add_config(media_types::v1_config(), blob.as_bytes(), HashMap::new())?; + } + Self::Dir(builder) => { + builder.add_config(media_types::v1_config(), blob.as_bytes(), HashMap::new())?; + } + } Ok(()) } - pub fn build(self) -> Result> { - Artifact::new(self.0.build()?) + /// Add an annotation to the manifest + pub fn add_annotation(&mut self, key: String, value: String) { + match self { + Self::Archive(builder) => { + builder.add_annotation(key, value); + } + Self::Dir(builder) => { + builder.add_annotation(key, value); + } + } + } + + /// Add source URL annotation (convenience method) + pub fn add_source(&mut self, source: &url::Url) { + self.add_annotation( + "org.opencontainers.image.source".to_string(), + source.to_string(), + ); + } + + /// Add description annotation (convenience method) + pub fn add_description(&mut self, description: String) { + self.add_annotation( + "org.opencontainers.image.description".to_string(), + description, + ); + } + + /// Finalise the builder and produce an [`Artifact`] with the same backend variant. + pub fn build(self) -> Result { + match self { + Self::Archive(builder) => { + let artifact = builder.build()?; + Ok(Artifact::Archive(artifact)) + } + Self::Dir(builder) => { + let artifact = builder.build()?; + Ok(Artifact::Dir(artifact)) + } + } } } diff --git a/rust/ommx/src/artifact/io.rs b/rust/ommx/src/artifact/io.rs new file mode 100644 index 000000000..463eff280 --- /dev/null +++ b/rust/ommx/src/artifact/io.rs @@ -0,0 +1,239 @@ +//! I/O operations for Artifact (load, save) + +use super::Artifact; +use crate::artifact::get_local_registry_path; +use anyhow::{anyhow, bail, ensure, Context, Result}; +use ocipkg::{ + image::{ + OciArchive, OciArchiveBuilder, OciArtifact, OciDir, OciDirBuilder, Remote, RemoteBuilder, + }, + ImageName, +}; +use std::{fs, ops::DerefMut, path::Path}; + +impl Artifact { + /// Load an artifact from local registry, pulling from remote if not found + /// + /// This method searches the local registry for the artifact in the following order: + /// 1. Check for `.ommx` archive file + /// 2. Check for OCI directory format + /// 3. If neither exists, pull from remote and save as archive (default format) + /// + /// # Arguments + /// + /// * `image_name` - The image name to load + /// + /// # Returns + /// + /// Returns the loaded artifact, prioritizing archive format when both exist. + pub fn load(image_name: &ImageName) -> Result { + let base_path = get_local_registry_path(image_name); + let archive_path = base_path.with_extension("ommx"); + let dir_path = &base_path; + + // Check for archive format first (preferred) + if archive_path.exists() { + log::debug!("Loading artifact from archive: {}", archive_path.display()); + match Self::from_oci_archive(&archive_path) { + Ok(artifact) => return Ok(artifact), + Err(e) => { + log::warn!( + "Failed to load archive at {}, trying directory format: {}", + archive_path.display(), + e + ); + } + } + } + + // Check for directory format (legacy support) + if dir_path.exists() && dir_path.is_dir() { + log::debug!("Loading artifact from directory: {}", dir_path.display()); + return Self::from_oci_dir(dir_path); + } + + // Neither format exists locally, pull from remote + log::info!("Artifact not found locally, pulling from remote: {image_name}"); + let mut remote_artifact = Self::from_remote(image_name.clone())?; + + // Save to local registry as archive (default format) + remote_artifact.save_as_archive(&archive_path)?; + + // Load the saved archive + Self::from_oci_archive(&archive_path) + } + + /// Save the artifact as an OCI archive file + /// + /// This method converts the artifact to archive format and saves it to the specified path. + /// + /// # Arguments + /// + /// * `path` - The path where the archive should be saved (typically with `.ommx` extension) + pub fn save_as_archive(&mut self, path: &Path) -> Result<()> { + if path.exists() { + bail!("Output file already exists: {}", path.display()); + } + + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).with_context(|| { + format!("Failed to create parent directory: {}", parent.display()) + })?; + } + + if let Self::Remote(remote) = self { + apply_basic_auth(remote); + } + + let image_name = self + .image_name() + .context("Cannot save artifact without image name")?; + let image_name = ImageName::parse(&image_name)?; + + let builder = OciArchiveBuilder::new(path.to_path_buf(), image_name)?; + + match self { + Self::Archive(a) => { + ocipkg::image::copy(a.deref_mut() as &mut OciArchive, builder)?; + } + Self::Dir(a) => { + ocipkg::image::copy(a.deref_mut() as &mut OciDir, builder)?; + } + Self::Remote(a) => { + ocipkg::image::copy(a.deref_mut() as &mut Remote, builder)?; + } + } + + *self = Self::from_oci_archive(path)?; + + Ok(()) + } + + /// Save the artifact as an OCI directory + pub fn save_as_dir(&mut self, path: &Path) -> Result<()> { + if path.exists() { + bail!("Output directory already exists: {}", path.display()); + } + + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).with_context(|| { + format!("Failed to create parent directory: {}", parent.display()) + })?; + } + + if let Self::Remote(remote) = self { + apply_basic_auth(remote); + } + + let image_name = self + .image_name() + .context("Cannot save artifact without image name")?; + let image_name = ImageName::parse(&image_name)?; + + let builder = OciDirBuilder::new(path.to_path_buf(), image_name)?; + + match self { + Self::Archive(a) => { + ocipkg::image::copy(a.deref_mut() as &mut OciArchive, builder)?; + } + Self::Dir(a) => { + ocipkg::image::copy(a.deref_mut() as &mut OciDir, builder)?; + } + Self::Remote(a) => { + ocipkg::image::copy(a.deref_mut() as &mut Remote, builder)?; + } + } + + *self = Self::from_oci_dir(path)?; + + Ok(()) + } + + /// Save the artifact to the local registry (defaults to archive format) + pub fn save(&mut self) -> Result<()> { + let image_name = self + .image_name() + .context("Cannot save artifact without image name")?; + let image_name = ImageName::parse(&image_name)?; + let base_path = get_local_registry_path(&image_name); + let archive_path = base_path.with_extension("ommx"); + + if archive_path.exists() { + log::debug!( + "Archive already exists; reusing: {}", + archive_path.display() + ); + *self = Self::from_oci_archive(&archive_path)?; + return Ok(()); + } + + if let Some(parent) = archive_path.parent() { + fs::create_dir_all(parent).with_context(|| { + format!("Failed to create registry directory: {}", parent.display()) + })?; + } + + self.save_as_archive(&archive_path) + } + + /// Pull the artifact from remote into the local registry (archive format) + pub fn pull(&mut self) -> Result<()> { + ensure!( + matches!(self, Self::Remote(_)), + "pull() is only supported for remote artifacts" + ); + self.save() + } + + /// Push the artifact to the remote registry + pub fn push(&mut self) -> Result<()> { + let image_name = self + .image_name() + .context("Cannot push artifact without image name")?; + let image_name = ImageName::parse(&image_name)?; + + match self { + Self::Archive(archive) => { + push_to_remote(archive.deref_mut(), image_name)?; + } + Self::Dir(dir) => { + push_to_remote(dir.deref_mut(), image_name)?; + } + Self::Remote(_) => { + bail!("Cannot push a Remote artifact without local data"); + } + } + + Ok(()) + } +} + +fn push_to_remote(image: &mut T, image_name: ImageName) -> Result<()> +where + T: ocipkg::image::Image, +{ + let mut builder = RemoteBuilder::new(image_name.clone())?; + if let Ok((domain, username, password)) = auth_from_env() { + builder.add_basic_auth(&domain, &username, &password); + } + ocipkg::image::copy(image, builder)?; + Ok(()) +} + +fn apply_basic_auth(remote: &mut OciArtifact) { + if let Ok((domain, username, password)) = auth_from_env() { + remote.add_basic_auth(&domain, &username, &password); + } +} + +fn auth_from_env() -> Result<(String, String, String)> { + if let (Ok(domain), Ok(username), Ok(password)) = ( + std::env::var("OMMX_BASIC_AUTH_DOMAIN"), + std::env::var("OMMX_BASIC_AUTH_USERNAME"), + std::env::var("OMMX_BASIC_AUTH_PASSWORD"), + ) { + log::info!("Using OMMX basic auth from environment for remote registry access"); + return Ok((domain, username, password)); + } + Err(anyhow!("Basic auth credentials not configured")) +} diff --git a/rust/ommx/src/artifact/layers.rs b/rust/ommx/src/artifact/layers.rs new file mode 100644 index 000000000..d80f98eb4 --- /dev/null +++ b/rust/ommx/src/artifact/layers.rs @@ -0,0 +1,196 @@ +//! Layer access methods for Artifact + +use super::Artifact; +use crate::artifact::{ + media_types, Config, InstanceAnnotations, ParametricInstanceAnnotations, SampleSetAnnotations, + SolutionAnnotations, +}; +use crate::v1; +use anyhow::{bail, ensure, Context, Result}; +use ocipkg::{ + distribution::MediaType, + image::Image, + oci_spec::image::{Descriptor, ImageManifest}, + Digest, +}; +use prost::Message as _; + +impl Artifact { + /// Get the manifest + pub fn get_manifest(&mut self) -> Result { + match self { + Self::Archive(a) => { + let manifest = a.get_manifest()?; + Self::validate_manifest(&manifest)?; + Ok(manifest) + } + Self::Dir(a) => { + let manifest = a.get_manifest()?; + Self::validate_manifest(&manifest)?; + Ok(manifest) + } + Self::Remote(a) => { + let manifest = a.get_manifest()?; + Self::validate_manifest(&manifest)?; + Ok(manifest) + } + } + } + + fn validate_manifest(manifest: &ImageManifest) -> Result<()> { + let ty = manifest + .artifact_type() + .as_ref() + .context("Not an OMMX Artifact")?; + ensure!( + *ty == media_types::v1_artifact(), + "Not an OMMX Artifact: {}", + ty + ); + Ok(()) + } + + /// Get layer descriptors filtered by media type + pub fn get_layer_descriptors(&mut self, media_type: &MediaType) -> Result> { + let manifest = self.get_manifest()?; + Ok(manifest + .layers() + .iter() + .filter(|desc| desc.media_type() == media_type) + .cloned() + .collect()) + } + + /// Get a specific layer by digest + pub fn get_layer(&mut self, digest: &Digest) -> Result<(Descriptor, Vec)> { + let layers = match self { + Self::Archive(a) => a.get_layers()?, + Self::Dir(a) => a.get_layers()?, + Self::Remote(a) => a.get_layers()?, + }; + for (desc, blob) in layers { + if desc.digest() == &digest.to_string() { + return Ok((desc, blob)); + } + } + bail!("Layer of digest {} not found", digest) + } + + /// Get blob by digest + pub fn get_blob(&mut self, digest: &Digest) -> Result> { + match self { + Self::Archive(a) => a.get_blob(digest), + Self::Dir(a) => a.get_blob(digest), + Self::Remote(a) => a.get_blob(digest), + } + } + + /// Get the config + pub fn get_config(&mut self) -> Result { + let (_desc, blob) = match self { + Self::Archive(a) => a.get_config()?, + Self::Dir(a) => a.get_config()?, + Self::Remote(a) => a.get_config()?, + }; + let config = serde_json::from_slice(&blob)?; + Ok(config) + } + + /// Get a solution by digest + pub fn get_solution(&mut self, digest: &Digest) -> Result<(v1::State, SolutionAnnotations)> { + let (desc, blob) = self.get_layer(digest)?; + ensure!( + desc.media_type() == &media_types::v1_solution(), + "Layer {digest} is not an ommx.v1.Solution: {}", + desc.media_type() + ); + Ok(( + v1::State::decode(blob.as_slice())?, + SolutionAnnotations::from_descriptor(&desc), + )) + } + + /// Get a sample set by digest + pub fn get_sample_set( + &mut self, + digest: &Digest, + ) -> Result<(v1::SampleSet, SampleSetAnnotations)> { + let (desc, blob) = self.get_layer(digest)?; + ensure!( + desc.media_type() == &media_types::v1_sample_set(), + "Layer {digest} is not an ommx.v1.SampleSet: {}", + desc.media_type() + ); + Ok(( + v1::SampleSet::decode(blob.as_slice())?, + SampleSetAnnotations::from_descriptor(&desc), + )) + } + + /// Get an instance by digest + pub fn get_instance(&mut self, digest: &Digest) -> Result<(v1::Instance, InstanceAnnotations)> { + let (desc, blob) = self.get_layer(digest)?; + ensure!( + desc.media_type() == &media_types::v1_instance(), + "Layer {digest} is not an ommx.v1.Instance: {}", + desc.media_type() + ); + Ok(( + v1::Instance::decode(blob.as_slice())?, + InstanceAnnotations::from_descriptor(&desc), + )) + } + + /// Get a parametric instance by digest + pub fn get_parametric_instance( + &mut self, + digest: &Digest, + ) -> Result<(v1::ParametricInstance, ParametricInstanceAnnotations)> { + let (desc, blob) = self.get_layer(digest)?; + ensure!( + desc.media_type() == &media_types::v1_parametric_instance(), + "Layer {digest} is not an ommx.v1.ParametricInstance: {}", + desc.media_type() + ); + Ok(( + v1::ParametricInstance::decode(blob.as_slice())?, + ParametricInstanceAnnotations::from_descriptor(&desc), + )) + } + + /// Get all solutions + pub fn get_solutions(&mut self) -> Result> { + let mut out = Vec::new(); + let layers = match self { + Self::Archive(a) => a.get_layers()?, + Self::Dir(a) => a.get_layers()?, + Self::Remote(a) => a.get_layers()?, + }; + for (desc, blob) in layers { + if desc.media_type() != &media_types::v1_solution() { + continue; + } + let solution = v1::State::decode(blob.as_slice())?; + out.push((desc, solution)); + } + Ok(out) + } + + /// Get all instances + pub fn get_instances(&mut self) -> Result> { + let mut out = Vec::new(); + let layers = match self { + Self::Archive(a) => a.get_layers()?, + Self::Dir(a) => a.get_layers()?, + Self::Remote(a) => a.get_layers()?, + }; + for (desc, blob) in layers { + if desc.media_type() != &media_types::v1_instance() { + continue; + } + let instance = v1::Instance::decode(blob.as_slice())?; + out.push((desc, instance)); + } + Ok(out) + } +} diff --git a/rust/ommx/src/artifact/tests.rs b/rust/ommx/src/artifact/tests.rs new file mode 100644 index 000000000..4f4813f14 --- /dev/null +++ b/rust/ommx/src/artifact/tests.rs @@ -0,0 +1,379 @@ +//! Tests for experimental Artifact API + +use super::{Artifact, Builder}; +use crate::artifact::{self, Config}; +use ocipkg::ImageName; +use std::{fs, path::PathBuf, sync::OnceLock}; +use uuid::Uuid; + +fn registry_root() -> PathBuf { + static ROOT: OnceLock = OnceLock::new(); + ROOT.get_or_init(|| { + let candidate = std::env::temp_dir().join(format!("ommx_test_registry_{}", Uuid::new_v4())); + fs::create_dir_all(&candidate).unwrap(); + if artifact::set_local_registry_root(candidate.clone()).is_ok() { + candidate + } else { + let root = artifact::get_local_registry_root().to_path_buf(); + fs::create_dir_all(&root).unwrap(); + root + } + }) + .clone() +} + +fn image_name(label: &str) -> ImageName { + ImageName::parse(&format!("example.com/test/{label}:{}", Uuid::new_v4())).unwrap() +} + +fn image_dir(image_name: &ImageName) -> PathBuf { + registry_root().join(image_name.as_path()) +} + +fn archive_path(image_name: &ImageName) -> PathBuf { + image_dir(image_name).with_extension("ommx") +} + +fn build_dir_artifact(image_name: &ImageName) -> PathBuf { + let dir = image_dir(image_name); + let mut builder = Builder::new_dir(dir.clone(), image_name.clone()).unwrap(); + builder.add_config(Config {}).unwrap(); + builder.build().unwrap(); + dir +} + +fn build_archive_artifact(image_name: &ImageName) -> PathBuf { + let archive = archive_path(image_name); + if let Some(parent) = archive.parent() { + fs::create_dir_all(parent).unwrap(); + } + let mut builder = Builder::new_archive(archive.clone(), image_name.clone()).unwrap(); + builder.add_config(Config {}).unwrap(); + builder.build().unwrap(); + archive +} + +fn cleanup(image_name: &ImageName) { + let archive = archive_path(image_name); + if archive.exists() { + let _ = fs::remove_file(&archive); + } + let dir = image_dir(image_name); + if dir.exists() { + let _ = fs::remove_dir_all(&dir); + } +} + +fn temp_output_dir(label: &str) -> PathBuf { + std::env::temp_dir().join(format!("ommx_test_output_{label}_{}", Uuid::new_v4())) +} + +#[test] +fn test_create_empty_oci_dir_artifact() { + // Create temporary directory + let temp_dir = std::env::temp_dir().join(format!("ommx_test_{}", uuid::Uuid::new_v4())); + fs::create_dir_all(&temp_dir).unwrap(); + + let dir_path = temp_dir.join("empty_artifact"); + + // For this test, create a minimal valid OCI dir structure manually + fs::create_dir_all(&dir_path).unwrap(); + fs::write( + dir_path.join("oci-layout"), + r#"{"imageLayoutVersion":"1.0.0"}"#, + ) + .unwrap(); + + // Create a minimal valid index.json with a manifest + let index_json = r#"{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.index.v1+json", + "manifests": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a", + "size": 200, + "annotations": { + "org.opencontainers.image.ref.name": "example.com/test/empty-dir:v1" + } + } + ] + }"#; + fs::write(dir_path.join("index.json"), index_json).unwrap(); + + // Create blobs directory and a minimal manifest + let blobs_dir = dir_path.join("blobs").join("sha256"); + fs::create_dir_all(&blobs_dir).unwrap(); + + let manifest_json = r#"{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "artifactType": "application/org.ommx.v1.artifact", + "config": { + "mediaType": "application/vnd.oci.image.config.v1+json", + "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a", + "size": 2 + }, + "layers": [] + }"#; + fs::write( + blobs_dir.join("44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a"), + manifest_json, + ) + .unwrap(); + + // Create minimal config blob (note: this overwrites the manifest, but we need unique digests) + // For simplicity, use different digest for config + let config_json = r#"{"schemaVersion":2}"#; + let config_digest = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; // sha256 of empty string + fs::write(blobs_dir.join(config_digest), config_json).unwrap(); + + // Verify directory structure exists + assert!(dir_path.exists(), "OCI directory should exist"); + assert!(dir_path.is_dir(), "Path should be a directory"); + assert!( + dir_path.join("oci-layout").exists(), + "oci-layout file should exist" + ); + assert!( + dir_path.join("index.json").exists(), + "index.json should exist" + ); + + // Test loading with experimental API + let result = Artifact::from_oci_dir(&dir_path); + assert!(result.is_ok(), "Failed to load OCI dir: {:?}", result.err()); + + let artifact = result.unwrap(); + match artifact { + Artifact::Dir(_) => {} // Expected + _ => panic!("Expected Dir variant"), + } + + fs::remove_dir_all(&temp_dir).unwrap(); +} + +#[test] +fn test_create_empty_oci_archive_artifact() { + // Create temporary directory + let temp_dir = std::env::temp_dir().join(format!("ommx_test_{}", uuid::Uuid::new_v4())); + fs::create_dir_all(&temp_dir).unwrap(); + + let image_name = ocipkg::ImageName::parse("example.com/test/empty-archive:v1").unwrap(); + let archive_path = temp_dir.join("empty_artifact.ommx"); + + // Build empty artifact in OCI archive format using experimental Builder + let mut builder = Builder::new_archive(archive_path.clone(), image_name.clone()).unwrap(); + builder.add_config(Config {}).unwrap(); + let _artifact = builder.build().unwrap(); + + // Verify archive file exists + assert!(archive_path.exists(), "OCI archive file should exist"); + assert!(archive_path.is_file(), "Path should be a file"); + assert!( + archive_path.extension().unwrap() == "ommx", + "Should have .ommx extension" + ); + + // Test loading with experimental API + let result = Artifact::from_oci_archive(&archive_path); + assert!( + result.is_ok(), + "Failed to load OCI archive: {:?}", + result.err() + ); + + let artifact = result.unwrap(); + match artifact { + Artifact::Archive(_) => {} // Expected + _ => panic!("Expected Archive variant"), + } + + fs::remove_dir_all(&temp_dir).unwrap(); +} + +#[test] +fn test_load_prefers_archive_when_available() { + let image_name = image_name("load-prefers-archive"); + build_dir_artifact(&image_name); + let _archive = build_archive_artifact(&image_name); + + let artifact = Artifact::load(&image_name).unwrap(); + assert!(matches!(artifact, Artifact::Archive(_))); + + drop(artifact); + cleanup(&image_name); +} + +#[test] +fn test_load_falls_back_to_directory_when_archive_invalid() { + let image_name = image_name("load-fallback"); + let dir = build_dir_artifact(&image_name); + if let Some(parent) = dir.parent() { + fs::create_dir_all(parent).unwrap(); + } + let archive = archive_path(&image_name); + if let Some(parent) = archive.parent() { + fs::create_dir_all(parent).unwrap(); + } + fs::write(&archive, b"invalid archive").unwrap(); + + let artifact = Artifact::load(&image_name).unwrap(); + assert!(matches!(artifact, Artifact::Dir(_))); + + drop(artifact); + cleanup(&image_name); +} + +#[test] +fn test_save_as_archive_from_dir_variant() { + let image_name = image_name("save-as-archive"); + let dir = build_dir_artifact(&image_name); + let archive = archive_path(&image_name); + if archive.exists() { + fs::remove_file(&archive).unwrap(); + } + + let mut artifact = Artifact::from_oci_dir(&dir).unwrap(); + artifact.save_as_archive(&archive).unwrap(); + assert!(archive.exists(), "archive file should be created"); + assert!(Artifact::from_oci_archive(&archive).is_ok()); + assert!(matches!(artifact, Artifact::Archive(_))); + + drop(artifact); + cleanup(&image_name); +} + +#[test] +fn test_save_as_archive_requires_nonexistent_path() { + let image_name = image_name("save-as-archive-clash"); + let dir = build_dir_artifact(&image_name); + let archive = archive_path(&image_name); + if let Some(parent) = archive.parent() { + fs::create_dir_all(parent).unwrap(); + } + fs::write(&archive, b"existing").unwrap(); + + let mut artifact = Artifact::from_oci_dir(&dir).unwrap(); + let err = artifact.save_as_archive(&archive).unwrap_err(); + assert!(err.to_string().contains("Output file already exists")); + + drop(artifact); + cleanup(&image_name); +} + +#[test] +fn test_save_creates_archive_in_registry() { + let image_name = image_name("save-default"); + let dir = build_dir_artifact(&image_name); + let archive = archive_path(&image_name); + if archive.exists() { + fs::remove_file(&archive).unwrap(); + } + + let mut artifact = Artifact::from_oci_dir(&dir).unwrap(); + artifact.save().unwrap(); + + assert!(archive.exists()); + assert!(matches!(artifact, Artifact::Archive(_))); + + drop(artifact); + cleanup(&image_name); +} + +#[test] +fn test_save_reuses_existing_archive() { + let image_name = image_name("save-reuse"); + let archive = build_archive_artifact(&image_name); + let original_len = fs::metadata(&archive).unwrap().len(); + + let mut artifact = Artifact::from_oci_archive(&archive).unwrap(); + artifact.save().unwrap(); + + assert!(matches!(artifact, Artifact::Archive(_))); + assert_eq!(fs::metadata(&archive).unwrap().len(), original_len); + + drop(artifact); + cleanup(&image_name); +} + +#[test] +fn test_save_as_dir_creates_directory() { + let image_name = image_name("save-as-dir"); + let archive = build_archive_artifact(&image_name); + let mut artifact = Artifact::from_oci_archive(&archive).unwrap(); + + let output = temp_output_dir("dir"); + artifact.save_as_dir(&output).unwrap(); + + assert!(output.exists() && output.is_dir()); + assert!(matches!(artifact, Artifact::Dir(_))); + + drop(artifact); + fs::remove_dir_all(&output).unwrap(); + cleanup(&image_name); +} + +#[test] +fn test_pull_requires_remote_variant() { + let image_name = image_name("pull-requires-remote"); + let archive = build_archive_artifact(&image_name); + let mut artifact = Artifact::from_oci_archive(&archive).unwrap(); + + let err = artifact.pull().unwrap_err(); + assert!(err.to_string().contains("pull() is only supported")); + + drop(artifact); + cleanup(&image_name); +} + +#[test] +fn test_annotations() { + let image_name = image_name("annotations"); + let archive = build_archive_artifact(&image_name); + let mut artifact = Artifact::from_oci_archive(&archive).unwrap(); + + let annotations = artifact.annotations().unwrap(); + assert!(annotations.is_empty() || !annotations.is_empty()); // Just verify it returns + + drop(artifact); + cleanup(&image_name); +} + +#[test] +fn test_layers() { + let image_name = image_name("layers"); + let archive = build_archive_artifact(&image_name); + let mut artifact = Artifact::from_oci_archive(&archive).unwrap(); + + let layers = artifact.layers().unwrap(); + assert_eq!(layers.len(), 0); // Empty artifact has no layers + + drop(artifact); + cleanup(&image_name); +} + +#[test] +fn test_builder_add_annotation() { + let image_name = image_name("builder-annotation"); + let archive = archive_path(&image_name); + if let Some(parent) = archive.parent() { + fs::create_dir_all(parent).unwrap(); + } + + let mut builder = Builder::new_archive(archive.clone(), image_name.clone()).unwrap(); + builder.add_config(Config {}).unwrap(); + builder.add_annotation("test.key".to_string(), "test.value".to_string()); + builder.add_annotation("another.key".to_string(), "another.value".to_string()); + let mut artifact = builder.build().unwrap(); + + let annotations = artifact.annotations().unwrap(); + assert_eq!(annotations.get("test.key"), Some(&"test.value".to_string())); + assert_eq!( + annotations.get("another.key"), + Some(&"another.value".to_string()) + ); + + drop(artifact); + cleanup(&image_name); +} diff --git a/rust/ommx/src/bin/ommx.rs b/rust/ommx/src/bin/ommx.rs index b18fca743..117758f0e 100644 --- a/rust/ommx/src/bin/ommx.rs +++ b/rust/ommx/src/bin/ommx.rs @@ -2,7 +2,7 @@ use anyhow::{bail, Result}; use clap::Parser; use colored::Colorize; use ocipkg::{oci_spec::image::ImageManifest, ImageName}; -use ommx::artifact::{get_image_dir, Artifact}; +use ommx::artifact::{get_local_registry_path, Artifact}; use std::path::{Path, PathBuf}; mod built_info { @@ -62,7 +62,13 @@ enum Command { /// List the images in the local registry List, - /// Get the directory where the image is stored + /// Get the base path for an image in the local registry + LocalRegistryPath { + /// Container image name + image_name: String, + }, + + /// Get the directory where the image is stored (deprecated: use local-registry-path instead) ImageDirectory { /// Container image name image_name: String, @@ -86,27 +92,51 @@ impl ImageNameOrPath { return Ok(Self::OciArchive(path.to_path_buf())); } if let Ok(name) = ImageName::parse(input) { - let path = get_image_dir(&name); - if path.exists() { + // Check for both oci-archive and oci-dir formats + let base_path = get_local_registry_path(&name); + let archive_path = base_path.with_extension("ommx"); + + // Priority: oci-archive > oci-dir + if archive_path.exists() && archive_path.is_file() { return Ok(Self::Local(name)); - } else { - return Ok(Self::Remote(name)); } + if base_path.exists() && base_path.is_dir() { + return Ok(Self::Local(name)); + } + return Ok(Self::Remote(name)); } bail!("Invalid input: {}", input) } fn get_manifest(&self) -> Result { let manifest = match self { - ImageNameOrPath::OciDir(path) => Artifact::from_oci_dir(path)?.get_manifest()?, + ImageNameOrPath::OciDir(path) => { + let mut artifact = Artifact::from_oci_dir(path)?; + artifact.get_manifest()? + } ImageNameOrPath::OciArchive(path) => { - Artifact::from_oci_archive(path)?.get_manifest()? + let mut artifact = Artifact::from_oci_archive(path)?; + artifact.get_manifest()? } ImageNameOrPath::Local(name) => { - let image_dir = get_image_dir(name); - Artifact::from_oci_dir(&image_dir)?.get_manifest()? + // Check for both formats, prioritizing oci-archive + let base_path = get_local_registry_path(name); + let archive_path = base_path.with_extension("ommx"); + + if archive_path.exists() && archive_path.is_file() { + let mut artifact = Artifact::from_oci_archive(&archive_path)?; + artifact.get_manifest()? + } else if base_path.exists() && base_path.is_dir() { + let mut artifact = Artifact::from_oci_dir(&base_path)?; + artifact.get_manifest()? + } else { + bail!("Artifact not found in local registry: {}", name) + } + } + ImageNameOrPath::Remote(name) => { + let mut artifact = Artifact::from_remote(name.clone())?; + artifact.get_manifest()? } - ImageNameOrPath::Remote(name) => Artifact::from_remote(name.clone())?.get_manifest()?, }; Ok(manifest) } @@ -168,9 +198,19 @@ fn main() -> Result<()> { artifact.push()?; } ImageNameOrPath::Local(name) => { - let image_dir = get_image_dir(&name); - let mut artifact = Artifact::from_oci_dir(&image_dir)?; - artifact.push()?; + // Check for both formats, prioritizing oci-archive + let base_path = get_local_registry_path(&name); + let archive_path = base_path.with_extension("ommx"); + + if archive_path.exists() && archive_path.is_file() { + let mut artifact = Artifact::from_oci_archive(&archive_path)?; + artifact.push()?; + } else if base_path.exists() && base_path.is_dir() { + let mut artifact = Artifact::from_oci_dir(&base_path)?; + artifact.push()?; + } else { + bail!("Artifact not found in local registry: {}", name) + } } ImageNameOrPath::Remote(name) => { bail!("Image not found in local: {}", name) @@ -180,24 +220,45 @@ fn main() -> Result<()> { Command::Pull { image_name } => { let name = ImageName::parse(image_name)?; let mut artifact = Artifact::from_remote(name)?; - artifact.pull()?; + artifact.pull()?; // pull() saves to local registry automatically } Command::Save { image_name, output } => { let name = ImageName::parse(image_name)?; - let image_dir = get_image_dir(&name); - let mut artifact = Artifact::from_oci_dir(&image_dir)?; - artifact.save(output)?; + // Check for both formats, prioritizing oci-archive + let base_path = get_local_registry_path(&name); + let archive_path = base_path.with_extension("ommx"); + + if archive_path.exists() && archive_path.is_file() { + // Convert oci-archive to oci-archive (copy) + std::fs::copy(&archive_path, output)?; + } else if base_path.exists() && base_path.is_dir() { + let mut artifact = Artifact::from_oci_dir(&base_path)?; + artifact.save_as_archive(output)?; + } else { + bail!("Artifact not found in local registry: {}", name) + } } Command::Load { path } => { let mut artifact = Artifact::from_oci_archive(path)?; - artifact.load()?; + artifact.save()?; // save() saves to local registry + } + + Command::LocalRegistryPath { image_name } => { + let name = ImageName::parse(image_name)?; + let path = get_local_registry_path(&name); + println!("{}", path.display()); } Command::ImageDirectory { image_name } => { + log::warn!( + "The 'image-directory' command is deprecated since 2.1.0. \ + Use 'local-registry-path' instead for dual format support." + ); let name = ImageName::parse(image_name)?; - let path = get_image_dir(&name); + #[allow(deprecated)] + let path = ommx::artifact::get_image_dir(&name); println!("{}", path.display()); } diff --git a/rust/ommx/src/dataset/miplib2017.rs b/rust/ommx/src/dataset/miplib2017.rs index 9dc4860c7..c429019ab 100644 --- a/rust/ommx/src/dataset/miplib2017.rs +++ b/rust/ommx/src/dataset/miplib2017.rs @@ -1,5 +1,5 @@ use crate::{ - artifact::{ghcr, Artifact, InstanceAnnotations}, + artifact::{ghcr, InstanceAnnotations}, v1::Instance, }; @@ -243,6 +243,8 @@ fn check_unsupported(name: &str) -> Result<()> { /// Load an instance from the MIPLIB 2017 dataset pub fn load(name: &str) -> Result<(Instance, InstanceAnnotations)> { + use crate::artifact::Artifact; + let annotations = instance_annotations(); ensure!( annotations.contains_key(name), @@ -251,7 +253,8 @@ pub fn load(name: &str) -> Result<(Instance, InstanceAnnotations)> { check_unsupported(name)?; let image_name = ghcr("Jij-Inc", "ommx", "miplib2017", name)?; - let mut artifact = Artifact::from_remote(image_name)?.pull()?; + let mut artifact = Artifact::from_remote(image_name)?; + artifact.pull()?; let mut instances = artifact.get_instances()?; ensure!( instances.len() == 1, diff --git a/rust/ommx/src/dataset/qplib.rs b/rust/ommx/src/dataset/qplib.rs index 7ac18acc7..ed96a4b2a 100644 --- a/rust/ommx/src/dataset/qplib.rs +++ b/rust/ommx/src/dataset/qplib.rs @@ -1,5 +1,5 @@ use crate::{ - artifact::{ghcr, Artifact, InstanceAnnotations}, + artifact::{ghcr, InstanceAnnotations}, v1::Instance, }; @@ -283,7 +283,8 @@ pub fn load(tag: &str) -> Result<(Instance, InstanceAnnotations)> { ); let image_name = ghcr("Jij-Inc", "ommx", "qplib", tag)?; - let mut artifact = Artifact::from_remote(image_name)?.pull()?; + let mut artifact = crate::artifact::Artifact::from_remote(image_name)?; + artifact.pull()?; let mut instances = artifact.get_instances()?; ensure!( instances.len() == 1, diff --git a/rust/ommx/src/lib.rs b/rust/ommx/src/lib.rs index a240ec2ba..17a26df08 100644 --- a/rust/ommx/src/lib.rs +++ b/rust/ommx/src/lib.rs @@ -260,6 +260,10 @@ pub mod parse; pub mod qplib; pub mod random; +// Experimental modules - API subject to change +// Experimental module has been promoted to stable +// pub mod experimental; + // Internal modules mod atol; mod bound;