fix: break dev-dependency cycle that blocks publishing iceberg-property-macro by kevinjqliu · Pull Request #3184 · apache/iceberg-rust

kevinjqliu · 2026-09-10T05:46:30Z

Which issue does this PR close?

Related to Release Iceberg Rust (0.11.0) #3034 (0.11.0 release tracking). Found while verifying 0.11.0 RC1.

What changes are included in this PR?

Fix publishing iceberg-property-macro as part of the workspace release.

iceberg-property-macro is new in 0.11.0 and must be published because iceberg depends on it. The macro crate also had a dev-dependency on iceberg declared with workspace = true, which gives it the workspace version.

When Cargo packages the macro crate for publishing, it tries to resolve iceberg ^0.11.0 from crates.io. That version does not exist yet because iceberg is published later in the workspace release, so cargo publish --workspace fails on the v0.11.0-rc.1 tag:

error: failed to prepare local package for uploading
  failed to select a version for the requirement `iceberg = "^0.11.0"`
  required by package `iceberg-property-macro v0.11.0`

This is the same failure mode that broke the 0.5.0 publish (#1325).

This PR:

Changes the dev-dependency to path-only (iceberg = { path = "../iceberg" }), so Cargo strips it when packaging. This matches the pattern already used for iceberg_test_utils across the workspace.
Marks iceberg-property-macro as internal to iceberg in its package description and README, since it is published only because iceberg depends on it.
Adds cargo publish --workspace --all-features --dry-run --no-verify to the check_standalone CI job, so publishing problems like dependency cycles and unresolvable versions are caught on the PR instead of during the post-vote release. --no-verify skips compiling each packaged crate, which took 13 minutes per PR.
Adds the full cargo publish --workspace --all-features --dry-run to dev/release/create_rc.sh as a --check_publish step, before any artifact or the RC tag is created, so a crate that cannot be published fails RC creation.
Adds the same full dry-run to publish.yml on RC tag pushes, which previously ran no steps, so the exact tag is also checked on the MSRV toolchain the real publish uses.

Follow-ups outside this PR: cherry-pick the fix and the create_rc.sh change to 0.11.x before RC2, since RC2 is created from the release branch. The macro crate's first publish will also need to use an API token, since crates.io does not allow Trusted Publishing for the first version of a new crate.

Are these changes tested?

cargo publish --workspace --all-features --dry-run fails on the current manifest and passes with this change, on both stable 1.95 (the publish workflow's toolchain) and the pinned nightly. The --no-verify variant fails on the current manifest with "circular dependency detected" in about 10 seconds.
dev/release/create_rc.sh with the new step: an uncommitted change inside a crate fails with FAILED: Check crates can be published before any artifact is created, a clean tree verifies all 11 crates, and --check_publish 0 skips the step.
cargo test -p iceberg-property-macro --all-features passes with the path-only dev-dependency, including the trybuild compile-fail tests and README doctests.
The new CI step runs on this PR. The publish.yml step only runs on an RC tag push, so it is untested until RC2.

AI Disclosure

Investigated and drafted with Claude Code; changes reviewed and verified by me.

🤖 Generated with Claude Code

…ty-macro `iceberg` depends on `iceberg-property-macro`, and the macro crate had a versioned dev-dependency on `iceberg` via `workspace = true`. When cargo packages the macro crate for publishing it must resolve `iceberg ^0.11.0` from crates.io, which does not exist until the next step, so `cargo publish --workspace` fails with "failed to select a version". This is the same failure that broke the 0.5.0 publish and it reproduces on the v0.11.0-rc.1 tag. Make the dev-dependency path-only so cargo strips it when packaging. Mark the macro crate as internal to `iceberg` in its description and README, and add a `cargo publish --workspace --dry-run` step to CI so an unpublishable crate fails on the PR instead of after the release vote. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot

🟢 Approval recommended

No unresolved blocking issues were identified.

Pull request overview

Fixes the publish-time dependency cycle for iceberg-property-macro and adds CI validation.

Changes:

Makes the iceberg dev-dependency path-only.
Documents the crate as internal.
Adds a workspace publish dry-run to CI.

File summaries

File	Description
`crates/property-macro/README.md`	Documents internal usage.
`crates/property-macro/Cargo.toml`	Removes the versioned dependency cycle.
`.github/workflows/ci.yml`	Validates workspace publishability.

Review details

Files reviewed: 3/3 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

kevinjqliu · 2026-09-10T05:49:08Z

+      # Dry-runs the publish.yml command to catch crates that cannot be published,
+      # such as packages missing files or dependency cycles between workspace crates.
+      - name: Check each crate can be published
+        run: cargo publish --workspace --all-features --dry-run


this will ensure we catch similar issue in CI time

this is perfect, I was about to suggest it. thanks for adding it!

note: we could skip the build if we want it faster with --no-verify, meaning it would just check everything resolves. but I think its worth building from the artifacts as implemented here

Do we need to run it for every pr? Our publish workflow works on rc, so I think it would be enough to check in on publish workflow?

@blackmwk Is your concern that it'll use up too much CI time?

We could add --no-verify, which reduces it just to resolution and *.crate file creation.

While we can move it to the publish workflow, I'd rather see us in a state where we're basically "ready" at any time to kick off a release. Front-loading things like crate setup sounds good to me.

@blackmwk Is your concern that it'll use up too much CI time?

Yes. How about this, we do this check with --no-verify on each pr, and do this check without --no-verify on publish workflow for rc?

i was debating this too. i think either works, just depends on preference. 😄

kevinjqliu · 2026-09-10T05:49:32Z


 categories = ["database"]
-description = "Property derive macro for Apache Iceberg Rust"
+description = "Apache Iceberg Rust property macros, internal to the iceberg crate"


i feel like its a good idea to call this out as "internal", but happy to revert

+1, I agree to add internal note to description.

kevinjqliu · 2026-09-10T05:49:43Z

-iceberg = { workspace = true }
+# Path-only so cargo strips it when packaging; a versioned dev-dependency would
+# create a publish cycle, since `iceberg` depends on this crate.
+iceberg = { path = "../iceberg" }


this is the main fix

dannycjones

lgtm! thanks for this fix and a proper guard against it happening in future

dannycjones · 2026-09-10T09:28:27Z

+      # Dry-runs the publish.yml command to catch crates that cannot be published,
+      # such as packages missing files or dependency cycles between workspace crates.
+      - name: Check each crate can be published
+        run: cargo publish --workspace --all-features --dry-run


this is perfect, I was about to suggest it. thanks for adding it!

dannycjones · 2026-09-10T09:30:27Z

+      # Dry-runs the publish.yml command to catch crates that cannot be published,
+      # such as packages missing files or dependency cycles between workspace crates.
+      - name: Check each crate can be published
+        run: cargo publish --workspace --all-features --dry-run


note: we could skip the build if we want it faster with --no-verify, meaning it would just check everything resolves. but I think its worth building from the artifacts as implemented here

dannycjones · 2026-09-10T09:30:53Z


 categories = ["database"]
-description = "Property derive macro for Apache Iceberg Rust"
+description = "Apache Iceberg Rust property macros, internal to the iceberg crate"


+1, I agree to add internal note to description.

The full `cargo publish --dry-run` compiles every packaged crate and took 13 minutes per PR. PR CI now runs it with `--no-verify`, which still catches dependency cycles, unresolvable versions, and missing metadata in seconds. The compiled dry-run moves to where it matters for a release: - `create_rc.sh` runs it before creating any artifact or the RC tag, so a crate that cannot be published fails RC creation. Opt out with `--check_publish 0`. - `publish.yml` runs it on RC tag pushes, which previously did nothing, so the exact tag is also checked on the MSRV toolchain the real publish uses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The RC creation script now blocks on the publish dry-run, so the workflow run on the tag is a secondary check that does not need a manual step. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

kevinjqliu · 2026-09-10T16:12:35Z

Updated the PR. We have several new gates

.github/workflows/ci.yml runs in PR CI and does a very simple and fast ( --dry-run --no-verify)
dev/release/create_rc.sh runs during the RC process with --dry-run
.github/workflows/publish.yml runs during publishing workflow to check again with --dry-run

dannycjones

LGTM.

Also verified that create_rc.sh works fine from a nested dir.

Co-authored-by: Daniel Carl Jones <danny@danielcarl.info>

CTTY

Thanks for catching and fixing this!

CTTY · 2026-09-10T22:18:12Z

        usage
        exit 0
        ;;
      --release_ref | --release-ref)


I think we should remove this option and always ask developers to run this on the HEAD that we want to release?

Currently there are logic like check_dependency_license() that always run on HEAD, and now we are adding check_crates_publishable

This is a separate issue though

@dannycjones

…rty-macro (#3199) fix: break dev-dependency cycle that blocks publishing iceberg-property-macro (#3184) * fix: break dev-dependency cycle that blocks publishing iceberg-property-macro `iceberg` depends on `iceberg-property-macro`, and the macro crate had a versioned dev-dependency on `iceberg` via `workspace = true`. When cargo packages the macro crate for publishing it must resolve `iceberg ^0.11.0` from crates.io, which does not exist until the next step, so `cargo publish --workspace` fails with "failed to select a version". This is the same failure that broke the 0.5.0 publish and it reproduces on the v0.11.0-rc.1 tag. Make the dev-dependency path-only so cargo strips it when packaging. Mark the macro crate as internal to `iceberg` in its description and README, and add a `cargo publish --workspace --dry-run` step to CI so an unpublishable crate fails on the PR instead of after the release vote. * ci: split publish dry-run between PR CI, RC tags, and RC creation The full `cargo publish --dry-run` compiles every packaged crate and took 13 minutes per PR. PR CI now runs it with `--no-verify`, which still catches dependency cycles, unresolvable versions, and missing metadata in seconds. The compiled dry-run moves to where it matters for a release: - `create_rc.sh` runs it before creating any artifact or the RC tag, so a crate that cannot be published fails RC creation. Opt out with `--check_publish 0`. - `publish.yml` runs it on RC tag pushes, which previously did nothing, so the exact tag is also checked on the MSRV toolchain the real publish uses. * docs: drop RC tag workflow verification note from release guide The RC creation script now blocks on the publish dry-run, so the workflow run on the tag is a secondary check that does not need a manual step. * docs: note requirements for create_rc.sh --check_publish * Apply suggestion from @dannycjones --------- (cherry picked from commit 0aeccb8) Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings September 10, 2026 05:46

Copilot started reviewing on behalf of kevinjqliu September 10, 2026 05:47

View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

kevinjqliu commented Sep 10, 2026

View reviewed changes

kevinjqliu requested review from CTTY and blackmwk September 10, 2026 05:54

This was referenced Sep 10, 2026

Release Iceberg Rust (0.11.0) #3034

Open

ci: check new crates are reserved on crates.io before merge #3185

Merged

dannycjones approved these changes Sep 10, 2026

View reviewed changes

kevinjqliu and others added 3 commits September 10, 2026 09:04

docs: drop RC tag workflow verification note from release guide

a9e25c7

The RC creation script now blocks on the publish dry-run, so the workflow run on the tag is a secondary check that does not need a manual step. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

docs: note requirements for create_rc.sh --check_publish

3db891c

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Merge branch 'main' into fix/property-macro-publish-cycle

bb506f0

kevinjqliu requested a review from dannycjones September 10, 2026 16:13

dannycjones approved these changes Sep 10, 2026

View reviewed changes

Comment thread website/src/release.md Resolved

Apply suggestion from @dannycjones

aa883e1

Co-authored-by: Daniel Carl Jones <danny@danielcarl.info>

CTTY approved these changes Sep 10, 2026

View reviewed changes

CTTY merged commit 0aeccb8 into apache:main Sep 10, 2026
23 checks passed

CTTY mentioned this pull request Sep 10, 2026

Remove --release_ref from create_rc.sh #3198

Closed

kevinjqliu deleted the fix/property-macro-publish-cycle branch September 10, 2026 22:55

Conversation

kevinjqliu commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

What changes are included in this PR?

Are these changes tested?

AI Disclosure

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dannycjones left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinjqliu commented Sep 10, 2026

Uh oh!

dannycjones left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CTTY left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kevinjqliu commented Sep 10, 2026 •

edited

Loading