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

Installation fails due to compiler flags #1

Closed
jonocarroll opened this issue Feb 24, 2024 · 11 comments
Closed

Installation fails due to compiler flags #1

jonocarroll opened this issue Feb 24, 2024 · 11 comments
Assignees

Comments

@jonocarroll
Copy link

This is odd, because I can see that there are no CRAN check fails, so presumably my own system has some peculiarity.

> install.packages("luajr")
Installing package into ‘/home/jono/R/x86_64-pc-linux-gnu-library/4.3’
(as ‘lib’ is unspecified)
trying URL 'https://cloud.r-project.org/src/contrib/luajr_0.1.6.tar.gz'
Content type 'application/x-gzip' length 1174922 bytes (1.1 MB)
==================================================
downloaded 1.1 MB

* installing *source* package ‘luajr’ ...
** package ‘luajr’ successfully unpacked and MD5 sums checked
** using staged installation
** libs
using C++ compiler: ‘g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0’
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -Iluajit/src      -fpic  -g -O2 -ffile-prefix-map=/build/r-base-H0vbME/r-base-4.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2  -c lua_api.cpp -o lua_api.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -Iluajit/src      -fpic  -g -O2 -ffile-prefix-map=/build/r-base-H0vbME/r-base-4.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2  -c parallel.cpp -o parallel.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -Iluajit/src      -fpic  -g -O2 -ffile-prefix-map=/build/r-base-H0vbME/r-base-4.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2  -c push_to.cpp -o push_to.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -Iluajit/src      -fpic  -g -O2 -ffile-prefix-map=/build/r-base-H0vbME/r-base-4.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2  -c registry_entry.cpp -o registry_entry.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -Iluajit/src      -fpic  -g -O2 -ffile-prefix-map=/build/r-base-H0vbME/r-base-4.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2  -c run_func.cpp -o run_func.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -Iluajit/src      -fpic  -g -O2 -ffile-prefix-map=/build/r-base-H0vbME/r-base-4.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2  -c setup.cpp -o setup.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -Iluajit/src      -fpic  -g -O2 -ffile-prefix-map=/build/r-base-H0vbME/r-base-4.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2  -c state.cpp -o state.o
make[1]: Entering directory '/tmp/RtmpDnfNIa/R.INSTALL24c8c1191a53cc/luajr/src/luajit'
==== Building LuaJIT 2.1 (amalgamation) ====
make -C src amalg
make[2]: Entering directory '/tmp/RtmpDnfNIa/R.INSTALL24c8c1191a53cc/luajr/src/luajit/src'
cc1: error: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Werror=format-security]
cc1: some warnings being treated as errors
make all "LJCORE_O=ljamalg.o"
make[3]: Entering directory '/tmp/RtmpDnfNIa/R.INSTALL24c8c1191a53cc/luajr/src/luajit/src'
cc1: error: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Werror=format-security]
cc1: some warnings being treated as errors
HOSTCC    host/minilua.o
cc1: error: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Werror=format-security]
cc1: some warnings being treated as errors
make[3]: *** [Makefile:719: host/minilua.o] Error 1
make[3]: Leaving directory '/tmp/RtmpDnfNIa/R.INSTALL24c8c1191a53cc/luajr/src/luajit/src'
make[2]: *** [Makefile:623: amalg] Error 2
make[2]: Leaving directory '/tmp/RtmpDnfNIa/R.INSTALL24c8c1191a53cc/luajr/src/luajit/src'
make[1]: *** [Makefile:165: amalg] Error 2
make[1]: Leaving directory '/tmp/RtmpDnfNIa/R.INSTALL24c8c1191a53cc/luajr/src/luajit'
make: *** [Makevars:19: luajit/src/libluajit.a] Error 2
ERROR: compilation failed for package ‘luajr’
* removing ‘/home/jono/R/x86_64-pc-linux-gnu-library/4.3/luajr’
Warning in install.packages :
  installation of package ‘luajr’ had non-zero exit status

The downloaded source packages are in
	‘/tmp/RtmpHnubmV/downloaded_packages’

I get the same errors trying with remotes::install_github("nicholasdavies/luajr") and if I clone the repo and try to build manually.

My global ~/.R/Makevars is empty, but my $R_HOME/etc/Makeconf does contain -Wformat -Werror=format-security and perhaps this isn't superseded during compilation of this package.

I'm running Pop!_OS but it's almost Ubuntu

> sessionInfo()
R version 4.3.2 (2023-10-31)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Pop!_OS 22.04 LTS

and have GNU Make

