Skip to content

Commit

Permalink
fix(ci): Fix verification workflow (#552)
Browse files Browse the repository at this point in the history
There are a few problems in the verification workflow:

- On Windows, we weren't exporting any symbols in the C data integration
test (when building with CMake)
- The R package testthat doesn't seem to support centos7, which we would
need to test the R package there
- The R package jsonlite seems to leak memory. One fix is
jeroen/jsonlite#442 , but in the meantime we can
ignore the leak since it doesn't come from us.
  • Loading branch information
paleolimbot authored Jul 9, 2024
1 parent e19c17f commit 2aa2e69
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
5 changes: 3 additions & 2 deletions .github/workflows/verify.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ jobs:
platform: "centos7",
arch: "amd64",
# Currently the Python on the centos7 image is 3.6, which does not support
# new enough setuptools to build the Python package.
compose_args: "-e NANOARROW_ACCEPT_IMPORT_GPG_KEYS_ERROR=1 -e TEST_PYTHON=0"
# new enough setuptools to build the Python package. Our test dependencies
# no longer support centos7 for R, either.
compose_args: "-e NANOARROW_ACCEPT_IMPORT_GPG_KEYS_ERROR=1 -e TEST_PYTHON=0 -e TEST_R=0"
}
- {
platform: "ubuntu",
Expand Down
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ if(NANOARROW_BUILD_TESTS OR NANOARROW_BUILD_INTEGRATION_TESTS)
set_target_properties(nanoarrow PROPERTIES POSITION_INDEPENDENT_CODE ON)
add_library(nanoarrow_c_data_integration SHARED
src/nanoarrow/integration/c_data_integration.cc)
target_compile_definitions(nanoarrow_c_data_integration PRIVATE NANOARROW_BUILD_DLL
NANOARROW_EXPORT_DLL)
target_include_directories(nanoarrow_c_data_integration
PUBLIC $<BUILD_INTERFACE:${NANOARROW_BUILD_INCLUDE_DIR}>
$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/src>
Expand Down
9 changes: 9 additions & 0 deletions valgrind.supp
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,12 @@
fun:malloc
fun:_PyObject_GC_NewVar
}

# Can be removed when https://github.com/jeroen/jsonlite/pull/442 is released
{
<jsonlite>:Leak in base64_encode
Memcheck:Leak
...
fun:base64_encode
fun:R_base64_encode
}

0 comments on commit 2aa2e69

Please sign in to comment.