Skip to content

homebrew: fix toolchain install without git metadata #1098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sbryngelson merged 8 commits into from
Dec 20, 2025
Merged

homebrew: fix toolchain install without git metadata #1098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
45090f5
homebrew: fix toolchain install without git metadata
sbryngelson Dec 19, 2025
5b18ea1
homebrew: scope pretend-version vars + update cantera to >=3.1.0
sbryngelson Dec 19, 2025
7778507
homebrew: add SETUPTOOLS_SCM_PRETEND_VERSION fallback for hatch-vcs
sbryngelson Dec 19, 2025
5438ad8
homebrew: fix rubocop style warning (use ENV.fetch)
sbryngelson Dec 19, 2025
8c8a522
homebrew: fix hash alignment for rubocop
sbryngelson Dec 19, 2025
4e09e9a
fix homebrew junk
sbryngelson Dec 20, 2025
f9ce23a
last fixes i hope
sbryngelson Dec 20, 2025
d576110
Merge branch 'master' into fix-release
sbryngelson Dec 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions .github/workflows/deploy-tap.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@ on:
paths:
- 'packaging/homebrew/mfc.rb'
- 'packaging/homebrew/README.md'
# Deploy to tap on push to master
# Deploy to tap on push to master (only when formula files changed)
push:
branches: [ master, homebrew-new ]
branches: [ master ]
paths:
- 'packaging/homebrew/mfc.rb'
- 'packaging/homebrew/README.md'
tags:
- 'v*.*.*'
# Allow manual trigger for testing
# Deploy to tap when a tag is created (no paths filter on tag creation)
create:
workflow_dispatch:

permissions:
Expand All @@ -25,6 +24,7 @@ jobs:
deploy-tap:
name: Audit and deploy formula
runs-on: macos-14
if: github.event_name != 'create' || github.event.ref_type == 'tag'
permissions:
contents: write
pull-requests: write
Expand All @@ -37,28 +37,38 @@ jobs:
- name: Determine event metadata
id: meta
run: |
if [[ "${GITHUB_REF_TYPE}" == "tag" ]]; then
VERSION="${GITHUB_REF_NAME#v}"
set -euo pipefail
EVENT_NAME="${{ github.event_name }}"
REF_NAME="${{ github.ref_name }}"

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}"
URL="https://github.com/${{ github.repository }}/archive/refs/tags/v${VERSION}.tar.gz"
else
# Extract URL from current formula to re-audit and sync
URL="$(grep -Eo 'https://github.com/.*/archive/refs/tags/v[0-9]+\.[0-9]+\.[0-9]+\.tar\.gz' packaging/homebrew/mfc.rb | head -n1)"
URL="$(grep -Eo 'https://github.com/[^"]+/archive/refs/tags/v[0-9]+\.[0-9]+\.[0-9]+\.tar\.gz' packaging/homebrew/mfc.rb | tail -n 1)"
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}')"
Copy link
Contributor

@qodo-code-review qodo-code-review bot Dec 20, 2025

Choose a reason for hiding this comment

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

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]

Suggested change
SHASUM="$(curl -sL "${URL}" | shasum -a 256 | awk '{print $1}')"
SHASUM="$(curl -sfL "${URL}" | shasum -a 256 | awk '{print $1}')"

echo "version=${VERSION}" >> $GITHUB_OUTPUT
echo "url=${URL}" >> $GITHUB_OUTPUT
echo "sha256=${SHASUM}" >> $GITHUB_OUTPUT
echo "Event: ${{ github.event_name }}" >> $GITHUB_STEP_SUMMARY
echo "Event: ${EVENT_NAME}" >> $GITHUB_STEP_SUMMARY
echo "Version: ${VERSION}" >> $GITHUB_STEP_SUMMARY
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
if [[ "${EVENT_NAME}" == "pull_request" ]]; then
echo "Mode: Audit only (PR)" >> $GITHUB_STEP_SUMMARY
else
echo "Mode: Audit and deploy" >> $GITHUB_STEP_SUMMARY
fi