> make -v 
GNU Make 4.3
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2020 Free Software Foundation, Inc.

I did manage to get the package to build by changing the following line

CC="$(CC)" CFLAGS="$(CFLAGS) $(CPICFLAGS) -DLUAJIT_ENABLE_LUA52COMPAT -O2 -fomit-frame-pointer -Wno-pedantic -Wformat=0 -fno-sanitize=all" \

replacing -Wformat=0 with -Wno-format -Wno-format-security, and this then builds successfully and runs great

library(luajr)
sum_of_squares <- lua_func("
    function(n)
        local val = 0
        for i=1, n do
            val = val + i*i
        end
        return val
    end")
sum_of_squares(10)
#> [1] 385

Created on 2024-02-24 with reprex v2.0.2

@nicholasdavies
Copy link
Owner

Hi Jonathan, thanks for posting the first issue for this package!

I agree it seems like your flags are not getting overridden, but unfortunately I haven't been able to reproduce this on my own setup. I'm not sure whether the right thing to do would be to incorporate your changes to Makevars.in, or to just have a note somewhere that there are some flags that make the LuaJIT build fail, and that these are among them. There may well be lots of gcc flags that would cause the LuaJIT build to fail. Do you know how common it is to have the -Wformat -Werror=format-security flags set?

Do you know whether you need both of -Wno-format -Wno-format-security in Makevars.in for the build to work? And did you try -Wformat=0 -Wno-format-security? (The GCC manual says that -Wno-format and -Wformat=0 are synonyms, but it's possible they may not be on other compilers...) I'm trying to narrow down which of the flags (or both) on your system is causing the problem.

Thanks,

Nick

@nicholasdavies nicholasdavies self-assigned this Feb 24, 2024
@jonocarroll
Copy link
Author

I'll run some tests to see which flags are necessary (at least on my system). I figured having the issue here could be of help to someone in the same boat.

I will also try running a matrix of linux jobs on rhub to see if I can trigger the issue elsewhere. I notice that CRAN only tested fedora.

Great package, by the way - with really nice vignettes. I'm curating this week's RWeekly and want to bring some attention to this package.

@nicholasdavies
Copy link
Owner

Thank you so much! I'm very much willing to add the required flags to Makevars.in -- the fact that this has come up as a problem so early on in the package release history suggests that it is likely to be a widespread issue, so I'm keen to fix it!

@nicholasdavies
Copy link
Owner

Hi Jonathan,

Do you have any update? I can't replicate this on my machine when using your custom CFLAGS, so I need your help to resolve this.

I think I have come up with a new idea that might resolve this in a more general way, and if it works I would only need you to do one test for me. Could you try compiling the package with -Wno-format -Wno-error=format instead of your -Wno-format -Wno-format-security? From testing on my machine (with clang), the -Wno-error=format cancels out -Werror=format-security, which should allow luajr to compile on your machine (if clang acts similarly to gcc here), but will also "fix" a broader set of custom CFLAGS.

My thinking is that other users could well have other -Werror=format-XXX flags set besides -Werror=format-security, so I'd rather have a more general fix in place.

Thanks

Nick

@jonocarroll
Copy link
Author

Sorry - I got distracted.

Just now I removed the installed package and confirmed that rebuilding failed. I tried Wno-format -Wno-error=format but got the same installation error.

As a reproduction of this, I tried in docker and can indeed reproduce it using the rocker/r-base (ubuntu) image

$ docker run -it rocker/r-base

[...]

R> install.packages("luajr")
==== Building LuaJIT 2.1 (amalgamation) ====
make -C src amalg
make[2]: Entering directory '/tmp/Rtmp2KdHFX/R.INSTALL1a2b404e7a/luajr/src/luajit/src'
cc1: error: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Werror=format-security]
cc1: some warnings being treated as errors
make all "LJCORE_O=ljamalg.o"
make[3]: Entering directory '/tmp/Rtmp2KdHFX/R.INSTALL1a2b404e7a/luajr/src/luajit/src'
cc1: error: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Werror=format-security]
cc1: some warnings being treated as errors
HOSTCC    host/minilua.o
cc1: error: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Werror=format-security]
cc1: some warnings being treated as errors
make[3]: *** [Makefile:719: host/minilua.o] Error 1
make[3]: Leaving directory '/tmp/Rtmp2KdHFX/R.INSTALL1a2b404e7a/luajr/src/luajit/src'
make[2]: *** [Makefile:623: amalg] Error 2
make[2]: Leaving directory '/tmp/Rtmp2KdHFX/R.INSTALL1a2b404e7a/luajr/src/luajit/src'
make[1]: *** [Makefile:165: amalg] Error 2
make[1]: Leaving directory '/tmp/Rtmp2KdHFX/R.INSTALL1a2b404e7a/luajr/src/luajit'
make: *** [Makevars:19: luajit/src/libluajit.a] Error 2
ERROR: compilation failed for package ‘luajr’
* removing ‘/usr/local/lib/R/site-library/luajr’

