cran check issues

[sckott]

got email from cran

Please correct all the issues (including the memory-access ones) ASAP.

https://cran.r-project.org/web/checks/check_results_jqr.html

The only platforms with errors are Solaris sparc https://www.r-project.org/nosvn/R.check/r-patched-solaris-sparc/jqr-00check.html and Solaris x86 https://www.r-project.org/nosvn/R.check/r-patched-solaris-x86/jqr-00check.html

A few other platforms having warnings, which are all

checking whether package ‘jqr’ can be installed ... WARNING
Found the following significant warnings:
  execute.c:55:21: warning: ISO C forbids zero-size array ‘entries’ [-Wpedantic]
  parser.y:178:72: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
  jq/execute.c:55:21: warning: ISO C forbids zero-size array ‘entries’ [-Wpedantic]
See http://www.r-project.org/nosvn/R.check/r-release-linux-x86_64/jqr-00install.html for details.

[richfitz]

These are super annoying to deal with, given they're in jq code, some of which is the autogenerated parser.

On OSX with clang and -pedantic I see

gcc -I/Library/Frameworks/R.framework/Resources/include -DNDEBUG -Ijq -pedantic -I/usr/local/include -I/usr/local/include/freetype2 -I/opt/X11/include -I"/Library/Frameworks/R.framework/Versions/3.2/Resources/library/BH/include" -I"/Library/Frameworks/R.framework/Versions/3.2/Resources/library/Rcpp/include"   -fPIC  -Wall -mtune=core2 -g -O2  -c jq/execute.c -o jq/execute.o
jq/execute.c:55:29: warning: zero size arrays are an extension
      [-Wzero-length-array]
  union frame_entry entries[0]; 
gcc -I/Library/Frameworks/R.framework/Resources/include -DNDEBUG -Ijq -pedantic -I/usr/local/include -I/usr/local/include/freetype2 -I/opt/X11/include -I"/Library/Frameworks/R.framework/Versions/3.2/Resources/library/BH/include" -I"/Library/Frameworks/R.framework/Versions/3.2/Resources/library/Rcpp/include"   -fPIC  -Wall -mtune=core2 -g -O2  -c jq/parser.c -o jq/parser.o
parser.y:178:38: warning: must specify at least one argument for '...' parameter
      of variadic macro [-Wgnu-zero-variadic-macro-arguments]
  return BLOCK(a, gen_call("format", BLOCK(gen_lambda(gen_const(fmt)))));

[sckott]

meaning that we'd have to fix jq code itself?

[richfitz]

Yeah. This isn't code we wrote. The issue is that because we link the library in directly we're subject to all the cran checks. But jq is so weird that we really can't list it as a SystemDependency because nobody will have it! (I don't think it even installs as a library by default).

[sckott]

@jeroenooms any idea about the Solaris errors? never used Solaris myself

I'm not sure what the way forward is here. Should we ask CRAN folks if they can forget about these warnings b/c it's in jq code and we don't have control over that?

I imagine Ripley won't let the Solaris errors slide?

[jeroen]

Best approach would be to try and get the jq errors fixed upstream. Which version of jq are you bundling?

[sckott]

we're using v1.4 of jq

[sckott]

so fixed in jq? hmm, i guess it could take a while, hopefully ripley doesn't make us take it off CRAN in meantime

[jeroen]

Looks like the problem in execute.c has been fixed in the latest version of jq: https://github.com/stedolan/jq/blob/master/src/execute.c#L67

[jeroen]

Why are you not including jq 1.5?

[sckott]

Why are you not including jq 1.5?

@richfitz i thought for some reason 1.5 wasn't working, correct?

[richfitz]

Yeah, I think I couldn't get it to compile with gcc on windows, from memory.

[sckott]

hmm, this might take a while. I'll ask for more time

[jeroen]

Actually jq is in debian and fedora and homebrew now, so could link to that instead of bundling it....

[sckott]

I guess that would take a re-write - would linking to jq instead still work on windows?

[richfitz]

So long as we can build a windows version of the library via Jeroen's stash of those that should work. That could give us 1.5 which would be nice, too.

[jeroen]

Sure I'll send a PR to review. Do you guys want to build with oniguruma support or are you not using that?

