Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Fix symbol export visibility #531

Merged
merged 16 commits into from
Jun 21, 2024
Merged

fix: Fix symbol export visibility #531

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
6def468
fix: Fix symbol export visibility
WillAyd Jun 17, 2024
580587f
Update Meson configuration
WillAyd Jun 17, 2024
02813af
Hidden visibility for C modules
WillAyd Jun 17, 2024
abfec33
Merge remote-tracking branch 'upstream/main' into symbol-visibility
WillAyd Jun 18, 2024
529b460
cleanups
WillAyd Jun 18, 2024
b5b2804
Updated Python regex in bootstrap
WillAyd Jun 18, 2024
3d95064
Error for libtype='both'
WillAyd Jun 18, 2024
dcb29ab
Revert "Error for libtype='both'"
WillAyd Jun 21, 2024
6f63161
Revert "Updated Python regex in bootstrap"
WillAyd Jun 21, 2024
9e0175f
Revert "cleanups"
WillAyd Jun 21, 2024
ebfc9fa
Revert "Hidden visibility for C modules"
WillAyd Jun 21, 2024
c3ea966
Merge remote-tracking branch 'upstream/main' into symbol-visibility
WillAyd Jun 21, 2024
1ddb081
Updates after simplification
WillAyd Jun 21, 2024
d5f6634
Refactor macros and Meson configuration
WillAyd Jun 21, 2024
71eca69
Use elif in macro
WillAyd Jun 21, 2024
25baf3d
Macro fix
WillAyd Jun 21, 2024
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
32 changes: 19 additions & 13 deletions meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,30 @@ configure_file(input: 'src/nanoarrow/nanoarrow_config.h.in',
output: 'nanoarrow_config.h',
configuration: conf_data)

# Windows shared libraries are not exporting the right symbols
# See https://github.com/mesonbuild/wrapdb/pull/1536#issuecomment-2136011408
# and https://github.com/apache/arrow-nanoarrow/issues/495

if host_machine.system() == 'windows'
libtype = 'static_library'
else
libtype = 'library'
libtype = get_option('default_library')
if libtype == 'both'
error('libtype "both" is not supported')
elif libtype == 'shared'
if meson.is_subproject()
add_project_arguments('-DNANOARROW_CONSUME_DLL', language: 'c')
add_project_arguments('-DNANOARROW_CONSUME_DLL', language: 'cpp')
else
add_project_arguments('-DNANOARROW_BUILD_DLL', language: 'c')
add_project_arguments('-DNANOARROW_BUILD_DLL', language: 'cpp')
endif
endif
endif

