Simplify GHC packaging by extracting bootstrap and build tools - #203
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughThis PR extracts alex, happy, and ghc-bootstrap as local build packages with Nickel specs and Bash build scripts, integrates them into ChangesGHC Toolchain Refactor
Sequence Diagram(s)sequenceDiagram
participant GHC_build_sh as packages/ghc/build.sh
participant GHC_bootstrap_sh as packages/ghc-bootstrap/build.sh
participant Alex_build_sh as packages/alex/build.sh
participant Happy_build_sh as packages/happy/build.sh
GHC_build_sh->>GHC_bootstrap_sh: invoke ghc-bootstrap build (version via build_args)
GHC_build_sh->>Alex_build_sh: invoke alex build (version via build_args)
GHC_build_sh->>Happy_build_sh: invoke happy build (version via build_args)
GHC_bootstrap_sh->>GHC_build_sh: install prebuilt GHC into $OUTPUT_DIR/usr and provide libtinfo
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/alex/build.sh`:
- Around line 1-12: The build script must change into the extracted, versioned
source directory before building and reference the version variable; update the
top of the script to cd into the directory named by $MINIMAL_ARG_VERSION (e.g.,
cd "${MINIMAL_ARG_VERSION}" or cd "$MINIMAL_ARG_VERSION") before invoking ghc
and the ./Setup commands (Setup.hs, ghc --make Setup.hs, ./Setup
configure/build/copy), ensuring $OUTPUT_DIR is still used for copy destdir.
- Around line 4-10: The build script always compiles "Setup.hs" even when only
"Setup.lhs" exists; update the logic to select the existing setup file and
compile that instead (e.g., determine a SETUP_FILE variable that is "Setup.hs"
if present, otherwise "Setup.lhs", and use that variable in the ghc --make and
./Setup commands). Ensure references to Setup.hs in the compile and run steps
are replaced by this selected setup filename so the path where only Setup.lhs is
provided succeeds.
In `@packages/ghc-bootstrap/build.sh`:
- Around line 27-35: The script hardcodes /usr/lib/libncursesw.so.6 when
creating the libtinfo.so.6 shim; update it to locate the real ncurses library
(search /usr/lib* and /usr/lib64 or use ldconfig -p/grep or gcc
-print-file-name) into a variable (e.g., NCURSES_PATH) and use that variable
when copying to "$OUTPUT_DIR/usr/lib/libtinfo.so.6" and to
"$GHC_LIB_DIR/libtinfo.so.6"; if no library is found, emit a clear error and
exit non‑zero. Ensure you reference OUTPUT_DIR, GHC_LIB_DIR and
MINIMAL_ARG_VERSION and replace the two cp -p /usr/lib/libncursesw.so.6
occurrences with cp -p "$NCURSES_PATH" ... .
In `@packages/ghc/build.sh`:
- Around line 42-46: The current export of LD_LIBRARY_PATH can produce a
trailing colon when LD_LIBRARY_PATH is unset, causing ld.so to treat it as the
current directory; update the export that sets LD_LIBRARY_PATH (the line using
PSEUDO_LIBS and export LD_LIBRARY_PATH) so it only inserts a ':' between
PSEUDO_LIBS and the existing LD_LIBRARY_PATH when LD_LIBRARY_PATH is non-empty
(i.e., append the old value conditionally via parameter expansion or an
if-check), ensuring no trailing empty entry is added.
In `@packages/happy/build.sh`:
- Around line 1-12: The build script doesn't cd into the extracted versioned
source directory nor reference the MINIMAL_ARG_VERSION variable; update
packages/happy/build.sh to first change directory into the extracted source
(e.g. cd "happy-$MINIMAL_ARG_VERSION" or whichever directory name pattern is
used) before running ghc and ./Setup commands, and replace any hardcoded paths
(if present) with references that use $MINIMAL_ARG_VERSION and keep OUTPUT_DIR
usage for copy/destdir; ensure the cd happens before checking/creating Setup.hs
and before running ghc and ./Setup configure/build/copy.
- Around line 4-10: The build script assumes Setup.hs is present when running
GHC; change the compile step to pick the existing setup file dynamically so
Setup.lhs is handled too: after the block that creates Setup.hs only when
neither file exists, invoke GHC with the actual setup filename (use Setup.lhs if
it exists, otherwise Setup.hs) for the "ghc --make" step, then continue to run
the generated ./Setup configure --prefix=/usr; update the reference to the
compiled filename in the subsequent commands to match the chosen source
(Setup.lhs or Setup.hs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bbb8e42f-b3de-44f1-b199-9d03f99c6dec
📒 Files selected for processing (8)
packages/alex/build.nclpackages/alex/build.shpackages/ghc-bootstrap/build.nclpackages/ghc-bootstrap/build.shpackages/ghc/build.nclpackages/ghc/build.shpackages/happy/build.nclpackages/happy/build.sh
|
/build |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ghc-bootstrap/build.sh`:
- Around line 58-68: The ldconfig pipeline in the NCURSES_PATH fallback can
cause the script to abort under set -euo pipefail; modify the assignment to
LDCONFIG_PATH so pipeline failures don't propagate (e.g. replace the pipeline
with a single awk that matches and exits: ldconfig -p | awk
'/libncursesw.so.6|libncurses.so.6/ {print $NF; exit}' or append "|| true" to
the command substitution), ensuring LDCONFIG_PATH becomes empty on no-match and
lets the subsequent if [ -f "$LDCONFIG_PATH" ] check run; update the block that
sets NCURSES_PATH from LDCONFIG_PATH accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1f448cc-0e6e-401d-ae99-83d1d19f8c27
📒 Files selected for processing (7)
packages/alex/build.nclpackages/alex/build.shpackages/ghc-bootstrap/build.nclpackages/ghc-bootstrap/build.shpackages/ghc/build.shpackages/happy/build.nclpackages/happy/build.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/alex/build.ncl
- packages/ghc-bootstrap/build.ncl
- packages/alex/build.sh
|
/build |
|
Hey @0chroma — good news, this is building now ✅ The Two small things to get it merge-ready once it's green:
Thanks for the GHC packaging cleanup! 🙌 |
… and safe LD_LIBRARY_PATH
Replace glob pattern usr/bin/* with explicit binary enumeration for the happy and alex packages to satisfy the enumerate bins checker.
7b17dd4 to
4ae6435
Compare
Summary
This PR significantly simplifies the
ghcpackage by extracting its inline bootstrap compiler and Haskell-based build tools (alexandhappy) into their own dedicated, reusable packages.Changes
ghc-bootstrap(packages/ghc-bootstrap/build.ncl,packages/ghc-bootstrap/build.sh):x86_64andaarch64architectures.libtinfo.so.6, and linker configuration in an isolated way.alex(packages/alex/build.ncl,packages/alex/build.sh):alexlexical analyzer generator, built from source usingghc-bootstrap.happy(packages/happy/build.ncl,packages/happy/build.sh):happyparser generator, built from source usingghc-bootstrap.ghc(packages/ghc/build.ncl,packages/ghc/build.sh):alexandhappy.packages/ghc/build.shfrom 130 lines to 63 lines.ghcpackage now simply depends onghc-bootstrap,alex, andhappyas standard build dependencies.Verification
min check --packages ghc-bootstrap,alex,happy,ghcto verify Nickel schemas and formatting.min patched-build ghc-bootstrapto verify bootstrap compiler extraction and linking.min patched-build alexandmin patched-build happyto verify they compile successfully usingghc-bootstrap.Summary by CodeRabbit
New Features
Refactor