zeroshade commented on a change in pull request #11206:
URL: https://github.com/apache/arrow/pull/11206#discussion_r718505637
##########
File path: ci/docker/debian-10-go-cgo.dockerfile
##########
@@ -0,0 +1,34 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review comment:
The `debian-10-go-cgo-python` dockerfile installs pyarrow and the
underlying go does not rely on linking against the libarrow.so objects.
This dockerfile installs libarrow directly via apt so that pkg-config can
find libarrow.so since the allocator piece directly needs to link against
libarrow.so to run which is explicitly different than the case for the
`cgo-python` one which in which Go has no dependency on libarrow since it only
needs the header files and .c/.cc files which are in the Go repo itself.
##########
File path: go/arrow/memory/cgo_allocator.go
##########
@@ -0,0 +1,108 @@
+// 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.
+
+// +build cgo
+// +build ccalloc
+
+package memory
+
+import (
+ "runtime"
+
+ cga "github.com/apache/arrow/go/arrow/memory/internal/cgoalloc"
+)
+
+// CgoArrowAllocator is an allocator which exposes the C++ memory pool class
+// from the Arrow C++ Library as an allocator for memory buffers to use in Go.
+// The build tag 'ccalloc' must be used in order to include it as it requires
+// linking against the arrow library.
+//
+// The primary reason to use this would be as an allocator when dealing with
+// exporting data across the cdata interface in order to ensure that the memory
+// is allocated safely on the C side so it can be held on the CGO side beyond
+// the context of a single function call. If the memory in use isn't allocated
+// on the C side, then it is not safe for any pointers to data to be held
outside
+// of Go beyond the context of a single Cgo function call as it will be
invisible
+// to the Go garbage collector and could potentially get moved without being
updated.
Review comment:
When making a CGO call, during the execution of it the Go garbage
collector will pin memory being used so that any pointers to go memory will
remain valid during the length of that cgo call. But the documentation is very
clear that it is *not* safe to let C/C++ maintain pointers to Go memory beyond
the context of that single call because after the cgo call returns, the Go
garbage collector is free to move memory around as necessary. Normally the
garbage collector can update any references in Go when it does this so that
everything continues to work just fine, but it cannot update any pointers in
C/C++ when it does this.
So the global container of exported arrays used by `releaseData` serves two
purposes:
1. Ensuring that during the context of a single CGO call that there is a
maintained reference to the Go objects so that it keeps the garbage collector
from cleaning it up
2. Allowing C/C++ to call `releaseData` to free the memory if we're handing
it off, or allowing the ability to hand off memory to let C/C++ own it and
control when it is released. this is only safe if the underlying buffers were
allocated by C and not Go allocated memory.
For more information about this specifically in terms of passing pointers
around: https://pkg.go.dev/cmd/cgo#hdr-Passing_pointers is the documentation on
CGO that i'm referencing.
##########
File path: go/arrow/memory/internal/cgoalloc/helpers.h
##########
@@ -0,0 +1,59 @@
+// 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.
+
+#pragma once
+
+#include <cstdint>
+#include <memory>
+
+// helper functions to be included by C++ code for interacting with Cgo
+
+// create_ref will construct a shared_ptr on the heap and return a pointer
+// to it. the returned uintptr_t can then be used with retrieve_instance
+// to get back the shared_ptr and object it refers to. This ensures that
+// the object outlives the exported function so that Go can use it.
+template <typename T>
+uintptr_t create_ref(std::shared_ptr<T> t) {
+ std::shared_ptr<T>* retained_ptr = new std::shared_ptr<T>(t);
+ return reinterpret_cast<uintptr_t>(retained_ptr);
+}
+
+// specialization for shared_ptrs to const objects
Review comment:
So i was running into an issue with something i was trying before but i
don't remember if i had tried that correctly. I don't think the const
specialization here is needed for *this* change so i can remove it for now and
then when i get to that point that i needed it i'll double check doing
`create_ref<const T>` and if that works, then i won't need to add this :smile:
##########
File path: go/arrow/memory/internal/cgoalloc/allocator.cc
##########
@@ -0,0 +1,75 @@
+// 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.
+
+// +build ccalloc
+
+#include "allocator.h"
+#include "arrow/memory_pool.h"
+#include "helpers.h"
+
+struct mem_holder {
+ std::unique_ptr<arrow::MemoryPool> pool;
+ arrow::MemoryPool* current_pool;
Review comment:
`arrow::default_memory_pool` returns a raw pointer, so if this is
holding a reference to the *default* memory pool, `current_pool` can be set to
the pointer without worry of deleting it. The `unique_ptr` is there for the
case where we're not using the `default_memory_pool`, allowing the logic to not
have to explicitly check which is in use when using the allocate/reallocate
functions since the raw pointer should always be valid regardless of if it's
using the default memory pool or not.
##########
File path: go/arrow/memory/internal/cgoalloc/allocator.cc
##########
@@ -0,0 +1,75 @@
+// 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.
+
+// +build ccalloc
+
+#include "allocator.h"
+#include "arrow/memory_pool.h"
+#include "helpers.h"
+
+struct mem_holder {
+ std::unique_ptr<arrow::MemoryPool> pool;
+ arrow::MemoryPool* current_pool;
Review comment:
I've added a comment explaining this, and also functions for referencing
the default memory pool (which was in my original version of this but didn't
make it into this cleaned up branch for whatever reason)
##########
File path: go/arrow/memory/internal/cgoalloc/allocator.go
##########
@@ -0,0 +1,99 @@
+// 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.
+
+// +build ccalloc
+
+package cgoalloc
+
+// #cgo !windows pkg-config: arrow
+// #cgo CXXFLAGS: -std=c++14
+// #cgo windows LDFLAGS: -larrow
+// #include "allocator.h"
+import "C"
+import (
+ "reflect"
+ "unsafe"
+)
+
+// CGOMemPool is an alias to the typedef'd uintptr from the allocator.h file
+type CGOMemPool = C.ArrowMemoryPool
+
+// CgoPoolAlloc allocates a block of memory of length 'size' using the memory
+// pool that is passed in.
+func CgoPoolAlloc(pool CGOMemPool, size int) []byte {
+ var out *C.uint8_t
+ C.arrow_pool_allocate(pool, C.int64_t(size),
(**C.uint8_t)(unsafe.Pointer(&out)))
+
+ var ret []byte
+ s := (*reflect.SliceHeader)(unsafe.Pointer(&ret))
+ s.Data = uintptr(unsafe.Pointer(out))
+ s.Len = size
+ s.Cap = size
+
+ return ret
+}
+
+// CgoPoolRealloc calls 'reallocate' on the block of memory passed in which
must
+// be a slice that was returned by CgoPoolAlloc or CgoPoolRealloc.
+func CgoPoolRealloc(pool CGOMemPool, size int, b []byte) []byte {
+ oldSize := C.int64_t(len(b))
+ data := (*C.uint8_t)(unsafe.Pointer(&b[0]))
+ C.arrow_pool_reallocate(pool, oldSize, C.int64_t(size), &data)
+
+ var ret []byte
+ s := (*reflect.SliceHeader)(unsafe.Pointer(&ret))
+ s.Data = uintptr(unsafe.Pointer(data))
+ s.Len = size
+ s.Cap = size
+
+ return ret
+}
+
+// CgoPoolFree uses the indicated memory pool to free a block of memory. The
+// slice passed in *must* be a slice which was returned by CgoPoolAlloc or
+// CgoPoolRealloc.
+func CgoPoolFree(pool CGOMemPool, b []byte) {
+ if len(b) == 0 {
Review comment:
fair point, i assumed that calling allocate with a 0 byte size wouldn't
actually allocate anything and thus there would be nothing to leak. i've added
a corresponding check to alloc and realloc to address this
##########
File path: go/arrow/memory/cgo_allocator.go
##########
@@ -0,0 +1,108 @@
+// 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.
+
+// +build cgo
+// +build ccalloc
+
+package memory
+
+import (
+ "runtime"
+
+ cga "github.com/apache/arrow/go/arrow/memory/internal/cgoalloc"
+)
+
+// CgoArrowAllocator is an allocator which exposes the C++ memory pool class
+// from the Arrow C++ Library as an allocator for memory buffers to use in Go.
+// The build tag 'ccalloc' must be used in order to include it as it requires
+// linking against the arrow library.
+//
+// The primary reason to use this would be as an allocator when dealing with
+// exporting data across the cdata interface in order to ensure that the memory
+// is allocated safely on the C side so it can be held on the CGO side beyond
+// the context of a single function call. If the memory in use isn't allocated
+// on the C side, then it is not safe for any pointers to data to be held
outside
+// of Go beyond the context of a single Cgo function call as it will be
invisible
+// to the Go garbage collector and could potentially get moved without being
updated.
Review comment:
So that's also a possibility, rather than using the memorypool exposed
by the arrow lib we could instead create our own allocator (or utilize a
different one) that allocates memory using C always rather than the
DefaultGoAllocator using Go-allocated memory. But that then changes some
semantics in terms of memory usage and potential performance characteristics
because CGO calls have a higher overhead.
If you aren't making calls to C or passing data around then using the Go
allocator is perfectly fine and performant (though if you're doing *a lot* of
allocations, there are a few libraries which have shown significant benefits to
using things like jemalloc and manually managing the memory via C calls but
this has to be done carefully to avoid paying large overheads in CGO calls and
avoid introducing extra dependencies that may not be needed).
I can definitely see a future enhancement to make the default allocator do
this, but I didn't want to change the default allocation behavior yet.
##########
File path: go/arrow/memory/cgo_allocator.go
##########
@@ -0,0 +1,108 @@
+// 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.
+
+// +build cgo
+// +build ccalloc
+
+package memory
+
+import (
+ "runtime"
+
+ cga "github.com/apache/arrow/go/arrow/memory/internal/cgoalloc"
+)
+
+// CgoArrowAllocator is an allocator which exposes the C++ memory pool class
+// from the Arrow C++ Library as an allocator for memory buffers to use in Go.
+// The build tag 'ccalloc' must be used in order to include it as it requires
+// linking against the arrow library.
+//
+// The primary reason to use this would be as an allocator when dealing with
+// exporting data across the cdata interface in order to ensure that the memory
+// is allocated safely on the C side so it can be held on the CGO side beyond
+// the context of a single function call. If the memory in use isn't allocated
+// on the C side, then it is not safe for any pointers to data to be held
outside
+// of Go beyond the context of a single Cgo function call as it will be
invisible
+// to the Go garbage collector and could potentially get moved without being
updated.
Review comment:
i don't want the vanilla `go get github.com/apache/arrow/go/arrow`
library to require having the C++ libarrow available to link against
##########
File path: go/arrow/memory/internal/cgoalloc/allocator.cc
##########
@@ -0,0 +1,75 @@
+// 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.
+
+// +build ccalloc
+
+#include "allocator.h"
+#include "arrow/memory_pool.h"
+#include "helpers.h"
+
+struct mem_holder {
+ std::unique_ptr<arrow::MemoryPool> pool;
+ arrow::MemoryPool* current_pool;
Review comment:
I actually added a function which exposes the `default_memory_pool` and
adds it as an optional pull in the package because it seemed like that made
sense to do.
##########
File path: go/arrow/memory/internal/cgoalloc/allocator.cc
##########
@@ -0,0 +1,75 @@
+// 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.
+
+// +build ccalloc
+
+#include "allocator.h"
+#include "arrow/memory_pool.h"
+#include "helpers.h"
+
+struct mem_holder {
+ std::unique_ptr<arrow::MemoryPool> pool;
+ arrow::MemoryPool* current_pool;
Review comment:
`holder->pool` is the `unique_ptr`, so when logging is disabled the call
to `release_ref<mem_holder>(pool)` would destroy the `mem_holder` which would
also destroy the memory pool instance created by `CreateDefault` when the
`unique_ptr` destructor runs. Unless I'm missing something?
##########
File path: go/arrow/memory/cgo_allocator.go
##########
@@ -0,0 +1,108 @@
+// 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.
+
+// +build cgo
+// +build ccalloc
+
+package memory
+
+import (
+ "runtime"
+
+ cga "github.com/apache/arrow/go/arrow/memory/internal/cgoalloc"
+)
+
+// CgoArrowAllocator is an allocator which exposes the C++ memory pool class
+// from the Arrow C++ Library as an allocator for memory buffers to use in Go.
+// The build tag 'ccalloc' must be used in order to include it as it requires
+// linking against the arrow library.
+//
+// The primary reason to use this would be as an allocator when dealing with
+// exporting data across the cdata interface in order to ensure that the memory
+// is allocated safely on the C side so it can be held on the CGO side beyond
+// the context of a single function call. If the memory in use isn't allocated
+// on the C side, then it is not safe for any pointers to data to be held
outside
+// of Go beyond the context of a single Cgo function call as it will be
invisible
+// to the Go garbage collector and could potentially get moved without being
updated.
Review comment:
That's fair, and i was considering something, though the drawback there
is that we don't want every allocation to go through C because CGO has a higher
overhead, so if you're making a lot of smaller allocations the overhead of the
CGO calls could pose a potential problem in order to be the default from a
performance standpoint. So I'd want to at a minimum hide it behind a build tag
just in case.
##########
File path: go/arrow/memory/cgo_allocator.go
##########
@@ -0,0 +1,108 @@
+// 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.
+
+// +build cgo
+// +build ccalloc
+
+package memory
+
+import (
+ "runtime"
+
+ cga "github.com/apache/arrow/go/arrow/memory/internal/cgoalloc"
+)
+
+// CgoArrowAllocator is an allocator which exposes the C++ memory pool class
+// from the Arrow C++ Library as an allocator for memory buffers to use in Go.
+// The build tag 'ccalloc' must be used in order to include it as it requires
+// linking against the arrow library.
+//
+// The primary reason to use this would be as an allocator when dealing with
+// exporting data across the cdata interface in order to ensure that the memory
+// is allocated safely on the C side so it can be held on the CGO side beyond
+// the context of a single function call. If the memory in use isn't allocated
+// on the C side, then it is not safe for any pointers to data to be held
outside
+// of Go beyond the context of a single Cgo function call as it will be
invisible
+// to the Go garbage collector and could potentially get moved without being
updated.
Review comment:
Yea, I figured it was simplest to use the memory pool that already
exists in the libarrow library (as I use the libarrow library in my future
changes to connect to the compute API), but I could potentially use something
like https://github.com/spinlock/jemalloc-go to just include jemalloc as a
go-getable dependency. Which i'll play around with and see how it does. I liked
the libarrrow memory pool because of the consistency it provided and feature
handling such as the logging proxy and the tracking of how many bytes had been
allocated in it, but it is likely pretty easy to do similar functionality and
use jemalloc directly. As long as consumers are using Reserve and otherwise
pre-allocating memory, it would cut down the number of cgo calls to mitigate
performance hits.
Alternately I could pursue looking into a slab style allocator that manages
much larger chunks of memory that it hands out as a way to amortize the number
of calls, but that's a separate thing to look into later.
##########
File path: go/arrow/memory/internal/cgoalloc/allocator.cc
##########
@@ -0,0 +1,75 @@
+// 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.
+
+// +build ccalloc
+
+#include "allocator.h"
+#include "arrow/memory_pool.h"
+#include "helpers.h"
+
+struct mem_holder {
+ std::unique_ptr<arrow::MemoryPool> pool;
+ arrow::MemoryPool* current_pool;
Review comment:
I do like that as a better simplification, nice thanks! Can you think of
any reason that it might be better to use `CreateDefault()`? Or does it make
more sense to just use the `default_memory_pool` anyways?
##########
File path: ci/docker/debian-go-cgo.dockerfile
##########
@@ -0,0 +1,32 @@
+# 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.
+
+ARG base
+FROM ${base}
+
+ENV DEBIAN_FRONTEND noninteractive
+
+# install libarrow-dev to link against with CGO
+RUN apt-get update -y -q && \
+ apt-get install -y -q --no-install-recommends ca-certificates lsb-release
wget && \
+ wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short
| tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename
--short).deb && \
+ apt-get install -y -q --no-install-recommends
./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb && \
Review comment:
At the moment it was intentional because I didn't want to go through
setting up building it with this test and for now I didn't expect there to be
cases where changes are being made to both. The drawback of course being that
this wouldn't catch a change in the C++ that breaks an API that is being used
in the .cc files here though given the desire to have as small a binding as
possible, I thought it was acceptable for now.
In order to make this safer, if desired, I could change this to actually
build master as part of this like is done in the python tests (note also that
`go-cgo-python` dockerfile also uses the latest pyarrow --only-binary for it's
test as opposed to building it there)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
