Skip to content

fix(shm): drop cfg_aliases macro, hand-roll build.rs cfg logic#2680

Merged
YuanYuYuan merged 3 commits into
eclipse-zenoh:mainfrom
ZettaScaleLabs:fix/cfg-aliases-nightly
Jul 16, 2026
Merged

fix(shm): drop cfg_aliases macro, hand-roll build.rs cfg logic#2680
YuanYuYuan merged 3 commits into
eclipse-zenoh:mainfrom
ZettaScaleLabs:fix/cfg-aliases-nightly

Conversation

@YuanYuYuan

@YuanYuYuan YuanYuYuan commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

commons/zenoh-shm/build.rs used the cfg_aliases! macro (crate cfg_aliases, pinned to 0.2.1, latest on crates.io) to compute a small set of target-platform boolean flags at build time. That macro's expansion trips rustc's semicolon_in_expressions_from_macros lint, which recently became a hard compile error on current nightly. A 3-line upstream fix exists (katharostech/cfg_aliases#15) but is unreleased. This breaks every nightly CI job (Generate documentation, Coverage (ubuntu-latest, nightly)) on main and every open PR.

Checking actual usage shows shm_external_lockfile is the only alias ever read outside build.rs (9 call sites in src/posix_shm/cleanup.rs and src/shm/unix.rs) — every other alias the macro produced was only an intermediate step in its own alias graph, never queried by any code. So this PR drops the cfg_aliases dependency entirely and computes that one boolean directly from CARGO_CFG_TARGET_OS, no macro, no crate dependency needed. The resulting shm_external_lockfile flag is identical to what the macro produced today (the BSD family, including all Apple targets, plus Redox).

Originally discovered as part of #2678, extracted here since it's an unrelated, pre-existing issue affecting the whole repo.

Key Changes

  • commons/zenoh-shm/build.rs: replace cfg_aliases! { ... } with a direct CARGO_CFG_TARGET_OS match computing only shm_external_lockfile. Also drops the cargo:rustc-check-cfg=cfg(apple_targets) / cfg(bsd) declarations — cfg_aliases! mechanically emits cargo:rustc-cfg=... for every alias name in its block, including intermediate composite ones like apple_targets/bsd, not just the final shm_external_lockfile, so those two declarations existed only to describe those extra emissions. Neither is ever read via #[cfg(...)] anywhere in the crate (grep -rn "apple_targets\|\bbsd\b" commons/zenoh-shm/src is empty), and the new script no longer emits them as flags at all — they're just local booleans folded directly into the one matches! — so there's nothing left to declare.
  • commons/zenoh-shm/Cargo.toml: remove the now-unused cfg_aliases build-dependency.

🏷️ Label-Based Checklist

Based on the labels applied to this PR, please complete these additional requirements:

Labels: bug, dependencies

🐛 Bug Fix Requirements

Since this PR is labeled as a bug fix, please ensure:

  • Root cause documented - see "Summary" above: cfg_aliases!'s macro expansion trips rustc's semicolon_in_expressions_from_macros lint, now a hard error on current nightly, with no released crate fix available.
  • Reproduction test added - no unit test is meaningful for a build-script/toolchain-lint failure; the reproduction is the existing nightly CI jobs (Generate documentation, Coverage (ubuntu-latest, nightly)) themselves, which already fail on main/any PR without this fix (confirmed on fix(liveliness): decouple send_interest replay from calling thread #2678 before this fix was extracted from it) and are expected to pass once this PR merges.
  • Test passes with fix - verified via this PR's own CI run; Generate documentation and Coverage (ubuntu-latest, nightly) are the jobs to watch.
  • Regression prevention - the same nightly CI jobs will fail again on any future regression of this build script, same as they did before this fix.
  • Fix is minimal - touches exactly one file's build-time cfg logic (commons/zenoh-shm/build.rs) plus removing the now-unused dependency line in commons/zenoh-shm/Cargo.toml; no other files changed, no workspace-root manifest changes.
  • Related bugs checked - confirmed via grep -rln "cfg_aliases" --include="Cargo.toml" . that commons/zenoh-shm was the only crate in this workspace depending on cfg_aliases directly; no other crate is affected. Also confirmed via grep -rn "shm_external_lockfile\|apple_targets\|\bbsd\b" commons/zenoh-shm/src that shm_external_lockfile is the only one of the macro's aliases ever read outside build.rs.

Why this matters: Bugs without tests often reoccur.

📦 Dependency Updates

Since this PR removes a dependency (not a version bump or a new dependency):

  • Changelog reviewed - N/A, the dependency (cfg_aliases) is removed entirely, not upgraded; there's no new changelog to review.
  • Security advisories checked - N/A, removing a dependency only shrinks the attack/advisory surface, never adds to it.
  • License compatible - N/A, no new dependency is introduced.
  • Tests pass - verified via GitHub Actions CI on this PR; no local toolchain was available to run cargo build/test directly in this session, so CI is the sole verification mechanism.
  • Transitive deps reviewed - N/A, no new transitive dependencies are introduced; Cargo.lock's existing cfg_aliases 0.1.1 entry is an unrelated transitive build-dependency of a different external crate, untouched by this change.
  • Version pinned appropriately - N/A, no version to pin; the dependency is removed, not versioned.
  • Update scope limited - yes, this PR touches exactly one dependency (cfg_aliases, dropped) in exactly one crate (commons/zenoh-shm).

Best practice: Keep dependency updates focused and test thoroughly.

Instructions:

  1. Check off items as you complete them (change - [ ] to - [x])
  2. The PR checklist CI will verify these are completed

This checklist updates automatically when labels change, but preserves your checked boxes.

cfg_aliases!'s macro expansion trips rustc's
semicolon_in_expressions_from_macros lint, a hard error on current
nightly. No fixed release of cfg_aliases exists on crates.io (0.2.1
is latest). Spell out the equivalent cfg detection manually instead
of depending on the macro, and drop the now-unused build-dependency.

Extracted from dev/liveliness-query-deadlock (eclipse-zenoh#2678),
where it was originally bundled by necessity because that PR's own CI
run hit this pre-existing, unrelated issue.
@YuanYuYuan YuanYuYuan added bug Something isn't working dependencies Pull requests that update a dependency file labels Jul 16, 2026
Checked actual usage: shm_external_lockfile is the only cfg alias
zenoh-shm queries anywhere outside build.rs (9 call sites in
posix_shm/cleanup.rs and shm/unix.rs) -- the other aliases
(dragonfly, ios, freebsd, macos, ..., apple_targets, bsd) only ever
existed as intermediate steps inside the alias graph, never read
directly by any code. No need for the cfg_aliases crate (or a patch
to it) at all: compute the one boolean the crate actually needs
directly from CARGO_CFG_TARGET_OS.

Supersedes the earlier hand-rolled 12-alias rewrite -- this is
smaller and has zero new dependency surface.
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.30%. Comparing base (e946e58) to head (93d122a).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2680      +/-   ##
==========================================
+ Coverage   74.09%   74.30%   +0.21%     
==========================================
  Files         414      414              
  Lines       62296    62301       +5     
==========================================
+ Hits        46157    46292     +135     
+ Misses      16139    16009     -130     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Removes the cfg_aliases build-dependency from commons/zenoh-shm and replaces the cfg_aliases! { ... } build-time alias graph with a small, direct CARGO_CFG_TARGET_OS-based computation to emit only the shm_external_lockfile cfg flag. This unblocks nightly builds affected by the macro expansion lint becoming a hard error.

Changes:

  • Replaced cfg_aliases! usage in commons/zenoh-shm/build.rs with a direct CARGO_CFG_TARGET_OS match to compute and emit shm_external_lockfile.
  • Removed the cfg_aliases = "0.2.1" build-dependency from commons/zenoh-shm/Cargo.toml.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
commons/zenoh-shm/build.rs Hand-rolls build-time cfg emission for shm_external_lockfile without cfg_aliases.
commons/zenoh-shm/Cargo.toml Drops the now-unused cfg_aliases build-dependency section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// family (including all Apple targets, which are BSD-derived) plus
// Redox. Computed directly instead of pulling in the `cfg_aliases`
// crate for a single derived flag.
let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@YuanYuYuan this is a fair comment, even if CARGO_CFG_TARGET_OS should be present.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I missed this since it doesn't show up in the GitHub mobile app. I will address it.

@YuanYuYuan
YuanYuYuan merged commit 7583f9a into eclipse-zenoh:main Jul 16, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants