Skip to content

Vendor stratisd in COPR#562

Merged
mulkieran merged 1 commit intostratis-storage:masterfrom
mulkieran:vendor-copr
Oct 10, 2025
Merged

Vendor stratisd in COPR#562
mulkieran merged 1 commit intostratis-storage:masterfrom
mulkieran:vendor-copr

Conversation

@mulkieran
Copy link
Copy Markdown
Member

@mulkieran mulkieran commented Oct 10, 2025

No description provided.

@mulkieran mulkieran self-assigned this Oct 10, 2025
@mulkieran mulkieran moved this to In Progress in 2025September Oct 10, 2025
@mulkieran mulkieran marked this pull request as ready for review October 10, 2025 17:19
@mulkieran mulkieran moved this from In Progress to In Review in 2025September Oct 10, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 10, 2025

Walkthrough

Removed conditional prep logic and cargo-generated buildrequires from the spec and simplified the %autosetup invocation to always include -a1 in mockbuild_test/stratisd.spec.

Changes

Cohort / File(s) Change Summary
Spec file: prep & autosetup
mockbuild_test/stratisd.spec
Deleted the conditional RHEL/COPR prep branch that ran %cargo_prep -v vendor and %generate_buildrequires/%cargo_generate_buildrequires; simplified %autosetup call from %autosetup -n stratisd-stratisd-v%{version} %{?rhel:-a1} to %autosetup -n stratisd-stratisd-v%{version} -a1.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Maintainer
    participant Spec
    rect #f8f9fa
    Note over Maintainer,Spec: Update removes conditional prep and always applies autosetup -a1
    end
    Maintainer->>Spec: invoke %autosetup -n stratisd-stratisd-v%{version} -a1
    Spec->>Maintainer: apply patches/unpack with -a1
    Note right of Spec: previous conditional vendor-prep and buildrequires generation removed
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibble at a spec so neat,
One branch gone, the prep's complete.
Autosetup hops on, steady and spry,
No vendor detours — straight to sky.
A rabbit clap — the build leaps high. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and succinctly describes the primary change—vendoring stratisd for COPR builds—aligning directly with the spec modifications in this pull request. It is specific to the main purpose of the update without extraneous details. This makes it easy for reviewers to understand the core intent at a glance.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 397a74d and 04257dd.

📒 Files selected for processing (1)
  • mockbuild_test/stratisd.spec (1 hunks)
🔇 Additional comments (1)
mockbuild_test/stratisd.spec (1)

88-90: Unconditional vendor extraction resolves COPR build issue.

Source1 is defined on line 16, %autosetup -a1 always extracts the vendor tarball, and %cargo_prep -v vendor (line 90) will succeed; no %{?rhel:} condition remains.


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

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 10, 2025

Walkthrough

The RPM spec adjusts a conditional to treat COPR builds like RHEL for vendorized cargo prep and moves build-requirements generation to only the non-RHEL, non-COPR path. No exported/public APIs are affected.

Changes

Cohort / File(s) Summary
RPM spec conditional and build macros
mockbuild_test/stratisd.spec
Expanded conditional from 0%{?rhel} to `0%{?rhel}

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant B as Builder
  participant RPM as RPM Macros
  participant Cargo as Cargo Prep

  B->>RPM: Evaluate build conditional
  alt RHEL or COPR
    RPM->>Cargo: %cargo_prep -v vendor
    note right of Cargo: Vendorized dependencies
  else Other distros
    RPM->>RPM: %generate_buildrequires
    RPM->>RPM: %cargo_generate_buildrequires ...
    note right of RPM: Build reqs generated at build-time
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

A bunny taps the spec with care,
Flips a flag—COPR, now you’re there.
RHEL’s path you also tread,
Vendor crates neatly fed.
Elsewhere, macros spin and weave—
Build-reqs bloom, then softly leave.
Hop, build, ship—🎯 achieved!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Vendor stratisd in COPR” clearly and concisely captures the primary change, which is extending the vendor-fetch behavior for stratisd to COPR builds, matching the updates in the spec file’s conditional logic.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 575579c and 8e83786.

📒 Files selected for processing (1)
  • mockbuild_test/stratisd.spec (1 hunks)
🔇 Additional comments (1)
mockbuild_test/stratisd.spec (1)

94-95: LGTM! Feature flags align with build commands.

The feature flags specified in %cargo_generate_buildrequires match the superset of features used throughout the %build section (line 99), ensuring all necessary build dependencies are generated for non-vendor builds.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 575579c and 8e83786.

📒 Files selected for processing (1)
  • mockbuild_test/stratisd.spec (1 hunks)
🔇 Additional comments (1)
mockbuild_test/stratisd.spec (1)

90-96: LGTM: Conditional expansion is syntactically correct.

The expansion of the conditional to include COPR builds aligns with the PR objective to vendor dependencies in COPR. The logic correctly separates the vendored build path (RHEL and COPR) from the dynamic build-requirements generation path (Fedora).

However, please ensure that COPR builds have access to Source1 (the vendor tarball) and that the extraction issue flagged in line 88 is addressed.

@mulkieran mulkieran force-pushed the vendor-copr branch 2 times, most recently from 397a74d to 04257dd Compare October 10, 2025 19:32
This makes it possible to build with Fedora-unavailable packages.

We can't test in Packit w/out this change.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran mulkieran merged commit 842386f into stratis-storage:master Oct 10, 2025
6 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in 2025September Oct 10, 2025
@mulkieran mulkieran deleted the vendor-copr branch October 10, 2025 20:21
@mulkieran mulkieran moved this to Done(1) in 2025October Oct 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done(1)

Development

Successfully merging this pull request may close these issues.

1 participant