[sckott]

awesome, thanks

[jeroen]

Ugh it looks like Debian/Ubuntu only provides the jq command line but not the libjq headers and library :-( So this might not work ...

[jeroen]

I'm talking to the maintainer of jq in Debian now to see if we can get it in there ...

[jeroen]

OK the Debian maintainer is going to look at it. Might take a few days. Once things are in Debian we can remove jq from jqr and never have to worry about it again.

[sckott]

great, Kurt said okay to me asking for more time...

[sckott]

@jeroenooms Kurt is getting impatient, and wants us to fix the compilation warnings like

Found the following significant warnings:
  execute.c:55:21: warning: ISO C forbids zero-size array ‘entries’ [-Wpedantic]
  parser.y:178:72: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
  jq/execute.c:55:21: warning: ISO C forbids zero-size array ‘entries’ [-Wpedantic]

and submit a new versions. Can we do that somewhat easily?

[jeroen]

Sure we can workaround these things until libjq becomes available.

[richfitz]

With #45 merged, this should probably be good to go, Scott.

[sckott]

thanks all, will submit patch version

[sckott]

submitted

[richfitz]

Brilliant, thanks Scott

[sckott]

Kurt rejected, said he still gets

* checking whether package ‘jqr’ can be installed ... [42s/42s] WARNING
Found the following significant warnings:
  execute.c:55:21: warning: ISO C forbids zero-size array ‘entries’ [-Wpedantic]
  parser.y:178:72: warning: ISO C99 requires at least one argument for the "..." in a variadic macro

Said

Debian testing with GCC 5.3, and have a ~/.R/Makevars with

CFLAGS = -g -O2 -Wall -Wno-unused -pedantic
CXXFLAGS = -g -O2 -Wall -Wno-unused -pedantic
CXX1XFLAGS = -g -O2 -Wall -Wno-unused -pedantic
FFLAGS = -g -O2 -fbounds-check -mtune=native
FCFLAGS = -g -O2 -fbounds-check -mtune=native

any ideas?

[sckott]

i tried these flags but couldn't get these warnings to show. I do have a different gcc version though

@jeroenooms

[jeroen]

I can't reproduce this either. It's really strange, those errors seem very unlikely on new package. It almost looks as if he was checking the wrong file or so. I'll have another look later...

[sckott]

thanks!

[jeroen]

I don't understand how this happened, but the new 0.2.2 version on cran does not include the fixes that are on master. Note that master on line execute.c:55 has:

union frame_entry entries[]; 

However the 0.2.2 version on CRAN still has the old version with the bug.

union frame_entry entries[0]; 

I suspect this line in your .gitignore might prevent you from pulling in the fixed sources, and you keep resubmitting old versions.

## jq sources
src/*.c

Not sure what that line is doing there.

So we are going to resubmit yet again, but with the fixed jqr sources. Perhaps after you generate the source package make sure to double check that it contains the fixes by extracting the tarball and inspecting execute.c:55 manually.

[sckott]

Looks like that line in gitignore was added here https://github.com/ropensci/jqr/commit/4c178f679caa538b70f2ce5805858e8e8b893136

[sckott]

I removed the src/*.c line and applied the backport fixes to *.c files

[sckott]

ripley emailed

First, it seems to have an unstated SystemRequirements which excludes
Sparc, and a malformed check in the code (it should start with #error).
  But the CRAN policies do say

'Package authors should make all reasonable efforts to provide
cross-platform portable code.'

and it seems all you really need is the endianness, which the R headers
do make available.

For the x86 machine, your test report is meaningless to me, so you at
least need to work on reporting some context.  (There is nothing more in
tests/testthat.Rout.fail.)  Also, that the CRAN policies require

   - Compiled code should never terminate the R process within which it
is running.

and it seems you are violating that

@jeroenooms

• What is the unstated SystemRequirements?
• I thought you did include a fix for Solaris - checks are here https://cran.r-project.org/web/checks/check_results_jqr.html
• Compiled code should never terminate the R process is this

 terminate called after throwing an instance of 'Rcpp::exception'
    what(): error: Invalid numeric literal (while parsing '...')

for x86 on solaris

The Solaris sparc check fails on install