nanoarrow_lib = build_target(
nanoarrow_lib = library(
'nanoarrow',
'src/nanoarrow/array.c',
'src/nanoarrow/schema.c',
'src/nanoarrow/array_stream.c',
'src/nanoarrow/utils.c',
install: true,
target_type: libtype,
gnu_symbol_visibility: 'hidden',
)

incdir = include_directories('src/')
Expand All @@ -80,13 +87,13 @@ nanoarrow_dep = declare_dependency(include_directories: [incdir],
if get_option('ipc')
flatcc_dep = dependency('flatcc')

nanoarrow_ipc_lib = build_target(
nanoarrow_ipc_lib = library(
'nanoarrow_ipc',
'src/nanoarrow/nanoarrow_ipc_decoder.c',
'src/nanoarrow/nanoarrow_ipc_reader.c',
dependencies: [nanoarrow_dep, flatcc_dep],
install: true,
target_type: libtype,
gnu_symbol_visibility: 'hidden',
)
nanoarrow_ipc_dep = declare_dependency(include_directories: [incdir],
link_with: nanoarrow_ipc_lib,
Expand All @@ -112,13 +119,13 @@ if needs_device
error('CUDA support with the Meson build system is not implemented')
endif

nanoarrow_device_lib = build_target(
nanoarrow_device_lib = library(
'nanoarrow_device',
sources: device_srcs,
dependencies: device_deps,
install: true,
target_type: libtype,
cpp_args: device_defines,
gnu_symbol_visibility: 'hidden',
Copy link
Member

Choose a reason for hiding this comment

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

You can check the effect of this with nm --extern-only, but I am not sure that in nanoarrow's case (where we've been very careful to mark everything internal as static or put it in a header and mark it as static inline) this applies.

)
endif

Expand Down Expand Up @@ -252,7 +259,6 @@ if get_option('tests')
endif
endif


if get_option('benchmarks')
subdir('dev/benchmarks')
endif
Expand Down
4 changes: 2 additions & 2 deletions python/bootstrap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ def _define_regexes(self):
r"(?P<type>struct|union|enum) (?P<name>Arrow[^ ]+) {(?P<body>[^}]*)}"
)
self.re_func_def = re.compile(
r"\n(static inline )?(?P<const>const )?(struct |enum )?"
r"(?P<return_type>[A-Za-z0-9_*]+) "
r"\n(NANOARROW_EXPORT )?(static inline )?(?P<const>const )?(struct |enum )?"
r"(?P<return_type>[A-Za-z0-9_*]+)\s"
r"(?P<name>Arrow[A-Za-z0-9]+)\((?P<args>[^\)]*)\);"
)
self.re_tagged_type = re.compile(
Expand Down
2 changes: 1 addition & 1 deletion src/apps/dump_stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ int main(int argc, char* argv[]) {

end = clock();
elapsed = (end - begin) / ((double)CLOCKS_PER_SEC);
fprintf(stdout, "Read %l" PRId64 " rows in %" PRId64 " batch(es) <%.06f seconds>\n",
fprintf(stdout, "Read %" PRId64 " rows in %" PRId64 " batch(es) <%.06f seconds>\n",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated but figure minor enough to toss in

row_count, batch_count, elapsed);

ArrowArrayStreamRelease(&stream);
Expand Down
8 changes: 4 additions & 4 deletions src/nanoarrow/integration/c_data_integration.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,25 +209,25 @@ static const char* ConvertError(ArrowErrorCode errno_code) {

int64_t nanoarrow_BytesAllocated() { return kBytesAllocated; }

DLL_EXPORT const char* nanoarrow_CDataIntegration_ExportSchemaFromJson(
NANOARROW_EXPORT const char* nanoarrow_CDataIntegration_ExportSchemaFromJson(
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think NANOARROW_DLL might be more appropriate (except for our own usage internally, this will most of the time expand to an import rather than an export).

(nanoarrow_BytesAllocated() also needs this attribute)

const char* json_path, ArrowSchema* out) {
ArrowErrorInit(&global_error);
return ConvertError(ExportSchemaFromJson(json_path, out, &global_error));
}

DLL_EXPORT const char* nanoarrow_CDataIntegration_ImportSchemaAndCompareToJson(
NANOARROW_EXPORT const char* nanoarrow_CDataIntegration_ImportSchemaAndCompareToJson(
const char* json_path, ArrowSchema* schema) {
ArrowErrorInit(&global_error);
return ConvertError(ImportSchemaAndCompareToJson(json_path, schema, &global_error));
}

DLL_EXPORT const char* nanoarrow_CDataIntegration_ExportBatchFromJson(
NANOARROW_EXPORT const char* nanoarrow_CDataIntegration_ExportBatchFromJson(
const char* json_path, int num_batch, ArrowArray* out) {
ArrowErrorInit(&global_error);
return ConvertError(ExportBatchFromJson(json_path, num_batch, out, &global_error));
}

DLL_EXPORT const char* nanoarrow_CDataIntegration_ImportBatchAndCompareToJson(
NANOARROW_EXPORT const char* nanoarrow_CDataIntegration_ImportBatchAndCompareToJson(
const char* json_path, int num_batch, ArrowArray* batch) {
ArrowErrorInit(&global_error);
return ConvertError(
Expand Down
17 changes: 6 additions & 11 deletions src/nanoarrow/integration/c_data_integration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,9 @@
#ifndef NANOARROW_INTEGRATION_C_DATA_INTEGRATION_H_INCLUDED
#define NANOARROW_INTEGRATION_C_DATA_INTEGRATION_H_INCLUDED

#include <nanoarrow/nanoarrow_types.h>
#include <stdint.h>

#if defined(_MSC_VER)
#define DLL_EXPORT __declspec(dllexport)
#else
#define DLL_EXPORT
#endif

#ifdef __cplusplus
extern "C" {
#endif
Expand Down Expand Up @@ -76,19 +71,19 @@ struct ArrowArray {
#endif // ARROW_C_DATA_INTERFACE
#endif // ARROW_FLAG_DICTIONARY_ORDERED

DLL_EXPORT const char* nanoarrow_CDataIntegration_ExportSchemaFromJson(
NANOARROW_EXPORT const char* nanoarrow_CDataIntegration_ExportSchemaFromJson(
const char* json_path, struct ArrowSchema* out);

DLL_EXPORT const char* nanoarrow_CDataIntegration_ImportSchemaAndCompareToJson(
NANOARROW_EXPORT const char* nanoarrow_CDataIntegration_ImportSchemaAndCompareToJson(
const char* json_path, struct ArrowSchema* schema);

DLL_EXPORT const char* nanoarrow_CDataIntegration_ExportBatchFromJson(
NANOARROW_EXPORT const char* nanoarrow_CDataIntegration_ExportBatchFromJson(
const char* json_path, int num_batch, struct ArrowArray* out);

DLL_EXPORT const char* nanoarrow_CDataIntegration_ImportBatchAndCompareToJson(
NANOARROW_EXPORT const char* nanoarrow_CDataIntegration_ImportBatchAndCompareToJson(
const char* json_path, int num_batch, struct ArrowArray* batch);

DLL_EXPORT int64_t nanoarrow_BytesAllocated(void);
NANOARROW_EXPORT int64_t nanoarrow_BytesAllocated(void);

#ifdef __cplusplus
}
Expand Down
Loading
Loading