From 4a25180f342cffb7ba350c15379fb3dc2297b8eb Mon Sep 17 00:00:00 2001 From: snkmcb <88454117+snkmcb@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:56:51 +0900 Subject: [PATCH 1/4] Run the BVH path from an installed product, which refuses every file WORKSPACE.md section 5 ships the motion profiles as product data for one stated reason: a converter with no profile available refuses every file it is given, which would make an artifact-only smoke of the BVH path impossible to pass. That has been a requirement without a test since v0.7.0. ost 0.22.3 supplied the missing mechanism ([[workspace.install_data]], data_files: 3) and report 36 section 4 said in as many words that the staging was what had been proven and not the run. This is the run, and it fails. The script packages the aggregate product, verifies it, installs it to a fresh prefix outside this repository, and drives motion_bvh_convert there with no --profile-dir and no USDVRM_MOTION_PROFILE_PATH. The profiles arrive byte-identically at share/usd-vrm-plugins/profiles/motion/ and the converter refuses the capture anyway. Two checks are worth naming. Every shipped profile is compared byte for byte against profiles/motion/, because the failure that shape replaces was a copy that had stopped being the file scripts/check_motion_profiles.py validates. And once the conversion succeeds the installed profile is moved aside and the same command re-run, which must now refuse -- so "the tool found a profile" cannot pass for "the tool found the one this product ships". It runs in release.yml beside the clean-install smoke, against the archive that lane just proved digest-reproducible rather than a fresh package of its own. That places it in the one workflow no pull request event runs, which is a standing caveat of that lane rather than a new one. Co-Authored-By: Claude Opus 5 --- .github/workflows/release.yml | 19 ++ scripts/artifact_only_bvh_smoke.py | 360 +++++++++++++++++++++++++++++ 2 files changed, 379 insertions(+) create mode 100644 scripts/artifact_only_bvh_smoke.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b484069..8867e83 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -309,6 +309,25 @@ jobs: - name: Verify the packaged artifacts, composed (clean-install smoke) shell: bash run: python scripts/clean_install_smoke.py --skip-build + # The other half of "the artifact works": the clean-install smoke opens + # avatars through the four bundles, and nothing until 2026-08-30 had run a + # *tool* member out of an installed product. WORKSPACE.md §5 ships the + # motion profiles as product data so that this can pass, and the first run + # of it failed -- the tool lands at `tools//bin/` and looked for + # `/../share/...`, which is one directory too shallow inside the + # product's own prefix. + # + # `--product` rather than a fresh package: the archive this step is about + # is the one the previous steps just built and proved digest-reproducible, + # and re-packaging here would smoke a different artifact than the one the + # release stages. + - name: Verify the packaged BVH path from the product alone + shell: bash + run: | + set -euo pipefail + product="$(jq -r '.data.product.archive' .ost-ci/package.json)" + if command -v cygpath > /dev/null; then product="$(cygpath -u "$product")"; fi + python scripts/artifact_only_bvh_smoke.py --product "$(dirname "$product")" - name: Verify the packaged VRMA reader independently shell: bash run: ost plugin test plugins/usdVrmaFileFormat --from-package --target ${{ matrix.platform }} --profile ${{ matrix.profile }} --up-to 5 --json diff --git a/scripts/artifact_only_bvh_smoke.py b/scripts/artifact_only_bvh_smoke.py new file mode 100644 index 0000000..1b53364 --- /dev/null +++ b/scripts/artifact_only_bvh_smoke.py @@ -0,0 +1,360 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: Apache-2.0 +"""Drive the BVH path from the *installed product* and nothing else. + +WORKSPACE.md §5 ships the motion profiles as product data — +`share/usd-vrm-plugins/profiles/motion/` — for one stated reason: *a converter +with no profile available refuses every file it is given, which would make an +artifact-only smoke test of the BVH path impossible to pass.* That sentence has +been a requirement without a test since v0.7.0. `ost` 0.22.3 supplied the +missing half of the mechanism (`[[workspace.install_data]]`, product manifest +`data_files: 3`), and report 36 §4 said in as many words that **the staging is +what was proven and not the run**. This is the run. + + ost plugin package --workspace --product -> the product dist + ost plugin product verify -> archive + every member checksum + ost plugin product install --prefix -> a fresh prefix outside the repo + /tools/motion_bvh/bin/motion_bvh_convert --profile + +What makes it an *artifact-only* run rather than another way of running the +tool: + +* the prefix is created outside the repository and the script refuses one + inside it, so nothing in this source tree can answer a lookup; +* `--profile-dir` is never passed and `USDVRM_MOTION_PROFILE_PATH` is removed + from the environment, so the profile is found by the tool's own installed + layout or it is not found at all; +* the only environment the run is given is the runtime `ost env` prints and the + activation the product itself declares in `openstrata.activation.json` — + which is what the install contract tells a consumer to use; +* the profile that answered is then *proved* to be the installed one: the file + is moved aside, the same command is re-run, and it must refuse. + +The last of those is the check worth keeping. A converter that found a profile +somewhere else on the host would pass every other assertion here, and "which +profile was used" is the one reproducible fact a conversion carries. + +Exit codes follow `clean_install_smoke.py`: 0 pass, 1 the smoke ran and an +assertion failed, 2 the harness itself is misconfigured. + +Usage: + python scripts/artifact_only_bvh_smoke.py # package, then run + python scripts/artifact_only_bvh_smoke.py --product dist/products/... + python scripts/artifact_only_bvh_smoke.py --keep +""" +from __future__ import annotations + +import argparse +import json +import os +import pathlib +import re +import shutil +import subprocess +import sys +import tempfile +from typing import NoReturn + +REPO_ROOT = pathlib.Path(__file__).resolve().parents[1] + +# The tool member and the executable inside it, as `openstrata.product.json` +# names them. Spelled out rather than discovered: a script that scanned the +# prefix for something convert-shaped would pass on the day the product stopped +# carrying this one. +TOOL_MEMBER = "motion_bvh" +TOOL_EXECUTABLE = "motion_bvh_convert" + +# Where the product's data must land. This is the contract WORKSPACE.md §5 +# states, `openstrata.toml`'s `[[workspace.install_data]]` maps to, and +# `ProfileLocator.cpp` derives from the executable's own path — three places +# that have to agree, which is why the destination is written here as a literal +# and compared rather than read out of the manifest. +PROFILE_DESTINATION = ("share", "usd-vrm-plugins", "profiles", "motion") + +# The committed export and the profile written from it, named for the reason +# `test_motion_bvh_convert.py` names them: a smoke that scanned a directory +# would pass on the day the file it is about stopped being there. +DEFAULT_BVH = ("libs/motionBvh/tests/corpus/recorded/redistributable/" + "mocopi-mobile-arm-raise-turn.bvh") +DEFAULT_PROFILE_ID = "mocopi-mobile-bvh-default-v1" + + +class Failures: + def __init__(self) -> None: + self.messages: list[str] = [] + + def check(self, condition: bool, message: str) -> bool: + if not condition: + self.messages.append(message) + return condition + + def report(self) -> int: + if not self.messages: + return 0 + for message in self.messages: + print(f"FAIL: {message}", file=sys.stderr) + return 1 + + +def run(cmd: list[str], **kw) -> subprocess.CompletedProcess: + print(f"$ {' '.join(cmd)}", flush=True) + return subprocess.run(cmd, check=True, text=True, **kw) + + +def ost_json(cmd: list[str]) -> dict: + proc = run(cmd, capture_output=True) + return json.loads(proc.stdout) + + +def fail_setup(msg: str) -> NoReturn: + """Exit 2 — the harness is misconfigured, which is not the same answer as + the product failing its own smoke.""" + print(f"SETUP: {msg}", file=sys.stderr) + raise SystemExit(2) + + +def workspace_target() -> tuple[str, str]: + """The platform and profile `openstrata.toml` requires. + + Read with a regex rather than a TOML parser so the script runs under any + Python this repository's lanes have, and read from the manifest rather than + passed in so the smoke cannot be pointed at a runtime the product was not + built against. + """ + text = (REPO_ROOT / "openstrata.toml").read_text(encoding="utf-8") + requires = re.search(r"^\[requires\]\s*$(.*?)(?=^\[|\Z)", text, + re.MULTILINE | re.DOTALL) + if requires is None: + fail_setup("openstrata.toml has no [requires] table") + body = requires.group(1) + values = {} + for key in ("platform", "profile"): + found = re.search(rf'^\s*{key}\s*=\s*"([^"]+)"', body, re.MULTILINE) + if found is None: + fail_setup(f"openstrata.toml [requires] has no {key}") + values[key] = found.group(1) + return values["platform"], values["profile"] + + +def package_product(ost: str, platform: str, profile: str) -> pathlib.Path: + """Package the workspace's aggregate product and return its dist directory. + + The tree must already be built — this is the packaging half only, exactly as + `release.yml` runs it after `ost build` and the per-bundle builds. + """ + result = ost_json([ost, "plugin", "package", "--workspace", "--product", + "--target", platform, "--profile", profile, "--json"]) + product = result["data"].get("product") + if not product: + fail_setup("`ost plugin package --workspace --product` reported no " + "product") + archive = pathlib.Path(product["archive"]) + print(f"packaged product {product['name']} {product.get('version', '')} " + f"-> {product['archive_digest']}") + return archive.parent + + +def apply_runtime_env(env: dict, ost: str, platform: str, + profile: str) -> None: + """The runtime the product was built against, as `ost env` reports it. + + Every entry is a prepend: `ost env` emits one row per path element, and a + row that replaced the caller's value would take `PATH` away from the + subprocess on Windows, where the C runtime lives on it. + """ + result = ost_json([ost, "env", platform, "--profile", profile, "--json"]) + for entry in result["data"]["env"]: + name, value = entry["name"], entry["value"] + existing = env.get(name) + env[name] = f"{value}{os.pathsep}{existing}" if existing else value + + +def apply_product_activation(env: dict, prefix: pathlib.Path) -> None: + """The product's own activation, from the file the install contract names. + + Read rather than reconstructed: the file says which variable each list of + paths belongs to, and on a POSIX host the loader variable is not `PATH`. + """ + activation_path = prefix / "openstrata.activation.json" + if not activation_path.is_file(): + fail_setup(f"the install wrote no activation file at {activation_path}") + activation = json.loads(activation_path.read_text(encoding="utf-8")) + variables = activation.get("environment", {}) + for key, paths_key in (("loader", "library_paths"), + ("plugin", "plugin_paths"), + ("python", "python_paths")): + name = variables.get(key) + if not name: + continue + values = [str(prefix / relative) + for relative in activation.get(paths_key, [])] + if not values: + continue + joined = os.pathsep.join(values) + existing = env.get(name) + env[name] = f"{joined}{os.pathsep}{existing}" if existing else joined + + +def check_profiles_installed(failures: Failures, + prefix: pathlib.Path) -> pathlib.Path: + """Every shipped profile reaches the prefix, byte for byte. + + Byte for byte rather than merely present, because the failure this replaces + was a *copy* that stopped being the file `scripts/check_motion_profiles.py` + validates (report 35 §4). A check that only counted files would have passed + against it. + """ + installed = prefix.joinpath(*PROFILE_DESTINATION) + source = REPO_ROOT / "profiles" / "motion" + if not failures.check(installed.is_dir(), + f"the product installed no {'/'.join(PROFILE_DESTINATION)}"): + return installed + for authored in sorted(source.glob("*.yaml")): + shipped = installed / authored.name + if not failures.check(shipped.is_file(), + f"{authored.name} did not reach the prefix"): + continue + failures.check(shipped.read_bytes() == authored.read_bytes(), + f"{authored.name} in the prefix is not the file " + f"profiles/motion/ holds") + return installed + + +def run_converter(tool: pathlib.Path, env: dict, bvh: pathlib.Path, + profile_id: str, + output: pathlib.Path) -> subprocess.CompletedProcess: + """The one command this whole script exists to run. + + No `--profile-dir`. The caller has already removed + `USDVRM_MOTION_PROFILE_PATH` from `env`, so the two directories left in the + search order are both derived from this executable's own location — and the + executable is inside the prefix, which is outside the repository. + """ + cmd = [str(tool), str(bvh), "--profile", profile_id, + "--output", str(output)] + print(f"$ {' '.join(cmd)}", flush=True) + return subprocess.run(cmd, env=env, text=True, capture_output=True) + + +def check_the_run(failures: Failures, tool: pathlib.Path, env: dict, + bvh: pathlib.Path, profile_id: str, + installed_profiles: pathlib.Path, + work: pathlib.Path) -> None: + output = work / "artifact-only-clip.usda" + result = run_converter(tool, env, bvh, profile_id, output) + if not failures.check( + result.returncode == 0, + f"the installed converter exited {result.returncode} with no " + f"--profile-dir. This is the artifact-only condition failing:\n" + f"{result.stdout}{result.stderr}"): + return + + if not failures.check(output.is_file(), + f"the converter reported success and wrote no " + f"{output.name}"): + return + clip = output.read_text(encoding="utf-8") + failures.check(profile_id in clip, + f"the clip does not record profileId {profile_id!r}") + failures.check("SkelAnimation" in clip, + "the clip carries no SkelAnimation") + print(result.stdout, end="") + + # And the negative: the file that answered was the installed one. + # + # Without this the smoke would pass on a host that happens to have a + # profile of the same id somewhere the search order reaches, which is the + # one way "it found its profile" can be true and mean nothing. + shipped = installed_profiles / f"{profile_id}.yaml" + hidden = installed_profiles / f"{profile_id}.yaml.hidden" + if not failures.check(shipped.is_file(), + f"{shipped} is missing, so the negative check cannot " + f"run"): + return + shipped.rename(hidden) + try: + result = run_converter(tool, env, bvh, profile_id, + work / "must-not-exist.usda") + failures.check( + result.returncode == 2, + f"with the installed profile moved aside the converter exited " + f"{result.returncode}, so the profile it read the first time was " + f"not the one this product ships") + finally: + hidden.rename(shipped) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--product", default=None, + help="an existing product dist directory, manifest.json " + "or .tar.zst; default packages the workspace") + parser.add_argument("--bvh", default=DEFAULT_BVH, + help="the recorded export to convert") + parser.add_argument("--profile-id", default=DEFAULT_PROFILE_ID, + help="the profile id the product must supply") + parser.add_argument("--keep", action="store_true", + help="keep the installed prefix for inspection") + parser.add_argument("--ost", default="ost", help="ost executable") + args = parser.parse_args() + + platform, profile = workspace_target() + bvh = (REPO_ROOT / args.bvh).resolve() + if not bvh.is_file(): + fail_setup(f"no BVH file at {bvh}") + + ost = args.ost + if args.product is None: + product = package_product(ost, platform, profile) + else: + product = pathlib.Path(args.product).resolve() + if not product.exists(): + fail_setup(f"no product at {product}") + + # Verify before install, in that order and both of them, because the + # install contract in `openstrata.product.json` names both and a smoke that + # skipped the first would be testing a different contract than the one a + # consumer is told to follow. + run([ost, "plugin", "product", "verify", str(product)]) + + scratch = pathlib.Path(tempfile.mkdtemp(prefix="vrm-artifact-only-")) + prefix = scratch / "prefix" + if REPO_ROOT in prefix.resolve().parents or prefix.resolve() == REPO_ROOT: + fail_setup(f"the install prefix is inside the repository: {prefix}") + + failures = Failures() + try: + run([ost, "plugin", "product", "install", "--prefix", str(prefix), + str(product)]) + + installed_profiles = check_profiles_installed(failures, prefix) + + suffix = ".exe" if os.name == "nt" else "" + tool = prefix / "tools" / TOOL_MEMBER / "bin" / (TOOL_EXECUTABLE + suffix) + if not failures.check( + tool.is_file(), + f"the product installed no {TOOL_EXECUTABLE} at {tool}"): + return failures.report() + + env = dict(os.environ) + # The whole point of the run. Its presence would make the smoke pass + # against a product that ships no profile at all. + env.pop("USDVRM_MOTION_PROFILE_PATH", None) + apply_runtime_env(env, ost, platform, profile) + apply_product_activation(env, prefix) + + check_the_run(failures, tool, env, bvh, args.profile_id, + installed_profiles, scratch) + finally: + if args.keep: + print(f"kept the installed prefix at: {prefix}") + else: + shutil.rmtree(scratch, ignore_errors=True) + + rc = failures.report() + print("artifact-only BVH smoke: " + ("PASS" if rc == 0 else "FAIL")) + return rc + + +if __name__ == "__main__": + sys.exit(main()) From 2f9a44e19c22de845d13eaaa6403207a4ab0708f Mon Sep 17 00:00:00 2001 From: snkmcb <88454117+snkmcb@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:56:59 +0900 Subject: [PATCH 2/4] Find the profile a product installs beside the tool that needs it motion_bvh_convert derives its profile directory from its own executable path, and its installed-prefix rule was /../share/usd-vrm-plugins/profiles/motion. That is correct for a `cmake --install` prefix and for a member archive unpacked on its own, and one directory too shallow for the aggregate product: `ost plugin product install` lands a tool member at /tools//bin/ while the product's own data goes to /share/. A converter that finds no profile refuses every file it is given, so the whole BVH path was unusable from a release artifact. Two installed layouts put the data in the same place relative to the prefix and the tool at different depths inside it, so one executable-relative rule can only ever serve one of them. The locator now carries both, and the repository rule that already climbed three directories is expressed in terms of the same prefix rather than repeating the arithmetic. None of this moves the destination. It is stated in openstrata.toml, in the root CMakeLists.txt, in WORKSPACE.md section 5 and in ProfileLocator.h, all four agree, and one of them was describing a different prefix -- which is the argument for the smoke rather than for more review. Co-Authored-By: Claude Opus 5 --- tools/motionBvh/src/ProfileLocator.cpp | 15 ++++++--- tools/motionBvh/src/ProfileLocator.h | 46 ++++++++++++++++++++------ 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/tools/motionBvh/src/ProfileLocator.cpp b/tools/motionBvh/src/ProfileLocator.cpp index c6af872..a30e226 100644 --- a/tools/motionBvh/src/ProfileLocator.cpp +++ b/tools/motionBvh/src/ProfileLocator.cpp @@ -135,13 +135,20 @@ ProfileSearchPath(const std::vector& extraDirs) const std::filesystem::path executableDir = ExecutableDirectory(); if (!executableDir.empty()) { + // /bin/ -> /share/... : a `cmake --install` + // prefix, and a member archive unpacked on its own. directories.push_back(executableDir.parent_path() / "share" / "usd-vrm-plugins" / "profiles" / "motion"); + // /tools//bin/ -> /share/... : an + // installed product. The two installed layouts agree about where the + // data is relative to the prefix and disagree about how deep the tool + // sits inside it, so each needs its own rule. + const std::filesystem::path prefixFromToolMember = + executableDir.parent_path().parent_path().parent_path(); + directories.push_back(prefixFromToolMember / "share" + / "usd-vrm-plugins" / "profiles" / "motion"); // tools//bin/ -> the repository root's profiles/motion. - directories.push_back(executableDir.parent_path() - .parent_path() - .parent_path() - / "profiles" / "motion"); + directories.push_back(prefixFromToolMember / "profiles" / "motion"); } return directories; } diff --git a/tools/motionBvh/src/ProfileLocator.h b/tools/motionBvh/src/ProfileLocator.h index 1742722..b2fd82a 100644 --- a/tools/motionBvh/src/ProfileLocator.h +++ b/tools/motionBvh/src/ProfileLocator.h @@ -37,22 +37,46 @@ // // 1. every `--profile-dir`, in the order given // 2. `USDVRM_MOTION_PROFILE_PATH`, a list in the platform's PATH separator -// 3. `/../share/usd-vrm-plugins/profiles/motion` — an install prefix -// 4. `/../../../profiles/motion` — this repository, whose tools stage +// 3. `/../share/usd-vrm-plugins/profiles/motion` — a `cmake --install` +// prefix, where the tools land in `/bin/` +// 4. `/../../../share/usd-vrm-plugins/profiles/motion` — an installed +// **product**, where `ost plugin product install` lands a tool member in +// `/tools//bin/` and the product's own data in +// `/share/` +// 5. `/../../../profiles/motion` — this repository, whose tools stage // their executables in `tools//bin/` // -// `share` in the third is literal on every platform, and that is the contract -// rather than an assumption about GNUInstallDirs: WORKSPACE.md §5 names -// `share/usd-vrm-plugins/profiles/motion/`, and both install rules that place -// these files spell it the same way for the same reason. A lookup that followed -// a configurable data directory while the packager's rule followed another would -// leave the converter finding nothing — and finding nothing means refusing every -// file it is given, which is a failure that looks like a broken build rather -// than like a misplaced directory. +// **The fourth is what an artifact-only run needs, and it was missing until +// 2026-08-30.** Both installed layouts put the profiles at the same place +// relative to the *prefix* and the tool at a different depth inside it, so one +// executable-relative rule can only ever serve one of them. The third was +// written against a member archive unpacked on its own — `/bin/` +// beside `/share/` — and WORKSPACE.md §5 recorded from that measurement +// that "the layout was agreed and only the staging was missing". The staging +// then arrived (`ost` 0.22.3's `[[workspace.install_data]]`), the first run +// from an installed product was performed, and the agreement turned out to be +// with a layout the product does not use. Which is the whole argument for +// writing the smoke rather than reasoning about the destination: this file +// named `/share/usd-vrm-plugins/profiles/motion` correctly and looked +// somewhere else. // -// The fourth is a convenience and is stated rather than hidden: a build tree +// `share` in the third and the fourth is literal on every platform, and that is +// the contract rather than an assumption about GNUInstallDirs: WORKSPACE.md §5 +// names `share/usd-vrm-plugins/profiles/motion/`, and every install rule that +// places these files spells it the same way for the same reason. A lookup that +// followed a configurable data directory while the packager's rule followed +// another would leave the converter finding nothing — and finding nothing means +// refusing every file it is given, which is a failure that looks like a broken +// build rather than like a misplaced directory. +// +// The fifth is a convenience and is stated rather than hidden: a build tree // that found no profile would send whoever ran it looking for a packaging bug // that is not there. +// +// The fourth and the fifth both climb three directories, so from a `bin/` +// directly under a prefix they reach *above* it. That is not new exposure — +// the fifth has always done it — and it costs nothing here, because the third +// is tried first and a correct `cmake --install` prefix answers there. #pragma once #include From 92332e2d272e921914338c5ab4689905a4f77501 Mon Sep 17 00:00:00 2001 From: snkmcb <88454117+snkmcb@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:57:08 +0900 Subject: [PATCH 3/4] Record what the artifact-only run measured, and where it has not run The passing run converts the committed 17-second mocopi export from the prefix alone: 853 frames at 50 Hz, 22 of 27 joints bound. WORKSPACE.md section 5 said "the layout was agreed and only the staging was missing". The agreement was real and with a different layout -- it was measured on a member archive unpacked on its own -- so the paragraph is replaced with what the run found rather than amended. The BVH track's item closes on two fixes in different layers, and the sequence is the point: the packaging ask was real, closing it was not sufficient, and only running the thing distinguished the two. RELEASE_NOTES_TEMPLATE.md still told a reader that no member carries data and to pass --profile a path. That has been stale since ost 0.22.3 and is now wrong: an installed product carries the profiles and the tool finds them with no flag. The member-archive route still needs the flag, and the template now separates the two. The v0.7.0 condition "both paths running from release artifacts alone" moves to in-progress rather than done: the recorded path runs, and the live path still needs the product composed with mocopi_record's own artifact. What is not claimed: this has been measured on a Windows workstation only. The smoke is a release.yml step, so the first Linux and macOS runs happen at a tag, and current.md's before-the-tag list now says so. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 43 +++++++++++++++++++ docs/architecture/WORKSPACE.md | 33 ++++++++++++--- docs/contributing/RELEASE_NOTES_TEMPLATE.md | 11 +++-- docs/roadmap/current.md | 47 +++++++++++++++------ docs/roadmap/recorded-motion-sources.md | 25 +++++++---- 5 files changed, 125 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6556930..d3e681b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,26 @@ Current schema contract version: **1**. ### Fixed +- **A tool member of the installed product could not find the product's own + data.** `motion_bvh_convert` derives its profile directory from its own + executable path, and its installed-prefix rule was + `/../share/usd-vrm-plugins/profiles/motion` — correct for a `cmake + --install` prefix and for a member archive unpacked on its own, and one + directory too shallow for the aggregate product, where `ost plugin product + install` lands a tool member at `/tools//bin/` while the + product's data goes to `/share/`. A converter that finds no profile + refuses every file it is given, so the whole BVH path was unusable from a + release artifact: the profiles shipped, byte-identical, to the directory + WORKSPACE.md §5 names, and the tool beside them looked somewhere else. The + locator now carries both installed layouts. + + **It was found by running it, and nothing else could have found it.** The + destination is stated in `openstrata.toml`, in the root `CMakeLists.txt`, in + WORKSPACE.md §5 and in `ProfileLocator.h`, all four agree, and one of them was + describing a different prefix. `ost` 0.22.3 supplied the staging in August and + report 36 §4 recorded in as many words that the staging was what had been + proven and not the run; this is the run, and it failed the first time. + - **A rotation too small to square came back un-normalised, in the two conversions that had not been fixed.** `GfQuatf::GetLength()` squares in float, and both `vrmAdapterVmc::ToCanonicalRotation` and @@ -130,6 +150,29 @@ Current schema contract version: **1**. ### Added +- **An artifact-only smoke for the BVH path** + (`scripts/artifact_only_bvh_smoke.py`), which closes the v0.7.0 release + condition *both paths running from release artifacts alone, profiles + included* for the recorded half. It packages the aggregate product, runs + `ost plugin product verify`, installs it to a fresh prefix **outside** this + repository, and converts a real 17-second mocopi export there — 853 frames at + 50 Hz through 22 bound joints — with no `--profile-dir`, no + `USDVRM_MOTION_PROFILE_PATH`, and nothing from this source tree on any search + path. + + Two of its checks are the ones worth naming. Every shipped profile is compared + **byte for byte** against `profiles/motion/`, because the failure that shape + replaces was a *copy* that had stopped being the file + `scripts/check_motion_profiles.py` validates. And after the conversion + succeeds the installed profile is moved aside and the same command is re-run, + which must now refuse — so "the tool found a profile" cannot pass for "the + tool found the one this product ships". + + It runs in `release.yml` beside the clean-install smoke, against the archive + that lane just proved digest-reproducible rather than a fresh package of its + own. That places it in the one workflow no PR event runs, which is a standing + caveat of that lane and not a new one. + - **`osc`, the OSC wire format once instead of once per adapter.** Packets, bundles and their flattening, addresses, type tags, arguments, and a refusal that names the byte and the address it refused at. It knows no address diff --git a/docs/architecture/WORKSPACE.md b/docs/architecture/WORKSPACE.md index f5511ae..721d0c9 100644 --- a/docs/architecture/WORKSPACE.md +++ b/docs/architecture/WORKSPACE.md @@ -727,13 +727,32 @@ rather than copied under any member root. The file that ships is the file `scripts/check_motion_profiles.py` validates, which is what `directories: [bin, share]` could not promise. -**What is proven is the staging, not yet the run.** The product archive carries -the profiles; nothing in this repository has yet extracted that product to a -prefix and driven `motion_bvh_convert` from it, so the artifact-only smoke this -paragraph exists to make possible is now *possible* rather than *passing*. That -distinction is the one to keep: v0.7.0 recorded the condition unmet because the -mechanism was missing, and the mechanism arriving is not the same thing as the -test existing +**The run happened on 2026-08-30 and it failed, which is why the distinction +above was worth keeping.** `scripts/artifact_only_bvh_smoke.py` packages the +product, verifies it, installs it to a prefix outside this repository, and +drives `motion_bvh_convert` there with no `--profile-dir` and no +`USDVRM_MOTION_PROFILE_PATH`. The profiles arrived exactly where this section +says — byte-identical to `profiles/motion/` — and the converter refused the +capture anyway, because `ost plugin product install` lands a tool member at +`/tools//bin/` and the locator looked at +`/../share/usd-vrm-plugins/profiles/motion`, one directory too shallow +inside the product's own prefix. + +The paragraph this replaces recorded that "the layout was agreed and only the +staging was missing", and the agreement was real but with a *different* layout: +it was measured on a member archive unpacked on its own, where the executable +does sit at `/bin/`. Two installed layouts put the data in the same place +relative to the prefix and the tool at different depths inside it, so an +executable-relative rule serves one of them at a time. The locator now carries +both, the smoke passes — 853 frames at 50 Hz, 22 bound joints, from the artifact +alone — and it proves the profile it read was the installed one by moving that +file aside and requiring the refusal to come back. + +**None of this changes the destination**, which is the part worth stating: the +contract in this section was right, `[[workspace.install_data]]` puts the files +there, and the defect was one reader of it. That is the argument for the smoke +rather than for more review — the search path was documented, the destination +was documented, the two were written from each other, and they still disagreed ([report 36](../reports/ost/36-2026-08-25-v0.22.3-canonical-runtimes-and-release-membership.md) §4). That split is the one to check when a future reader arrives: a reader is in the diff --git a/docs/contributing/RELEASE_NOTES_TEMPLATE.md b/docs/contributing/RELEASE_NOTES_TEMPLATE.md index 6f941a5..d4c83ac 100644 --- a/docs/contributing/RELEASE_NOTES_TEMPLATE.md +++ b/docs/contributing/RELEASE_NOTES_TEMPLATE.md @@ -50,10 +50,13 @@ VRMA motion clips as canonical `UsdSkelAnimation` data. > §5 keeps optional network and SDK dependencies out of the core distribution — > and they build from the source archive. > -> **No member carries data.** `motion_bvh_convert` needs a producer profile from -> `share/usd-vrm-plugins/profiles/motion/`, which reaches no archive, so run it -> from a `cmake --install` prefix or pass `--profile` a path. The profiles ship -> in the source archive. +> **The product carries the motion profiles.** `motion_bvh_convert` needs a +> producer profile, and `ost plugin product install --prefix ` puts the +> shipped ones at `/share/usd-vrm-plugins/profiles/motion/` where the +> tool finds them with no flag — verified per release by +> `scripts/artifact_only_bvh_smoke.py`. No *member* archive carries them: a +> `motion_bvh` archive unpacked on its own is its two executables, so that route +> still needs a `cmake --install` prefix or a `--profile` path. Each binary bundle carries a `buildInfo.json` stamp in its resources (git commit / build OS / compiler / build type / OpenUSD version / schema diff --git a/docs/roadmap/current.md b/docs/roadmap/current.md index f35f24d..9fca4b4 100644 --- a/docs/roadmap/current.md +++ b/docs/roadmap/current.md @@ -193,6 +193,10 @@ checks are listed rather than remembered: - [ ] the CHANGELOG names the **architecture** changes, not only the features: two shared libraries extracted, a third adapter, and every adapter's package config gaining a dependency it was missing; +- [ ] the artifact-only BVH smoke is green on **Linux and macOS**, not only on + the workstation that wrote it — it is a `release.yml` step, so a tag is + the first time those two cells run it, and the `workflow_dispatch --ref` + dry run below is what turns that from a surprise into a measurement; - [ ] `scripts/check_docs.py`, `check_motion_profiles.py` and `verify_corpus.py` are green, and `release.yml` is dry-run with `workflow_dispatch --ref` before the tag — a green PR lane proves nothing about it. @@ -228,13 +232,25 @@ checks are listed rather than remembered: adapter artifact in CI, so whether a release carries them is open ([report 36](../reports/ost/36-2026-08-25-v0.22.3-canonical-runtimes-and-release-membership.md) §2, §3). -- ⬜ **The profiles reach the product; the smoke that would prove it does not - exist.** Both halves of the staging are closed — the plain-CMake install - (2026-08-05) and the packaged one (2026-08-25, `ost` 0.22.3's - `[[workspace.install_data]]`, `data_files: 3`). **What remains is the test**: - nothing has extracted the product to a prefix and driven `motion_bvh_convert` - from it, so the entry stays open on a written smoke rather than on a missing - mechanism +- ✅ **The profiles reach the product, and the smoke that proves it found a + defect** *(2026-08-30: `scripts/artifact_only_bvh_smoke.py`)*. Both halves of + the staging were already closed — the plain-CMake install (2026-08-05) and the + packaged one (2026-08-25, `ost` 0.22.3's `[[workspace.install_data]]`, + `data_files: 3`) — and this entry stayed open on the test rather than on a + mechanism. **The first run of that test failed.** The profiles installed + byte-identically to `share/usd-vrm-plugins/profiles/motion/` and + `motion_bvh_convert` refused the capture anyway: `ost plugin product install` + lands a tool member at `/tools//bin/`, and the locator's + installed-prefix rule was `/../share/…`, which is the layout of a *member + archive* unpacked on its own. The fix is one more search-path rule in + [ProfileLocator.cpp](../../tools/motionBvh/src/ProfileLocator.cpp); the run is + 853 frames at 50 Hz through 22 bound joints, from the artifact alone, and it + ends by moving the installed profile aside and requiring the refusal to come + back — so "it found *a* profile" cannot pass for "it found the one this + product ships". Wired into `release.yml` beside the clean-install smoke, on + all three of that lane's cells — which means it inherits that lane's standing + caveat: **no PR event runs it**, so the only host it has been measured on is a + Windows workstation, and the first Linux and macOS runs happen at a tag ([report 36](../reports/ost/36-2026-08-25-v0.22.3-canonical-runtimes-and-release-membership.md) §4). ### Carried out of v0.7.0 — evidence an operator produces @@ -258,13 +274,16 @@ None of these closes by writing code, and each is stated with what it costs. with hashes, every measured statistic and no bytes — a session is a real person's motion and a skeleton packet is a body measurement of that person. Getting a publishable one needs the vendor's `BVH Sender`, not a device. -- ⬜ **Both paths running from release artifacts alone, profiles included.** - No longer blocked on the toolchain. Every member the two paths need is in the - product, and as of `ost` 0.22.3 so are the profiles. `mocopi_record` is not — - it left the aggregate by declaration when the exclusion stopped being a - version pin — so this run composes the product with the adapter's own artifact, - which `ost library package` can now produce. It stays open because the run has - not been performed. +- 🚧 **Both paths running from release artifacts alone, profiles included.** + **The recorded path is done** (2026-08-30): `motion_bvh_convert` converts a + real mocopi export from an installed product prefix with nothing from this + source tree on any search path, and the profiles it uses are the product's own + — see the packaging entry above. What is left is the **live** path, and it is + left for the reason it always was rather than a new one: `mocopi_record` is + not in the aggregate — it left by declaration when the exclusion stopped being + a version pin — so that run composes the product with the adapter's own + artifact, which `ost library package` can now produce, and nothing has + performed it. ### Still Motion Phase G diff --git a/docs/roadmap/recorded-motion-sources.md b/docs/roadmap/recorded-motion-sources.md index ba8b446..78f0906 100644 --- a/docs/roadmap/recorded-motion-sources.md +++ b/docs/roadmap/recorded-motion-sources.md @@ -587,22 +587,29 @@ Still open: characterisation, so a later rule has to change that test before it changes the behaviour, and `motion_retarget` names the bone on stderr rather than losing it in silence. -- ⬜ **The profiles reach a packaged product; the artifact-only smoke does not - exist.** Both halves of the staging are closed — the root project installs - `profiles/motion/*.yaml` to `share/usd-vrm-plugins/profiles/motion/` - (2026-08-05, verified against a scratch prefix), and `ost` 0.22.3's - `[[workspace.install_data]]` gives the mapping a product-level owner, with the - aggregate reporting `data_files: 3` +- ✅ **The profiles reach a packaged product and the artifact-only smoke runs it** + *(2026-08-30, `scripts/artifact_only_bvh_smoke.py`)*. Both halves of the + staging closed first — the root project installs `profiles/motion/*.yaml` to + `share/usd-vrm-plugins/profiles/motion/` (2026-08-05, verified against a + scratch prefix), and `ost` 0.22.3's `[[workspace.install_data]]` gives the + mapping a product-level owner, with the aggregate reporting `data_files: 3` ([report 36](../reports/ost/36-2026-08-25-v0.22.3-canonical-runtimes-and-release-membership.md) §4). - Through v0.7.0 it did not: a `motion_bvh` archive was its two executables and + Through v0.7.0 neither did: a `motion_bvh` archive was its two executables and its descriptor, so a converter unpacked from a product found nothing on its executable-relative search path and refused every file it was given — the specific consequence [WORKSPACE.md §5](../architecture/WORKSPACE.md) put the profiles beside the tools to prevent, and an `ost` ask rather than something a `--profile-dir` flag closes, because "works if you pass a flag naming a directory the artifact does not contain" is not an artifact-only smoke. - **What stops BVH-3 closing is now only that nobody has extracted the product - and run the converter from it.** + + **The smoke then failed for a second reason, which was ours.** The data + arrived byte-identically and the converter still refused: an installed product + puts a tool member at `/tools//bin/`, and the locator's + installed-prefix rule assumed `/bin/`. So this entry closes on two + fixes in different layers, and the sequence is the point — the packaging ask + was real, closing it was not sufficient, and only running the thing + distinguished the two. The passing run converts the committed 17-second mocopi + export from the prefix alone: 853 frames at 50 Hz, 22 of 27 joints bound. Two measured facts worth keeping, because both are easy to assume the other way: From 4f15f78de937b5126bd8b7ba0c219dff9cb8b14f Mon Sep 17 00:00:00 2001 From: snkmcb <88454117+snkmcb@users.noreply.github.com> Date: Sun, 30 Aug 2026 12:14:41 +0900 Subject: [PATCH 4/4] Refuse a profile that is not inside the prefix the tool was installed to Five review findings, and the first is a defect the fix introduced. Both three-parent search rules -- the installed-product one added here and the repository one it was written beside, which had always been unguarded -- were offered from any executable location. From a `cmake --install` prefix, /bin/, they climb two levels above the prefix, which is where a sibling install of this product puts its own share/. Neither CMake rule that places the profiles is unconditional, so a prefix without its own copy is reachable, and the outcome is not a refusal: it is a conversion reading some other prefix's profile. That is the near-miss the no-default-profile rule exists to prevent, arriving through the search path instead of through a flag. Reproduced before fixing: an executable at a/b/prefix/bin/ with a profile only at a/share/... converted 853 frames instead of refusing. Both rules are now offered only when the executable really is in a tools//bin/, and the same reproduction now refuses and lists one directory. The `cmake --install` layout and the product layout both still resolve. The other four: * the negative check in the smoke -- its headline assertion -- tested only `returncode == 2`, which convert_main.cpp returns for every refusal about the command. A foreign same-id profile that was malformed would have satisfied it and meant the opposite. Measured: such a profile exits 2 with "expected a closing ']'" and no "was found", so the message is what discriminates, and the check now requires it. * the byte-for-byte profile comparison passed having compared zero files if the glob matched nothing. It now fails on the empty list, verified by calling it against an empty authored directory. * release.yml read .data.product.archive without `// empty`, so a missing key would print "null", dirname would give ".", and the smoke would be aimed at the repository root. Same `// empty` plus guard the staging step below already has. * tools/motionBvh/README.md still documented the four-entry search order, and check_docs.py does not reach it. Also corrects WORKSPACE.md's account of the v0.7.0 measurement: it named /share/... as the first directory searched, which was true of a member archive unpacked on its own and read as a general claim. Co-Authored-By: Claude Opus 5 --- .github/workflows/release.yml | 10 ++++++- CHANGELOG.md | 11 ++++++++ docs/architecture/WORKSPACE.md | 10 ++++--- scripts/artifact_only_bvh_smoke.py | 25 +++++++++++++++--- tools/motionBvh/README.md | 30 +++++++++++++++------ tools/motionBvh/src/ProfileLocator.cpp | 36 +++++++++++++++++++------- tools/motionBvh/src/ProfileLocator.h | 26 ++++++++++++++++--- 7 files changed, 119 insertions(+), 29 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8867e83..0e73b37 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -325,8 +325,16 @@ jobs: shell: bash run: | set -euo pipefail - product="$(jq -r '.data.product.archive' .ost-ci/package.json)" + product="$(jq -r '.data.product.archive // empty' .ost-ci/package.json)" if command -v cygpath > /dev/null; then product="$(cygpath -u "$product")"; fi + # `// empty` and this guard together, for the reason the staging step + # below has the same pair: a bare `jq -r` on a missing key prints the + # string "null", `dirname` turns that into ".", and the smoke would be + # pointed at the repository root and fail with a message about the + # wrong thing. + if [ -z "$product" ] || [ ! -f "$product" ]; then + echo "::error title=release packaging::workspace product archive is missing" ; exit 1 + fi python scripts/artifact_only_bvh_smoke.py --product "$(dirname "$product")" - name: Verify the packaged VRMA reader independently shell: bash diff --git a/CHANGELOG.md b/CHANGELOG.md index d3e681b..888571a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,17 @@ Current schema contract version: **1**. WORKSPACE.md §5 names, and the tool beside them looked somewhere else. The locator now carries both installed layouts. + Both three-parent rules — the new one and the repository one beside it, which + had always been unguarded — are now offered **only when the executable really + is in a `tools//bin/`**. From a `cmake --install` prefix they would + otherwise climb two levels *above* it, which is where a sibling install of + this product puts its own `share/`, and the result is not a refusal but a + conversion reading another prefix's profile. That is the near-miss the + no-default-profile rule exists to prevent, arriving through the search path + instead of through a flag. Caught in review before either rule shipped, and + reproduced first: an executable at `a/b/prefix/bin/` with a profile only at + `a/share/…` converted instead of refusing. + **It was found by running it, and nothing else could have found it.** The destination is stated in `openstrata.toml`, in the root `CMakeLists.txt`, in WORKSPACE.md §5 and in `ProfileLocator.h`, all four agree, and one of them was diff --git a/docs/architecture/WORKSPACE.md b/docs/architecture/WORKSPACE.md index 721d0c9..e2ed4f3 100644 --- a/docs/architecture/WORKSPACE.md +++ b/docs/architecture/WORKSPACE.md @@ -702,9 +702,13 @@ smoke test of the BVH path impossible to pass. --install` met it.** A packaged product did not: `ost` packaged a tool member out of the `directories:` its descriptor declared, had no notion of a data-only member, and the measured `motion_bvh` archive was exactly its two executables -and its descriptor. Unpacked and run, the converter refused a real capture and -named `/share/usd-vrm-plugins/profiles/motion` as the first directory it -looked in — so the layout was agreed and only the staging was missing. Declaring +and its descriptor. Unpacked and run — a *member* archive, on its own, so the +executable sat at `/bin/` — the converter refused a real capture and +named `/share/usd-vrm-plugins/profiles/motion` as the first directory it +looked in, which read at the time as the layout being agreed and only the +staging being missing. The qualification is added in hindsight and the next +paragraph is why: that is one of two installed layouts, and the tool searched +the product's first only after 2026-08-30. Declaring `directories: [bin, share]` did stage it, and was rejected: `directories:` names subdirectories of the *member root*, so it would have put the layer's data inside one tool's directory and the copy that shipped would have stopped being the file diff --git a/scripts/artifact_only_bvh_smoke.py b/scripts/artifact_only_bvh_smoke.py index 1b53364..d0b0d90 100644 --- a/scripts/artifact_only_bvh_smoke.py +++ b/scripts/artifact_only_bvh_smoke.py @@ -209,7 +209,15 @@ def check_profiles_installed(failures: Failures, if not failures.check(installed.is_dir(), f"the product installed no {'/'.join(PROFILE_DESTINATION)}"): return installed - for authored in sorted(source.glob("*.yaml")): + authored_profiles = sorted(source.glob("*.yaml")) + # A comparison over an empty list passes having compared nothing, which is + # the shape of green this whole script exists to distrust. + if not failures.check( + bool(authored_profiles), + f"{source} holds no *.yaml, so the byte comparison below would " + f"check nothing and report success"): + return installed + for authored in authored_profiles: shipped = installed / authored.name if not failures.check(shipped.is_file(), f"{authored.name} did not reach the prefix"): @@ -275,11 +283,20 @@ def check_the_run(failures: Failures, tool: pathlib.Path, env: dict, try: result = run_converter(tool, env, bvh, profile_id, work / "must-not-exist.usda") + # Exit 2 alone is not enough. `convert_main.cpp` returns it for every + # refusal about the command or something it named -- a profile that + # will not parse, a profile whose id disagrees with the request, an + # output that cannot be created. So a *foreign* profile of this id that + # happened to be malformed would satisfy an exit-code-only check and + # tell us the opposite of what this check is for. The refusal has to be + # the one that says the file is not there. + not_found = f"no profile '{profile_id}' was found" failures.check( - result.returncode == 2, + result.returncode == 2 and not_found in result.stderr, f"with the installed profile moved aside the converter exited " - f"{result.returncode}, so the profile it read the first time was " - f"not the one this product ships") + f"{result.returncode} and did not report {not_found!r}, so the " + f"profile it read the first time was not the one this product " + f"ships:\n{result.stdout}{result.stderr}") finally: hidden.rename(shipped) diff --git a/tools/motionBvh/README.md b/tools/motionBvh/README.md index 5d94bc9..0d38a2b 100644 --- a/tools/motionBvh/README.md +++ b/tools/motionBvh/README.md @@ -110,14 +110,28 @@ first hit wins: 1. every `--profile-dir`, in the order given 2. `USDVRM_MOTION_PROFILE_PATH`, a list in the platform's PATH separator -3. `/../share/usd-vrm-plugins/profiles/motion` — an install prefix -4. `/../../../profiles/motion` — this repository - -The third is why a packaged artifact works with no flags at all: the profiles -ship beside the tools, and a converter with none available refuses every file it -is given. A request that *is* a path is opened as given. The difference matters -in one more place: when you name an id, the file's own `id` must match it, so a -profile renamed on disk cannot make a conversion record an id it never read. +3. `/../share/usd-vrm-plugins/profiles/motion` — a `cmake --install` + prefix, where the tools land in `/bin/` +4. `/../../../share/usd-vrm-plugins/profiles/motion` — an installed + product, where `ost plugin product install` puts a tool member in + `/tools//bin/` and the product's data in `/share/` +5. `/../../../profiles/motion` — this repository + +The last two are offered only when the executable really is in a +`tools//bin/`; from anywhere else they would climb past a prefix +entirely, and finding a *different* prefix's profile is worse than finding +none. + +The third and the fourth are why an artifact works with no flags at all: the +profiles ship beside the tools, and a converter with none available refuses +every file it is given. Which of the two answers depends on how the artifact was +installed, and `scripts/artifact_only_bvh_smoke.py` is what keeps the product +half honest — it installs the product to a clean prefix and converts a real +export there with neither the flag nor the environment variable set. + +A request that *is* a path is opened as given. The difference matters in one +more place: when you name an id, the file's own `id` must match it, so a profile +renamed on disk cannot make a conversion record an id it never read. ### Report and exit status diff --git a/tools/motionBvh/src/ProfileLocator.cpp b/tools/motionBvh/src/ProfileLocator.cpp index a30e226..406396e 100644 --- a/tools/motionBvh/src/ProfileLocator.cpp +++ b/tools/motionBvh/src/ProfileLocator.cpp @@ -140,15 +140,33 @@ ProfileSearchPath(const std::vector& extraDirs) directories.push_back(executableDir.parent_path() / "share" / "usd-vrm-plugins" / "profiles" / "motion"); // /tools//bin/ -> /share/... : an - // installed product. The two installed layouts agree about where the - // data is relative to the prefix and disagree about how deep the tool - // sits inside it, so each needs its own rule. - const std::filesystem::path prefixFromToolMember = - executableDir.parent_path().parent_path().parent_path(); - directories.push_back(prefixFromToolMember / "share" - / "usd-vrm-plugins" / "profiles" / "motion"); - // tools//bin/ -> the repository root's profiles/motion. - directories.push_back(prefixFromToolMember / "profiles" / "motion"); + // installed product, and this repository's own build tree. The two + // installed layouts agree about where the data is relative to the + // prefix and disagree about how deep the tool sits inside it, so each + // needs its own rule. + // + // **Both of these are gated on the tool actually being inside a + // `tools//bin/`**, and the guard is load-bearing rather than + // tidiness. Climbing three parents unguarded from a `cmake --install` + // prefix — `/bin/` — lands two levels *above* the prefix, + // which is where a sibling install of this product puts its own + // `share/`. The result would not be a refusal: it would be a + // conversion reading some other prefix's profile, which is the + // "subtly misassembled rather than absent" outcome this whole path is + // shaped to prevent. Found by review on 2026-08-30, before either rule + // had shipped, and reproduced: an executable at `a/b/prefix/bin/` with + // a profile only at `a/share/...` converted instead of refusing. + const std::filesystem::path memberDir = executableDir.parent_path(); + const std::filesystem::path toolsDir = memberDir.parent_path(); + if (executableDir.filename() == "bin" && toolsDir.filename() == "tools" + && !memberDir.filename().empty()) { + const std::filesystem::path prefix = toolsDir.parent_path(); + directories.push_back(prefix / "share" / "usd-vrm-plugins" + / "profiles" / "motion"); + // tools//bin/ -> the repository root's + // profiles/motion. + directories.push_back(prefix / "profiles" / "motion"); + } } return directories; } diff --git a/tools/motionBvh/src/ProfileLocator.h b/tools/motionBvh/src/ProfileLocator.h index b2fd82a..8a0e9e5 100644 --- a/tools/motionBvh/src/ProfileLocator.h +++ b/tools/motionBvh/src/ProfileLocator.h @@ -46,6 +46,10 @@ // 5. `/../../../profiles/motion` — this repository, whose tools stage // their executables in `tools//bin/` // +// The last two are offered **only when the executable really is in a +// `tools//bin/`**, and both are absent from the search path — and from +// the refusal's list — when it is not. +// // **The fourth is what an artifact-only run needs, and it was missing until // 2026-08-30.** Both installed layouts put the profiles at the same place // relative to the *prefix* and the tool at a different depth inside it, so one @@ -73,10 +77,24 @@ // that found no profile would send whoever ran it looking for a packaging bug // that is not there. // -// The fourth and the fifth both climb three directories, so from a `bin/` -// directly under a prefix they reach *above* it. That is not new exposure — -// the fifth has always done it — and it costs nothing here, because the third -// is tried first and a correct `cmake --install` prefix answers there. +// **That condition is why the fourth and the fifth are guarded rather than +// unconditional.** Both climb three directories, so from a `bin/` directly +// under a prefix — the `cmake --install` layout — they reach two levels +// *above* it, which is exactly where a sibling install of this product puts +// its own `share/`. Saying "the third is tried first and a correct prefix +// answers there" is not an answer: a prefix installed without its own +// profiles is reachable, because both CMake rules that place them are guarded +// on `profiles/motion` existing. What that costs is not a refusal but a +// conversion reading another prefix's profile — a near-miss producing motion +// that is *subtly misassembled rather than absent*, which is the outcome the +// no-default-profile rule exists to prevent, arriving through the search path +// instead of through a flag. The guard was added on review, before either rule +// shipped, after reproducing it: an executable at `a/b/prefix/bin/` with a +// profile only at `a/share/…` converted instead of refusing. +// +// It tightens the fifth as well, which had always climbed unguarded. In this +// repository the executable is at `tools//bin/` and the rule is +// unchanged; outside one it now declines to guess. #pragma once #include