Skip to content

Commit

Permalink
ARROW-4911: [R] Progress towards completing windows support
Browse files Browse the repository at this point in the history
Support for building [R] package in Windows x64/x86.

Fixes https://issues.apache.org/jira/browse/ARROW-4911 which helps us work towards releasing in CRAN: https://issues.apache.org/jira/browse/ARROW-3204.

Author: Romain Francois <romain@rstudio.com>
Author: Javier Luraschi <javierluraschi@hotmail.com>
Author: Romain Francois <romainfrancois@MacBook-Pro.localdomain>
Author: Romain François <romain@purrple.cat>

Closes #3932 from javierluraschi/bugfix/port-win-fixes and squashes the following commits:

5806d5e <Romain Francois> cran comments
a44d1f5 <Romain Francois> using configure and Makevars.* from @jeroen
30ab279 <Romain Francois> not yet supported
797c3b5 <Romain Francois> merge javierluraschi/bugfix/port-win-fixes
21e0d9d <Romain Francois> Revert "enable parquet on mac. from https://github.com/jeroen/arrow/commit/833a2dc7554b6dcee364ff39206ed9a7d9fe59e0"
06858d0 <Romain Francois> revert configure and Makevars.in changes so that it works fine on mac os
892af4d <Romain Francois> enable parquet on mac. from https://github.com/jeroen/arrow/commit/833a2dc7554b6dcee364ff39206ed9a7d9fe59e0
f008baa <Romain François> Merge branch 'master' into bugfix/port-win-fixes
b0fd448 <Romain Francois> Remove incorrect stuff from configure/make setup, from @jeroen. use version 0.13.0 of C++ arrow lib on windows, enable parquet
7927aa0 <Romain Francois> add @jeroen as author and use 0.13.0 as the version to prepare for cran release
1d50c30 <Romain Francois> lint
59025b6 <Romain Francois> use int64_t instead of size_t
e2e451e <Romain Francois> usigned comparison
d1ba6dd <Romain Francois> initialize value.
169918f <Romain Francois> split int_cast in versions for signed and unsigned
ee21495 <Romain Francois> using R_xlen_t instead of size_t. causes trouble in win32
cc12f9c <Javier Luraschi> Merge branch 'master' into bugfix/port-win-fixes
aa91e2b <Javier Luraschi> modivy R's makevars.win file on release script
9dcc347 <Javier Luraschi> fix apache rat warnings
14bce9c <Javier Luraschi>  Progress towards completing windows support
  • Loading branch information
romainfrancois committed Apr 11, 2019
1 parent 264dc56 commit 76e1bc5
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 53 deletions.
1 change: 1 addition & 0 deletions dev/release/rat_exclude_files.txt
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions r/.Rbuildignore
Expand Up @@ -9,3 +9,4 @@ Dockerfile
.*\.tar\.gz
^windows
clang_format.sh
^cran-comments\.md$
3 changes: 2 additions & 1 deletion 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'.
Expand Down
32 changes: 8 additions & 24 deletions r/configure
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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"
Expand All @@ -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
27 changes: 27 additions & 0 deletions 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.
5 changes: 2 additions & 3 deletions r/src/Makevars.in
Expand Up @@ -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@
21 changes: 6 additions & 15 deletions r/src/Makevars.win
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion r/src/array.cpp
Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions r/src/array__to_vector.cpp
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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());
}
}
Expand Down Expand Up @@ -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 {
Expand Down
22 changes: 19 additions & 3 deletions r/src/array_from_vector.cpp
Expand Up @@ -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");
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 76e1bc5

Please sign in to comment.