Closed eddelbuettel closed 3 years ago
This is not an R-hub config, it is coming from the default CRAN installer:
❯ R-4.1 CMD config CC
clang -mmacosx-version-min=10.13
So I am not sure what we can do about this. Maybe you can change it in the package Makevars
file?
Ah, thanks for CC hint. Odd that it is asymmetric between x86_64 and M1 then.
@s-u is this in fact the staus quo, i.e. 10.13 on non-M1 but no constraint on M1?
I will try and and see if I can set CC and CXX* appropriately. I had so far only fiddled with PKG_CPPFLAGS and PKG_LIBS.
The first macOS version that supports M1 is 11.0.
There is some more, in case you need it:
❯ grep mmacosx-version /Library/Frameworks/R.framework/Versions/4.1/Resources/etc/Makeconf
# configure 'CC=clang -mmacosx-version-min=10.13' 'CXX=clang++ -mmacosx-version-min=10.13' 'OBJC=clang -mmacosx-version-min=10.13' 'FC=gfortran -mmacosx-version-min=10.13' 'F77=gfortran -mmacosx-version-min=10.13' 'CFLAGS=-Wall -g -O2' 'CXXFLAGS=-Wall -g -O2' 'OBJCFLAGS=-Wall -g -O2' 'FCFLAGS=-Wall -g -O2' 'F77FLAGS=-Wall -g -O2' '--enable-memory-profiling' '--x-libraries=/opt/X11/lib' '--x-includes=/opt/X11/include' '--enable-R-framework' '--build=x86_64-apple-darwin17.0' 'build_alias=x86_64-apple-darwin17.0' 'PKG_CONFIG_PATH=/usr/lib/pkgconfig:/usr/local/lib/pkgconfig:/opt/X11/lib/pkgconfig'
CC = clang -mmacosx-version-min=10.13
CXX = clang++ -mmacosx-version-min=10.13 -std=gnu++14
CXX11 = clang++ -mmacosx-version-min=10.13
CXX14 = clang++ -mmacosx-version-min=10.13
CXX17 = clang++ -mmacosx-version-min=10.13
CXX20 = clang++ -mmacosx-version-min=10.13
FC = gfortran -mmacosx-version-min=10.13
OBJC = clang -mmacosx-version-min=10.13
OBJCXX = clang++ -mmacosx-version-min=10.13
This is for R 4.1.x.
Yes, thanks, checked here and am now trying with CXX17
set appropriately (as we also set CXX_STD
to it).
And FWIW our package builds just fine at M1builder.
No luck yet (and I added a head -10 src/Makevars
to the end of configure.ac
to more easily check):
378#> ## We need C++17 to use TileDB's C++ API
379#> CXX_STD = CXX17
380#> ## For macOS aka Darwin need to set minimum version 10.14 for macOS
381#> CXX17 = clang++ -mmacosx-version-min=10.13
382#> ## We need the TileDB Headers
383#> PKG_CPPFLAGS = -I../inst/include/ -I../inst/tiledb/include
384#> ## We also need the TileDB library
385#> ** libs
386#> clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/uservLYeankA/R/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c RcppExports.cpp -o RcppExports.o
387#> clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/uservLYeankA/R/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c arrowio.cpp -o arrowio.o
388#> In file included from arrowio.cpp:29:
389#> In file included from ../inst/tiledb/include/tiledb/arrowio:92:
390#> ../inst/tiledb/include/tiledb/arrow_io_impl.h:391:45: error: call to unavailable member function 'value': introduced in macOS 10.14
391#>
but that may be on my end. R CMD check --as-cran
does not complain about trying to set CXX17
so I'll try some more.
It is as I (vaguely) remembered that you cannot override from within:
378#> ## We need C++17 to use TileDB's C++ API
379#> CXX_STD = CXX17
380#> ## For macOS aka Darwin need to set minimum version 10.14 for macOS
381#> CXX17 = clang++ -mmacosx-version-min=10.14
382#> ## We need the TileDB Headers
383#> PKG_CPPFLAGS = -I../inst/include/ -I../inst/tiledb/include
384#> ## We also need the TileDB library
385#> ** libs
386#> clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/userp3WH9jR3/R/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c RcppExports.cpp -o RcppExports.o
387#> clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/userp3WH9jR3/R/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c arrowio.cpp -o arrowio.o
A colleague suggested we inject MACOSX_DEPLOYMENT_TARGET=10.13
to maybe for a failure at the CI setting (for macOS still at Azure) but no luck: there it still builds. Gaaa.
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c RcppExports.cpp -o RcppExports.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c arrowio.cpp -o arrowio.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c deprecation.cpp -o deprecation.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c durations.cpp -o durations.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c libtiledb.cpp -o libtiledb.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c nullable.cpp -o nullable.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c utilities.cpp -o utilities.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup -single_module -multiply_defined suppress -L/Library/Frameworks/R.framework/Resources/lib -L/usr/local/lib -o tiledb.so RcppExports.o arrowio.o deprecation.o durations.o libtiledb.o nullable.o utilities.o -ltiledb -L../inst/tiledb/lib -Wl,-rpath,$ORIGIN/../tiledb/lib -F/Library/Frameworks/R.framework/.. -framework R -Wl,-framework -Wl,CoreFoundation
ld: warning: dylib (../inst/tiledb/lib/libtiledb.dylib) was built for newer macOS version (10.14) than being linked (10.13)
if [ "" != "Windows_NT" ] && [ `uname -s` = 'Darwin' ] && [ -f ../inst/tiledb/lib/libtiledb.dylib ] && [ -f tiledb.so ]; then install_name_tool -change libz.1.dylib @rpath/libz.1.dylib ../inst/tiledb/lib/libtiledb.dylib; install_name_tool -add_rpath @loader_path/../tiledb/lib tiledb.so; fi
installing to /Users/runner/work/1/s/tiledb.Rcheck/00LOCK-tiledb/00new/tiledb/libs
** R
** inst
We were able to sort it out via src/Makevars.in
and proper compiler and linker flags. Let's see if CRAN lets us spec a minimum version but I did state it in the system requirements and C++17 is not per se ruled out and used by others ... fingers crossed.
For reference, our changes are in the TileDB-Inc/TileDB-R repo here and will be in the next version, likely 0.9.8.
In general, any hard requirement on something that is not on available on 10.13 will fail, since the CRAN build is actually done on macOS 10.13. If it is an optional requirement (weak linking, for example) then that will work if the code expects it. If I recall binary format did not change in 10.14 so in principle binaries are compatible (so long as there is not some link-time symbol dependency issue).
@eddelbuettel I figured I'll try, but I don't even get as far as linking on the CRAN build machine (TileDB-R master @3310f07):
* installing *source* package ‘tiledb’ ...
** using staged installation
checking for g++... g++
checking whether the C++ compiler works... yes
[...]
downloading x86_64 TileDB library...
installing TileDB for Darwin:17.7.0...
downloading https://github.com/TileDB-Inc/TileDB/releases/download/2.4.3/tiledb-macos-x86_64-2.4.3-5cb0824.tar.gz
using inst/tiledb/{lib,include}
[...]
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -I'/Volumes/Builds/packages/high-sierra-x86_64/Rlib/4.1/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c utilities.cpp -o utilities.o
shmem.cpp:40:10: fatal error: 'filesystem' file not found
#include <filesystem>
^~~~~~~~~~~~
1 error generated.
make: *** [shmem.o] Error 1
make: *** Waiting for unfinished jobs....
ERROR: compilation failed for package ‘tiledb’
AFAIR filesystem is an optional part of the standard and only reliably present with Boost. We can take this to a TileDB-R issue or e-mail if you wish to discuss further...
It works for us (e.g. in CI at Azure Pipelines) when inject the proper minimum style, i.e. step one is
if test x"${uname}" = x"Darwin"; then
AC_MSG_CHECKING([for Darwin minimum version override])
CXX17_MACOS="-mmacosx-version-min=10.14"
AC_MSG_RESULT([${CXX17_MACOS}])
fi
[...]
AC_SUBST([CXX17_MACOS])
and step is just
## We need C++17 to use TileDB's C++ API
CXX_STD = CXX17
## For macOS aka Darwin need to set minimum version 10.14 for macOS
PKG_CXX17FLAGS = @CXX17_MACOS@
## We need the TileDB Headers
PKG_CPPFLAGS = -I../inst/include/ @TILEDB_INCLUDE@
## We also need the TileDB library
PKG_LIBS = @CXX17_MACOS@ @TILEDB_LIBS@ @TILEDB_RPATH@
And we use two parts of C++17 that are more advanced (than say the vanilla C++17 from Lemire in RcppSimdJson): std::optional
(in our Arrow interface) and std::fs
(for convenience).
Our core library standard is now C++17. R 4.2.0 will default to C++14 as a minimum so might be nice to support more than the old 11.13 here.
For example one such build is at here at Azure and then 'min 10.14' to the right wins over the 'min 10.13' from your setup:
config.status: creating src/Makevars
** libs
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -mmacosx-version-min=10.14 -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c RcppExports.cpp -o RcppExports.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -mmacosx-version-min=10.14 -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c arrowio.cpp -o arrowio.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -mmacosx-version-min=10.14 -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c deprecation.cpp -o deprecation.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -mmacosx-version-min=10.14 -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c durations.cpp -o durations.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -mmacosx-version-min=10.14 -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c libtiledb.cpp -o libtiledb.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -mmacosx-version-min=10.14 -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c nullable.cpp -o nullable.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -mmacosx-version-min=10.14 -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c shmem.cpp -o shmem.o
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include/ -I../inst/tiledb/include -mmacosx-version-min=10.14 -I'/Users/runner/work/1/R/library/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c utilities.cpp -o utilities.o
shmem.cpp:43:19: warning: unused variable 'debug' [-Wunused-const-variable]
static const bool debug = false;
^
1 warning generated.
clang++ -mmacosx-version-min=10.13 -std=gnu++17 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup -single_module -multiply_defined suppress -L/Library/Frameworks/R.framework/Resources/lib -L/usr/local/lib -o tiledb.so RcppExports.o arrowio.o deprecation.o durations.o libtiledb.o nullable.o shmem.o utilities.o -mmacosx-version-min=10.14 -ltiledb -L../inst/tiledb/lib -Wl,-rpath,$ORIGIN/../tiledb/lib -F/Library/Frameworks/R.framework/.. -framework R -Wl,-framework -Wl,CoreFoundation
if [ "" != "Windows_NT" ] && [ `uname -s` = 'Darwin' ] && [ -f ../inst/tiledb/lib/libtiledb.dylib ] && [ -f tiledb.so ]; then install_name_tool -change libz.1.dylib @rpath/libz.1.dylib ../inst/tiledb/lib/libtiledb.dylib; install_name_tool -add_rpath @loader_path/../tiledb/lib tiledb.so; fi
installing to /Users/runner/work/1/s/tiledb.Rcheck/00LOCK-tiledb/00new/tiledb/libs
** R
** inst
@eddelbuettel But that is a 10.15 machine.
It cannot work on CRAN and on R-hub's High Sierra builders because they are actually 10.13 machines.
So if CRAN publishes this, then the package will not have (intel) macOS binaries on CRAN. If that is important at all.
We have made no decision on R 4.2.0 target yet, it's a bit early. Unfortunately, the old macOS numbers are not going down as quickly as one would hope, that's mostly due to the fallout of the Catalina (10.15) release which broke so many things (mostly due to security measures) that there are many hold-outs unwilling to upgrade. We'll discuss the plans probably early next year.
That said, the version-min is mostly irrelevant. It does not affect the compilation, it is only used to flag the compatibility of the resulting binary. What is more relevant is what is available and hence the above error.
Ah well, it works in M1. :disappointed: Would be a bummer to lose all Intel macOS builds.
This is all for the soon-but-not-quite release of TileDB 2.5.0. It looks like I will prepare another release of the tiledb
R package using only TileDB 2.4.3 which should build as before.
@gaborcsardi Can I take the macos-highsierra-release-cran
RHub machine as a as-faithful-as-we-can-given-sparse-information proxy for CRAN? If we get it to build there, it will build at CRAN?
@gaborcsardi Can I take the macos-highsierra-release-cran RHub machine as a as-faithful-as-we-can-given-sparse-information proxy for CRAN? If we get it to build there, it will build at CRAN?
Obviously, it is impossible to know for sure, but yes, almost surely it will.
@gaborcsardi Can I take the macos-highsierra-release-cran RHub machine as a as-faithful-as-we-can-given-sparse-information proxy for CRAN? If we get it to build there, it will build at CRAN?
Obviously, it is impossible to know for sure, but yes, almost surely it will.
So to your and @s-u 's best judgement, this build from a few hours ago would provide an existence proof?
https://builder.r-hub.io/status/tiledb_0.9.7.9.tar.gz-82f9ae2b5ba14e94988f8dee9b9ec84f
and email 'picture'
@s-u Are am I missing something that is different here?
Yes, I think that should work then.
So is this just the addition of -mmacosx-version-min=10.14
or you made some other changes?
Essentially just that, as recommended by a colleague yesterday. In both the compiler and linker instructions. It took me a few runs to properly propagate it to the src/Makevars
; I may have accidently modified a draft version outside the package once or twice and uploaded for no-ops. The key was to 'fake the test' (for Linux) in configure.ac
so that I could check (on Linux) that the correct src/Makevars
resulted. Then it ... just worked. Looks like Xtools uses max(...several min versions listed...)
so we get 10.14.
Big big thanks to you and RHub for that. Could not have done this without it. Builders help so much (as does @s-u 's nice M1 builder which is of course from this century running 11.0).
Interesting. FWIW this is the compiler on R-hub's High Sierra:
R-hubs-Mac-2:~ rhub$ clang --version
Apple LLVM version 10.0.0 (clang-1000.10.44.4)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
This is the latest xcode (command line tools, really) for High Sierra AFAICT. So if CRAN has the same (which it should based on https://cran.r-project.org/web/checks/check_flavors.html), then it should work there as well.
@eddelbuettel ... if you point me to the package tar ball I can easily check ... (I used master
above since I didn't know what else to pick).
Sure but tarball of what ?
Our TileDB sources? That would be the branch with (draft) TileDB 2.5.0 preps here that the current (sole) PR is about.
Or did you mean the RHub build? For that you follow the same URL I gave earlier to access the logs, and this link to get artifacts including tar.gz sources and .tgz binary.
We haven't actually produced a package tarball (but I can make you one if you like) and I have hence not mentioned one either.
(And yes I was off re 4.2.0 above -- I confused it with R 4.1.0 which already defaults to C++14 whereas I keep thinking we would only get there next April. Not so, and much better! But that only is wind in my sails here and even more reason to see if we can move on from macOS 10.13 as a binding constraint....)
@s-u you can take the tarball from here, this is the one that was succesful: https://artifacts.r-hub.io/tiledb_0.9.7.9.tar.gz-82f9ae2b5ba14e94988f8dee9b9ec84f/
Yep I belatedly added the artifacts link too in https://github.com/r-hub/rhub/issues/493#issuecomment-966711621. BTW would be sweet if the (html log) page had the link too.
@gaborcsardi thanks, yes, that's exactly what I was asking for and, yes, my "manual" run on the CRAN build machine with that tar ball worked fine (Status: 1 NOTE
which is just the installed size NOTE).
So ... then we're good and asking for minimum version 10.14 is in fact viable?
(Release at CRAN has a "configure.ac
needs updating but that is old news I fixed many PRs ago; but that is one of the reason I'd rather upload sooner rather than later but given that we will switch to C++17 with TileDB 2.5.0 it would be nice to ascertain that macOS x86_64 builds at CRAN will not be a hurdle....)
@eddelbuettel FWIW you're now forcing the min version even on Big Sur builds where it will force older APIs (i.e. you are decreasing the min version used). Currently, it's not actually causing any issues there, but If you really wanted to do this properly, check in configure
whether it is needed based on the failure you got would be best.
Re 10.14 target - technically, it's not reliable - Apple is explicitly flagging some C++ features as available from 10.14 only (it's a tag on the libc++ version), but empirical evidence suggests that lot of those features are already working in 10.13 (but not in 10.12 which we don't care about). So I'd say the pragmatic approach is to make sure that the code path is tested and if it works on macOS 10.13 then it's fine. However. we cannot enable 10.14 as a blanket option, because we don't actually know which features are not implemented in libc++ on 10.13.
Effectively we have exactly two macOS builds: x86_64, and M1. It did occur to me too that the above is not needed for M1 (as your M1build nicely shows). I already retrieve machine
from uname
so I can control for that easily.
With that, we may then have a way forward: you keep 10.13 as the base layer (as we know changes take years here given the size of CRAN) and we can try to get by with 10.14 as an option -- until users come screaming and try to tar and feather both you and us. And that point we reconvene and see who will volunteer...
And I added that so now only Darwin && x86_64 select 10.14. See e.g. this log.
We are now at 2.5.0-rc1 which is all fine at our end in eleven builds, and also in a (still manual) M1builder upload which shows that the conditional injection of the 10.14 standard works fine.
We have a package that fails at RHub (using '"macos-highsierra-release-cran") because its
clang++ -mmacosx-version-min=10.13
is too old, our build needs 10.14 or later. It does build at the new M1builder machine which may reflect current CRAN minimums better.Would it be possible to either roll the macOS machines forward or add another one?