homebrew: fix toolchain install without git metadata #1098
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe deployment workflow now distinguishes between tag creation and tag push events with separate triggers, modifying event metadata extraction and adding conditional guards. The Homebrew formula is bumped to v5.1.5, expands Python build dependencies, and refines version handling through setuptools_scm environment variables with improved build isolation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
codeant-ai
bot
added
the
size:M
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Nitpicks 🔍
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| VERSION="$(echo "${URL}" | sed -E 's/.*v([0-9]+\.[0-9]+\.[0-9]+)\.tar\.gz/\1/')" | ||
| fi | ||
| SHASUM="$(curl -sL "${URL}" | shasum -a 256 | awk '{print $1}')" |
There was a problem hiding this comment.
Suggestion: Add the -f flag to the curl command to ensure the workflow fails on HTTP errors like 404, preventing the generation of an incorrect checksum. [possible issue, importance: 8]
| SHASUM="$(curl -sL "${URL}" | shasum -a 256 | awk '{print $1}')" | |
| SHASUM="$(curl -sfL "${URL}" | shasum -a 256 | awk '{print $1}')" |
| # GitHub release tarballs without a .git directory. Use --no-build-isolation so the build | ||
| # backend can see our environment variables, and set SETUPTOOLS_SCM_PRETEND_VERSION which | ||
| # hatch-vcs respects when VCS metadata is unavailable. | ||
| pretend_env = { |
There was a problem hiding this comment.
Suggestion: Replace the manual environment variable save/restore logic with Homebrew's with_env helper for cleaner and safer temporary environment scoping. [general, importance: 7]
There was a problem hiding this comment.
Pull request overview
This PR fixes Homebrew formula installation failures that occur when building MFC's Python toolchain from release tarballs without git metadata. The toolchain uses VCS-based versioning (hatch-vcs), which requires git history or explicit version configuration.
- Adds setuptools-scm and hatch-vcs dependencies with environment variable overrides to support building without git metadata
- Updates Cantera version constraint to match toolchain/pyproject.toml requirements
- Enhances GitHub Actions workflow to properly handle tag creation events
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packaging/homebrew/mfc.rb | Updates formula to v5.1.5, adds VCS build dependencies, implements SETUPTOOLS_SCM_PRETEND_VERSION environment variable handling with save/restore pattern, and uses --no-build-isolation flag for pip install |
| .github/workflows/deploy-tap.yml | Adds 'create' event trigger for tag deployments, updates conditional logic to handle both create and push tag events, improves grep pattern for URL extraction, and removes 'homebrew-new' branch from deployment targets |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
|
||
| # Now build MFC with pre-configured venv | ||
| # Set VIRTUAL_ENV so mfc.sh uses existing venv instead of creating new one | ||
| ENV["VIRTUAL_ENV"] = venv |
There was a problem hiding this comment.
Suggestion: Assigning a Pathname object to an environment variable (ENV["VIRTUAL_ENV"] = venv) can produce a non-string value in ENV; convert the Pathname to a string to ensure the environment variable contains a proper path string. [type error]
Severity Level: Minor
| ENV["VIRTUAL_ENV"] = venv | |
| ENV["VIRTUAL_ENV"] = venv.to_s |
Why it matters? ⭐
venv is a Pathname (libexec/"venv"). Setting ENV with an explicit string (venv.to_s) is clearer and avoids any ambiguity about value types stored in ENV. In practice Pathname#to_s will be called in many contexts, but making it explicit is a small, harmless improvement that prevents subtle surprises on environments or Ruby versions that expect true String values.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packaging/homebrew/mfc.rb
**Line:** 81:81
**Comment:**
*Type Error: Assigning a Pathname object to an environment variable (`ENV["VIRTUAL_ENV"] = venv`) can produce a non-string value in ENV; convert the Pathname to a string to ensure the environment variable contains a proper path string.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/deploy-tap.yml (1)
58-58: Add-fflag tocurlto fail on HTTP errors.If the tarball URL returns a 404 or other HTTP error, the current command silently computes a checksum of the error page. Adding
-fensures the workflow fails fast with a clear error.🔎 Proposed fix
- SHASUM="$(curl -sL "${URL}" | shasum -a 256 | awk '{print $1}')" + SHASUM="$(curl -sfL "${URL}" | shasum -a 256 | awk '{print $1}')"packaging/homebrew/mfc.rb (1)
81-81: Convert Pathname to string for ENV assignment.
venvis aPathnameobject (libexec/"venv"). While Ruby will implicitly convert it in most contexts, explicitly calling.to_savoids any ambiguity and potential issues with environment variable handling.🔎 Proposed fix
- ENV["VIRTUAL_ENV"] = venv + ENV["VIRTUAL_ENV"] = venv.to_s
🧹 Nitpick comments (1)
.github/workflows/deploy-tap.yml (1)
44-51: Duplicated URL construction and inconsistent environment variable reference.
- The URL construction logic is identical in both branches (lines 47 and 51) — consider consolidating.
- Line 48 uses
${GITHUB_REF_TYPE:-}(shell environment variable) while line 44 checksEVENT_NAME(derived from GitHub context). For consistency and reliability, prefer using the GitHub context${{ github.ref_type }}directly.🔎 Proposed consolidation
- if [[ "${EVENT_NAME}" == "create" ]]; then - # Tag creation event - VERSION="${REF_NAME#v}" - URL="https://github.com/${{ github.repository }}/archive/refs/tags/v${VERSION}.tar.gz" - elif [[ "${GITHUB_REF_TYPE:-}" == "tag" ]]; then - # Tag push event - VERSION="${REF_NAME#v}" + if [[ "${EVENT_NAME}" == "create" ]] || [[ "${{ github.ref_type }}" == "tag" ]]; then + # Tag creation or tag push event + VERSION="${REF_NAME#v}" URL="https://github.com/${{ github.repository }}/archive/refs/tags/v${VERSION}.tar.gz" else
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/deploy-tap.yml(3 hunks)packaging/homebrew/mfc.rb(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`
Applied to files:
packaging/homebrew/mfc.rb
🧬 Code graph analysis (1)
packaging/homebrew/mfc.rb (1)
toolchain/mfc/common.py (1)
system(34-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Full Installation Test
- GitHub Check: Build & Publish
🔇 Additional comments (8)
.github/workflows/deploy-tap.yml (3)
16-18: Appropriate guard forcreateevent.The
createevent triggers on both tag and branch creation. The job-level condition on line 27 correctly filters to only run for tag creation events. This is a clean approach.
27-27: Good defensive check for tag-only execution.The condition
github.event_name != 'create' || github.event.ref_type == 'tag'correctly ensures the job runs for all non-create events OR only for tag creation (not branch creation). This prevents unintended runs when branches are created.
71-71: Condition aligns with new triggers.The formula update now correctly triggers on both
create(tag creation) andgithub.ref_type == 'tag'(tag push), matching the dual-trigger design.packaging/homebrew/mfc.rb (5)
32-36: Required build dependencies for VCS-less installs.Adding
setuptools-scm,hatchling,hatch-vcs, andeditablesenables building the toolchain from release tarballs that lack.gitmetadata. This aligns with the--no-build-isolationapproach below.
49-73: Manual environment save/restore is verbose but functional.The pattern correctly saves, sets, and restores the
SETUPTOOLS_SCM_PRETEND_VERSION*variables. While Homebrew'swith_envhelper could simplify this, the current approach works and handles theensurecleanup properly.Note: Setting three variants (
_FOR_MFC,_FOR_mfc, and the fallback) covers setuptools-scm's package name normalization behavior.
64-64:--no-build-isolationis necessary here.Without this flag, pip creates an isolated build environment where the
SETUPTOOLS_SCM_PRETEND_VERSION*variables would not be visible to the build backend. This is the correct fix for VCS-less builds.
83-87: Good defensive setup for potential toolchain reinstalls.Setting the pretend-version variables again before running
mfc.shensures consistent versioning if the build script attempts to reinstall or verify the toolchain.
9-10: Version bump and SHA256 hash are correct.The tarball URL and SHA256 for v5.1.5 have been verified against the actual GitHub release archive. The hash matches the downloaded artifact.
1 participant
User description
PR Type
Enhancement, Bug fix
Description
Fix toolchain installation without git metadata by setting environment variables
Add build dependencies (setuptools-scm, hatch-vcs, hatchling, editables)
Update Cantera dependency to >=3.1.0 with binary-only installation
Refactor GitHub Actions workflow to handle tag creation events properly
Scope pretend-version environment variables and implement proper cleanup
Diagram Walkthrough
File Walkthrough
mfc.rb
Fix VCS-based toolchain builds without git metadatapackaging/homebrew/mfc.rb
editables
SETUPTOOLS_SCM_PRETEND_VERSION to handle VCS-less builds
access to environment variables
deploy-tap.yml
Refactor workflow to use tag creation events.github/workflows/deploy-tap.yml
type)
create event
creation
events
push events
CodeAnt-AI Description
homebrew: install toolchain when tarballs lack git metadata and update formula + deploy workflow
What Changed
Impact
✅ Homebrew installs succeed for release tarballs without .git metadata✅ Avoids local Cantera source builds that previously failed on CI/machines✅ Formula deploys trigger correctly on tag creation and on master updates💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.