Skip to content
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

Strengthen CRAN_Release.cmd to find UBSAN errors in data.table.Rcheck #5509

Merged
merged 4 commits into from
Nov 2, 2022

Conversation

mattdowle
Copy link
Member

@mattdowle mattdowle commented Nov 2, 2022

Closes #5496

Made many attempts to capture UBSAN errors on stderr but nothing worked. sink()-ing type="message" does work but the UBSAN errors still appeared in the output rather than the sink'd file. Also tried a Linux only stat /proc/self/fd/2 before and after the test to see if the timestamp of stderr had increased, with no luck. Setting UBSAN_OPTIONS=log_path=.... is possible but seems too UBSAN specific; I was hoping to capture anything on stderr to include ASAN too for example and any other tool that might arise in future.

So in the end, just strengthened CRAN_Release.cmd to do a full R CMD check under UBSAN/ASAN rather than just test.data.table(). So now .Rd examples and vignettes are covered too. And included a grep on data.table.Rcheck/*.Rout for the UBSAN errors. Can refine it in future.

@mattdowle mattdowle added this to the 1.14.5 milestone Nov 2, 2022
@mattdowle mattdowle mentioned this pull request Nov 2, 2022
@mattdowle mattdowle merged commit 3f19061 into master Nov 2, 2022
@mattdowle mattdowle deleted the stderr branch November 2, 2022 02:15
isTRUE(.Machine$sizeof.longdouble==0) # check noLD is being tested
options(repos = "http://cloud.r-project.org")
install.packages(c("bit64","xts","nanotime","R.utils","yaml")) # minimum packages needed to not skip any tests in test.data.table()
# install.packages(c("curl","knitr")) # for `R CMD check` when not strict. Too slow to install when strict
install.packages(c("bit64", "bit", "curl", "R.utils", "xts","nanotime", "zoo", "yaml", "knitr", "rmarkdown"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe set Ncpus if this is really so slow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not so slow anymore. Didn't time it but under 30 mins I think. Could be that clang's ubsan is faster than gcc's perhaps. I tended to use gcc in the past. Anyway, Ncpus is a good idea, should use it regardless.

Copy link
Member

@MichaelChirico MichaelChirico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. getting closer to what CRAN actually does seems the best route, even a few extra hours spent building/testing is worth it

@jangorecki jangorecki modified the milestones: 1.14.9, 1.15.0 Oct 29, 2023
ben-schwen added a commit that referenced this pull request Dec 21, 2023
MichaelChirico added a commit that referenced this pull request Dec 22, 2023
* add multiple containers

add symlinks

add RUN

use echo to create .Rprofile

add workdir

symlink + echo

add dependencies for check

add char vec

update echo

update dockerfile

fix FROM

update dirs

use rocker image

rollback to jans clang image

change docker order

change flags

update makevars

update flags

add workdir

change C version

clang working

rm local .Rprofile

* tidy spaces

* update flags to #5509

* Robust deps install, use files over echo, tidy

* no ARG

* Wrong context?

* need to COPY DESCRIPTION. Try more WORKDIR.

* attempt to fix Makevars, .Rprofile

* Try copying .Rprofile to /root. Remove ~/GitHub for now.

* same changes to gcc container

* Try adding REdtiorSupport extension by default

* load cc conditionally

* Don't install languageserver by default (slow)

* udpate image and makevars

* drop native

* terminal newline

* whitespace

* whitespace

---------

Co-authored-by: Michael Chirico <michaelchirico4@gmail.com>
Co-authored-by: Michael Chirico <chiricom@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test() could catch UBSAN errors
3 participants