setup.sh: drop the python3 R-source patch step (fixed upstream in #360) - #26
Merged
Conversation
setup.sh tweaked the copied R binding sources (zzz.R, *KrigingClass.R) with two inline `python3` heredocs. On a minimal build image with no python3 (e.g. rocker/r-ver), `set -eo pipefail` made the first heredoc abort setup.sh *before* it copied src/*.cpp and NAMESPACE. R CMD build then wrote a default NAMESPACE and Rcpp::compileAttributes() had no sources to scan, so the installed package shipped with zero .Call bindings (new_KrigingFit, optim_set_*, ...). Every Kriging() call then failed at runtime with `could not find function "new_KrigingFit"`. Only a clean install hits this (install_github, or CRAN-version mismatch via a loader that pins a tag); a host that already has rlibkriging installed never re-runs setup.sh, which is why it isn't seen on dev machines. - Port both heredocs to tools/patch-r-sources.R. R is always present when building an R package, and setup.sh already shells out to Rscript elsewhere. Output is byte-identical to the python on the current zzz.R and every *KrigingClass.R. - Copy src/*.cpp and NAMESPACE *before* the patch step, and run the patch with `|| echo WARNING` so a failure in this cosmetic step can never again yield a bindings-less package with a default NAMESPACE. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A5w4ZLUZpoHhtsbGyWcVtc
The original python heredoc would append a second `#' unlink(outfile)` if run over an already-patched KrigingClass.R (the inserted line itself matches `#' .*[,(]outfile`). Skip lines that already contain `unlink(outfile)` so re-running tools/setup.sh is a no-op -- relevant once the same fix lands in libKriging/libKriging#360 and the source already carries the line. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A5w4ZLUZpoHhtsbGyWcVtc
…#360 libKriging/libKriging#360 moved the three tweaks (zzz.R ::: self-call, setOldClass("WarpKriging"), unlink(outfile) in save/load.Kriging examples) into the binding sources. With src/libK bumped to include it there is nothing left to patch after the copy: - remove tools/patch-r-sources.R and the patch invocation from setup.sh - src/libK: 6abd0dd -> 3a52334 (master incl. #360), tools/gitmodules-shas updated to match Net effect of this PR is now just: setup.sh no longer runs the two python3 heredocs (or any interpreter) to post-process the copied R sources, and src/*.cpp + NAMESPACE are copied up front so a future post-copy step can never again abort setup.sh before them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A5w4ZLUZpoHhtsbGyWcVtc
yannrichet
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Follow-up to libKriging/libKriging#360 (merged).
Background
tools/setup.shpost-processed the copied R binding sources with two inlinepython3heredocs — cosmetic fixes only (anR CMD check:::NOTE, asetOldClassstartup warning,#' unlink(outfile)in examples). On a build image withoutpython3(e.g.rocker/r-ver),set -eo pipefailabortedsetup.shbefore it copiedsrc/*.cppandNAMESPACE;R CMD buildthen fell back to a default NAMESPACE,Rcpp::compileAttributes()had nothing to scan, and the package installed with zero.Callbindings — everyKriging()call failing at runtime withcould not find function "new_KrigingFit". Only clean installs hit it (install_github, or a loader pinning a tag past the CRAN version).Change
#360 moved all three tweaks into the libKriging binding sources, so there is nothing left to patch after the copy:
tools/setup.sh— remove bothpython3heredoc blocks.src/*.cppandNAMESPACEare now copied immediately after the R sources, so no post-copy step can abort before them.tools/patch-r-sources.R(was the interim Rscript port; no longer needed).src/libK:6abd0dd→3a52334(libKrigingmasterincl. #360);tools/gitmodules-shasupdated to match.Net:
tools/setup.sh−71 lines, no interpreter dependency, and the bindings-less-package failure mode is gone.(The branch has a few intermediate commits from when this PR carried the Rscript port — squash-merge for a clean history.)
🤖 Generated with Claude Code
https://claude.ai/code/session_01A5w4ZLUZpoHhtsbGyWcVtc