mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Check root Python script formatting in CI (#25165)
## Why Python files under `scripts/` were not covered by the repository formatting recipe or the CI formatting job, so formatting drift could merge unnoticed. ## What - Add a dedicated `scripts/pyproject.toml` and `scripts/uv.lock` so root-script formatting uses a locked Ruff version. - Extend `just fmt` to format root Python scripts and add `fmt-scripts-check` for CI. - Run `just fmt-scripts-check` from `.github/workflows/ci.yml`, installing `uv` through SHA-pinned `astral-sh/setup-uv` while retaining the `uv` `0.11.3` pin. - Apply Ruff formatting to the root Python scripts, including `scripts/just-shell.py`, and extend `sdk/python/tests/test_artifact_workflow_and_binaries.py` to cover the root formatting recipe. - Update `AGENTS.md` so agents run `just fmt` after code changes anywhere in the repository. ## Validation - Extended the existing Python SDK workflow test to assert that `just fmt` includes root Python scripts.
This commit is contained in:
committed by
GitHub
Unverified
parent
c3cdf3c007
commit
281b416c44
@@ -99,7 +99,9 @@ def resolve_zstd_command(
|
||||
|
||||
|
||||
def write_zip_archive(package_dir: Path, archive_path: Path) -> None:
|
||||
with zipfile.ZipFile(archive_path, "w", compression=zipfile.ZIP_DEFLATED) as archive:
|
||||
with zipfile.ZipFile(
|
||||
archive_path, "w", compression=zipfile.ZIP_DEFLATED
|
||||
) as archive:
|
||||
for path in package_entries(package_dir):
|
||||
relative_path = path.relative_to(package_dir)
|
||||
if path.is_dir():
|
||||
@@ -109,4 +111,7 @@ def write_zip_archive(package_dir: Path, archive_path: Path) -> None:
|
||||
|
||||
|
||||
def package_entries(package_dir: Path) -> list[Path]:
|
||||
return sorted(package_dir.rglob("*"), key=lambda path: path.relative_to(package_dir).as_posix())
|
||||
return sorted(
|
||||
package_dir.rglob("*"),
|
||||
key=lambda path: path.relative_to(package_dir).as_posix(),
|
||||
)
|
||||
|
||||
@@ -44,8 +44,7 @@ def build_source_binaries(
|
||||
variant,
|
||||
build_entrypoint=entrypoint_bin is None,
|
||||
build_bwrap=spec.is_linux and bwrap_bin is None,
|
||||
build_codex_command_runner=spec.is_windows
|
||||
and codex_command_runner_bin is None,
|
||||
build_codex_command_runner=spec.is_windows and codex_command_runner_bin is None,
|
||||
build_codex_windows_sandbox_setup=spec.is_windows
|
||||
and codex_windows_sandbox_setup_bin is None,
|
||||
)
|
||||
@@ -138,7 +137,9 @@ def validate_prebuilt_resource_inputs(
|
||||
)
|
||||
|
||||
|
||||
def resolve_output_path(explicit_path: Path | None, default_path: Path | None) -> Path | None:
|
||||
def resolve_output_path(
|
||||
explicit_path: Path | None, default_path: Path | None
|
||||
) -> Path | None:
|
||||
if explicit_path is not None:
|
||||
return explicit_path.resolve()
|
||||
|
||||
|
||||
@@ -44,8 +44,7 @@ def parse_args() -> argparse.Namespace:
|
||||
type=Path,
|
||||
default=argparse.SUPPRESS,
|
||||
help=(
|
||||
"Output directory to create as the package root. Defaults to a new "
|
||||
"temporary directory."
|
||||
"Output directory to create as the package root. Defaults to a new temporary directory."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
@@ -72,8 +71,7 @@ def parse_args() -> argparse.Namespace:
|
||||
"--cargo-profile",
|
||||
default="dev-small",
|
||||
help=(
|
||||
"Cargo profile for source-built package artifacts. Use release for "
|
||||
"release packages."
|
||||
"Cargo profile for source-built package artifacts. Use release for release packages."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
@@ -169,7 +167,9 @@ def main() -> int:
|
||||
)
|
||||
prepare_package_dir(package_dir, force=args.force)
|
||||
build_package_dir(package_dir, version, variant, spec, inputs)
|
||||
validate_package_dir(package_dir, variant, spec, include_zsh=inputs.zsh_bin is not None)
|
||||
validate_package_dir(
|
||||
package_dir, variant, spec, include_zsh=inputs.zsh_bin is not None
|
||||
)
|
||||
|
||||
for archive_output in args.archive_output:
|
||||
archive_path = archive_output.resolve()
|
||||
|
||||
@@ -17,7 +17,9 @@ LAYOUT_VERSION = 1
|
||||
def prepare_package_dir(package_dir: Path, *, force: bool) -> None:
|
||||
if package_dir.exists():
|
||||
if not package_dir.is_dir():
|
||||
raise RuntimeError(f"Package output exists and is not a directory: {package_dir}")
|
||||
raise RuntimeError(
|
||||
f"Package output exists and is not a directory: {package_dir}"
|
||||
)
|
||||
if any(package_dir.iterdir()):
|
||||
if not force:
|
||||
raise RuntimeError(
|
||||
|
||||
@@ -27,7 +27,9 @@ class SourceBinariesForTargetTest(unittest.TestCase):
|
||||
[],
|
||||
)
|
||||
|
||||
def test_linux_package_with_prebuilt_entrypoint_and_bwrap_builds_nothing(self) -> None:
|
||||
def test_linux_package_with_prebuilt_entrypoint_and_bwrap_builds_nothing(
|
||||
self,
|
||||
) -> None:
|
||||
self.assertEqual(
|
||||
source_binaries_for_target(
|
||||
TARGET_SPECS["x86_64-unknown-linux-musl"],
|
||||
@@ -40,7 +42,9 @@ class SourceBinariesForTargetTest(unittest.TestCase):
|
||||
[],
|
||||
)
|
||||
|
||||
def test_windows_package_with_prebuilt_entrypoint_and_helpers_builds_nothing(self) -> None:
|
||||
def test_windows_package_with_prebuilt_entrypoint_and_helpers_builds_nothing(
|
||||
self,
|
||||
) -> None:
|
||||
self.assertEqual(
|
||||
source_binaries_for_target(
|
||||
TARGET_SPECS["x86_64-pc-windows-msvc"],
|
||||
|
||||
+18
-11
@@ -43,8 +43,7 @@ def resolve_codex_v8_cargo_env(
|
||||
return {}
|
||||
if archive_override or binding_override:
|
||||
raise RuntimeError(
|
||||
"Cargo package builds need RUSTY_V8_ARCHIVE and "
|
||||
"RUSTY_V8_SRC_BINDING_PATH set together."
|
||||
"Cargo package builds need RUSTY_V8_ARCHIVE and RUSTY_V8_SRC_BINDING_PATH set together."
|
||||
)
|
||||
|
||||
artifacts = fetch_codex_v8_artifacts(spec, cache_root=cache_root)
|
||||
@@ -61,12 +60,13 @@ def fetch_codex_v8_artifacts(
|
||||
cache_root: Path | None = None,
|
||||
) -> RustyV8ArtifactPair:
|
||||
if spec.is_windows:
|
||||
raise RuntimeError(f"No Codex-built V8 release artifacts for target: {spec.target}")
|
||||
raise RuntimeError(
|
||||
f"No Codex-built V8 release artifacts for target: {spec.target}"
|
||||
)
|
||||
|
||||
version = version or resolved_v8_crate_version()
|
||||
release_url = (
|
||||
"https://github.com/openai/codex/releases/download/"
|
||||
f"rusty-v8-v{version}"
|
||||
f"https://github.com/openai/codex/releases/download/rusty-v8-v{version}"
|
||||
)
|
||||
target = spec.target
|
||||
cache_dir = (cache_root or default_cache_root()) / f"rusty-v8-{version}-{target}"
|
||||
@@ -98,7 +98,9 @@ def resolved_v8_crate_version() -> str:
|
||||
}
|
||||
)
|
||||
if len(versions) != 1:
|
||||
raise RuntimeError(f"Expected exactly one resolved v8 version, found: {versions}")
|
||||
raise RuntimeError(
|
||||
f"Expected exactly one resolved v8 version, found: {versions}"
|
||||
)
|
||||
return versions[0]
|
||||
|
||||
|
||||
@@ -111,18 +113,21 @@ def load_checksums(checksums_path: Path, artifact_names: set[str]) -> dict[str,
|
||||
lines = checksums_path.read_text(encoding="utf-8").splitlines()
|
||||
if len(lines) != len(artifact_names):
|
||||
raise RuntimeError(
|
||||
f"Expected {len(artifact_names)} V8 checksums in {checksums_path}, "
|
||||
f"found {len(lines)}."
|
||||
f"Expected {len(artifact_names)} V8 checksums in {checksums_path}, found {len(lines)}."
|
||||
)
|
||||
|
||||
for line in lines:
|
||||
parts = line.split(maxsplit=1)
|
||||
if len(parts) != 2:
|
||||
raise RuntimeError(f"Invalid V8 checksum line in {checksums_path}: {line!r}")
|
||||
raise RuntimeError(
|
||||
f"Invalid V8 checksum line in {checksums_path}: {line!r}"
|
||||
)
|
||||
|
||||
digest, artifact_name = parts[0], parts[1].strip()
|
||||
if len(digest) != 64 or any(char not in "0123456789abcdef" for char in digest):
|
||||
raise RuntimeError(f"Invalid V8 checksum digest in {checksums_path}: {digest}")
|
||||
raise RuntimeError(
|
||||
f"Invalid V8 checksum digest in {checksums_path}: {digest}"
|
||||
)
|
||||
if artifact_name not in artifact_names:
|
||||
raise RuntimeError(
|
||||
f"Unexpected V8 checksum artifact in {checksums_path}: {artifact_name}"
|
||||
@@ -146,7 +151,9 @@ def ensure_valid_artifact(artifact: Path, checksum: str, url: str) -> None:
|
||||
return
|
||||
|
||||
artifact.unlink(missing_ok=True)
|
||||
raise RuntimeError(f"Codex-built V8 artifact {artifact} failed checksum validation.")
|
||||
raise RuntimeError(
|
||||
f"Codex-built V8 artifact {artifact} failed checksum validation."
|
||||
)
|
||||
|
||||
|
||||
def has_checksum(path: Path, expected: str) -> bool:
|
||||
|
||||
Reference in New Issue
Block a user