CI: let a downstream SDK publish its own copy of this library - #37
Conversation
zenoh-java and zenoh-kotlin have always published their snapshot from main only - gated on refs/heads/main, on every merge there plus the weekday 06:00 nightly, never from a branch or a pull request. This repository has no publication in CI at all: publish.yml is workflow_call, and release.yml is the only caller, so the only builds that ever reached Central are two hand-dispatched rehearsals (1.9.0-rc4, 1.9.0-rc8) under versions nothing tracks. Apply the same rule here. <version.txt>-SNAPSHOT from main, through the same publish.yml a rehearsal runs, so the upload path stays exercised between releases and a consumer has something to resolve before the first release exists. One mutable coordinate set, rewritten in place.
There was a problem hiding this comment.
I found two blockers and one security/reliability issue:
-
P1 — The downstream bootstrap fails for the currently pinned commit. The reusable workflow checks out
inputs.branchbefore running Gradle, so the qualifier/stamp implementation must exist in that historical source revision. Companion zenoh-java PR #525 currently passese75529c; at that revision Gradle ignoresversionQualifierand emits neither the qualified version norzenoh.flatJniCommit. The dry-run consequently publishes1.9.0-SNAPSHOT, while the smoke test requests1.9.0-java-SNAPSHOT, so the first downstream run fails. Please either bump the downstream pin to the eventual #37 merge commit and enforce/document that minimum revision, or make qualification and stamping independent of the checked-out source revision. -
P1 — Mutable refs can produce a falsely stamped, mixed-commit artifact. Every matrix/job resolves
inputs.branchindependently, and the input explicitly accepts branch names. During this roughly half-hour workflow,maincan move after native jobs check out commit A but before the publication job checks out commit B. The final POM is then stamped as B even though packaged native libraries may be from A; a downstream stamp check can incorrectly reuse a JNI-incompatible artifact. Please resolve the source ref once to an immutable SHA, then use that SHA for every checkout and for the POM stamp. -
P2 —
version-qualifieris interpolated directly into Bash. At the consumer version construction, the workflow-call input is inserted into executable script text. Quotes, newlines, or shell syntax can alter or execute the script; the unquotedQUALIFIER_ARGalso permits argument splitting. Please pass the input through an environment variable, validate it against a narrow qualifier format, and construct the Gradle arguments safely.
The PR also currently conflicts with main. When resolving the dry-run command conflict, please retain both shell: bash and -PprebuiltAndroidLibs=true from main, while adding QUALIFIER_ARG; taking the PR side alone reintroduces the Windows/NDK failures fixed by #34/#35.
Focused local validation did pass for the new Gradle behavior itself: all three POMs used 1.9.0-java-SNAPSHOT and carried the expected head commit, and release-plus-qualifier was rejected as intended.
— Codex (GPT-5)
Review of #37. - **Mixed-commit artifacts.** `branch` accepts a branch name and every job resolved it independently, over a workflow that runs about half an hour: `main` moving mid-run would package native libraries from one commit into an artifact whose POM is stamped with another, and a downstream stamp check would read that as current and reuse a JNI-incompatible build. `resolve-source` now resolves it to a commit once — `git ls-remote` for a branch or tag, as-is for a SHA, the caller's own commit when nothing is given — and every checkout takes that. - **The downstream pin has to be new enough.** The qualifier and the stamp are produced by the *checked-out* `build.gradle.kts`, so a caller pinning an older commit would publish an unqualified, unstamped artifact and then fail resolving a coordinate nobody uploaded. Checked before anything is built, with a message saying what to do. - **Injection.** `version-qualifier` is caller input and was interpolated into script text, unquoted. It now travels by environment variable, is validated against `^[a-z0-9]+(-[a-z0-9]+)*$` before any job builds, and is expanded as a single array element rather than spliced into a command line. Also resolves the conflict with main, keeping #34/#35's `shell: bash` and `-PprebuiltAndroidLibs=true` on the dry-run publish.
4ea98bf to
7bbf0f7
Compare
|
Rebased on 2 — mixed-commit artifacts. Agreed, and it applies to the release path too: 3 — injection. qualifier=()
[[ -z $VERSION_QUALIFIER ]] || qualifier=("-PversionQualifier=$VERSION_QUALIFIER")
./gradlew … "${qualifier[@]}"It is also validated once, in 'java' accept
'my-sdk' accept
'Java' REJECT
'a b' REJECT
'$(id)' REJECT
'java; rm -rf /' REJECT1 — the downstream bootstrap. Correct, and it cannot be fixed on this side alone: the qualifier and the stamp are produced by the checked-out So the minimum revision is enforced instead, before any of the half-hour matrix runs: - name: Check the source can publish a qualified copy
if: inputs.version-qualifier != ''
run: grep -q versionQualifier build.gradle.kts || { echo "::error::… predates versionQualifier support; pin a commit that has it"; exit 1; }A stale grep there fails a build rather than publishing a wrong artifact, which is the direction to fail in. PUBLISHING.md says the same thing in prose. The downstream half is on eclipse-zenoh/zenoh-java#525, which cannot merge before this one regardless — it calls inputs that do not exist on |
milyin
left a comment
milyin
left a comment
There was a problem hiding this comment.
Re-review of 7bbf0f7: the P1 blockers from my first review are addressed. Source revisions are now shared across jobs, old incompatible pins fail before the expensive build, qualifier input no longer enters executable shell text, and the conflict resolution retained the Windows/NDK fixes.
I found three remaining P2 issues:
-
Plain tag names can resolve to the wrong ref.
git ls-remote ... "$SOURCE_REF" | head -1uses Git ref-pattern matching, which matches suffixes rather than selecting an exact branch or tag. For example,1.9.0-rc8returns bothrefs/heads/release/dry-run/1.9.0-rc8andrefs/tags/1.9.0-rc8, then selects the branch because it appears first. Those happen to point to the same commit today, but if they diverge the workflow silently publishes the wrong revision. The simplest robust construction is to checkout once inresolve-sourceusingactions/checkout, then exposegit rev-parse HEAD; alternatively resolve fully qualified branch/tag refs and reject ambiguity. -
A release-plus-qualifier mistake still wastes the entire native build. The preflight validates qualifier syntax but does not require
snapshot: true. Withsnapshot: falseand a qualifier, all six desktop targets plus the Android build complete before the consumer-test Gradle configuration finally rejects the combination. Please reject this inresolve-source, beside the syntax check. -
The caller's expected base version is not checked against the pinned source. This workflow derives the coordinate from the checked-out
version.txt, while zenoh-java independently expects the fixed value in itszenohFlatJniVersion. If lockfile sync pins a flat-JNI commit after its base version moves, this workflow publishes (for example)1.10.0-java-SNAPSHOTwhile the SDK still resolves1.9.0-java-SNAPSHOT. The build then fails only after publishing the wrong coordinate. Please pass and verify an expected base/full version, or make the downstream dependency version derive from the pinned source.
One coordination item remains: zenoh-java PR #525 still pins e75529c, so the new compatibility gate will intentionally reject it until that pin is updated after #37 merges.
The current ordinary CI matrix is green, and the focused POM/version checks from the first review still apply because the Gradle implementation is unchanged.
— Codex (GPT-5)
Re-review of #37. - **`git ls-remote <name>` matches by ref pattern**, so a tag name could resolve to `refs/heads/release/dry-run/<name>` instead. `resolve-source` now checks out with the same `actions/checkout` every job below uses and reports `git rev-parse HEAD`, so the ref resolves exactly once and exactly the way it would have there. Shorter than the ls-remote branch it replaces, and it subsumes the second checkout the capability check needed. - **A qualifier with `snapshot: false`** was rejected by build.gradle.kts, but not until the consumer test configured — after six desktop targets and the Android build. Rejected in the preflight now, with an explicit `branch`, which a qualified copy also needs: without one the checkout takes the default branch, which is the moving target pinning was meant to avoid. - **`expected-base-version`**: the coordinate comes from the checked-out version.txt while the caller names a fixed version of its own. Let the pin move past a version bump here and the run publishes 1.10.0-java-SNAPSHOT to an SDK still resolving 1.9.0-java-SNAPSHOT, failing only after the upload. The caller passes what it expects and the mismatch is caught in the preflight.
Its coordinate comes from its own version.txt, ours from gradle.properties. A pin that moves past a version bump there would publish 1.10.0-java-SNAPSHOT while this SDK still resolves 1.9.0-java-SNAPSHOT — and only fail afterwards. flat-jni-copy.bash now emits the base version and ci.yml passes it as expected-base-version, so eclipse-zenoh/zenoh-flat-jni#37's preflight catches the mismatch before anything is built.
|
All three addressed in 9d56dd6, with the caller half on eclipse-zenoh/zenoh-java#525 (4d718e7). 1 — ref-pattern matching. Right, and 2 — release plus qualifier. Moved into the preflight, beside the syntax check. While there, qualifier='java' snapshot=true branch='main' -> accept
qualifier='java' snapshot=false branch='main' -> not a snapshot
qualifier='java' snapshot=true branch='' -> no branch
qualifier='Java' snapshot=true branch='main' -> bad qualifier3 — base version. Added Deriving the SDK's dependency version from the pinned source instead would need Gradle to resolve a coordinate it cannot know at configuration time, so the check is the cheaper direction. On the caller side $ bash ci/scripts/flat-jni-copy.bash
commit=e75529ce3758401ce213456e7b8e4e5667635cf8 base=1.9.0 rebuild=trueCoordination. Confirmed — the gate rejecting |
milyin
left a comment
milyin
left a comment
There was a problem hiding this comment.
Re-review of 9d56dd6: the previous tag-resolution, delayed rejection, and version-comparison findings are addressed correctly. Two P2 issues remain:
-
branchis again interpolated directly into executable Bash. In theCheck what was asked forstep,[[ -n "${{ inputs.branch }}" ]]pastes a workflow-call input into script text before checkout. Git permits branch names containing quotes and shell metacharacters (for example,git check-ref-format 'refs/heads/x";id;#'succeeds), and this step is reached before the ref has to exist. Please pass the branch through an environment variable and test that variable, as is already done forversion-qualifier. -
expected-base-versionremains optional for qualified publications. The comparison runs only when the value is nonempty. A qualified caller that omits it therefore recreates the exact version-drift failure this input was added to prevent: the workflow can publish the pinned source's new base version while the caller resolves its old fixed coordinate. Please require a nonemptyexpected-base-versionwheneverversion-qualifieris present, then compare it withversion.txt.
The rest of the latest change looks sound. The diff check passes; completed CI jobs are green, with several jobs still running at the time of review.
Coordination remains unchanged: zenoh-java #525 must update its current e75529c pin after #37 merges, because the compatibility gate intentionally rejects that older source.
— Codex (GPT-5)
Re-review of #37. - `branch` was interpolated into the preflight's own `[[ -n … ]]`, and git permits a branch name with quotes and metacharacters — `git check-ref-format 'refs/heads/x";id;#'` succeeds, and this step runs before the ref has to exist. It goes through the environment now, like the qualifier. So does the qualifier in the version-mismatch message, which had the same shape. - `expected-base-version` was compared only when non-empty, so a qualified caller that omitted it recreated exactly the drift the input was added to catch. It is now required whenever a qualifier is given, and the comparison is unconditional.
|
Both fixed in 2e8c027. 1 — There is now no caller-supplied string left in any caller strings in run blocks: none
all run blocks parse2 — optional '1.9.0' -> accept
'' -> REJECTzenoh-java#525 already passes it, so nothing changes on that side. |
milyin
left a comment
milyin
left a comment
There was a problem hiding this comment.
Re-review of 2e8c027: no further findings.
The two findings from the previous pass are resolved:
branchnow travels throughSOURCE_REFrather than being interpolated into executable script text. The qualifier and version-mismatch message follow the same rule.expected-base-versionis now mandatory whenever a qualified copy is requested, and its comparison with the pinned source'sversion.txtis unconditional.
I also rechecked the earlier fixes: source resolution happens once through actions/checkout, every build job consumes the resulting immutable commit, qualifier syntax and snapshot-only use are rejected before cross-compilation, incompatible historical source pins fail early, Gradle arguments are constructed as array elements, and the dry-run path retains shell: bash plus -PprebuiltAndroidLibs=true.
The full CI matrix is green, git diff --check passes, and the focused POM/version behavior validated in the first review is unchanged.
The remaining zenoh-java #525 pin update from e75529c to a post-#37 commit is merge coordination, not a defect in this PR: the new preflight intentionally makes that dependency explicit.
— Codex (GPT-5)
…t-cache Three things the review asked for, all of them the rule the rest of the org already follows: - A `ci` aggregate job, as in zenoh-java. Branch protection lists required checks by job name, and three of the four jobs are 3-OS matrices, so without it the list has to enumerate nine names that change whenever the matrix does. The snapshot publication now gates on that one job instead of on four. - Third-party actions pinned to a commit, version in a trailing comment, as in zenoh-pico#1281. A tag is mutable; a moved tag runs code nobody reviewed. eclipse-zenoh/* actions are ours and stay on a branch. This also picked up four majors that had gone stale - setup-java v3/v4 and setup-gradle v3 were on retired Node runtimes. setup-gradle stops at v5: v6 moved caching into a proprietary component under Gradle's own terms of use, which is not ours to accept for an Eclipse project. - No toolchain action. rust-toolchain.toml pins the channel, the runner ships rustup, and `rustup show` installs what the file says - which is what publish.yml already did. The workflow named 1.97.1 a second time, a pin that could disagree with the file; the components move into the file with it. - swatinem/rust-cache in place of three hand-rolled actions/cache blocks per job: keyed on the toolchain and the lockfile, and it prunes what a plain `path: target` cache grows forever.
The triggers follow the main zenoh repository, as the review asked, with one deliberate difference and one addition. - `release/*` on push. A release is built from a branch the shared create-release-branch action creates, and CI never ran on it. The dry-run branches that same action produces are excluded. - `pull_request` on every branch, so a backport against a release branch gets CI too. Not `push` on every branch: with pull_request already on, that runs the nine-job matrix twice per branch. - The weekday nightly stays, which zenoh has no need of. This repository tracks zenoh, zenoh-ext and zenoh-flat by git *branch*, so a nightly build is the only thing that catches one of them moving under us between merges. The publication rides along on a trigger that has to exist anyway. - `concurrency`, which zenoh has and this repository did not. Cancelling reaches a snapshot publication in flight, and that is safe: no build resolves the coordinate this publishes, so a cancelled upload leaves an artifact nobody consumes, in a mutable repository with no staging state to unwind. That last point is the one the comments got wrong, so they are rewritten here. Downstream SDKs publish their own copy of this library from the commit each one pins - they do not resolve ours, and their CI does not wait on it. What this publication is for is the upload path itself: signing, credentials and what Central accepts are otherwise exercised only when someone dispatches a release by hand, which makes a release the moment a break in them is discovered.
The previous commit justified the schedule by saying a nightly build is what
catches zenoh, zenoh-ext or zenoh-flat moving under us, since all three are
`branch = "main"` dependencies. That is wrong. Cargo.lock pins each to a commit:
source = "git+https://github.com/eclipse-zenoh/zenoh.git?branch=main#773126fd…"
and Cargo re-resolves a git dependency only on `cargo update` or a missing lock
entry, so a timed build rebuilds exactly what the last merge built. Upstream
drift arrives here as a lockfile-sync pull request, which runs CI like anything
else.
What is left is an expired Central token or GPG key, caught only in a week with
no merges at all - every merge already exercises them. That is a thin canary
against five ten-target publications a week, so the trigger goes. Dropping it
also makes the trigger block exactly the one the review asked for, with no
difference left to defend. workflow_dispatch still runs the path on demand, and
a weekly schedule would buy the canary at a fifth of the cost if it is ever
wanted.
PUBLISHING.md records the reasoning, because "why does this repository not have
the nightly the other JVM repositories have" is a question that will be asked
again.
#37 is now stacked on this branch rather than merging before it, so the PUBLISHING.md link to "Downstream copies of the snapshot" and the ci.yml comment naming it both pointed at something that would not exist on main until #37 lands. The statement they support - that the SDKs publish their own copies and do not resolve ours - stands on its own, so it stays and the pointer goes. #37 adds the link back in the commit that adds the section.
#37 is now stacked on this branch rather than merging before it, so the PUBLISHING.md link to "Downstream copies of the snapshot" and the ci.yml comment naming it both pointed at something that would not exist on main until #37 lands. The statement they support - that the SDKs publish their own copies and do not resolve ours - stands on its own, so it stays and the pointer goes. #37 adds the link back in the commit that adds the section.
A downstream snapshot cannot depend on this repository's CI having run, and
its dependency has to be the commit it compiled against. Both follow if the
SDK builds and publishes its own copy from the commit it pins — which needs
three things this workflow did not have.
- `source-repository`: a called reusable workflow runs with the caller's
context, so the unqualified checkouts fetched the *caller's* repository.
Defaults to `github.repository`, so a fork still builds its own sources.
- `version-qualifier`: inserted before `-SNAPSHOT`, so the copies do not
overwrite each other or ours. Rejected for a non-snapshot: releases come
from here only.
- `zenoh.flatJniCommit` in every POM, from `git rev-parse HEAD`, so a
publisher can tell whether the published copy is already current without
downloading a 39 MB jar. Absent outside a git checkout, which reads as
unknown and makes a comparing consumer rebuild rather than reuse.
Verified locally: `generatePomFileFor{Jvm,KotlinMultiplatform}Publication
-PSNAPSHOT -PversionQualifier=java` produce 1.9.0-java-SNAPSHOT carrying the
commit property, and the same without -PSNAPSHOT fails the guard.
Review of #37. - **Mixed-commit artifacts.** `branch` accepts a branch name and every job resolved it independently, over a workflow that runs about half an hour: `main` moving mid-run would package native libraries from one commit into an artifact whose POM is stamped with another, and a downstream stamp check would read that as current and reuse a JNI-incompatible build. `resolve-source` now resolves it to a commit once — `git ls-remote` for a branch or tag, as-is for a SHA, the caller's own commit when nothing is given — and every checkout takes that. - **The downstream pin has to be new enough.** The qualifier and the stamp are produced by the *checked-out* `build.gradle.kts`, so a caller pinning an older commit would publish an unqualified, unstamped artifact and then fail resolving a coordinate nobody uploaded. Checked before anything is built, with a message saying what to do. - **Injection.** `version-qualifier` is caller input and was interpolated into script text, unquoted. It now travels by environment variable, is validated against `^[a-z0-9]+(-[a-z0-9]+)*$` before any job builds, and is expanded as a single array element rather than spliced into a command line. Also resolves the conflict with main, keeping #34/#35's `shell: bash` and `-PprebuiltAndroidLibs=true` on the dry-run publish.
Re-review of #37. - **`git ls-remote <name>` matches by ref pattern**, so a tag name could resolve to `refs/heads/release/dry-run/<name>` instead. `resolve-source` now checks out with the same `actions/checkout` every job below uses and reports `git rev-parse HEAD`, so the ref resolves exactly once and exactly the way it would have there. Shorter than the ls-remote branch it replaces, and it subsumes the second checkout the capability check needed. - **A qualifier with `snapshot: false`** was rejected by build.gradle.kts, but not until the consumer test configured — after six desktop targets and the Android build. Rejected in the preflight now, with an explicit `branch`, which a qualified copy also needs: without one the checkout takes the default branch, which is the moving target pinning was meant to avoid. - **`expected-base-version`**: the coordinate comes from the checked-out version.txt while the caller names a fixed version of its own. Let the pin move past a version bump here and the run publishes 1.10.0-java-SNAPSHOT to an SDK still resolving 1.9.0-java-SNAPSHOT, failing only after the upload. The caller passes what it expects and the mismatch is caught in the preflight.
Re-review of #37. - `branch` was interpolated into the preflight's own `[[ -n … ]]`, and git permits a branch name with quotes and metacharacters — `git check-ref-format 'refs/heads/x";id;#'` succeeds, and this step runs before the ref has to exist. It goes through the environment now, like the qualifier. So does the qualifier in the version-mismatch message, which had the same shape. - `expected-base-version` was compared only when non-empty, so a qualified caller that omitted it recreated exactly the drift the input was added to catch. It is now required whenever a qualifier is given, and the comparison is unconditional.
Stacked on #36, which pins every third-party action in the three workflows, this branch arrives with one exception: the checkout in the `resolve-source` job it introduces, which did not exist on #36's base and so could not be pinned there. Same SHA and trailing-comment format as the other seventeen. The PUBLISHING.md pointer to "Downstream copies of the snapshot" comes back here too. #36 carries the sentence without it, because the section it names is the one this branch adds.
2e8c027 to
8ba3237
Compare
Its coordinate comes from its own version.txt, ours from gradle.properties. A pin that moves past a version bump there would publish 1.10.0-java-SNAPSHOT while this SDK still resolves 1.9.0-java-SNAPSHOT — and only fail afterwards. flat-jni-copy.bash now emits the base version and ci.yml passes it as expected-base-version, so eclipse-zenoh/zenoh-flat-jni#37's preflight catches the mismatch before anything is built.
* CI: publish a snapshot from main, same rule as the other JVM repos
zenoh-java and zenoh-kotlin have always published their snapshot from main
only - gated on refs/heads/main, on every merge there plus the weekday 06:00
nightly, never from a branch or a pull request. This repository has no
publication in CI at all: publish.yml is workflow_call, and release.yml is the
only caller, so the only builds that ever reached Central are two hand-dispatched
rehearsals (1.9.0-rc4, 1.9.0-rc8) under versions nothing tracks.
Apply the same rule here. <version.txt>-SNAPSHOT from main, through the same
publish.yml a rehearsal runs, so the upload path stays exercised between
releases and a consumer has something to resolve before the first release
exists. One mutable coordinate set, rewritten in place.
* CI: review follow-ups - status-check job, pinned actions, rustup, rust-cache
Three things the review asked for, all of them the rule the rest of the org
already follows:
- A `ci` aggregate job, as in zenoh-java. Branch protection lists required
checks by job name, and three of the four jobs are 3-OS matrices, so without
it the list has to enumerate nine names that change whenever the matrix does.
The snapshot publication now gates on that one job instead of on four.
- Third-party actions pinned to a commit, version in a trailing comment, as in
zenoh-pico#1281. A tag is mutable; a moved tag runs code nobody reviewed.
eclipse-zenoh/* actions are ours and stay on a branch. This also picked up
four majors that had gone stale - setup-java v3/v4 and setup-gradle v3 were
on retired Node runtimes. setup-gradle stops at v5: v6 moved caching into a
proprietary component under Gradle's own terms of use, which is not ours to
accept for an Eclipse project.
- No toolchain action. rust-toolchain.toml pins the channel, the runner ships
rustup, and `rustup show` installs what the file says - which is what
publish.yml already did. The workflow named 1.97.1 a second time, a pin that
could disagree with the file; the components move into the file with it.
- swatinem/rust-cache in place of three hand-rolled actions/cache blocks per
job: keyed on the toolchain and the lockfile, and it prunes what a plain
`path: target` cache grows forever.
* CI: triggers and concurrency, and what the snapshot is actually for
The triggers follow the main zenoh repository, as the review asked, with one
deliberate difference and one addition.
- `release/*` on push. A release is built from a branch the shared
create-release-branch action creates, and CI never ran on it. The dry-run
branches that same action produces are excluded.
- `pull_request` on every branch, so a backport against a release branch gets
CI too. Not `push` on every branch: with pull_request already on, that runs
the nine-job matrix twice per branch.
- The weekday nightly stays, which zenoh has no need of. This repository tracks
zenoh, zenoh-ext and zenoh-flat by git *branch*, so a nightly build is the
only thing that catches one of them moving under us between merges. The
publication rides along on a trigger that has to exist anyway.
- `concurrency`, which zenoh has and this repository did not. Cancelling
reaches a snapshot publication in flight, and that is safe: no build resolves
the coordinate this publishes, so a cancelled upload leaves an artifact
nobody consumes, in a mutable repository with no staging state to unwind.
That last point is the one the comments got wrong, so they are rewritten here.
Downstream SDKs publish their own copy of this library from the commit each one
pins - they do not resolve ours, and their CI does not wait on it. What this
publication is for is the upload path itself: signing, credentials and what
Central accepts are otherwise exercised only when someone dispatches a release
by hand, which makes a release the moment a break in them is discovered.
* CI: drop the nightly, and correct the reason given for it
The previous commit justified the schedule by saying a nightly build is what
catches zenoh, zenoh-ext or zenoh-flat moving under us, since all three are
`branch = "main"` dependencies. That is wrong. Cargo.lock pins each to a commit:
source = "git+https://github.com/eclipse-zenoh/zenoh.git?branch=main#773126fd…"
and Cargo re-resolves a git dependency only on `cargo update` or a missing lock
entry, so a timed build rebuilds exactly what the last merge built. Upstream
drift arrives here as a lockfile-sync pull request, which runs CI like anything
else.
What is left is an expired Central token or GPG key, caught only in a week with
no merges at all - every merge already exercises them. That is a thin canary
against five ten-target publications a week, so the trigger goes. Dropping it
also makes the trigger block exactly the one the review asked for, with no
difference left to defend. workflow_dispatch still runs the path on demand, and
a weekly schedule would buy the canary at a fifth of the cost if it is ever
wanted.
PUBLISHING.md records the reasoning, because "why does this repository not have
the nightly the other JVM repositories have" is a question that will be asked
again.
* docs: do not point forward to a section this branch does not add
#37 is now stacked on this branch rather than merging before it, so the
PUBLISHING.md link to "Downstream copies of the snapshot" and the ci.yml
comment naming it both pointed at something that would not exist on main until
#37 lands. The statement they support - that the SDKs publish their own copies
and do not resolve ours - stands on its own, so it stays and the pointer goes.
#37 adds the link back in the commit that adds the section.
* docs: name the repository that still has a nightly, not "the others"
"The other JVM repositories in the org publish their snapshot on a weekday 06:00
schedule" was true when written and is not any more: zenoh-java#526 drops its
nightly on exactly the argument this section makes - its Cargo.lock pins the
zenoh-flat-jni commit, so a timed build there rebuilds what the last merge built
too. zenoh-kotlin is the one that is left, so name it, and record that the
reasoning travelled rather than being a local peculiarity.
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: milyin <1909657+milyin@users.noreply.github.com>
Its coordinate comes from its own version.txt, ours from gradle.properties. A pin that moves past a version bump there would publish 1.10.0-java-SNAPSHOT while this SDK still resolves 1.9.0-java-SNAPSHOT — and only fail afterwards. flat-jni-copy.bash now emits the base version and ci.yml passes it as expected-base-version, so eclipse-zenoh/zenoh-flat-jni#37's preflight catches the mismatch before anything is built.
eclipse-zenoh/zenoh-flat-jni#37 is merged, so `publish.yml@main` now accepts `source-repository`, `version-qualifier` and `expected-base-version`, and every POM it publishes carries `zenoh.flatJniCommit`. The pin here was e75529c, which predates all of that: the preflight added by that PR rejects it by design, and the workflow this branch adds could not start at all while the inputs it names were not on main. Only the zenoh-flat-jni line moves. `cargo update -p zenoh-flat-jni --precise` also re-resolved the zenoh git dependencies to the tip of their branch, which this branch has no business carrying: zenoh-flat-jni@6b5c04c was tested against 773126fd, and the lockfile-sync bot is what moves that rev in step with Zenoh's own. They are pinned back, leaving a one-line diff.
* CI: publish the zenoh-flat-jni the snapshot was built against The nightly snapshot publication has never got past compiling: it resolves `zenoh-flat-jni:1.9.0`, and nothing of zenoh-flat-jni has ever been published under any version. Naming its own `1.9.0-SNAPSHOT` instead would fix the symptom and break two things — this repository's CI would then wait on that repository's CI, and the coordinate always holds the tip of *its* main while we compile against the commit `Cargo.lock` pins. JNI being a binary contract, that mismatch surfaces as `UnsatisfiedLinkError` at runtime, not as a build failure. So the publication publishes what it depends on. On `main` — every merge plus the weekday nightly, as before — it builds zenoh-flat-jni from the pinned commit and uploads it as `1.9.0-java-SNAPSHOT`, then builds and uploads the SDK against that. Self-sufficient, because it uses that repository's *source at a commit we choose*, never an artifact its CI produced; coherent, because the dependency our POM names is what we compiled against. - `flat_jni_pin` reads the pin from `Cargo.lock` and the commit stamp from the three published coordinates. Rebuilding means ten cross-compiled targets and half an hour, and the pin moves about once a day, so it happens only when the published copy is not already that commit. Anything missing or unreadable reads as "not ours" and rebuilds. - `publish_flat_jni_copy` calls zenoh-flat-jni's own publication workflow rather than duplicating its build matrix. `uses:` cannot hold an expression, so the workflow file comes from its `main` and the pin goes in `branch:` — the coupling is to a file in that repository, never to a run of its CI. - `-java` keeps our copy from overwriting the one zenoh-flat-jni publishes or zenoh-kotlin's; the three can legitimately pin different commits at once. The name is fixed, so it is overwritten rather than accumulated. - `--refresh-dependencies` on both SDK invocations: Gradle caches changing modules for 24 hours and setup-gradle restores that cache, so without it the SDK could compile against yesterday's copy while publishing a POM naming the coordinate that now holds today's. - `ci/consumer-smoke-test` resolves the published snapshot from a clean build with no connection to this one and takes a key expression through JNI. That is the only check that the POM, the transitive dependency and the native library work for someone who is not us. - `bump-and-tag.bash` now checks the value `gradle.properties` ends up with: main inherits a snapshot, so *omitting* `zenoh-flat-jni-version` is how a release would reach one, which the input-only check did not cover. - CI runs on `main` are serialized rather than cancelled. Two uploads cannot be made atomic, and cancelling mid-publication is what splits them; PUBLISHING.md says so rather than claiming coherence by construction. The test job is untouched: it still builds zenoh-flat-jni from source through the composite build, which keeps that path from rotting. Verified end to end locally through Maven Local: the qualified copy publishes with its commit stamp, the SDK compiles and publishes against it, and an outside consumer resolves both and prints `zenoh-java smoke test OK`. Needs eclipse-zenoh/zenoh-flat-jni's `source-repository`/`version-qualifier` inputs on `main` first. * CI: tell zenoh-flat-jni which base version we expect Its coordinate comes from its own version.txt, ours from gradle.properties. A pin that moves past a version bump there would publish 1.10.0-java-SNAPSHOT while this SDK still resolves 1.9.0-java-SNAPSHOT — and only fail afterwards. flat-jni-copy.bash now emits the base version and ci.yml passes it as expected-base-version, so eclipse-zenoh/zenoh-flat-jni#37's preflight catches the mismatch before anything is built. * CI: publish the commit that triggered the run, and check the copy is whole Review of #525. - **The SDK publication was resolving `main`, not the run's commit.** Concurrency queues a newer run; it does not hold the branch still. So run A could read Cargo.lock at A, spend half an hour building the JNI copy from A's pin, and then publish SDK source B against it — recreating exactly the mismatch this job exists to prevent, and persisting if B's queued run later fails. It now passes `github.sha`. - **Three matching stamps were not proof of a finished publication.** The POM is uploaded before the Gradle module metadata and before the jar or aar, so a run that died in between would leave three readable stamps, no module metadata for a consumer to resolve a variant against, and a decision to skip rebuilding — permanently, since every later run reads the same three stamps. Nothing downstream would catch it: the consumer smoke test runs on Linux and the broken variant could be the Android one. The decision now also requires each coordinate's metadata to advertise `pom`, `module` and its binary (`jar`, or `aar` for Android), which is what a complete upload lists. Verified against the real 1.9.0-rc8-SNAPSHOT, and the two ways to be unfinished are in the self-test. - **PUBLISHING.md contradicted itself.** The rehearsal sections still said an empty `zenoh-flat-jni-version` falls back to an unreleased `1.9.0` and dies while compiling, and sent operators to zenoh-flat-jni's own rehearsal snapshot. The fallback is now our own `1.9.0-java-SNAPSHOT`, which resolves — what it cannot do is reach a live release, and it is `bump-and-tag.bash` that stops that. "No consumer test" was also listed as a known gap; the gap now is only that a *release* candidate is not consumable, being in staging. * CI: require the whole coordinate to be at one build, not merely present Review of #525. The presence check missed the case that matters more than a first publication: an overwrite that failed part-way. Every <snapshotVersion> in a snapshot's maven-metadata.xml carries its own <value>, updated as that file lands. After the first complete publication all three extensions are listed and stay listed, so an overwrite that replaced the POM and then failed leaves the POM at build N+1 with the module metadata and the binary still at N. The old check saw three extensions, read the N+1 POM, found the pin in its stamp, and skipped rebuilding — permanently, and with a coordinate split across two JNI builds. So the check is now agreement rather than presence: the unclassified pom, module and binary entries must all name the same <timestamp>-<buildNumber>, and the POM is fetched by that name, so a metadata entry pointing at a file that never landed reads as no stamp and rebuilds. `timestamped_name` became `snapshot_value`, since the build identifier rather than one file name is what is being compared. The self-test gains the split case, which the previous fixtures could not express. Checked against the published 1.9.0-rc8-SNAPSHOT: all three coordinates whole, at 1.9.0-rc8-20260810.012355-1. * docs: stop describing a nightly this repository no longer has #526 removes both weekday schedules - CI's 06:00 snapshot run and release.yml's 00:00 dry run - so three sentences added here describe a trigger that will not exist: - the ci.yml comment above the publishing jobs, "every merge there plus the weekday nightly above"; - "every merge to `main` and the weekday nightly upload a mutable pre-release build"; - the rehearsal section, which offered the nightly release run as an example of leaving `zenoh-flat-jni-version` empty. The last one was the weakest of the three anyway: that run failed while compiling every night, because an empty input fell back to a version not on Maven Central. What this branch changes about it is the part worth saying - the fallback is now our own published copy, so an empty field is a sound default rather than a guaranteed failure. The crons themselves are left to #526; touching them here would only conflict. * CI: pin the actions this branch adds #526 pins every third-party action in these workflows; the four steps added here - the checkout in flat_jni_pin, and the checkout, setup-java and setup-gradle in consumer_test - were written against the unpinned form and would land on main as the only tags left. Same commits, same trailing comments. * Repin zenoh-flat-jni at the commit that carries the publication inputs eclipse-zenoh/zenoh-flat-jni#37 is merged, so `publish.yml@main` now accepts `source-repository`, `version-qualifier` and `expected-base-version`, and every POM it publishes carries `zenoh.flatJniCommit`. The pin here was e75529c, which predates all of that: the preflight added by that PR rejects it by design, and the workflow this branch adds could not start at all while the inputs it names were not on main. Only the zenoh-flat-jni line moves. `cargo update -p zenoh-flat-jni --precise` also re-resolved the zenoh git dependencies to the tip of their branch, which this branch has no business carrying: zenoh-flat-jni@6b5c04c was tested against 773126fd, and the lockfile-sync bot is what moves that rev in step with Zenoh's own. They are pinned back, leaving a one-line diff.
* CI: pin third-party actions, fix the triggers, drop both nightlies The same three changes eclipse-zenoh/zenoh-java#526 made, for the same reasons. - Every third-party action pinned to a commit, version in a trailing comment. A tag is mutable, and these workflows hold the signing key and the Central token. eclipse-zenoh/* actions are ours and stay on a branch. Pinning also picked up stale majors: setup-java v4 -> v5.7.0, setup-gradle v4 -> v5.0.2, upload-artifact v4 -> v7.0.1, actions-gh-pages v3 -> v4.1.0. setup-gradle stops at v5: v6 moved caching into a proprietary component under Gradle's own terms of use, which is not ours to accept for an Eclipse project. - Triggers follow the main zenoh repository. `push` on main and release branches - a release is built from a branch create-release-branch makes, and CI never ran on it - and `pull_request` on every branch, so a backport against a release branch gets CI. Not `push` on every branch as well: with pull_request on, that ran the whole matrix twice per branch. - Both nightlies go. ci.yml's would have rebuilt exactly what the last merge built, because Cargo.lock pins zenoh-flat-jni to a commit and Cargo re-resolves a git dependency only on `cargo update` - upstream drift arrives as a lockfile-sync pull request instead. release.yml's was worse than useless: a scheduled run passes no inputs, so every night it resolved the unreleased fallback in gradle.properties and died compiling. Releases and rehearsals are both deliberate acts, and both are `Run workflow`. * CI: publish the zenoh-flat-jni the snapshot was built against Closes the zenoh-kotlin half of eclipse-zenoh/zenoh-java#524, the same way eclipse-zenoh/zenoh-java#525 closes the other. gradle.properties named `zenoh-flat-jni:1.9.0`, a version that exists in no form, so the snapshot publication died in compileKotlinJvm every time it ran. Naming zenoh-flat-jni's own `1.9.0-SNAPSHOT` instead would fix the symptom and break two things: this repository's CI would wait on that repository's CI, and that coordinate always holds the tip of *its* main while this SDK compiles against the commit Cargo.lock pins. JNI being a binary contract, the mismatch surfaces as UnsatisfiedLinkError at runtime rather than as a build failure. The rule instead: **the publication publishes what it depends on.** On main, on every merge, build zenoh-flat-jni from the pinned commit, upload it as 1.9.0-kotlin-SNAPSHOT, then build and upload the SDK against that. - `flat_jni_pin` reads the pin from Cargo.lock and the commit stamp from all three published coordinates. Rebuilding is ten cross-compiled targets and about half an hour, and the pin moves roughly once a day, so it happens only when the published copy is not already that commit. Anything missing or unreadable reads as "not ours" and rebuilds - the safe direction. - `publish_flat_jni_copy` calls zenoh-flat-jni's own publication workflow rather than duplicating its six-target/four-ABI matrix, which is how the two would drift. - `-kotlin` keeps our copy from overwriting the one zenoh-flat-jni publishes or zenoh-java's; the three can legitimately pin different commits at once. The name is fixed, so it is overwritten rather than accumulated. - `--refresh-dependencies` on both SDK invocations. Gradle caches changing modules for 24 hours and setup-gradle restores that cache, so without it a run could upload copy B, compile against cached copy A, and publish a POM naming the coordinate that now resolves to B. - `ci/consumer-smoke-test` - a separate Gradle build with no path, project or composite connection to this one. It resolves the published snapshot from the snapshot repository and takes a key expression through JNI. Kotlin rather than Java because KeyExpr.tryFrom returns a Result, which an inline value class makes awkward to call from Java. - `concurrency` with cancel-in-progress false. The two uploads are not atomic, and cancelling a run mid-publication is exactly what leaves them naming different commits. - The release guard in bump-and-tag.bash now checks the value gradle.properties ends up with rather than the workflow input, because main inherits a snapshot and omitting the input is how a release would reach one. The pin moves to 6b5c04c, eclipse-zenoh/zenoh-flat-jni#37's merge commit: the earlier e75529c predates the version-qualifier inputs and the commit stamp this depends on, and that repository's preflight rejects it by design. Only that line of Cargo.lock moves - `cargo update -p` also re-resolved the zenoh git dependencies to their branch tip, which is the lockfile-sync bot's job, so they are pinned back.
Merge order
This PR is stacked on #36 — its base is
ci/snapshot-publish, notmain, sothe diff above is this branch's own changes only and GitHub will retarget it to
mainthe moment #36 merges.Merge CI: publish a snapshot from main, same rule as the other JVM repos #36. It carries the snapshot publication and the CI hygiene from
@diogomatsubara's review, including pinning every third-party action in the
three workflows.
Merge this PR. It pins the one action CI: publish a snapshot from main, same rule as the other JVM repos #36 could not reach — the checkout
in the
resolve-sourcejob this branch introduces — sopublish.ymllands onmainwith nothing unpinned.Repin CI: publish the zenoh-flat-jni the snapshot was built against zenoh-java#525 — required, not tidying:
Its
Cargo.lockstill pinse75529c, which predates bothversionQualifierand the commit stamp, so the preflight added here rejects it by design.
Separately, that PR's workflow cannot start at all today: it calls
publish.yml@mainwith inputs that are not onmainuntil this merges, andGitHub validates those before creating any job — which is why every run there
so far ends in
startup_failure. Both clear together.Rerun #525's CI, confirm green, then merge it. That is the first run which
produces a real signal for that branch.
The first merge to zenoh-java's
mainafterwards is the first end-to-endexercise, and the expensive one: nothing is published yet, so the pin check asks
for a rebuild and this workflow runs its full ten-target matrix (~30 min) before
the SDK is published and consumed. It bootstraps itself — nothing needs
publishing by hand first.
Why
eclipse-zenoh/zenoh-java#524. That repository's nightly snapshot publication has
never got past compiling — it resolves
zenoh-flat-jni:1.9.0, and nothing ofthis library has ever been published under any version.
Pointing it at our own
1.9.0-SNAPSHOTwould fix the symptom and break twothings: its CI would then wait on ours, and that coordinate always holds the tip
of our
mainwhile it compiles against the commit itsCargo.lockpins. JNIbeing a binary contract, that mismatch surfaces as
UnsatisfiedLinkErroratruntime rather than as a build failure.
The construction that avoids both is for each SDK to build and publish its own
copy of this library, from the commit it pins. That needs three things this
workflow did not have.
What changes
source-repository— a called reusable workflow runs with the caller'scontext, so the four unqualified
actions/checkoutsteps fetched the caller'srepository and then tried to build a Cargo project that is not in it. Defaults
to
github.repository, so a fork still builds its own sources.version-qualifier— inserted before-SNAPSHOT, so the copies do notoverwrite each other or ours:
Rejected for a non-snapshot publication: releases come from here only. It
reaches the dry-run consumer test as well as the upload, so that test exercises
the coordinates that actually get published.
zenoh.flatJniCommitin every POM, fromgit rev-parse HEAD. Rebuilding acopy is ten cross-compiled targets and about half an hour, so a downstream
publisher needs to know whether the published copy is already the commit it
pins — and reading a POM property costs ~4.7 kB against a 39 MB jar. It is
absent outside a git checkout, which reads as unknown and makes a comparing
consumer rebuild rather than reuse.
Every publication of this repository carries the stamp, including the
unqualified
1.9.0-SNAPSHOT, so any consumer can ask which commit a mutableversion came from.
Verification
The root
kotlinMultiplatformpublication produces the same version and stamp.Published to Maven Local under the qualifier and consumed from there by
zenoh-java: the SDK compiles against it and an outside consumer loads the native
library through it. See the companion PR.
Companion
eclipse-zenoh/zenoh-java's
ci/self-sufficient-snapshotcalls this workflow. Itcannot start until these inputs are on
main, which is what the merge orderabove sequences.