From c82efaf1a979719dd3f6217b33bfb82c86079904 Mon Sep 17 00:00:00 2001 From: Vladimir Yakunin Date: Fri, 11 Sep 2026 21:06:39 +0000 Subject: [PATCH] Record the installed positronic revision in an episode, not the working tree's An episode's `writer.git` and a run's `git.positronic` read the git state around the process: the working directory, and the repository that contains `site-packages`. On a rig that runs an installed wheel from a venv inside a checkout, both name that checkout. Every episode then carries the same stale sha, and a recording cannot say which code wrote it. `get_package_git_state` reads the installed distribution's `direct_url.json` (PEP 610). A wheel built from a VCS URL names the commit it was built from. An editable install names the checkout it imports from. Any other install records no revision. `git.current` still records the working directory. Ticket: Positronic-Robotics/internal#1295 #refs --- positronic/dataset/local_dataset.py | 4 +- positronic/dataset/tests/test_episode.py | 24 +++++- positronic/utils/__init__.py | 16 ++-- positronic/utils/git.py | 46 +++++++++- positronic/utils/tests/test_git.py | 103 +++++++++++++++++++++++ 5 files changed, 180 insertions(+), 13 deletions(-) create mode 100644 positronic/utils/tests/test_git.py diff --git a/positronic/dataset/local_dataset.py b/positronic/dataset/local_dataset.py index a3bea7df7..f50ad718f 100644 --- a/positronic/dataset/local_dataset.py +++ b/positronic/dataset/local_dataset.py @@ -17,7 +17,7 @@ import numpy as np import pyarrow.parquet as pq -from positronic.utils.git import get_git_state +from positronic.utils.git import get_package_git_state from positronic.utils.lazy import LazyDict from .dataset import ConcatDataset, Dataset, DatasetWriter @@ -73,7 +73,7 @@ def _cached_env_writer_info() -> dict: info['version'] = importlib_metadata.version('positronic') except Exception: info['version'] = '' - git_state = get_git_state() + git_state = get_package_git_state() if git_state is not None: info['git'] = git_state return info diff --git a/positronic/dataset/tests/test_episode.py b/positronic/dataset/tests/test_episode.py index 1ba823014..2a1b467d1 100644 --- a/positronic/dataset/tests/test_episode.py +++ b/positronic/dataset/tests/test_episode.py @@ -6,9 +6,10 @@ import pytest from positronic.dataset import Episode -from positronic.dataset.local_dataset import UNFINISHED_MARKER, DiskEpisode, DiskEpisodeWriter +from positronic.dataset.local_dataset import UNFINISHED_MARKER, DiskEpisode, DiskEpisodeWriter, _cached_env_writer_info from positronic.dataset.tests.test_video import assert_frames_equal, create_frame from positronic.dataset.transforms.episode import Derive, FromValue, Get, Group, Identity +from positronic.utils.tests.test_git import WHEEL_COMMIT, git_repo, install_as, vcs_wheel def test_episode_writer_and_reader_basic(tmp_path): @@ -126,11 +127,28 @@ def test_episode_meta_written_and_exposed(tmp_path): formatted_size = f'{m["size_mb"]:.2f}' assert isinstance(formatted_size, str) assert formatted_size.replace('.', '', 1).isdigit() - # git info present when running inside a git repo; skip strict assertions otherwise + # git info present when positronic is installed from a checkout or a VCS wheel if 'git' in m['writer']: git = m['writer']['git'] assert isinstance(git, dict) - assert {'commit', 'branch', 'dirty'}.issubset(git.keys()) + assert {'commit', 'dirty'}.issubset(git.keys()) + + +def test_episode_written_by_an_installed_wheel_records_that_wheel_revision(tmp_path, monkeypatch): + cwd_head = git_repo(tmp_path / 'cwd') + monkeypatch.chdir(tmp_path / 'cwd') + install_as(monkeypatch, vcs_wheel()) + _cached_env_writer_info.cache_clear() + try: + with DiskEpisodeWriter(tmp_path / 'ep') as w: + w.append('a', 1, 1000) + finally: + _cached_env_writer_info.cache_clear() + + git = DiskEpisode(tmp_path / 'ep').meta['writer']['git'] + assert git['commit'] == WHEEL_COMMIT + assert git['commit'] != cwd_head + assert git['dirty'] is False def test_episode_writer_marks_unfinished_and_clears_on_close(tmp_path): diff --git a/positronic/utils/__init__.py b/positronic/utils/__init__.py index 684cab8eb..0703aaa34 100644 --- a/positronic/utils/__init__.py +++ b/positronic/utils/__init__.py @@ -15,7 +15,7 @@ from positronic import __file__ as pkg_init_file from positronic.utils.checkpoints import get_latest_checkpoint, list_checkpoints from positronic.utils.frozen_dict import frozen_keys_dict, frozen_view -from positronic.utils.git import get_git_diff, get_git_state +from positronic.utils.git import get_git_diff, get_git_state, get_package_checkout, get_package_git_state # Positronic's public S3 bucket, registered under the name 'PUBLIC' here in a low-level module # every S3 entrypoint imports, so the `s3://PUBLIC@positronic-public/...` URL form resolves to @@ -168,8 +168,10 @@ def run_metadata(patterns: list[str] | None = None, add_git_diff: bool = True, a - python: Python version - platform: Platform string - package_version: Positronic package version (if available) - - git: Git state (commit, branch, dirty flag) - - git_diff: Git diff for uncommitted changes matching patterns + - git.positronic: the installed positronic revision (a wheel names the commit it was built + from; an editable install names its checkout) + - git.current: the git state of the working directory, where it differs + - git.*.diff: Git diff for uncommitted changes matching patterns - environment: Environment information (VIRTUAL_ENV, uv.lock presence, docker info) """ @@ -186,12 +188,12 @@ def run_metadata(patterns: list[str] | None = None, add_git_diff: bool = True, a pkg_dir = Path(pkg_init_file).resolve().parent - # Check if package directory is in a different git repo - pkg_git_state = get_git_state(workdir=pkg_dir) + pkg_git_state = get_package_git_state() if pkg_git_state: metadata['git.positronic'] = pkg_git_state - if add_git_diff: - git_diff = get_git_diff(workdir=pkg_dir, patterns=patterns) + pkg_checkout = get_package_checkout() + if add_git_diff and pkg_checkout is not None: + git_diff = get_git_diff(workdir=pkg_checkout, patterns=patterns) if git_diff: metadata['git.positronic.diff'] = git_diff diff --git a/positronic/utils/git.py b/positronic/utils/git.py index 03c02c352..45bf36fea 100644 --- a/positronic/utils/git.py +++ b/positronic/utils/git.py @@ -5,8 +5,12 @@ information cannot be determined. """ +import json import subprocess +from importlib import metadata as importlib_metadata from pathlib import Path +from urllib.parse import urlparse +from urllib.request import url2pathname def get_git_state(workdir: Path | None = None) -> dict[str, str | bool] | None: @@ -64,4 +68,44 @@ def get_git_diff(workdir: Path | None = None, patterns: list[str] | None = None) return None -__all__ = ['get_git_state', 'get_git_diff'] +def get_package_checkout(distribution: str = 'positronic') -> Path | None: + """Return the checkout an editable install of ``distribution`` imports from, or None. + + A wheel, a PyPI install and a missing distribution all answer None. + """ + direct_url = _direct_url(distribution) + if direct_url is None or not direct_url.get('dir_info', {}).get('editable'): + return None + return Path(url2pathname(urlparse(direct_url['url']).path)) + + +def get_package_git_state(distribution: str = 'positronic') -> dict[str, str | bool] | None: + """Return the git revision of the installed ``distribution``, or None if it has none. + + A wheel built from a VCS URL answers with the commit its ``direct_url.json`` names (PEP 610). + An editable install answers with the state of the checkout it imports from. Any other install + has no revision. The git repository around ``site-packages`` never answers: a venv inside a + checkout would name that checkout, which is not the code in the process. + """ + direct_url = _direct_url(distribution) + if direct_url is None: + return None + vcs = direct_url.get('vcs_info') + if vcs is not None: + state: dict[str, str | bool] = {'commit': vcs['commit_id'], 'dirty': False, 'url': direct_url['url']} + if 'requested_revision' in vcs: + state['requested_revision'] = vcs['requested_revision'] + return state + checkout = get_package_checkout(distribution) + return get_git_state(workdir=checkout) if checkout is not None else None + + +def _direct_url(distribution: str) -> dict | None: + try: + text = importlib_metadata.distribution(distribution).read_text('direct_url.json') + except importlib_metadata.PackageNotFoundError: + return None + return json.loads(text) if text else None + + +__all__ = ['get_git_state', 'get_git_diff', 'get_package_checkout', 'get_package_git_state'] diff --git a/positronic/utils/tests/test_git.py b/positronic/utils/tests/test_git.py new file mode 100644 index 000000000..904e16adb --- /dev/null +++ b/positronic/utils/tests/test_git.py @@ -0,0 +1,103 @@ +import importlib.metadata +import json +import subprocess +from pathlib import Path + +import pytest + +from positronic import utils +from positronic.utils.git import get_git_state, get_package_checkout, get_package_git_state + +WHEEL_COMMIT = '08b08698e11f081c5c8b3f82b44835be3f331a60' + + +class _Distribution: + version = '0.0.0' + + def __init__(self, direct_url: dict | None): + self._direct_url = direct_url + + def read_text(self, filename: str) -> str | None: + assert filename == 'direct_url.json' + return None if self._direct_url is None else json.dumps(self._direct_url) + + +def install_as(monkeypatch, direct_url: dict | None) -> None: + monkeypatch.setattr(importlib.metadata, 'distribution', lambda name: _Distribution(direct_url)) + + +def vcs_wheel() -> dict: + return { + 'url': 'https://github.com/Positronic-Robotics/positronic.git', + 'vcs_info': {'vcs': 'git', 'commit_id': WHEEL_COMMIT, 'requested_revision': 'main'}, + } + + +def git_repo(path: Path) -> str: + path.mkdir(parents=True, exist_ok=True) + env = {'GIT_AUTHOR_NAME': 't', 'GIT_AUTHOR_EMAIL': 't@t', 'GIT_COMMITTER_NAME': 't', 'GIT_COMMITTER_EMAIL': 't@t'} + run = lambda *args: subprocess.run(['git', '-C', str(path), *args], check=True, capture_output=True, env=env) # noqa: E731 + run('init', '-q') + (path / 'f').write_text(str(path)) + run('add', 'f') + run('commit', '-q', '-m', 'init') + state = get_git_state(workdir=path) + assert state is not None + return str(state['commit']) + + +@pytest.fixture +def cwd_repo(tmp_path, monkeypatch) -> str: + """The process runs inside a git checkout that is not the installed positronic.""" + head = git_repo(tmp_path / 'cwd') + monkeypatch.chdir(tmp_path / 'cwd') + return head + + +def test_a_wheel_built_from_a_vcs_url_names_the_commit_it_was_built_from(cwd_repo, monkeypatch): + install_as(monkeypatch, vcs_wheel()) + + state = get_package_git_state() + + assert state is not None + assert state == { + 'commit': WHEEL_COMMIT, + 'dirty': False, + 'url': 'https://github.com/Positronic-Robotics/positronic.git', + 'requested_revision': 'main', + } + assert state['commit'] != cwd_repo + assert get_package_checkout() is None + + +def test_an_editable_install_names_its_checkout_not_the_working_directory(cwd_repo, tmp_path, monkeypatch): + checkout = tmp_path / 'checkout' + head = git_repo(checkout) + install_as(monkeypatch, {'url': checkout.as_uri(), 'dir_info': {'editable': True}}) + + state = get_package_git_state() + + assert state is not None + assert state == get_git_state(workdir=checkout) + assert state['commit'] == head != cwd_repo + assert get_package_checkout() == checkout + + +def test_an_install_with_no_origin_has_no_revision(cwd_repo, monkeypatch): + install_as(monkeypatch, None) + assert get_package_git_state() is None + + install_as(monkeypatch, {'url': 'file:///nowhere', 'archive_info': {}}) + assert get_package_git_state() is None + + +def test_run_metadata_records_the_installed_revision_and_no_diff_for_a_wheel(cwd_repo, monkeypatch): + install_as(monkeypatch, vcs_wheel()) + Path('f').write_text('changed') + + metadata = utils.run_metadata(add_uv_lock=False) + + assert metadata['git.positronic']['commit'] == WHEEL_COMMIT + assert 'git.positronic.diff' not in metadata + assert metadata['git.current']['commit'] == cwd_repo + assert metadata['git.current']['dirty'] is True