These flags are a bit beyond my knowledge, but I threw the problem at ChatGPT and it suggests setting

-Wformat -Wformat-security 

which I can confirm does work, and presumably doesn't disable any of the error checking. In fact, just setting -Wformat in Makevars.in is sufficient (since -Wformat-security was coming from my default flags) - was there a reason you were turning that off? Looking again at my $R_HOME/etc/Makeconf I have -Wformat -Werror=format-security set so I suppose the issue was that -Wformat=0 is incompatible with the existing -Werror=format-security.

I can't check that in the R session of the docker image because the package sets its own flags, but one could drop to a shell and rebuild there for testing.

It's interesting that your GH Actions doesn't complain - I'm not sure what the difference is - perhaps the setup for that action doesn't set the same default compiler flags.

@nicholasdavies
Copy link
Owner

Thanks Jon! With the help of your notes, I can now reproduce the error using Docker. I'll see what I can do from here.

I disabled the warnings in the first place because (1) the warnings otherwise generated during the build of the embedded LuaJIT library — which I am not responsible for — are false positives, and therefore not helpful; (2) I was following advice in the "R packages book" to eliminate R CMD check warnings even if the fix is somewhat hacky.

Thanks

Nick

@nicholasdavies
Copy link
Owner

OK! Removing -Wformat entirely seems to have eliminated the error you found, with no ill effects that I can identify with either the Github build actions I have enabled or devtools::check_rhub().

It's possible that this change will reintroduce some obscure CRAN check issue that I can't reproduce myself, but I guess I'll find out when I update the package. Either way, I'll update here.

Thanks for your help! I have acknowledged your contribution in NEWS.md; if you have a spare minute to look at that, I would really appreciate you letting me know if that's appropriate, as I'm new to R community standards.

@jonocarroll
Copy link
Author

Removing -Wformat entirely seems to have eliminated the error you found

Do you mean removing -Wformat=0?

Ah - looking at the commits, 9cd58b1 probably fixed the issue because it "reset" the flag, and removing that entirely should be equivalent, so I'd say go with not messing with -Wformat at all if it doesn't cause any issues to do so. Great to hear it was helpful.

Thanks for the mention in NEWS - that looks great.

I'll keep playing around and see if there's any way I can usefully contribute. Do you have any plans for future improvements?

@nicholasdavies
Copy link
Owner

Do you mean removing -Wformat=0?

Yes, that's what I meant — first I tried replacing it with -Wformat as you suggested, then I tried eliminating the flag altogether, and both produced the same outcome so I went with the latter as the simplest solution.

Do you have any plans for future improvements?

The main one specifically for this package is that I'd like to have a similar mechanism to Rcpp for automatically importing Lua code into R, similarly to how the // [[Rcpp::export]] tag works. Doing this well will require some thought, though, primarily because neither Lua nor R are strongly typed, which means that the mechanism for bringing R objects into Lua and vice-versa requires more explicit grammar that is not directly supported in either language — contrast this with Rcpp, in which you can define function arguments and return values as Rcpp::NumericVector, Rcpp::NumericMatrix, etc. directly in C++.

"Arg codes" in luajr is how this is currently implemented, but the specific details of this are probably not as good as they can be.

Do you have any ideas?

@jonocarroll
Copy link
Author

Thinking of the other cross-language examples I know...

  • C: R's default .Call() mechanism relies on various #include <R*.h> to define types.

  • C++: {Rcpp} defines strong types and a translation to native structures.

  • Rust: {extendr} and {savvy} are the two major approaches and (IIRC) they have type-translations for the major R types to their own strongly typed versions.

  • Julia: {JuliaCall} perhaps looks the most like this package in that it does string evals rather than loading a source file, though it does have a simple julia_source() that just sources the file on the Julia side.

It's a shame there's no CRAN Task View for language interop.

I'll play around with this and see if I get anything interesting working.

@nicholasdavies
Copy link
Owner

Well, e-mail me first, as I have my own plans :)

nicholas.davies@lshtm.ac.uk.

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

No branches or pull requests

2 participants