Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions dev/release/00-prepare.sh
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ update_versions() {
git add DESCRIPTION
cd -

cd "${SOURCE_DIR}/../../r/src"
sed -i.bak -E -e \
"s/^VERSION = .+/VERSION = ${r_version}/" \
Makevars.win
rm -f Makevars.win.bak
git add Makevars.win
cd -

cd "${SOURCE_DIR}/../../ruby"
sed -i.bak -E -e \
"s/^ VERSION = \".+\"/ VERSION = \"${version}\"/g" \
Expand Down
2 changes: 2 additions & 0 deletions r/.Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ LICENSE.md
lint.sh
Dockerfile
.*\.tar\.gz
^windows
clang_format.sh
2 changes: 1 addition & 1 deletion r/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ inst/doc
.Rproj.user
.Rhistory
src/Makevars

windows/
2 changes: 0 additions & 2 deletions r/NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ export(list_of)
export(mmap_create)
export(mmap_open)
export(null)
export(print.integer64)
export(read_arrow)
export(read_csv_arrow)
export(read_feather)
Expand All @@ -148,7 +147,6 @@ export(read_schema)
export(read_table)
export(record_batch)
export(schema)
export(str.integer64)
export(struct)
export(table)
export(time32)
Expand Down
4 changes: 4 additions & 0 deletions r/R/compression.R
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ compression_codec <- function(type = "GZIP") {

#' Compressed output stream
#'
#' @details This function is not supported in Windows.
#'
#' @param stream Underlying raw output stream
#' @param codec a codec
#' @export
CompressedOutputStream <- function(stream, codec = compression_codec("GZIP")){
if (.Platform$OS.type == "windows") stop("'CompressedOutputStream' is unsupported in Windows.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because of missing compression libraries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct. We can probably solve this by figuring out which are the missing dependencies and adding support to compile them in Mingw, but I much rather suggest we disable this functionality in the 0.13 in Windows.


UseMethod("CompressedOutputStream")
}

Expand Down
2 changes: 0 additions & 2 deletions r/R/reexports-bit64.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
# under the License.

#' @importFrom bit64 print.integer64
#' @export
bit64::print.integer64

#' @importFrom bit64 str.integer64
#' @export
bit64::str.integer64
3 changes: 3 additions & 0 deletions r/man/CompressedOutputStream.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 47 additions & 0 deletions r/src/Makevars.win
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

VERSION = 0.12.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
CXX_STD = CXX11

PKG_LIBS = \
-L$(ARROW_LIBS) \
$(OTHER_LIBS) \
-lparquet -larrow -lthrift -lboost_regex-mt-s -ldouble-conversion -lz -lws2_32

#all: clean
all: $(SHLIB)

$(OBJECTS): winlibs

winlibs:
"${R_HOME}/bin${R_ARCH_BIN}/Rscript.exe" "../tools/winlibs.R" $(VERSION)

clean:
rm -f $(SHLIB) $(OBJECTS)
2 changes: 2 additions & 0 deletions r/src/array__to_vector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,8 @@ class Converter_Time : public Converter {
return 1000000;
case TimeUnit::NANO:
return 1000000000;
default:
return 0;
}
}
};
Expand Down
2 changes: 2 additions & 0 deletions r/src/array_from_vector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,8 @@ inline int64_t get_time_multiplier(TimeUnit::type unit) {
return 1000000;
case TimeUnit::NANO:
return 1000000000;
default:
return 0;
}
}

Expand Down
2 changes: 2 additions & 0 deletions r/tests/testthat/test-compressed.R
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
context("arrow::io::Compressed.*Stream")

test_that("can write Buffer to CompressedOutputStream and read back in CompressedInputStream", {
if (.Platform$OS.type == "windows") skip("Unsupported")

buf <- buffer(as.raw(sample(0:255, size = 1024, replace = TRUE)))

tf1 <- tempfile()
Expand Down
26 changes: 26 additions & 0 deletions r/tools/winlibs.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

# Download static arrow from rwinlib
VERSION <- commandArgs(TRUE)
if(!file.exists(sprintf("../windows/arrow-%s/include/arrow/api.h", VERSION))){
if(getRversion() < "3.3.0") setInternet2()
download.file(sprintf("https://github.com/rwinlib/arrow/archive/v%s.zip", VERSION), "lib.zip", quiet = TRUE)
dir.create("../windows", showWarnings = FALSE)
unzip("lib.zip", exdir = "../windows")
unlink("lib.zip")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely scary-looking and appears to introduce a dependency on binaries in a foreign repository, which is an attack vector. For example, a malicious actor could inject Arrow libraries into this repository with malicious code in them that sends them files from your %HOME% directory (this is what already happened to the NPM community). I don't know who has access to this repository and if anyone who does has their keys stolen or their GitHub account compromised (maybe they don't have 2FA enabled) then we are in trouble

If we're going to have dependencies on libraries such as these, at minimum we should have a SHA256/SHA512 hash for what we expect their contents to be to prevent malicious code from getting shipped.

Of course, a preferable solution is to build libraries locally based on the source code in the archive

Can you open some follow up JIRA issues about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.