Skip to content

Add namespaced variables for cross-compilation in libblkid-rs - #220

Open
jbaublitz wants to merge 1 commit into
stratis-storage:masterfrom
jbaublitz:issue-project-583
Open

jbaublitz wants to merge 1 commit into
stratis-storage:masterfrom
jbaublitz:issue-project-583

Conversation

@jbaublitz

@jbaublitz jbaublitz commented Sep 8, 2025

Copy link
Copy Markdown
Member

Related to stratis-storage/project#583

Summary by CodeRabbit

  • New Features

    • Build now honors three LIBBLKID_RS_PKG_CONFIG_* environment variables to influence pkg-config discovery (path, libdir, sysroot) during builds.
  • Bug Fixes

    • More reliable detection of libblkid in nonstandard prefixes and cross-compilation or custom build environments, reducing build failures.

@jbaublitz jbaublitz added this to the libblkid-rs-0.4.2 milestone Sep 8, 2025
@jbaublitz jbaublitz self-assigned this Sep 8, 2025
@jbaublitz jbaublitz added the enhancement New feature or request label Sep 8, 2025
@coderabbitai

coderabbitai Bot commented Sep 8, 2025

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 10ae829a-53fa-4ca9-b16f-2424d86ee517

📥 Commits

Reviewing files that changed from the base of the PR and between a2cd1b8 and 8552131.

📒 Files selected for processing (1)
  • libblkid-rs-sys/build.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • libblkid-rs-sys/build.rs

Walkthrough

The build script now maps three custom environment variables to their pkg-config counterparts before invoking pkg_config::probe("blkid"), which can alter how libblkid is discovered. No public APIs or other control flow were changed.

Changes

Cohort / File(s) Summary of changes
Build script: pkg-config env propagation
libblkid-rs-sys/build.rs
Reads LIBBLKID_RS_PKG_CONFIG_PATH, LIBBLKID_RS_PKG_CONFIG_LIBDIR, LIBBLKID_RS_PKG_CONFIG_SYSROOT_DIR and, if set, assigns them to PKG_CONFIG_PATH, PKG_CONFIG_LIBDIR, PKG_CONFIG_SYSROOT_DIR respectively before calling pkg_config::probe("blkid"). Missing variables are ignored.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Dev as Cargo Build
    participant B as build.rs
    participant Env as Environment
    participant PC as pkg-config
    participant Sys as System linker

    Dev->>B: Run build script
    B->>Env: Read LIBBLKID_RS_PKG_CONFIG_* vars
    alt Vars present
        B->>Env: Set PKG_CONFIG_* accordingly
        note right of B: Mapping occurs only if present
    else Vars absent
        note right of B: No changes to environment
    end
    B->>PC: probe("blkid")
    PC->>Env: Use PKG_CONFIG_* for search
    PC-->>B: blkid metadata (include/lib paths)
    B-->>Dev: Emit cargo:rustc-link-* directives
    Dev->>Sys: Link with discovered libblkid
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nudge the paths with gentle paws,
Three whispers guide pkg-config’s laws.
From burrowed env to linker’s light,
blkid is found—ears perked, just right.
A hop, a probe, a tidy trace—
Carrots secured, builds fall in place. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add namespaced variables for cross-compilation in libblkid-rs' directly and accurately reflects the main change: introducing LIBBLKID_RS_PKG_CONFIG_* environment variables for cross-compilation purposes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@mulkieran

Copy link
Copy Markdown
Member

@coderabbitai configuration

@coderabbitai

coderabbitai Bot commented Sep 9, 2025

Copy link
Copy Markdown
CodeRabbit configuration
language: en-US
tone_instructions: ''
early_access: false
enable_free_tier: true
reviews:
  profile: chill
  request_changes_workflow: false
  high_level_summary: true
  high_level_summary_placeholder: '@coderabbitai summary'
  high_level_summary_in_walkthrough: false
  auto_title_placeholder: '@coderabbitai'
  auto_title_instructions: ''
  review_status: true
  commit_status: true
  fail_commit_status: false
  collapse_walkthrough: false
  changed_files_summary: true
  sequence_diagrams: true
  estimate_code_review_effort: true
  assess_linked_issues: true
  related_issues: true
  related_prs: true
  suggested_labels: true
  auto_apply_labels: false
  suggested_reviewers: true
  auto_assign_reviewers: false
  poem: true
  labeling_instructions: []
  path_filters: []
  path_instructions: []
  abort_on_close: true
  disable_cache: false
  auto_review:
    enabled: true
    auto_incremental_review: true
    ignore_title_keywords: []
    labels: []
    drafts: false
    base_branches: []
    ignore_usernames: []
  finishing_touches:
    docstrings:
      enabled: true
    unit_tests:
      enabled: true
  pre_merge_checks:
    docstrings:
      mode: warning
      threshold: 80
    title:
      mode: warning
      requirements: ''
    description:
      mode: warning
    issue_assessment:
      mode: warning
    custom_checks: []
  tools:
    ast-grep:
      rule_dirs: []
      util_dirs: []
      essential_rules: true
      packages: []
    shellcheck:
      enabled: true
    ruff:
      enabled: true
    markdownlint:
      enabled: true
    github-checks:
      enabled: true
      timeout_ms: 90000
    languagetool:
      enabled: true
      enabled_rules: []
      disabled_rules: []
      enabled_categories: []
      disabled_categories: []
      enabled_only: false
      level: default
    biome:
      enabled: true
    hadolint:
      enabled: true
    swiftlint:
      enabled: true
    phpstan:
      enabled: true
      level: default
    phpmd:
      enabled: true
    phpcs:
      enabled: true
    golangci-lint:
      enabled: true
    yamllint:
      enabled: true
    gitleaks:
      enabled: true
    checkov:
      enabled: true
    detekt:
      enabled: true
    eslint:
      enabled: true
    flake8:
      enabled: true
    rubocop:
      enabled: true
    buf:
      enabled: true
    regal:
      enabled: true
    actionlint:
      enabled: true
    pmd:
      enabled: true
    cppcheck:
      enabled: true
    semgrep:
      enabled: true
    circleci:
      enabled: true
    clippy:
      enabled: true
    sqlfluff:
      enabled: true
    prismaLint:
      enabled: true
    pylint:
      enabled: true
    oxc:
      enabled: true
    shopifyThemeCheck:
      enabled: true
    luacheck:
      enabled: true
    brakeman:
      enabled: true
    dotenvLint:
      enabled: true
    htmlhint:
      enabled: true
    checkmake:
      enabled: true
    osvScanner:
      enabled: true
chat:
  art: true
  auto_reply: true
  integrations:
    jira:
      usage: auto
    linear:
      usage: auto
knowledge_base:
  opt_out: false
  web_search:
    enabled: true
  code_guidelines:
    enabled: true
    filePatterns: []
  learnings:
    scope: auto
  issues:
    scope: auto
  jira:
    usage: auto
    project_keys: []
  linear:
    usage: auto
    team_keys: []
  pull_requests:
    scope: auto
  mcp:
    usage: auto
    disabled_servers: []
code_generation:
  docstrings:
    language: en-US
    path_instructions: []
  unit_tests:
    path_instructions: []

@packit-as-a-service

Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/stratis-storage-libblkid-rs-220
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@packit-as-a-service

Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/stratis-storage-libblkid-rs-220
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@jbaublitz

Copy link
Copy Markdown
Member Author

@mulkieran After reviewing this issue, I believe that this may still be a useful PR to let people cross-compile these dependencies. Currently, I believe there's no way to specify the cross-compilation options for libblkid as a dependency as the environment variable would apply to all dependencies that use pkg-config. This change should allow specifically targeting libblkid. I intend to expand the pattern to other binding libraries.

@mulkieran

Copy link
Copy Markdown
Member

I'm fine w/ the code, but I realized we should really document this for users.

@mulkieran mulkieran moved this from In Review to In Progress in 2026January Feb 2, 2026
@mulkieran mulkieran removed this from 2026January Feb 17, 2026
@mulkieran mulkieran moved this to In Progress in 2026February Feb 17, 2026
@mulkieran mulkieran removed this from 2026February Mar 2, 2026
@mulkieran mulkieran moved this to In Progress in 2026March Mar 2, 2026
@mulkieran mulkieran moved this from In Progress to Pending in 2026March Apr 11, 2026
@mulkieran mulkieran removed this from 2026March Apr 11, 2026
@mulkieran mulkieran moved this to Pending in 2026April Apr 11, 2026
@mulkieran

Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 1, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mulkieran mulkieran added this to 2026May May 1, 2026
@mulkieran mulkieran removed this from 2026April May 1, 2026
@mulkieran mulkieran moved this to In Review in 2026May May 1, 2026

@mulkieran mulkieran left a comment

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.

@jbaublitz I think all that needs to be done is to add a note in the README explaining what those particular environment variables are for and then we can merge.

@mulkieran mulkieran moved this from In Review to In Progress in 2026May May 6, 2026
@mulkieran mulkieran removed this from 2026May Jun 1, 2026
@mulkieran mulkieran moved this to In Progress in 2026June Jun 1, 2026
@mulkieran mulkieran removed this from 2026June Jul 6, 2026
@mulkieran mulkieran moved this to In Progress in 2026July Jul 6, 2026
@mulkieran mulkieran removed this from 2026July Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants