-
Notifications
You must be signed in to change notification settings - Fork 5
refactor(build): Improve SConstruct maintainability and add R auto-detection #7
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.
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
Open
quinnjr
wants to merge
7
commits into
FIUBioRG:master
Choose a base branch
from
quinnjr:feature/sconstruct-cleanup
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tection ## Summary Major refactoring of the SCons build system for improved maintainability, reduced cyclomatic complexity, and better R library detection. ## Changes ### SConstruct Improvements - Reorganized into clear sections with constants, functions, and main entry point - Broke down monolithic build logic into well-named, single-purpose functions - Replaced hardcoded option registration with data-driven configuration - Extracted platform-specific logic into dedicated functions - Separated configuration and build phases for clarity ### Reduced Cyclomatic Complexity - Split large functions into smaller, focused helpers - Used data-driven approaches instead of nested conditionals - Eliminated conditionals in loops where possible - Created lookup tables for repeated configuration patterns ### R Library Auto-Detection - Added automatic detection of R, Rcpp, and RInside paths - Detection priority: environment variables → R/Rscript queries → fallback paths - Added CheckRPackages SCons test for package verification - Build now fails with helpful message if R packages are missing - Prints detection results during configuration ### build_support.py Enhancements - Added RConfig dataclass for R configuration management - Added detect_r_config() for automatic R library detection - Added check_r_packages() for R package verification - Split Python version detection into single-purpose functions - Added get_perl_ldopts() helper function ### build_config.py Enhancements - Added pre-computed boolean platform flags (is_darwin, is_linux, is_alpine) - Split platform detection into focused helper functions - Added type hints throughout Co-authored-by: Claude <noreply@anthropic.com>
Contributor
Author
|
Please merge the Java/Rust support PR before this one to avoid conflicts in the SConstruct and build support files. |
- Resolve merge conflicts in SConstruct and build_support.py - Integrate Java language support from upstream into cleaned-up structure - Add configure_java() function following the modular pattern - Add --without-java option to OPTIONS list
- Add build_java_plugins() to compile Java plugins when Java is enabled - Add _compile_java_plugins_in_folder() helper to find and compile *Plugin.java files - Compile all .java files in plugin directory using javac
- Add JavaConfig dataclass for Java installation paths - Detect JAVA_HOME from environment, javac/java symlinks, update-alternatives, and fallback paths - Support AlmaLinux/RHEL, Debian/Ubuntu, Arch Linux, and macOS - Find JNI include directories and libjvm library path - Add CheckJava() SCons custom test for Java configuration - Add detect_java_config() for programmatic Java detection
- Remove -D prefix from WITH_PERL and WITH_R in CPPDEFINES SCons automatically adds -D when generating compiler flags, so -DWITH_PERL would result in -D-DWITH_PERL (invalid) - Fix Export() call that passed a dict when env_cuda was None Export() expects a string, not a dictionary
Previously HAVE_PYTHON was unconditionally added to CPPDEFINES, causing compilation to fail when --without-python was specified because Py.cxx would still try to include Python.h. Now HAVE_PYTHON is only added when configure_python() successfully enables Python support.
- Add user R library path (~R/library) to fallback search paths - Update _run_r_command to include R_LIBS_USER environment variable so Rscript can find user-installed packages (Rcpp, RInside) - Fix GCC-specific compiler flags for clang compatibility: - Use basename check to distinguish g++ from clang++ - Only add -fno-gnu-unique and --param=ssp-buffer-size=4 for GCC - Fix --export-dynamic to -Wl,--export-dynamic for linker
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Major refactoring of the SCons build system for improved maintainability, reduced cyclomatic complexity, and automatic R library detection.
Motivation
The original SConstruct file had grown complex and difficult to maintain, with hardcoded paths, nested conditionals, and duplicated code. This refactoring addresses these issues while also improving the R configuration experience.
Changes
🏗️ SConstruct Improvements
📉 Reduced Cyclomatic Complexity
🔍 R Library Auto-Detection
R_HOME,RCPP_INCLUDE_DIR,RINSIDE_INCLUDE_DIR,RINSIDE_LIB_DIR)R.home()andsystem.file()CheckRPackagesSCons test for R package verification📦 build_support.py Enhancements
RConfigdataclass for R configuration managementdetect_r_config()for automatic R library detectioncheck_r_packages()for R package verificationget_perl_ldopts()helper function⚙️ build_config.py Enhancements
is_darwin,is_linux,is_alpine)Testing
The refactored build system maintains backward compatibility with existing build commands:
Co-authored-by
Co-authored-by: Claude noreply@anthropic.com