Realign the binding with zenoh-flat HEAD - #16
Conversation
zenoh-flat renamed part of its surface and moved several handle types to
value forms, so this binding's declarations named items that no longer
exist and `Registry::resolve` refused the whole generation. Nothing new is
bound here: this is the same surface, remapped.
Renames: `keyexpr_get_str` -> `keyexpr_as_str`, `zbytes_as_bytes` ->
`zbytes_to_bytes`, `*_get_keyexpr` -> `*_get_key_expr`, and the
`*_get_zid` / `*_get_eid` pairs -> a single `*_get_id` returning
`EntityGlobalId`. `config_new_from_json` is gone (json5 remains). The
Kotlin method names follow their source idents, as they always have, so
`getStr` becomes `asStr` and `asBytes` becomes `toBytes`; nothing
hand-written referenced either.
Handles that became values, each now declared as what it is rather than as
an opaque handle plus accessors:
* `Timestamp` — `ptr_class!` + `timestamp_get_ntp64` / `_get_id` becomes
`data_class!`, so it crosses as leaves reassembled in Kotlin bytecode.
* `SourceInfo` replaces `sample_get_source_{zid,eid,sn}`; `EntityGlobalId`
replaces `reply_get_replier_{zid,eid}`. Optionality now lives on the
whole value, which is what the source crate models.
* `ZenohId`'s `zenoh_id_to_bytes` accessor is redundant — the blob IS the
value class's `bytes` property.
* `Selector` — `session_get` takes the whole selector, so its
`.split_on_param("key_expr")` no longer has a param to split.
* `RecoveryMode` became a data-carrying enum, so it is `sealed_class!`.
Test fallout, both from source-crate signature changes rather than from
this binding: `encoding_get_schema` yields raw bytes now, so the encoding
correspondence test transcodes UTF-8 at the boundary (its whole corpus is
text); `zenoh_id_to_string` is fallible, so the zid test takes the typed
`onError` and drops the all-zero id — those bytes are not an identifier,
so there is no native rendering to correspond to.
The zenoh-flat CI pin moves to 3f431b6b, whose surface this generates
against byte-for-byte.
The ~70 zenoh-flat items added alongside these changes (links, transports,
timestamp stacks, the `*_to_struct` value forms, publisher matching
listeners) stay UNBOUND and therefore visible as `skipping undeclared`
warnings. They are pending work, not acknowledged exclusions, so they do
not belong in the `ignore` list.
Requires milyin/prebindgen#205: `RecoveryMode`'s `Duration` payload needs
the converter stage chain that PR restores at sum-payload leaves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The committed artifacts were generated before prebindgen picked up the boundary aliasing guards (#200) and the converter stage-chain fix (#205), so a plain `cargo build` no longer reproduced them. The one behavioral change is in `session.get`: `Selector` carries its key expression as a nested handle, so the new guards now reject a call that passes the same native resource twice — as `this` and the selector's key expression, or as the selector's key expression and the encoding. Those are consumed-handle aliases, which the previous generation let through. The Rust side is cosmetic only: a chain-less converter call is now wrapped in a block, which is how the composed form degenerates when a type has no conversion stages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The newly generated byte-backed value types do not have value equality. For example, against this PR's generated JAR: val a = Timestamp(1uL, byteArrayOf(1, 2, 3))
val b = Timestamp(1uL, byteArrayOf(1, 2, 3))
check(a == b)fails. I also reproduced the same behavior for Kotlin arrays compare by identity, so the generated Could we add content-based |
|
This API realignment currently breaks the existing zenoh-java transition integration branch. I checked out The compile fails across the changed surface, including The repository-local checks are green, but they do not exercise the documented Maven/composite consumer, so merging this independently leaves the current integration branch uncompilable. Could PR #482 be updated/linked with the matching consumer adaptation and this downstream compile added to the coordinated merge verification? |
`Timestamp`, `ZenohId`, `EntityGlobalId` and `SourceInfo` compared by array IDENTITY, so two identically built values were unequal — the shape a consumer hits keying a map on a peer id or comparing a sample's timestamp. Fixed in the generator (milyin/prebindgen#207); this is the regeneration plus the regression test. `ZenohId` is no longer a `@JvmInline value class`. Kotlin 1.9 reserves `equals`/`hashCode` members on a value class, so an inline one cannot carry the value equality its Rust counterpart has; it is a plain `data class` now. The JNI ABI is unchanged — the externs already took `ByteArray` and the wrapper already passed `.bytes`. `ValueEqualityTest` covers the reported cases and their nesting, asserting `HashSet` de-duplication rather than only `==`: Kotlin's `data class` codegen special-cases arrays in `hashCode` but not in `equals`, so equal hashes prove nothing on their own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Confirmed and fixed. Generator fix in milyin/prebindgen#207; this branch carries the regeneration plus Measured firstBefore writing any code, I probed Kotlin 1.9.0 with the exact shapes:
Your
|
The value blob is gone from the generator (milyin/prebindgen), so `ZenohId` is declared `data_class!`: a plain value with one fixed-width byte field, which now crosses as a Kotlin `ByteArray` through the generic fixed-size array support rather than as the raw memory image of the Rust struct. The Kotlin API is unchanged. `data class ZenohId(val bytes: ByteArray)` either way, with the same content `equals`/`hashCode`/`toString`; the ctor property loses an explicit `public` that a data-class property has by default anyway. `zidString()` and the SDKs' `inner.bytes` keep working. What this buys: the JVM no longer holds a `repr(Rust)` struct's memory image (padding included, layout unguaranteed), and the decode no longer `read_unaligned`s caller-supplied bytes after only a length check — `Copy` never implied every bit pattern was a valid value. One cold-path cost: `session.zid()` and the other bare `ZenohId` returns now go through the `__ZenohIdBuilder` upcall, where the blob returned its wire directly. Nested uses (`EntityGlobalId.zid` inside `SourceInfo`, the sample callback's hot path) ride their parent's single `fromParts` and are unaffected. Tracked as milyin/prebindgen#208 together with packing a small array into scalar slots. 18/18 JVM tests, fmt + clippy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two jobs in the same run could see different generators, and a run could see a generator older than its own cache. `branch = "main"` was resolved independently by each of the four jobs, so a prebindgen merge landing mid-run split the run across two generators. Resolve main to a commit once, in a `resolve-prebindgen` job, and pin every job to that rev. Still always the latest main — just one of them per run. The `target/` cache was worse. zenoh-flat's build-script OUT_DIR lives at `target/<profile>/build/zenoh-flat-<hash>/out`; the proc-macro only ever *adds* uniquely-named `.jsonl` files there (`create_new`, never truncating), and `Source::read_group` reads every matching file in the directory and dedups by record name in `read_dir` order. A `target/` restored from a run with different sources therefore hands the generator the union of old and new records: items deleted upstream survive, and collisions resolve arbitrarily. The prefix `restore-keys` made that the normal case, since the key hashed `Cargo.lock` files that are gitignored and never exist. In run 30292836956, Lint and Build both pinned zenoh-flat at 3f431b6b yet reported different line numbers for the same file (`advanced_subscriber/mod.rs:164` vs `:184`, `session/mod.rs:262` vs `:327`) — two different stale unions. So key the target caches on both source revisions and drop the prefix restore-keys: a hit is coherent, a miss is a clean build. Hoist the zenoh-flat pin to `env.ZENOH_FLAT_REF` so the four copies cannot drift, and echo the rewritten dependency lines to make the resolved generator visible in the log. This is a workaround for the generator reading stale sibling files; the directory-hygiene fix belongs in prebindgen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
zenoh-flat renamed part of its surface and moved several handle types to
value forms (ZettaScaleLabs/zenoh-flat-jni#16). Mostly mechanical, with one
change that reaches the public API.
## A reply timestamp needs a real originating-node id
`ReplyOptions.timeStamp` was an Apache commons-net `TimeStamp`: an NTP64
instant and nothing else. Zenoh's timestamp is the pair `(ntp64, id)`, and it
orders and de-duplicates on the pair — so the missing half was supplied by the
JNI layer, which built one with a **random** id:
Timestamp::new(NTP64(timestamp_ntp_64 as u64), ID::rand()) // zenoh-jni/src/query.rs:82
The time looked right while the id was untethered from any node. zenoh-flat
removed that (ZettaScaleLabs/zenoh-flat#47) and `query_reply_success` now takes
a whole `Timestamp`, so there is no longer anywhere for a fabricated id to
come from — which is the point.
`io.zenoh.time.Timestamp` is that pair, mirroring `zenoh::time::Timestamp`.
`ReplyOptions.timeStamp` and `Sample.timestamp` carry it instead of the
commons-net type. **This is source-breaking**: a caller now writes
options.setTimeStamp(Timestamp.ofNtp64(ntp64, session.info().zid()));
`ofNtp64` exists because the primary constructor takes `ULong`, which Java
cannot express; it takes the same 64 bits `TimeStamp(long)` does, so
`TimeStamp.getCurrentTime().ntpValue()` still feeds it. The id is the
replying session's — the reply does originate here — and `Sample.timestamp`
now surfaces the sender's id, which was previously discarded on receive.
The library no longer depends on commons-net; the tests and the ZQueryable
example still use it, as an NTP64 clock.
## Mechanical
* `keyexpr_get_str` -> `asStr`, `zbytes_as_bytes` -> `toBytes`.
* `session_get` takes a whole `Selector`, so the folded (key_expr, parameters)
pair is gone. A Selector holds an owned key-expr handle with no string arm,
so `KeyExpr.intoJniHandle()` materializes one for a string-backed key
expression — the slot trio cannot express this case.
* An encoding schema is raw bytes: zenoh transmits it verbatim and does not
require UTF-8. This SDK's `Encoding` carries a String, so it is encoded at
the boundary on send and decoded lossily on receive, rather than throwing on
a received message.
* A sample's source and a reply's replier arrive as whole `SourceInfo` /
`EntityGlobalId` values, so optionality lives on the value rather than on a
leading zid leaf.
* `config_new_from_json` is gone as an invented constructor (base zenoh has no
`from_json`). `Config.fromJson` parses via JSON5, of which JSON is a subset,
so every input accepted before still parses to the same config.
CI pins move to zenoh-flat 3f431b6b and zenoh-flat-jni 5e0ae509.
112 JVM tests pass; examples compile.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit dropped them on a mistaken diagnosis of mine, which this
corrects.
I read run 30292836956's Lint and Build jobs reporting different source
locations for the same pinned zenoh-flat (`advanced_subscriber/mod.rs:164` vs
`:184`) as a stale `target/` feeding the generator a union of old and new
JSONL records. It is not. Both jobs report the *same* 18 unresolved types;
only the call site each is attributed to differs, because `Source::read_group`
dedups records into a `HashMap` and returns `into_values()`, so when several
functions share an unresolved type — several take an `impl Fn(Miss)`, several
reference `ZenohId` — which one is blamed is unordered. And the hygiene works:
`init_prebindgen_out_dir()` wipes the directory on every build-script run, and
a source change does re-run it, verified by renaming a `#[prebindgen]` item and
watching the old record disappear.
So there was no corruption to protect against, and keying the cache to an exact
prebindgen commit with no fallback bought nothing while making every prebindgen
merge — several a day right now — a cold rebuild of zenoh and its dependency
tree. Cargo's own fingerprinting is what keeps an incremental build correct; a
cache key only has to avoid an absurd mismatch.
The exact key stays keyed on both source revisions, which is still an
improvement on the old one: that hashed `Cargo.lock` files that are gitignored
and never exist, so it was the constant `${{ runner.os }}-cargo-build-target-`
and never varied at all. Now a run prefers its own revisions, falls back to the
same zenoh-flat, then to any.
The `resolve-prebindgen` job is unaffected and stands on its own: pinning every
job in a run to one resolved `main` removes a real race where a merge landing
mid-run splits the run across two generators.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
zenoh-flat renamed part of its surface and moved several handle types to
value forms (ZettaScaleLabs/zenoh-flat-jni#16). Mostly mechanical, with one
change that reaches the public API. Mirrors the same realignment in
zenoh-java.
## A reply timestamp needs a real originating-node id
`Query.reply(timestamp = ...)` took an Apache commons-net `TimeStamp`: an
NTP64 instant and nothing else. Zenoh's timestamp is the pair `(ntp64, id)`,
and it orders and de-duplicates on the pair — so the missing half was supplied
by the JNI layer, which built one with a **random** id:
Timestamp::new(NTP64(timestamp_ntp_64 as u64), ID::rand()) // zenoh-jni/src/query.rs:82
The time looked right while the id was untethered from any node. zenoh-flat
removed that (ZettaScaleLabs/zenoh-flat#47) and `query_reply_success` now takes
a whole `Timestamp`, so there is no longer anywhere for a fabricated id to
come from — which is the point.
`io.zenoh.time.Timestamp` is that pair, mirroring `zenoh::time::Timestamp`.
The reply/replyDel `timestamp` parameters and `Sample.timestamp` carry it
instead of the commons-net type. **This is source-breaking**: a caller now
writes
query.reply(keyExpr, payload, timestamp = Timestamp.ofNtp64(ntp64, zid))
`ofNtp64` exists because the primary constructor takes `ULong`, which Java
cannot express; it takes the same 64 bits `TimeStamp(long)` does, so
`TimeStamp.getCurrentTime().ntpValue()` still feeds it. The id is the
replying session's — the reply does originate here — and `Sample.timestamp`
now surfaces the sender's id, which was previously discarded on receive.
The library no longer depends on commons-net; the tests and the ZQueryable
example still use it, as an NTP64 clock.
## Mechanical
* `keyexpr_get_str` -> `asStr`, `zbytes_as_bytes` -> `toBytes`.
* `session_get` takes a whole `Selector`, so the folded (key_expr, parameters)
pair is gone. A Selector holds an owned key-expr handle with no string arm,
so `KeyExpr.intoJniHandle()` materializes one for a string-backed key
expression — the slot trio cannot express this case.
* An encoding schema is raw bytes: zenoh transmits it verbatim and does not
require UTF-8. This SDK's `Encoding` carries a String, so it is encoded at
the boundary on send and decoded lossily on receive, rather than throwing on
a received message.
* A sample's source and a reply's replier arrive as whole `SourceInfo` /
`EntityGlobalId` values, so optionality lives on the value rather than on a
leading zid leaf.
* `RecoveryMode` is a data-carrying enum, so the choice and its payload are one
value rather than a `(period, flag)` pair in which only one of the two was
ever meaningful. `RecoveryConfig.retentionPeriod` has no counterpart on the
SDK config and stays absent, as it effectively was before.
* `config_new_from_json` is gone as an invented constructor (base zenoh has no
`from_json`). `Config.fromJson` parses via JSON5, of which JSON is a subset,
so every input accepted before still parses to the same config.
CI pins move to zenoh-flat 3f431b6b and zenoh-flat-jni 5e0ae509.
121 JVM tests pass; examples compile.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Re-reviewed current head Verification:
The two earlier findings are addressed. No further findings from this pass. |
ZettaScaleLabs/zenoh-flat-jni#16 merged as 498ba26, so the pin moves off the PR branch commit it was tracking. The squash merge's tree is byte-identical to the branch tip this was verified against, and 498ba26 pins zenoh-flat at the same 3f431b6b already pinned here, so nothing but the SHA changes. 112 JVM tests pass against it; examples compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ZettaScaleLabs/zenoh-flat-jni#16 merged as 498ba26, so the pin moves off the PR branch commit it was tracking. The squash merge's tree is byte-identical to the branch tip this was verified against, and 498ba26 pins zenoh-flat at the same 3f431b6b already pinned here, so nothing but the SHA changes. 121 JVM tests pass against it; examples compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
zenoh-flat renamed part of its surface and moved several handle types to value
forms, so this binding's declarations named items that no longer exist and
Registry::resolverefused the whole generation (12 missing declarations, 14missing ignores). Nothing new is bound here — the same surface, remapped.
Renames
keyexpr_get_str→keyexpr_as_str,zbytes_as_bytes→zbytes_to_bytes,*_get_keyexpr→*_get_key_expr, and every*_get_zid/*_get_eidpair →a single
*_get_idreturningEntityGlobalId.config_new_from_jsonis gone(json5 remains).
Kotlin method names follow their source idents, as they always have, so
getStrbecomes
asStrandasBytesbecomestoBytes. Nothing hand-written referencedeither.
Handles that became values
Each is now declared as what it is, rather than as an opaque handle plus
accessors:
Timestamp—ptr_class!+timestamp_get_ntp64/_get_idbecomesdata_class!, so it crosses as leaves reassembled in Kotlin bytecode.SourceInforeplacessample_get_source_{zid,eid,sn};EntityGlobalIdreplaces
reply_get_replier_{zid,eid}. Optionality now lives on the wholevalue, which is what the source crate models.
ZenohId—zenoh_id_to_bytesis redundant: the blob is the value'sbytesproperty.Selector—session_gettakes the whole selector, so its.split_on_param("key_expr")no longer has a param to split.RecoveryModebecame a data-carrying enum →sealed_class!.Test fallout
Both from source-crate signature changes rather than from this binding:
encoding_get_schemayields raw bytes now, soEncodingCorrespondenceTesttranscodes UTF-8 at the boundary — its whole corpus is text.
zenoh_id_to_stringis fallible, soZenohIdCorrespondenceTesttakes thetyped
onErrorand drops the all-zero id: those bytes are not an identifier,so there is no native rendering to correspond to.
What is deliberately NOT here
The ~70 zenoh-flat items added alongside these changes (links, transports,
timestamp stacks, the
*_to_structvalue forms, publisher matching listeners)stay unbound, and therefore visible as
skipping undeclaredwarnings. Theyare pending work, not acknowledged exclusions, so they do not belong in the
ignorelist — happy to move them if you'd rather have a quiet build.Generator dependencies (all landed)
This branch tracks generator changes in milyin/prebindgen, and was regenerated
three times as they merged:
RecoveryMode'sDurationpayload needs it at sum-payload leaves; without it the generatedRust does not compile.
Timestamp,ZenohId,EntityGlobalIdandSourceInfocompared by array identity, so twoidentically built values were unequal. Regenerated, plus a regression test.
arrays.
ZenohIdis consequently declareddata_class!rather than as a rawmemory image of the Rust struct. The Kotlin API is unchanged —
data class ZenohId(val bytes: ByteArray)either way.All three are in prebindgen
main, and CI (which resolves prebindgen from thatbranch) is green.
Verification
cargo buildclean,gradle test18/18, fmt + clippy clean. The zenoh-flat CIpin moves to
3f431b6b; the generated artifacts here are byte-identical whethergenerated against that commit or against the local checkout.
🤖 Generated with Claude Code