- name: Update formula (for tag events)
if: github.ref_type == 'tag'
if: github.event_name == 'create' || github.ref_type == 'tag'
run: |
/usr/bin/sed -i '' "s@^ url \".*\"@ url \"${{ steps.meta.outputs.url }}\"@" packaging/homebrew/mfc.rb
/usr/bin/sed -i '' "s@^ sha256 \".*\"@ sha256 \"${{ steps.meta.outputs.sha256 }}\"@" packaging/homebrew/mfc.rb
Expand Down
22 changes: 16 additions & 6 deletions packaging/homebrew/mfc.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
class Mfc < Formula
desc "Exascale multiphase/multiphysics compressible flow solver"
homepage "https://mflowcode.github.io/"
url "https://github.com/MFlowCode/MFC/archive/refs/tags/v5.1.0.tar.gz"
sha256 "4684bee6a529287f243f8929fb7edb0dfebbb04df7c1806459761c9a6c9261cf"
url "https://github.com/MFlowCode/MFC/archive/refs/tags/v5.1.5.tar.gz"
sha256 "229ba4532d9b31e54e7db67cc6c6a4c069034bb143be7c57cba31c5a56fe6a0b"
license "MIT"
head "https://github.com/MFlowCode/MFC.git", branch: "master"

Expand All @@ -29,7 +29,11 @@ def install
# Create Python virtual environment inside libexec (inside Cellar for proper bottling)
venv = libexec/"venv"
system Formula["python@3.12"].opt_bin/"python3.12", "-m", "venv", venv
system venv/"bin/pip", "install", "--upgrade", "pip", "setuptools", "wheel", "setuptools-scm"
system venv/"bin/pip", "install", "--upgrade",
"pip", "setuptools", "wheel",
"setuptools-scm",
"hatchling", "hatch-vcs",
"editables"

# Install Cantera from PyPI using pre-built wheel (complex package, doesn't need custom flags)
# Cantera has CMake compatibility issues when building from source with newer CMake versions
Expand All @@ -42,8 +46,9 @@ def install
# Keep toolchain in buildpath for now - mfc.sh needs it there
#
# MFC's toolchain uses VCS-derived versioning (via Hatch/hatch-vcs) and Homebrew builds from
# GitHub release tarballs without a .git directory. Scope pretend-version env vars tightly
# to avoid polluting subsequent pip installs.
# 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 = {
Copy link
Contributor

@qodo-code-review qodo-code-review bot Dec 20, 2025

Choose a reason for hiding this comment

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

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]

"SETUPTOOLS_SCM_PRETEND_VERSION_FOR_MFC" => version.to_s,
"SETUPTOOLS_SCM_PRETEND_VERSION_FOR_mfc" => version.to_s,
Expand All @@ -56,7 +61,7 @@ def install
end

begin
system venv/"bin/pip", "install", "-e", buildpath/"toolchain"
system venv/"bin/pip", "install", "--no-build-isolation", "-e", buildpath/"toolchain"
ensure
pretend_env.each_key do |k|
if saved_env[k].nil?
Expand All @@ -75,6 +80,11 @@ def install
# Set VIRTUAL_ENV so mfc.sh uses existing venv instead of creating new one
ENV["VIRTUAL_ENV"] = venv
Copy link

@codeant-ai codeant-ai bot Dec 20, 2025

Choose a reason for hiding this comment

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

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 ⚠️

Suggested change
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.


# Also set pretend-version env vars for mfc.sh in case it tries to reinstall toolchain
ENV["SETUPTOOLS_SCM_PRETEND_VERSION_FOR_MFC"] = version.to_s
ENV["SETUPTOOLS_SCM_PRETEND_VERSION_FOR_mfc"] = version.to_s
ENV["SETUPTOOLS_SCM_PRETEND_VERSION"] = version.to_s

# Build MFC using pre-configured venv
# Must run from buildpath (MFC root directory) where toolchain/ exists
Dir.chdir(buildpath) do
Expand Down
Loading