chore: bump openjd crate versions - #357
Conversation
| openjd-expr = "0.6.0" | ||
| openjd-model = "0.6.0" | ||
| openjd-sessions = "0.5.5" | ||
| openjd-model = "0.6.1" |
There was a problem hiding this comment.
THIRD-PARTY-LICENSES.txt was not regenerated for this bump. It still records the old versions:
2538:** openjd-model; version 0.6.0 -- https://crates.io/crates/openjd-model
2539:** openjd-sessions; version 0.5.5 -- https://crates.io/crates/openjd-sessions
cargo about renders that section from the workspace Cargo.lock, so the THIRD-PARTY-LICENSES check job in .github/workflows/rust_quality.yml (which runs scripts/check_third_party_licenses.sh) should fail on this diff. Every previous crate bump in this repo updated the file alongside Cargo.lock (e.g. 7222ce3, c6f7805).
Running ./scripts/check_third_party_licenses.sh --update and committing the result should resolve it.
4d9ce6d to
52d7b5f
Compare
| ** openjd-expr; version 0.6.0 -- https://crates.io/crates/openjd-expr | ||
| ** openjd-model; version 0.6.0 -- https://crates.io/crates/openjd-model | ||
| ** openjd-sessions; version 0.5.5 -- https://crates.io/crates/openjd-sessions | ||
| ** openjd-model; version 0.6.1 -- https://crates.io/crates/openjd-model |
There was a problem hiding this comment.
The regenerated file drops two license sections that have nothing to do with this version bump:
** wasi; version 0.11.1+wasi-snapshot-preview1under the Apache-2.0 WITH LLVM-exception text (old lines 819–1049) — the--- LLVM Exceptions to the Apache 2.0 License ----block is now gone from the file entirely.** encoding_rs; version 0.8.35under the BSD-3-Clause text that was harvested from the crate’s source file (the//-commented variant with theGB18030_2022_OVERRIDE_PUAtable, old lines 2639–2699).
Neither crate changed: both are still in Cargo.lock at the same versions (encoding_rs 0.8.35 via encoding_rs_io ← serde-saphyr, wasi 0.11.1 via getrandom 0.2.17/mio), and this diff only touches the openjd-model/openjd-sessions entries. Attribution is not lost (wasi still appears in the plain Apache-2.0 group, encoding_rs in the Apache-2.0 and clean BSD-3-Clause groups), so this looks like cargo about picking a different license variant / license-file source than it did for the committed baseline — i.e. a cargo-about version difference on the machine that ran --update, not a dependency change.
That matters because nothing pins the tool: scripts/check_third_party_licenses.sh just requires cargo-about on PATH, and .github/workflows/rust_quality.yml:204 runs cargo install cargo-about --locked --features cli, which resolves to whatever the latest published version is at job time. --locked pins cargo-about’s own dependencies, not cargo-about itself, so the generated output can drift between the contributor’s machine and CI, and between CI runs over time — producing spurious THIRD-PARTY-LICENSES check failures on diffs that never touched dependencies.
Two things worth confirming before merge:
- That these two deletions are intended and reviewed as a licensing change, rather than incidental fallout — dropping the LLVM-exception text is fine only because
wasiis also offered under plain Apache-2.0. - Consider pinning the tool (
cargo install cargo-about --locked --features cli --version X.Y.Z) in both the workflow and the script’s doc comment so regeneration is reproducible.
Bump openjd-rs crate dependencies to latest published versions: - openjd-model: 0.6.0 -> 0.6.1 - openjd-sessions: 0.5.5 -> 0.5.6 (openjd-expr already pinned at 0.6.0 on mainline.) Signed-off-by: Sean Tang <171081544+seant-aws@users.noreply.github.com>
52d7b5f to
9489d68
Compare
| openjd-expr = "0.6.0" | ||
| openjd-model = "0.6.0" | ||
| openjd-sessions = "0.5.5" | ||
| openjd-model = "0.6.1" |
There was a problem hiding this comment.
This bump pulls in a model-layer validation change that the pure-Python v0 reference does not have, so it silently creates a v0/v1 divergence.
openjd-model 0.6.1 includes openjd-rs#360, whose model half "reject[s] NUL in template-declared environment variable values so openjd check catches it at validation time" (structure.rs). After this bump, decode_job_template through the binding rejects a template whose environments[].variables value contains a YAML-escaped NUL.
The v0 model still accepts it. EnvironmentVariableValueString (src/openjd/model/v2023_09/_model.py:1954) only caps length at 2048, and _validate_variables (:2023) only rejects an empty map -- there is no NUL check anywhere in src/. Grepping for the escaped NUL codepoint across src/ finds only two unrelated hits, in _variable_reference_validation.py and _openjd_rs.pyi.
So the same template now validates under openjd.model v0 and fails under openjd.model._v1. That is exactly the class of thing AGENTS.md:184 (reference parity) and AGENTS.md:188 (known gaps) ask to be tracked, and there is no coverage on either side: no test in test/openjd/model_v1/ exercising a NUL in an environment variable value, and nothing in test/openjd/model_v1/test_known_gaps.py recording the gap. The v0 suite does test NUL rejection for other string types (test/openjd/model_v0/v2023_09/test_strings.py:156 and four other sites), so the omission here reads as unported rather than deliberate.
Note the contrast with the other half of this bump: openjd-rs#364 (EXPR type-name case gating) was ported to Python first, in #350 -- which is this PR's base commit. #360 appears to have no Python counterpart, so the bump lands the Rust half unaccompanied.
Worth deciding which way to close it before merge: port the NUL rejection to the v0 model (matching what #350 did for the case fix), or add an xfail in test_known_gaps.py recording the divergence. Either way a v1 test asserting the new ModelValidationError message and field path would pin the binding's behaviour, per AGENTS.md:178.
| openjd-expr = "0.6.0" | ||
| openjd-model = "0.6.0" | ||
| openjd-sessions = "0.5.5" | ||
| openjd-model = "0.6.1" |
There was a problem hiding this comment.
The v1 parity tests for the EXPR type-name-case rule belong in this PR, and are missing.
openjd-model 0.6.1 is the release that carries openjd-rs#364, so this bump is the commit where the binding's behaviour actually changes on three surfaces: job parameterDefinitions, step taskParameterDefinitions, and environment-template parameterDefinitions. Before it, the binding accepted type: string on a base template with no extensions and rejected type: int even with EXPR; after it, both are gated correctly.
The base commit (#350) landed the matching Python change plus 447 lines of v0 tests:
test/openjd/model_v0/v2023_09/test_parameter_space.py:779TestTaskParameterTypeNameCase-- all four spelling/extension combinations across all five task parameter typestest/openjd/model_v0/v2023_09/test_list_parameters.py:115-- the same four cases for job parameterstest/openjd/model_v0/v2023_09/test_environment_template.py-- the environment-template surface
There is no v1 counterpart for any of them. Grepping test/openjd/model_v1/ for miscased|case-insensitive|case_insensitive|type_name_case returns nothing, and test_known_gaps.py has no entry either. AGENTS.md:184 asks for a v1 equivalent for every v0 reference test; the reason the v1 side was legitimately empty at #350 was that the binding did not yet implement the rule. This PR removes that reason.
That matters more than usual here because the two implementations were wrong in opposite directions on different parameter kinds, and #364 also tightened the fold from to_uppercase to to_ascii_uppercase (so INT spelled with U+0131 dotless-i is no longer accepted). Nothing in this repo currently pins any of that through the binding, so a future regression on either side -- or a drift in the error text, which AGENTS.md:178 asks tests to assert -- would go unnoticed.
Mirroring the three v0 classes into test/openjd/model_v1/, asserting the ModelValidationError message and field path, would close it.
Bump openjd-rs crate dependencies to latest published versions:
openjd-expr already at 0.6.0 on mainline.
How was this change tested?
cargo build --all-targets✅cargo clippy -D warnings✅hatch run test— 5702 passed, 94.25% coverage ✅hatch run lint✅