diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..5c6f711 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,17 @@ +--- +# Stable low-noise gate for pre-commit. Keep this profile focused on +# correctness and a few numerics/performance checks that are useful for this +# codebase without depending heavily on LLVM-version-specific style policies. +Checks: >- + clang-diagnostic-*, + clang-analyzer-*, + bugprone-integer-division, + bugprone-incorrect-roundings, + bugprone-suspicious-memory-comparison, + bugprone-too-small-loop-variable, + bugprone-undefined-memory-manipulation, + performance-inefficient-vector-operation, + performance-move-const-arg +WarningsAsErrors: "" +FormatStyle: none +SystemHeaders: false diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml index e85f2fe..df463ce 100644 --- a/.github/workflows/test-coverage.yaml +++ b/.github/workflows/test-coverage.yaml @@ -57,7 +57,7 @@ jobs: # Fail if error if not on PR, or if on PR and token is given # fail_ci_if_error: ${{ github.event_name != 'pull_request' || secrets.CODECOV_TOKEN }} fail_ci_if_error: false - files: RcppTskit/cobertura.xml + files: RcppTskit/cobertura.xml # generated during the process plugins: noop disable_search: true token: ${{ secrets.CODECOV_TOKEN }} diff --git a/README.md b/README.md index 8d6e91e..82395de 100644 --- a/README.md +++ b/README.md @@ -128,115 +128,4 @@ vignette("RcppTskit_intro") ## Development -### Code of Conduct - -Please note that the `RcppTskit` project is released with a -[Contributor Code of Conduct](https://contributor-covenant.org/version/2/1/CODE_OF_CONDUCT.html). -By contributing to this project, you agree to abide by its terms. - -### Clone - -First clone the repository and step into the directory: - -``` -git clone https://github.com/HighlanderLab/RcppTskit.git -cd RcppTskit -``` - -### Pre-commit install - -We use [pre-commit](https://pre-commit.com) hooks to ensure code quality. -Specifically, we use: -* [air](https://github.com/posit-dev/air) to format `R` code, -* [jarl](https://github.com/etiennebacher/jarl) to lint `R` code, -* [clang-format](https://clang.llvm.org/docs/ClangFormat.html) to format `C/C++` code, and -* [clang-tidy](https://clang.llvm.org/extra/clang-tidy/) to lint `C/C++` code. - -To install the hooks, run: - -``` -pre-commit install --install-hooks -pre-commit install --hook-type pre-push -``` - -Run these once per clone. -This enables automatic checks on `commit` and `push`. - -### tskit - -If you plan to update `tskit`, follow instructions in `extern/README.md`. - -### RcppTskit - -Then open `RcppTskit` package directory in your favourite `R` editor -(Positron, RStudio, text-editor-of-your-choice, etc.) and implement your changes. - -You should routinely `R CMD check` your changes (in `R`): - -``` -# Note that the RcppTskit R package is in the RcppTskit sub-directory -setwd("path/to/RcppTskit/RcppTskit") - -# Check -devtools::check() - -# Install -devtools::install() - -# Test -devtools::test() - -# Test coverage -cov <- covr::package_coverage(clean = TRUE) -covr::report(cov) -``` - -Alternatively check your changes from the command line: - -``` -# Note that the RcppTskit package is in the RcppTskit sub-directory -cd path/to/RcppTskit/RcppTskit - -# Check -R CMD build RcppTskit -R CMD check RcppTskit_*.tar.gz - -# Install -R CMD INSTALL RcppTskit_*.tar.gz -``` - -On Windows, replace `tar.gz` with `zip`. - -### Pre-commit run - -When committing your changes, -`pre-commit` hooks should kick-in automatically -to ensure code quality. -Manually, you can run them using: - -``` -pre-commit autoupdate # to update the hooks -pre-commit run # on changed files -pre-commit run --all-files # on all files -pre-commit run # just a specific hook -pre-commit run --all-files # ... on all files -# see also --hook-stage option -``` - -### Continuous integration - -We use Github Actions to run continuous integration (CI) checks -on each push and pull request. -Specifically, we run: -* [R CMD check](.github/workflows/R-CMD-check.yaml) on multiple platforms - (see curent status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/R-CMD-check.yaml)), -* [documentation generation](.github/workflows/document.yaml) - (see current status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/document.yaml)), -* [pre-commit hooks](.github/workflows/pre-commit.yaml) - (see current status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/pre-commit.yaml)), and -* [test coverage](.github/workflows/test-coverage.yaml) - (see current status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/test-coverage.yaml)). - -[R universe for RcppTskit](https://highlanderlab.r-universe.dev/RcppTskit) -also provides another set of checks - see [here](https://highlanderlab.r-universe.dev/RcppTskit#checktable). -These are provided after new code is pushed or merged into this repository. +See [README_DEVEL.md](README_DEVEL.md) diff --git a/README_DEVEL.md b/README_DEVEL.md new file mode 100644 index 0000000..eaf1d68 --- /dev/null +++ b/README_DEVEL.md @@ -0,0 +1,414 @@ +# `RcppTskit` development + +## Code of Conduct + +Please note that the `RcppTskit` project is released with a +[Contributor Code of Conduct](https://contributor-covenant.org/version/2/1/CODE_OF_CONDUCT.html). +By contributing to this project, +you agree to abide by its terms. + +## Introduction + +This document describes a practical workflow for contributing to `RcppTskit`. +It assumes that you are comfortable working in a terminal and +with terminal-based tools. +The examples use macOS, +but Linux workflows should be very similar, and +Windows users can usually follow the same steps from WSL. + +## Setup + +Complete this section once per clone. +Everything before `RcppTskit changes, checks, and contributing` is setup. + +### Fork and clone `RcppTskit` (with `gh`) + +The simplest approach is to use `gh` +(assuming you have it installed or you can install it now): + +```sh +# Authenticate +gh auth login + +# Three actions in one command: +# 1) fork RcppTskit to your GitHub organisation, +# 2) clone the forked repository to your computer, and +# 3) set upstream and origin remote repositories +gh repo fork HighlanderLab/RcppTskit --clone --remote=true + +# Explore the settings and contents +cd RcppTskit +git remote -v +ls -1 +``` + +### Fork and clone `RcppTskit` (with `git`) + +If you prefer not to use `gh`, +open in your browser, +fork the repository with the `Fork` button, +and then clone the repository: + +```sh +git clone https://github.com/HighlanderLab/RcppTskit.git +cd RcppTskit +``` + +Then set the remote repositories such that `upstream` points to +`HighlanderLab/RcppTskit.git` and `origin` points to your fork: + +```sh +git remote rename origin upstream +git remote add origin git@github.com:/RcppTskit.git +git fetch upstream +git fetch origin +git remote -v +ls -1 +``` + +### Git remote repositories + +From `git remote -v` you should see output like this: + +```text +upstream git@github.com:HighlanderLab/RcppTskit.git (fetch) +upstream git@github.com:HighlanderLab/RcppTskit.git (push) +origin git@github.com:/RcppTskit.git (fetch) +origin git@github.com:/RcppTskit.git (push) +``` + +From `ls -1` you should see output like this: + +```text +CODE_OF_CONDUCT.md +extern +RcppTskit +README.md +README_DEVEL.md +``` + +### `pre-commit` and code quality tools + +We use [pre-commit](https://pre-commit.com) hooks +to run formatting and linting checks before commits and pushes. +The current hook set includes: + +* [air](https://github.com/posit-dev/air) to format `R` code +* [jarl](https://github.com/etiennebacher/jarl) to lint `R` code +* [clang-format](https://clang.llvm.org/docs/ClangFormat.html) to format `C/C++` code +* [clang-tidy](https://clang.llvm.org/extra/clang-tidy/) to lint `C/C++` code + +Install these tools with your preferred package manager before contributing. +One macOS setup looks like this: + +```sh +# Install pre-commit +brew install pre-commit + +# Install air +brew install air + +# Install jarl +# Follow the installation instructions on the jarl project page + +# Install clang-format +brew install clang-format + +# Install llvm for clang-tidy +brew install llvm + +# Add llvm tools to PATH +echo 'export PATH="/opt/homebrew/opt/llvm/bin:$PATH"' >> ~/.zshrc +``` + +Restart your terminal so the updated `PATH` is available. + +Then install the hooks: + +```sh +pre-commit install --install-hooks +pre-commit install --hook-type pre-push +``` + +This completes the setup section. + +## `RcppTskit` changes, checks, and contributing + +Once you have forked, cloned, and configured the remotes, +you are ready to contribute. + +### Open an issue + +Before starting substantial work, +open an issue at +. +Include a clear title and enough detail for others to understand your goal. Make a note of the issue number, for example `#123`. + +### Create a branch from `upstream/main` + +Before implementing changes, create a branch from the latest upstream branch. +In most cases this should be `upstream/main`. +Use `upstream/devel` only if the work is explicitly meant to target `devel`. + +```sh +# Update local knowledge of both remotes +git fetch origin --prune +git fetch upstream --prune + +# Create and switch to a new branch from upstream/main +# Replace bugfix-123 with something meaningful for your work +git switch -c bugfix-123 upstream/main + +# Confirm where HEAD points +git log +``` + +From `git log` you should see that `HEAD` points to your new branch +and that the branch starts from `upstream/main`. + +``` +commit ... (HEAD -> bugfix, upstream/main, upstream/HEAD, main) +Author: ... +Date: ... +``` + +If you expect a long-running change or +want to keep multiple branches isolated, +consider using a separate `git worktree`. + +### Implement changes + +Open the `RcppTskit/` package directory in your preferred editor, +review the relevant code, and implement the change. + +Add or update tests for every behavior change. +If the change is user-visible, +also update `RcppTskit/NEWS.md`. + +Do not edit autogenerated files (`man` folder and `RcppExports.*`). +These will be regenerated as part of the `R CMD check`. + +### Testing changes + +We use the `testthat` framework. +Relevant tests are in `RcppTskit/tests/testthat`. + +During development, +start with focused tests for the area you changed. +Before opening or updating a pull request: +1) run the full test suite, +2) ensure package checks pass, and +3) check test coverage. +All of these are described below. + +### `R CMD check` + +Run checks routinely while you work. + +In `R`, we recommend: + +```r +# The git repository contains the R package in the RcppTskit/ sub-directory. +setwd("path/to/RcppTskit/RcppTskit") + +# Source the latest version, including changes +devtools::load_all() + +# Focused tests while iterating +devtools::test(filter = "relevant-tests") + +# Full tests +devtools::test() + +# Package checks +devtools::check(vignette = FALSE) # faster +devtools::check() + +# Install if you want to try the package interactively +devtools::install() + +# Inspect test coverage +cov <- covr::package_coverage(clean = TRUE) +covr::report(cov) +``` + +From the package directory, +the equivalent terminal commands are: + +```sh +cd path/to/RcppTskit/RcppTskit +Rscript -e "devtools::test(filter = 'relevant-tests')" +Rscript -e "devtools::test()" +Rscript -e "devtools::check(vignette = FALSE)" +Rscript -e "devtools::check()" +``` + +If you prefer to work directly with `R CMD`, +run these commands from the package directory: + +```sh +cd path/to/RcppTskit +R CMD build RcppTskit +R CMD check RcppTskit_*.tar.gz +R CMD INSTALL RcppTskit_*.tar.gz +``` + +On Windows, replace `tar.gz` with `zip`. + +### Contributing your changes + +Once your tests and checks are passing, +stage and commit your changes. +Including the issue number in the commit message is helpful. + +```sh +git add the_files_you_changed +git commit -m "A clear, short message #123" +git log +``` + +`git commit` will also run the `pre-commit` hooks. A typical output looks like +this: + +```text +> git commit -m "Implemented foo() #123" +trim trailing whitespace..................................................Passed +fix end of files..........................................................Passed +mixed line ending.........................................................Passed +check yaml............................................(no files to check)Skipped +check for added large files...............................................Passed +check for merge conflicts.................................................Passed +air format................................................................Passed +jarl lint.................................................................Passed +clang-format..........................................(no files to check)Skipped +check sync between cpp and hpp options and defaults...(no files to check)Skipped +[bugfix-123 1dc2232] Implemented foo() #123 + 1 file changed, 1 insertion(+) + create mode 100644 foo.txt +``` + +### Pre-commit + +`pre-commit` hooks should run automatically when you commit/push. +You can also run them manually: + +```sh +pre-commit autoupdate # update hook versions +pre-commit run # run hooks on changed files +pre-commit run --all-files # run hooks on the entire repository +pre-commit run # run one hook on changed files +pre-commit run --all-files # run one hook on the entire repository +``` + +### Pushing to origin and opening a pull request + +Before you push, +rebase your branch onto the latest `upstream/main` +(or `upstream/devel` if that is your target branch): + +```sh +git fetch upstream main +git rebase upstream/main +``` + +If the rebase stops because of conflicts: + +1. Run `git status` to see which files are conflicted. +2. Edit each conflicted file and remove the conflict markers. +3. Stage the resolved files with `git add `. +4. Continue with `git rebase --continue`. +5. Repeat until the rebase finishes. + +If the rebase becomes confusing or the conflict set is larger than expected, +abort it and start again: + +```sh +git rebase --abort +``` + +At that point, +the safest recovery path is usually to create a fresh branch from the latest `upstream/main` +and then move your commits across with `git cherry-pick `, +or simply re-apply the changes manually to the files if that is simpler. +Another option with many local commits +is to squash them (see below) into one +and then rebasing. + +After a successful rebase, rerun the relevant tests and checks. +If you already pushed the branch to `origin`, update it with: + +```sh +git push --force-with-lease origin bugfix-123 +``` + +If this is the first push for the branch, use: + +```sh +git push -u origin bugfix-123 +``` + +Git will print a URL like +/RcppTskit/pull/new/bugfix-123>. +Open that link and create a pull request. +A draft pull request is fine if you want feedback before the work is complete. + +Mention the issue number `#123` in the pull request body +so GitHub can link the PR and the issue. +If the branch already has an open pull request, +any further pushes to `origin/bugfix-123` automatically update it. + +### Continuous integration + +When you open a pull request, +GitHub Actions runs continuous integration (CI) checks on each push and pull request. +We run: + +* [R CMD check](.github/workflows/R-CMD-check.yaml) on multiple platforms + (see current status + [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/R-CMD-check.yaml)) +* [documentation generation](.github/workflows/document.yaml) + (see current status + [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/document.yaml)) +* [pre-commit hooks](.github/workflows/pre-commit.yaml) + (see current status + [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/pre-commit.yaml)) +* [test coverage](.github/workflows/test-coverage.yaml) + (see current status + [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/test-coverage.yaml)) + +[R-universe for RcppTskit](https://highlanderlab.r-universe.dev/RcppTskit) also +provides another set of checks; see +[here](https://highlanderlab.r-universe.dev/RcppTskit#checktable). These appear +after new code is pushed or merged into the repository. + +### Squashing commits + +It is common to accumulate several commits while implementing a change. +We ask you to squash them into one before merging by using: + +```sh +git reset --soft HEAD~n +git commit -m "A clear, short message #123" +git push --force-with-lease origin bugfix-123 +``` + +Replace `n` with the number of commits you want to squash. +You can count them from the pull request commit list. + +This completes the `RcppTskit` changes, checks, and contributing section. + +## `tskit C` update + +If you plan to update the vendored `tskit` C library, +follow the instructions in `extern/README.md` and +do not edit vendored copies by hand. +Any changes to that code should go to `tskit` upstream. + +## More advanced and thorough testing + +The repository pins a low-noise `.clang-tidy` profile. +For occasional broader `C/C++` audits, +use `--config-file=RcppTskit/tools/clang_tidy_audit.yaml` +with `clang-tidy` or `RcppTskit/tools/clang_tidy.py`. +See `RcppTskit/notes_pkg_dev.Rmd` for a set of unpolished notes. +The notes also include suggestions on debugging `C/C++` code. diff --git a/RcppTskit/notes_pkg_dev.Rmd b/RcppTskit/notes_pkg_dev.Rmd index 146b3e6..a3e6cbc 100644 --- a/RcppTskit/notes_pkg_dev.Rmd +++ b/RcppTskit/notes_pkg_dev.Rmd @@ -93,8 +93,25 @@ PATCH version when you make backward compatible bug fixes ## Prepare for release ``` -use_upkeep_issue(year = NULL) # https://usethis.r-lib.org/reference/use_upkeep_issue.html -use_release_issue(version = NULL) # https://usethis.r-lib.org/reference/use_release_issue.html + +setwd("ggs_path_to/RcppTskit/RcppTskit/RcppTskit") +# yes, RcppTskit/RcppTskit/RcppTskit is correct! ;) +getwd() +dir() + +# https://usethis.r-lib.org/reference/use_upkeep_issue.html +use_upkeep_issue(year = NULL) +# ... this did not work for me +usethis::with_project(".", usethis::use_upkeep_issue(year = NULL)) +# ... this did work for me +usethis::with_project(".", usethis::use_upkeep_issue(year = NULL), force=TRUE) + +# https://usethis.r-lib.org/reference/use_release_issue.html +# ... this did not work for me +usethis::with_project(".", usethis::use_release_issue("0.3.0")) +# ... this did work for me +usethis::with_project(".", usethis::use_release_issue("0.3.0"), force=TRUE) + # just follow tasks in there;) ``` @@ -150,36 +167,6 @@ To ignore multiple lines use `# nocov start/stop`. In `Rcpp` code use `// # nocov ...`. -### Debugging C/C++ - -In `src/Makevars.in` uncomment debug flags ... - -From terminal run: - -``` -# Go to package folder -R_HOME="/Library/Frameworks/R.framework/Resources" -R_BIN="$R_HOME/bin/exec/R" -ASAN="/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/17/lib/darwin/libclang_rt.asan_osx_dynamic.dylib" -UBSAN="/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/17/lib/darwin/libclang_rt.ubsan_osx_dynamic.dylib" - -env R_HOME="$R_HOME" \ - DYLD_INSERT_LIBRARIES="$ASAN:$UBSAN" \ - ASAN_OPTIONS="abort_on_error=1,detect_leaks=0,verbosity=1" \ - UBSAN_OPTIONS="print_stacktrace=1" \ - "$R_BIN" --vanilla -``` - -In the R session (not from Rstudio/Positron!): - -``` -# Load + compile into the current ASan session (no child R load) -devtools::load_all() - -# Run tests in the same process -devtools::test() -``` - ### Air formatter of R code https://usethis.r-lib.org/reference/use_air.html @@ -247,6 +234,13 @@ pre-commit run clang-format --all-files https://clang.llvm.org/extra/clang-tidy/ +The repo uses two `clang-tidy` profiles: + +* `.clang-tidy`: stable low-noise pre-commit gate focused on correctness, + numerics, and a few performance checks. +* `tools/clang_tidy_audit.yaml`: broader manual audit profile for occasional + sweeps; do not require this profile to be warning-free on every commit. + ``` echo $(brew --prefix llvm)/bin/clang-tidy # /opt/homebrew/opt/llvm/bin/clang-tidy @@ -254,6 +248,7 @@ export CLANG_TIDY="$(brew --prefix llvm)/bin/clang-tidy" ./tools/clang_tidy.py src/RcppTskit.cpp ./RcppTskit/tools/clang_tidy.py RcppTskit/src/test_tsk_abort_stderr.cpp -- -system-headers '-header-filter=.*' -extra-arg=-Wno-unknown-warning-option ./tools/clang_tidy.py src/test_tsk_abort_stderr.cpp -- -system-headers '-header-filter=.*' -extra-arg=-Wno-unknown-warning-option +./tools/clang_tidy.py src/RcppTskit.cpp -- --config-file=tools/clang_tidy_audit.yaml ``` or @@ -263,6 +258,36 @@ pre-commit run clang-tidy src/RcppTskit.cpp pre-commit run clang-tidy --all-files ``` +### Debugging C/C++ + +In `src/Makevars.in` uncomment debug flags ... + +From terminal run: + +``` +# Go to package folder +R_HOME="/Library/Frameworks/R.framework/Resources" +R_BIN="$R_HOME/bin/exec/R" +ASAN="/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/17/lib/darwin/libclang_rt.asan_osx_dynamic.dylib" +UBSAN="/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/17/lib/darwin/libclang_rt.ubsan_osx_dynamic.dylib" + +env R_HOME="$R_HOME" \ + DYLD_INSERT_LIBRARIES="$ASAN:$UBSAN" \ + ASAN_OPTIONS="abort_on_error=1,detect_leaks=0,verbosity=1" \ + UBSAN_OPTIONS="print_stacktrace=1" \ + "$R_BIN" --vanilla +``` + +In the R session (not from Rstudio/Positron!): + +``` +# Load + compile into the current ASan session (no child R load) +devtools::load_all() + +# Run tests in the same process +devtools::test() +``` + ### GitHub actions ``` diff --git a/RcppTskit/tools/clang_tidy.py b/RcppTskit/tools/clang_tidy.py index 9ce921d..55e8880 100755 --- a/RcppTskit/tools/clang_tidy.py +++ b/RcppTskit/tools/clang_tidy.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 import os +import re import shlex import shutil import subprocess @@ -7,6 +8,9 @@ from pathlib import Path +VAR_REF_RE = re.compile(r"\$\(([^()]+)\)|\$\{([^{}]+)\}") + + def run(cmd): return subprocess.check_output(cmd, text=True).strip() @@ -88,19 +92,85 @@ def read_makevars(path): def dedupe(seq): out, seen = [], set() - for item in seq: + paired_flags = {"-F", "-I", "-iquote", "-isystem"} + i = 0 + while i < len(seq): + item = seq[i] + if item in paired_flags and i + 1 < len(seq): + pair = (item, seq[i + 1]) + if pair not in seen: + out.extend(pair) + seen.add(pair) + i += 2 + continue if item not in seen: out.append(item) seen.add(item) + i += 1 return out +def find_makevars(src_dir): + for name in ("Makevars", "Makevars.in"): + path = src_dir / name + if path.exists(): + return path + return None + + +def make_r_config_getter(r_bin): + cache = {} + + def getter(var): + if var not in cache: + try: + cache[var] = r_cmd_config(r_bin, var) + except Exception: + cache[var] = "" + return cache[var] + + return getter + + +def expand_makevars_vars(value, vars_map, context, r_config): + cache = {} + + def resolve(name, stack): + if name in cache: + return cache[name] + if name in stack: + return "" + + if name in context: + raw = context[name] + elif name in vars_map: + raw = vars_map[name] + else: + raw = os.environ.get(name) or r_config(name) + + if raw is None: + raw = "" + + next_stack = stack | {name} + expanded = VAR_REF_RE.sub( + lambda match: resolve(match.group(1) or match.group(2), next_stack), + raw, + ) + cache[name] = expanded + return expanded + + return VAR_REF_RE.sub( + lambda match: resolve(match.group(1) or match.group(2), set()), + value, + ) + + def normalize_path_flags(flags, base_dir): out = [] i = 0 while i < len(flags): token = flags[i] - if token in {"-I", "-isystem", "-iquote"}: + if token in {"-F", "-I", "-isystem", "-iquote"}: if i + 1 < len(flags): path = flags[i + 1] if not os.path.isabs(path): @@ -108,6 +178,13 @@ def normalize_path_flags(flags, base_dir): out.extend([token, path]) i += 2 continue + if token.startswith("-F") and len(token) > 2: + path = token[2:] + if not os.path.isabs(path): + path = str((base_dir / path).resolve()) + out.append("-F" + path) + i += 1 + continue if token.startswith("-I") and len(token) > 2: path = token[2:] if not os.path.isabs(path): @@ -133,11 +210,15 @@ def normalize_path_flags(flags, base_dir): return out -def makevars_flags(src_dir, r_home, lang): - makevars = src_dir / "Makevars" - if not makevars.exists(): - makevars = src_dir / "Makevars.in" - vars_map = read_makevars(makevars) +def makevars_context(vars_map, r_home): + context = {} + if r_home: + context["R_HOME"] = r_home + context["R_INCLUDE_DIR"] = str(Path(r_home) / "include") + return context + + +def makevars_flags(src_dir, vars_map, context, r_config, lang): keys = ["PKG_CPPFLAGS"] if lang == "c": keys.append("PKG_CFLAGS") @@ -149,12 +230,78 @@ def makevars_flags(src_dir, r_home, lang): val = vars_map.get(key) if not val: continue - if r_home: - val = val.replace("$(R_HOME)", r_home).replace("${R_HOME}", r_home) - flags += shlex.split(val) + expanded = expand_makevars_vars(val, vars_map, context, r_config).strip() + if expanded: + flags += shlex.split(expanded) return normalize_path_flags(flags, src_dir) +def cxx_std_setting(vars_map, context, r_config): + value = vars_map.get("CXX_STD", "").strip() + if not value: + return "" + return expand_makevars_vars(value, vars_map, context, r_config).strip() + + +def compiler_command(r_config, lang, cxx_std): + if lang == "c": + command = r_config("CC") + else: + command = r_config(cxx_std or "CXX") or r_config("CXX") + return shlex.split(command) if command else [] + + +def compiler_flag_vars(lang, cxx_std): + if lang == "c": + return ("CFLAGS", "") + if cxx_std: + return (f"{cxx_std}FLAGS", f"{cxx_std}STD") + return ("CXXFLAGS", "") + + +def parse_include_search_paths(output): + paths = [] + in_search_list = False + for line in output.splitlines(): + stripped = line.strip() + if stripped in { + '#include "..." search starts here:', + "#include <...> search starts here:", + }: + in_search_list = True + continue + if in_search_list and stripped == "End of search list.": + break + if not in_search_list or not stripped: + continue + if stripped.endswith("(framework directory)"): + path = stripped[: -len("(framework directory)")].strip() + if path and os.path.isdir(path): + paths.extend(["-F", path]) + continue + if os.path.isdir(stripped): + paths.extend(["-isystem", stripped]) + return paths + + +def compiler_include_flags(compiler, lang, std_flags): + if not compiler: + return [] + cmd = compiler + ["-E", "-x", lang] + std_flags + ["-v", "-"] + try: + proc = subprocess.run( + cmd, + input="", + text=True, + capture_output=True, + check=False, + ) + except OSError: + return [] + output = "\n".join(part for part in (proc.stderr, proc.stdout) if part) + return parse_include_search_paths(output) + + def build_flags(r_bin, lang, root): flags = [] r_home = None @@ -165,21 +312,27 @@ def build_flags(r_bin, lang, root): pkg = root / "RcppTskit" src_dir = pkg / "src" + makevars = find_makevars(src_dir) + vars_map = read_makevars(makevars) if makevars else {} + r_config = make_r_config_getter(r_bin) + context = makevars_context(vars_map, r_home) + cxx_std = cxx_std_setting(vars_map, context, r_config) if lang != "c" else "" - flags += makevars_flags(src_dir, r_home, lang) + flags += makevars_flags(src_dir, vars_map, context, r_config, lang) - cppflags = r_cmd_config(r_bin, "CPPFLAGS") + cppflags = r_config("CPPFLAGS") if cppflags: flags += shlex.split(cppflags) - if lang == "c": - cflags = r_cmd_config(r_bin, "CFLAGS") - if cflags: - flags += shlex.split(cflags) + flags_var, std_var = compiler_flag_vars(lang, cxx_std) + compiler_flags = r_config(flags_var) + if compiler_flags: + flags += shlex.split(compiler_flags) + if std_var: + std_flags = shlex.split(r_config(std_var)) + flags += std_flags else: - cxxflags = r_cmd_config(r_bin, "CXXFLAGS") - if cxxflags: - flags += shlex.split(cxxflags) + std_flags = [] rcpp_inc = rcpp_include_dir(r_bin) if rcpp_inc: @@ -196,6 +349,12 @@ def build_flags(r_bin, lang, root): if r_home: flags.append(f"-I{Path(r_home) / 'include'}") + flags += compiler_include_flags( + compiler_command(r_config, lang, cxx_std), + lang, + std_flags, + ) + flags = normalize_path_flags(flags, src_dir) if "-DNDEBUG" not in flags: diff --git a/RcppTskit/tools/clang_tidy_audit.yaml b/RcppTskit/tools/clang_tidy_audit.yaml new file mode 100644 index 0000000..a39270f --- /dev/null +++ b/RcppTskit/tools/clang_tidy_audit.yaml @@ -0,0 +1,13 @@ +--- +# Broader manual audit profile for occasional sweeps. This is intentionally +# wider than the pre-commit gate and may surface issues that require triage. +Checks: >- + clang-diagnostic-*, + clang-analyzer-*, + bugprone-*, + performance-*, + portability-*, + -bugprone-easily-swappable-parameters +WarningsAsErrors: "" +FormatStyle: none +SystemHeaders: false