Skip to content

Conversation

@echobt
Copy link
Contributor

@echobt echobt commented Feb 5, 2026

Summary

The previous fix (PR #100) didn't fully disable the audio feature because the workspace dependency still had default-features enabled.

Changes

This changes the workspace dependency for cortex-tui to have default-features = false, allowing cortex-cli to properly control which features are enabled via its own feature flags.

Verification

# Without audio (no alsa-sys)
cargo tree -p cortex-cli --no-default-features --features cortex-tui -i alsa-sys
# error: package ID specification `alsa-sys` did not match any packages

# With audio (alsa-sys included)
cargo tree -p cortex-cli -i alsa-sys
# alsa-sys v0.3.1
# └── alsa v0.9.1
#     └── cpal v0.15.3
#         └── rodio v0.19.0
#             └── cortex-tui v0.0.7
#                 └── cortex-cli v0.0.7

Impact

This fix allows the musl static builds to succeed by properly excluding alsa-sys when the audio feature is disabled.

The previous fix (PR #100) didn't fully disable the audio feature because
the workspace dependency still had default-features enabled.

This changes the workspace dependency for cortex-tui to have
default-features = false, allowing cortex-cli to properly control
which features are enabled via its own feature flags.
@greptile-apps
Copy link

greptile-apps bot commented Feb 5, 2026

Greptile Overview

Greptile Summary

This PR completes the fix from PR #100 by adding default-features = false to the cortex-tui workspace dependency definition. This resolves the musl build issue where alsa-sys was still being included despite using --no-default-features.

Key Changes:

  • Modified Cargo.toml:198 to add default-features = false to the cortex-tui workspace dependency

Technical Context:
The previous PR made the audio feature optional in cortex-tui but didn't address workspace dependency resolution. In Cargo workspaces, when a dependency is defined in [workspace.dependencies] without default-features = false, any crate using that dependency will have its default features enabled, even if the crate specifies --no-default-features. This caused alsa-sys to be pulled in for musl builds, which is incompatible with musl libc.

Verification:
The author's verification commands confirm that:

  • Without audio: cargo tree correctly shows alsa-sys is not present
  • With audio: alsa-sys is properly included in the dependency chain

This change allows cortex-cli to properly control feature flags when building without default features for musl static builds.

Confidence Score: 5/5

  • This PR is safe to merge - it's a minimal, well-understood fix to the workspace dependency configuration
  • Single-line change that correctly addresses a Cargo workspace dependency resolution issue. The fix follows Rust best practices for workspace dependencies and is verified by the author's cargo tree tests.
  • No files require special attention

Important Files Changed

Filename Overview
Cargo.toml Added default-features = false to cortex-tui workspace dependency to properly control feature flags

Sequence Diagram

sequenceDiagram
    participant CI as CI Build
    participant Cargo as Cargo Build
    participant Workspace as Workspace Deps
    participant CLI as cortex-cli
    participant TUI as cortex-tui
    
    Note over CI,TUI: Previous behavior (PR #100)
    CI->>Cargo: cargo build --no-default-features --features cortex-tui
    Cargo->>Workspace: Resolve cortex-tui dependency
    Workspace->>TUI: Load with default-features=true (implicit)
    TUI->>TUI: Enable default = ["audio"]
    TUI->>TUI: Pull in rodio -> cpal -> alsa-sys
    Note over TUI: ❌ alsa-sys still included!
    
    Note over CI,TUI: New behavior (this PR)
    CI->>Cargo: cargo build --no-default-features --features cortex-tui
    Cargo->>Workspace: Resolve cortex-tui dependency
    Workspace->>TUI: Load with default-features=false (explicit)
    TUI->>CLI: Respect CLI's feature selection
    CLI->>TUI: Only enable cortex-tui feature (not audio)
    TUI->>TUI: Skip audio feature
    Note over TUI: ✅ alsa-sys excluded!
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@echobt echobt merged commit df805a7 into main Feb 5, 2026
15 checks passed
@echobt echobt deleted the fix/release-musl-audio-feature branch February 5, 2026 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant