Skip to content

Commit 312a0ff

Browse files
committed
refactor: Implement all PR review suggestions for Homebrew formula
Addressed three additional suggestions from AI PR reviewer: 1. **Use libexec for mfc.sh** (Importance: 9) - Changed from prefix.install to libexec.install for mfc.sh - Updated wrapper to reference #{libexec}/mfc.sh - Follows Homebrew best practices for executable scripts 2. **Remove hardcoded compiler variables** (Importance: 7) - Removed ENV["FC"], ENV["CC"], ENV["CXX"] settings - Removed ENV["BOOST_INCLUDE"] from build step - Let Homebrew's superenv handle compiler setup via gcc dependency - Kept BOOST_INCLUDE only in wrapper script where needed at runtime 3. **Enhanced functional testing** (Importance: 6) - Added full toolchain test: runs actual example case - Tests 'mfc run' with 1D_sodshocktube example - Verifies entire workflow works end-to-end - Updated assertions to check libexec/mfc.sh location These changes make the formula more idiomatic and robust.
1 parent d1ccc91 commit 312a0ff

File tree

1 file changed

+9
-11
lines changed

1 file changed

+9
-11
lines changed

packaging/homebrew/mfc.rb

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,8 @@ class Mfc < Formula
1717
depends_on "openblas"
1818

1919
def install
20-
# Set up environment for MFC
21-
ENV["BOOST_INCLUDE"] = "#{Formula["boost"].opt_include}"
22-
ENV["FC"] = "gfortran"
23-
ENV["CC"] = "gcc"
24-
ENV["CXX"] = "g++"
25-
2620
# MFC uses a Python wrapper script for building
21+
# Homebrew's superenv handles compiler setup via gcc dependency
2722
system "./mfc.sh", "build",
2823
"-t", "pre_process", "simulation", "post_process",
2924
"-j", ENV.make_jobs
@@ -33,8 +28,8 @@ def install
3328
bin.install "build/install/bin/simulation"
3429
bin.install "build/install/bin/post_process"
3530

36-
# Install mfc.sh script to prefix
37-
prefix.install "mfc.sh"
31+
# Install mfc.sh script to libexec (for executable scripts)
32+
libexec.install "mfc.sh"
3833

3934
# Install Python toolchain
4035
prefix.install "toolchain"
@@ -46,7 +41,7 @@ def install
4641
(bin/"mfc").write <<~EOS
4742
#!/bin/bash
4843
export BOOST_INCLUDE="#{Formula["boost"].opt_include}"
49-
exec "#{prefix}/mfc.sh" "$@"
44+
exec "#{libexec}/mfc.sh" "$@"
5045
EOS
5146
chmod 0755, bin/"mfc"
5247
end
@@ -82,9 +77,12 @@ def caveats
8277
system bin/"pre_process", "-h"
8378
system bin/"simulation", "-h"
8479

85-
# Test that mfc.sh is accessible
86-
assert_predicate prefix/"mfc.sh", :exist?
80+
# Test that mfc.sh is accessible in libexec
81+
assert_predicate libexec/"mfc.sh", :exist?
8782
assert_predicate prefix/"toolchain", :exist?
83+
84+
# Test running a simple example case to verify full toolchain
85+
system bin/"mfc", "run", "#{pkgshare}/examples/1D_sodshocktube/case.py"
8886
end
8987
end
9088

0 commit comments

Comments
 (0)