Skip to content

chore: bump openjd crate versions - #357

Merged
seant-aws merged 1 commit into
OpenJobDescription:mainlinefrom
seant-aws:bump-openjd-crates-0.6
Sep 9, 2026
Merged

chore: bump openjd crate versions#357
seant-aws merged 1 commit into
OpenJobDescription:mainlinefrom
seant-aws:bump-openjd-crates-0.6

Conversation

@seant-aws

Copy link
Copy Markdown
Contributor

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 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

@seant-aws
seant-aws marked this pull request as ready for review September 9, 2026 21:43
@seant-aws
seant-aws requested a review from a team as a code owner September 9, 2026 21:43
Comment thread rust-bindings/Cargo.toml
openjd-expr = "0.6.0"
openjd-model = "0.6.0"
openjd-sessions = "0.5.5"
openjd-model = "0.6.1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

leongdl
leongdl previously approved these changes Sep 9, 2026
Comment thread THIRD-PARTY-LICENSES.txt
** 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regenerated file drops two license sections that have nothing to do with this version bump:

  • ** wasi; version 0.11.1+wasi-snapshot-preview1 under 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.35 under the BSD-3-Clause text that was harvested from the crate’s source file (the //-commented variant with the GB18030_2022_OVERRIDE_PUA table, 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_ioserde-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:

  1. 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 wasi is also offered under plain Apache-2.0.
  2. 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>
@seant-aws
seant-aws force-pushed the bump-openjd-crates-0.6 branch from 52d7b5f to 9489d68 Compare September 9, 2026 22:15
Comment thread rust-bindings/Cargo.toml
openjd-expr = "0.6.0"
openjd-model = "0.6.0"
openjd-sessions = "0.5.5"
openjd-model = "0.6.1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread rust-bindings/Cargo.toml
openjd-expr = "0.6.0"
openjd-model = "0.6.0"
openjd-sessions = "0.5.5"
openjd-model = "0.6.1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:779 TestTaskParameterTypeNameCase -- all four spelling/extension combinations across all five task parameter types
  • test/openjd/model_v0/v2023_09/test_list_parameters.py:115 -- the same four cases for job parameters
  • test/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.

@seant-aws
seant-aws merged commit 1696154 into OpenJobDescription:mainline Sep 9, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants