ARROW-4911: [R] Progress towards completing windows support

dev/release/rat_exclude_files.txt

@@ -188,6 +188,7 @@ r/arrow.Rproj
 r/README.md
 r/README.Rmd
 r/man/*.Rd
+r/cran-comments.md
 .gitattributes
 ruby/red-arrow/.yardopts
 rust/arrow/test/data/*.csv

r/.Rbuildignore

@@ -9,3 +9,4 @@ Dockerfile
 .*\.tar\.gz
 ^windows
 clang_format.sh
+^cran-comments\.md$

r/DESCRIPTION

@@ -1,10 +1,11 @@
 Package: arrow
 Title: R Integration to 'Apache' 'Arrow'
-Version: 0.13.0.9000
+Version: 0.13.0
 Authors@R: c(
     person("Romain", "François", email = "romain@rstudio.com", role = c("aut", "cre")),
     person("Javier", "Luraschi", email = "javier@rstudio.com", role = c("ctb")),
     person("Jeffrey", "Wong", email = "jeffreyw@netflix.com", role = c("ctb")),
+    person("Jeroen", "Ooms", email = "jeroen@berkeley.edu", role = c("aut")),
     person("Apache Arrow", email = "dev@arrow.apache.org", role = c("aut", "cph"))
   )
 Description: R Integration to 'Apache' 'Arrow'.

r/configure

@@ -27,29 +27,17 @@
 
 # Library settings
 PKG_CONFIG_NAME="arrow parquet"
-PKG_DEB_NAME="arrow"
-PKG_RPM_NAME="arrow"
-PKG_CSW_NAME="arrow"
+PKG_DEB_NAME="(unsuppored)"
+PKG_RPM_NAME="(unsuppored)"
 PKG_BREW_NAME="apache-arrow"
 PKG_TEST_HEADER="<arrow/api.h>"
-PKG_LIBS=""
+PKG_LIBS="-larrow -lparquet"
 
 # Use pkg-config if available
 pkg-config --version >/dev/null 2>&1
 if [ $? -eq 0 ]; then
-  PKGCONFIG_CFLAGS=$(pkg-config --cflags --silence-errors arrow)
-  if [ $? -ne 0 ]; then
-    echo "Apache Arrow C++ was not found using pkg-config"
-    exit 1
-  fi
-  PKGCONFIG_LIBS=$(pkg-config --libs arrow)
-  PKGCONFIG_CFLAGS_PARQUET=$(pkg-config --cflags --silence-errors parquet)
-  if [ $? -eq 0 ]; then
-    PKGCONFIG_CFLAGS="${PKGCONFIG_CFLAGS} ${PKGCONFIG_CFLAGS_PARQUET} -DARROW_R_WITH_PARQUET"
-    PKGCONFIG_LIBS="${PKGCONFIG_LIBS} $(pkg-config --libs parquet)"
-  fi
-else
-  PKG_LIBS="-larrow -lparquet"
+  PKGCONFIG_CFLAGS=`pkg-config --cflags --silence-errors ${PKG_CONFIG_NAME}`
+  PKGCONFIG_LIBS=`pkg-config --libs ${PKG_CONFIG_NAME}`
 fi
 
 # Note that cflags may be empty in case of success
@@ -68,7 +56,7 @@ elif [[ "$OSTYPE" == "darwin"* ]]; then
     curl -sfL "https://jeroen.github.io/autobrew/$PKG_BREW_NAME" > autobrew
     source autobrew
   fi
-  PKG_CFLAGS="-I$BREWDIR/opt/$PKG_BREW_NAME/include"
+  PKG_CFLAGS="-I$BREWDIR/opt/$PKG_BREW_NAME/include -DARROW_R_WITH_PARQUET"
   PKG_LIBS="-L$BREWDIR/opt/$PKG_BREW_NAME/lib $PKG_LIBS"
 fi
 
@@ -83,9 +71,8 @@ CXX11STD=$("${R_HOME}"/bin/R CMD config CXX11STD)
 CPPFLAGS=$("${R_HOME}"/bin/R CMD config CPPFLAGS)
 
 # If libarrow uses the old GLIBCXX ABI, so we have to use it too
-PKG_CXXFLAGS="$C_VISIBILITY"
 if [ "$ARROW_USE_OLD_CXXABI" ]; then
-  PKG_CXXFLAGS="$PKG_CXXFLAGS -D_GLIBCXX_USE_CXX11_ABI=0"
+  $PKG_CFLAGS="$PKG_CFLAGS -D_GLIBCXX_USE_CXX11_ABI=0"
 fi
 
 # Test configuration
@@ -95,9 +82,6 @@ echo "#include $PKG_TEST_HEADER" | ${CXXCPP} ${CPPFLAGS} ${PKG_CFLAGS} ${CXX11FL
 if [ $? -ne 0 ]; then
   echo "------------------------- ANTICONF ERROR ---------------------------"
   echo "Configuration failed because $PKG_CONFIG_NAME was not found. Try installing:"
-  echo " * deb: $PKG_DEB_NAME (Debian, Ubuntu, etc)"
-  echo " * rpm: $PKG_RPM_NAME (Fedora, CentOS, RHEL)"
-  echo " * csw: $PKG_CSW_NAME (Solaris)"
   echo " * brew: $PKG_BREW_NAME (Mac OSX)"
   echo "If $PKG_CONFIG_NAME is already installed, check that 'pkg-config' is in your"
   echo "PATH and PKG_CONFIG_PATH contains a $PKG_CONFIG_NAME.pc file. If pkg-config"
@@ -108,7 +92,7 @@ if [ $? -ne 0 ]; then
 fi
 
 # Write to Makevars
-sed -e "s|@cflags@|$PKG_CFLAGS|" -e "s|@libs@|$PKG_LIBS|" -e "s|@pkgcxxflags@|$PKG_CXXFLAGS|" src/Makevars.in > src/Makevars
+sed -e "s|@cflags@|$PKG_CFLAGS|" -e "s|@libs@|$PKG_LIBS|" src/Makevars.in > src/Makevars
 
 # Success
 exit 0

r/cran-comments.md

@@ -0,0 +1,27 @@
+## Test environments
+* local OS X install, R 3.5.3
+* win-builder (devel and release)
+
+## R CMD check results
+
+0 errors | 0 warnings | 1 note
+
+* This is a new release.
+
+## Platform support
+
+This package supports Windows and macOS but not Linux.
+
+The Arrow project is cross-language development platform
+for in-memory data, it spans several languages and
+their code base is quite large (about 150K lines of C
+sources and more than 600K lines across all languages).
+
+In the future, the Apache Arrow project will release
+binaries in the official Fedora and Debian repos;
+we're working on hard on this, but due to the size,
+this is likely to be implemented until next year.
+
+In the meantime, R users can install the Linux binaries
+from custom repos or build Arrow from source when using
+Linux.

r/src/Makevars.in

@@ -16,7 +16,6 @@
 # under the License.
 
 PKG_CPPFLAGS=@cflags@
-PKG_CXXFLAGS=@pkgcxxflags@
+PKG_CXXFLAGS=$(CXX_VISIBILITY)
 CXX_STD=CXX11
-PKG_LIBS=@libs@  -Wl,-rpath,/usr/local/lib
-#CXXFLAGS="-D_GLIBCXX_USE_CXX11_ABI=0"
+PKG_LIBS=@libs@

r/src/Makevars.win

@@ -15,24 +15,15 @@
 # specific language governing permissions and limitations
 # under the License.
 
-VERSION = 0.13.0.9000
-
-ifeq "$(ARROW_PATH)" ""
-  RWINLIB = ../windows/arrow-$(VERSION)
-	ARROW_INCLUDE = $(RWINLIB)/include
-	ARROW_LIBS = $(RWINLIB)/lib$(subst gcc,,$(COMPILED_BY))$(R_ARCH)
-	OTHER_LIBS = -L$(RWINLIB)/lib$(R_ARCH)
-else
-	ARROW_INCLUDE = $(ARROW_PATH)/include
-	ARROW_LIBS = $(ARROW_PATH)/lib
-endif
-
-PKG_CPPFLAGS = -I$(ARROW_INCLUDE) -DARROW_STATIC -DPARQUET_STATIC
+VERSION = 0.13.0
+RWINLIB = ../windows/arrow-$(VERSION)
+PKG_CPPFLAGS = -I$(RWINLIB)/include -DARROW_STATIC -DPARQUET_STATIC \
+	-DARROW_R_WITH_PARQUET
 CXX_STD = CXX11
 
 PKG_LIBS = \
-	-L$(ARROW_LIBS) \
-	$(OTHER_LIBS) \
+	-L$(RWINLIB)/lib$(subst gcc,,$(COMPILED_BY))$(R_ARCH) \
+	-L$(RWINLIB)/lib$(R_ARCH) \
 	-lparquet -larrow -lthrift -lboost_regex-mt-s -ldouble-conversion -lz -lws2_32
 
 #all: clean

r/src/array.cpp

@@ -99,7 +99,7 @@ LogicalVector Array__Mask(const std::shared_ptr<arrow::Array>& array) {
   LogicalVector res(no_init(n));
   arrow::internal::BitmapReader bitmap_reader(array->null_bitmap()->data(),
                                               array->offset(), n);
-  for (size_t i = 0; i < array->length(); i++, bitmap_reader.Next()) {
+  for (int64_t i = 0; i < n; i++, bitmap_reader.Next()) {
     res[i] = bitmap_reader.IsSet();
   }
   return res;

r/src/array__to_vector.cpp

@@ -113,7 +113,7 @@ Status SomeNull_Ingest(SEXP data, R_xlen_t start, R_xlen_t n,
   if (array->null_count()) {
     arrow::internal::BitmapReader bitmap_reader(array->null_bitmap()->data(),
                                                 array->offset(), n);
-    for (size_t i = 0; i < n; i++, bitmap_reader.Next(), ++p_data, ++p_values) {
+    for (R_xlen_t i = 0; i < n; i++, bitmap_reader.Next(), ++p_data, ++p_values) {
       *p_data = bitmap_reader.IsSet() ? lambda(*p_values) : default_value<RTYPE>();
     }
   } else {
@@ -251,11 +251,11 @@ class Converter_Boolean : public Converter {
       arrow::internal::BitmapReader null_reader(array->null_bitmap()->data(),
                                                 array->offset(), n);
 
-      for (size_t i = 0; i < n; i++, data_reader.Next(), null_reader.Next(), ++p_data) {
+      for (R_xlen_t i = 0; i < n; i++, data_reader.Next(), null_reader.Next(), ++p_data) {
         *p_data = null_reader.IsSet() ? data_reader.IsSet() : NA_LOGICAL;
       }
     } else {
-      for (size_t i = 0; i < n; i++, data_reader.Next(), ++p_data) {
+      for (R_xlen_t i = 0; i < n; i++, data_reader.Next(), ++p_data) {
         *p_data = data_reader.IsSet();
       }
     }
@@ -472,12 +472,12 @@ class Converter_Decimal : public Converter {
       internal::BitmapReader bitmap_reader(array->null_bitmap()->data(), array->offset(),
                                            n);
 
-      for (size_t i = 0; i < n; i++, bitmap_reader.Next(), ++p_data) {
+      for (R_xlen_t i = 0; i < n; i++, bitmap_reader.Next(), ++p_data) {
         *p_data = bitmap_reader.IsSet() ? std::stod(decimals_arr.FormatValue(i).c_str())
                                         : NA_REAL;
       }
     } else {
-      for (size_t i = 0; i < n; i++, ++p_data) {
+      for (R_xlen_t i = 0; i < n; i++, ++p_data) {
         *p_data = std::stod(decimals_arr.FormatValue(i).c_str());
       }
     }
@@ -514,7 +514,7 @@ class Converter_Int64 : public Converter {
     if (array->null_count()) {
       internal::BitmapReader bitmap_reader(array->null_bitmap()->data(), array->offset(),
                                            n);
-      for (size_t i = 0; i < n; i++, bitmap_reader.Next(), ++p_data) {
+      for (R_xlen_t i = 0; i < n; i++, bitmap_reader.Next(), ++p_data) {
         *p_data = bitmap_reader.IsSet() ? p_values[i] : NA_INT64;
       }
     } else {

r/src/array_from_vector.cpp

@@ -182,7 +182,8 @@ using internal::checked_cast;
 
 namespace internal {
 
-template <typename T, typename Target>
+template <typename T, typename Target,
+          typename std::enable_if<std::is_signed<Target>::value, Target>::type = 0>
 Status int_cast(T x, Target* out) {
   if (x < std::numeric_limits<Target>::min() || x > std::numeric_limits<Target>::max()) {
     return Status::Invalid("Value is too large to fit in C integer type");
@@ -191,6 +192,21 @@ Status int_cast(T x, Target* out) {
   return Status::OK();
 }
 
+template <typename T>
+struct usigned_type;
+
+template <typename T, typename Target,
+          typename std::enable_if<std::is_unsigned<Target>::value, Target>::type = 0>
+Status int_cast(T x, Target* out) {
+  // we need to compare between unsigned integers
+  uint64_t x64 = x;
+  if (x64 < 0 || x64 > std::numeric_limits<Target>::max()) {
+    return Status::Invalid("Value is too large to fit in C integer type");
+  }
+  *out = static_cast<Target>(x);
+  return Status::OK();
+}
+
 template <typename Int>
 Status double_cast(Int x, double* out) {
   *out = static_cast<double>(x);
@@ -297,7 +313,7 @@ struct Unbox<Type, enable_if_integer<Type>> {
       if (*p == na) {
         builder->UnsafeAppendNull();
       } else {
-        CType value;
+        CType value = 0;
         RETURN_NOT_OK(internal::int_cast(*p, &value));
         builder->UnsafeAppend(value);
       }
@@ -375,7 +391,7 @@ struct Unbox<FloatType> {
       if (*p == NA_INTEGER) {
         builder->UnsafeAppendNull();
       } else {
-        float value;
+        float value = 0;
         RETURN_NOT_OK(internal::float_cast(*p, &value));
         builder->UnsafeAppend(value);
       }