-
-
Notifications
You must be signed in to change notification settings - Fork 217
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
Fix unwind tests on win-builder #801
Conversation
Ok -- I had turned this into a 0.14.7.1 and it just passed -- hooray! https://win-builder.r-project.org/251187D9Ozo1/00check.log (R-release) I'll just stay away from R Hub then... |
BTW what I had in modified inst/unitTests/runit.misc.R
@@ -18,6 +18,7 @@
# along with Rcpp. If not, see <http://www.gnu.org/licenses/>.
.runThisTest <- Sys.getenv("RunAllRcppTests") == "yes"
+.onLinux <- .Platform$OS.type == "unix" && unname(Sys.info()["sysname"]) == "Linux"
if (.runThisTest) { and then use |
This sounds reasonable. I completely disabled it because I don't understand the win-builder failure but on the other hand it does seem to consistently work on Linux. |
Oh for fuck's sake. I just cleaned it all up and called it 0.12.14.8, and 'check time: 0' means it died AGAIN. Just like on r-hub.
Over to, @lionel- . That is the pristine master branch. |
And of course the same on r-devel:
|
I think I will switch my hobby to herding goats or something as this is beyond comprehension. 0.12.14.7.1 passed, but 0.12.14.8 does not, yet there is just about nothing material in the diff: Binary files Rcpp-0.12.14.7.1/build/Rcpp.pdf and Rcpp-0.12.14.8/build/Rcpp.pdf differ
diff -ru Rcpp-0.12.14.7.1/ChangeLog Rcpp-0.12.14.8/ChangeLog
--- Rcpp-0.12.14.7.1/ChangeLog 2018-01-14 15:35:29.000000000 -0600
+++ Rcpp-0.12.14.8/ChangeLog 2018-01-14 17:05:14.000000000 -0600
@@ -1,8 +1,16 @@
2018-01-14 Dirk Eddelbuettel <edd@debian.org>
+ * DESCRIPTION (Version, Date): New minor version 0.12.14.8
+
* inst/include/Rcpp/traits/is_na.h (Rcpp): Also speed up
Rcpp::is_na<Rcomplex>
+2018-01-14 Lionel Henry <lionel@rstudio.com>
+
+ * inst/unitTests/cpp/stack.cpp: Move stack unwinding tests from misc.cpp
+ * inst/unitTests/runit.stack.R: Move stack unwinding tests from misc.R;
+ disaggregate stack unwinding tests; also disable for now
+
2018-01-11 Kirill Müller <krlmlr@mailbox.org>
* inst/include/Rcpp/traits/is_na.h: Speed up Rcpp::is_na<double>()
diff -ru Rcpp-0.12.14.7.1/DESCRIPTION Rcpp-0.12.14.8/DESCRIPTION
--- Rcpp-0.12.14.7.1/DESCRIPTION 2018-01-14 15:46:03.000000000 -0600
+++ Rcpp-0.12.14.8/DESCRIPTION 2018-01-14 17:34:01.000000000 -0600
@@ -1,6 +1,6 @@
Package: Rcpp
Title: Seamless R and C++ Integration
-Version: 0.12.14.7.1
+Version: 0.12.14.8
Date: 2018-01-14
Author: Dirk Eddelbuettel, Romain Francois, JJ Allaire, Kevin Ushey, Qiang Kou,
Nathan Russell, Douglas Bates and John Chambers
@@ -24,4 +24,4 @@
MailingList: Please send questions and comments regarding Rcpp to rcpp-devel@lists.r-forge.r-project.org
RoxygenNote: 6.0.1
NeedsCompilation: yes
-Packaged: 2018-01-14 21:46:03.86255 UTC; edd
+Packaged: 2018-01-14 23:34:01.211863 UTC; edd
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-attributes.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-attributes.pdf differ
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-extending.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-extending.pdf differ
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-FAQ.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-FAQ.pdf differ
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-introduction.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-introduction.pdf differ
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-jss-2011.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-jss-2011.pdf differ
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-modules.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-modules.pdf differ
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-package.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-package.pdf differ
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-quickref.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-quickref.pdf differ
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-sugar.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-sugar.pdf differ
Binary files Rcpp-0.12.14.7.1/inst/doc/Rcpp-unitTests.pdf and Rcpp-0.12.14.8/inst/doc/Rcpp-unitTests.pdf differ
diff -ru Rcpp-0.12.14.7.1/inst/NEWS.Rd Rcpp-0.12.14.8/inst/NEWS.Rd
--- Rcpp-0.12.14.7.1/inst/NEWS.Rd 2018-01-14 15:35:29.000000000 -0600
+++ Rcpp-0.12.14.8/inst/NEWS.Rd 2018-01-14 17:06:19.000000000 -0600
@@ -11,6 +11,9 @@
set an initial format string (Dirk in \ghpr{777} fixing \ghit{776}).
\item The 'new' Date and Datetime vectors now have \code{is_na} methods
too. (Dirk in \ghpr{783} fixing \ghit{781}).
+ \item Protect more temporary \code{SEXP} objects produced by \code{wrap}
+ (Kevin in \ghpr{784}).
+ \item Use public R APIs for \code{new_env} (Kevin in \ghpr{785}).
\item Evaluation of R code is now safer when compiled against R
3.5 (you also need to explicitly define \code{RCPP_PROTECTED_EVAL}
before including \code{Rcpp.h}). Longjumps of all kinds (condition
@@ -36,6 +39,10 @@
\item Permit compilation on ANDROID (Kenny Bell in \ghpr{796}).
\item Improve support for NVCC, the CUDA compiler (Iñaki Ucar in
\ghpr{798} addressing \ghit{797}).
+ \item Speed up tests for NA and NaN (Kirill and Dirk in \ghpr{799} and
+ \ghpr{800}).
+ \item Rearrange stack unwind test code, keep test disabled for now (Lionel
+ in \ghpr{801}).
}
\item Changes in Rcpp Attributes:
\itemize{ |
Sounds like it's undeterministic :/ Did this also happen with a version anterior to the protection stuff? |
I am so bloody confused. I am re-submitting 0.12.14.7.1 as 0.12.14.9 just to check. It passed as the former, I just want to rule out that the 5 tokens matterd. "Usually" we do a.b.c for cran, and that does fewer checks, and a,b,c.d for tests. The comparison should just be > 3 and not have an impact. But I am generally confused now. |
Does anybody have an idea how to repair this to use the code? @lionel- @krlmlr @kevinushey ? At best as I can tell, we seem to be in a semi-random situation where the code sometimes fails on Windows, and sometimes it doesn't. My inclination to undo #789 / condition it away. Better suggestions? |
So the tarball of the current master
WTF? |
I would vote that we take out the protected evaluation stuff for now altogether. FWIW when running the Rcpp unit tests within RStudio on my macOS machine, I see that 'sometimes' things blow up (RStudio gets into an infinite loop printing error messages) so there may well be something non-deterministic that's causing errors here. (Maybe something that should be getting protected from the garbage collector isn't?) Since @lionel- is on vacation I'll try posting to R-devel to see if they have any comments. |
I was thinking that too. Ie post-process Lionel's patch with a few |
So far I have 3 out of 5 successful builds of b52f690 on WinBuilder. |
I've contacted Luke Tierney to ask for some more guidance. FWIW I saw this ASAN error locally today with my
so I suspect there may be something going on (a missed |
@eddelbuettel: right; I think we should just guard the protected eval with #if false // disabled until dust around R_UnwindProtect has settled or something similar. |
I think I add an Note that I am pretty certain that I also saw crappy outcomes with R-release. How about your experiments on macOS? Trouble with both, or only R-devel? |
All 5 builds good. Was I testing the wrong version? |
No, you just have better karma than us. Who knows maybe it also depends on the state of the machine. Did you send five to both R versions, or just one each? |
To both. |
And you got ten winners? Astounding ... |
With check times of ~1500s, though. That's different from CRAN's check times of 500s max, but might be a different machine too. |
Something is going over there because I got several reports about a top-level file |
I saw these too, but then it seems fairly unlikely that these have any effect (except for the longer check times, of course). But who knows? Are you getting e-mails from WinBuilder even when I change the e-mail address when submitting? |
No, and I am pretty sure I got mine (as I ended up with new/unused minor versions). But "they" did some rejigging over the break anyway (or tried to) so who knows. |
@kevinushey About the ASAN warning, it looks like one of the SEXP in the SETCAR in @krlmlr Maybe the double time is because on r-devel the examples and tests are run on both i386 and x64 arch. On r-release only the examples are run on both arch. |
@lionel- No, |
@lionel- the ASAN warning is not just a warning -- it leads to a (intermittent) crash / infinite loop / unpredictable behavior when run in R without sanitizers available. The API also does not work on Windows with 32bit versions of R. |
Got it, I'm not familiar with this ASAN/UBSAN thing yet. Since it's about one of the SEXP being a C NULL (if I understand the error correctly), I don't think this is about a protection issue though. |
I recently watched a truly awesome talk on the sanitizers -- including the newer ones on memory races and what not. But I can't find it right now :-/ Likely from cppcon, maybe from 2016. The various *SAN are the closest we have to a free lunch in C++ land. CRAN is right in pushing their use. |
No description provided.