Skip to content

Commit 202b8f1

Browse files
committed
squashing. things should work. add a proper message
1 parent 6c3bf16 commit 202b8f1

6 files changed

Lines changed: 23 additions & 137 deletions

File tree

cpp/meson.options

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,18 +90,6 @@ option(
9090
description: 'Arbitrary string that identifies the kind of package (for informational purposes)',
9191
)
9292
option('parquet', type: 'feature', description: 'Build the Parquet libraries')
93-
option(
94-
'parquet_build_dbps_libs',
95-
type: 'feature',
96-
value: 'enabled',
97-
description: 'Build DBPS external libraries',
98-
)
99-
option(
100-
'parquet_dbps_interface_include_dir',
101-
type: 'string',
102-
value: '',
103-
description: 'Path to DBPS interface headers directory (must contain dbpa_interface.h and enums.h, typically .../DataBatchProtectionService/src/common). Used by Parquet external DBPA encryption when building with Meson.',
104-
)
10593
option(
10694
'parquet_build_executables',
10795
type: 'feature',

cpp/src/arrow/meson.build

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -385,17 +385,13 @@ if needs_filesystem
385385
{'BUILD_PERFORMANCE_TESTS': 'FALSE'},
386386
{'BUILD_SAMPLES': 'FALSE'},
387387
{'BUILD_TESTING': 'FALSE'},
388+
{'BUILD_WINDOWS_UWP': 'TRUE'},
388389
{'CMAKE_UNITY_BUILD': 'FALSE'},
389390
{'DISABLE_AZURE_CORE_OPENTELEMETRY': 'TRUE'},
390391
{'ENV{AZURE_SDK_DISABLE_AUTO_VCPKG}': 'TRUE'},
391392
{'WARNINGS_AS_ERRORS': 'FALSE'},
392393
)
393-
if host_machine.system() == 'windows'
394-
azure_opt.add_cmake_defines({'BUILD_WINDOWS_UWP': 'TRUE'})
395-
endif
396-
if host_machine.system() != 'windows'
397-
azure_opt.append_compile_args('cpp', '-fPIC')
398-
endif
394+
azure_opt.append_compile_args('cpp', '-fPIC')
399395
azure_proj = cmake.subproject('azure', options: azure_opt)
400396

401397
azure_dep = declare_dependency(
@@ -636,11 +632,13 @@ if needs_testing
636632

637633
gtest_dep = dependency('gtest')
638634
gtest_main_dep = dependency('gtest_main')
635+
gtest_dep = dependency('gtest')
639636
gmock_dep = dependency('gmock')
640637
else
641638
filesystem_dep = disabler()
642639
gtest_dep = disabler()
643640
gtest_main_dep = disabler()
641+
gtest_dep = disabler()
644642
gmock_dep = disabler()
645643
endif
646644

cpp/src/parquet/encryption/external/dbpa_test_agent.cc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,7 @@ DBPATestAgent::~DBPATestAgent() {}
149149

150150
// Export function for creating new instances from shared library
151151
extern "C" {
152-
// Keep this symbol visible and retained so dlopen()+dlsym() based tests work reliably
153-
// under various linkers/flags (e.g. LTO / section GC).
154-
PARQUET_EXPORT DataBatchProtectionAgentInterface*
155-
create_new_instance() {
152+
DataBatchProtectionAgentInterface* create_new_instance() {
156153
return new parquet::encryption::external::DBPATestAgent();
157154
}
158155
}

cpp/src/parquet/encryption/external/test_utils.cc

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#include <filesystem>
2121
#include <string>
2222
#include <vector>
23-
#include <iostream>
2423

2524
#ifdef __APPLE__
2625
# include <mach-o/dyld.h>
@@ -58,21 +57,6 @@ std::string TestUtils::GetExecutableDirectory() {
5857
}
5958

6059
std::string TestUtils::GetTestLibraryPath() {
61-
// Strong override: reuse the same env var as the Python tooling
62-
// (`python/scripts/base_app.py`): DBPA_LIBRARY_PATH.
63-
//
64-
// This allows CI/build systems to provide the exact path to the DBPA agent shared
65-
// library, avoiding reliance on executable-path heuristics or current working directory.
66-
const char* explicit_path = std::getenv("DBPA_LIBRARY_PATH");
67-
if (explicit_path && explicit_path[0]) {
68-
std::string p(explicit_path);
69-
if (std::filesystem::exists(p)) {
70-
return p;
71-
}
72-
throw std::runtime_error("DBPA_LIBRARY_PATH is set but the file does not exist: " +
73-
p);
74-
}
75-
7660
// Check for environment variable to override the executable directory
7761
const char* cwd_override = std::getenv("PARQUET_TEST_LIBRARY_CWD");
7862
std::string base_path;
@@ -167,17 +151,7 @@ std::string TestUtils::GetTestLibraryPath() {
167151
}
168152
}
169153

170-
// Provide a detailed error to make CI failures diagnosable.
171-
std::string msg = "Could not find DBPA test agent library. Tried:\n";
172-
for (const auto& filename : possible_filenames) {
173-
for (const auto& directory : possible_directories) {
174-
msg += " - " + (directory + filename) + "\n";
175-
}
176-
}
177-
msg += "PARQUET_TEST_LIBRARY_CWD=";
178-
msg += (cwd_override && cwd_override[0]) ? cwd_override : "<unset>";
179-
msg += "\n";
180-
throw std::runtime_error(msg);
154+
throw std::runtime_error("test_utils.cc: Could not find DBPATestAgent library ");
181155
}
182156

183157
} // namespace parquet::encryption::external::test

cpp/src/parquet/encryption/external_dbpa_encryption.cc

Lines changed: 2 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -62,17 +62,8 @@ std::unique_ptr<dbps::external::DataBatchProtectionAgentInterface> LoadAndInitia
6262
// Step 1: Get path to the shared library
6363
auto it = configuration_properties.find(SHARED_LIBRARY_PATH_KEY);
6464
if (it == configuration_properties.end()) {
65-
std::string msg = "Required configuration key '" + SHARED_LIBRARY_PATH_KEY +
66-
"' not found in configuration_properties. Present keys: ";
67-
bool first = true;
68-
for (const auto& kv : configuration_properties) {
69-
if (!first) msg += ", ";
70-
first = false;
71-
msg += kv.first;
72-
}
73-
if (first) {
74-
msg += "<none>";
75-
}
65+
const auto msg = "Required configuration key '" + SHARED_LIBRARY_PATH_KEY +
66+
"' not found in configuration_properties";
7667
ARROW_LOG(ERROR) << msg;
7768
throw ParquetException(msg);
7869
}
@@ -439,19 +430,6 @@ ExternalDBPAEncryptorAdapter* ExternalDBPAEncryptorAdapterFactory::GetEncryptor(
439430
auto app_context = external_file_encryption_properties->app_context();
440431
auto connection_config_for_algorithm = configuration_properties.at(algorithm);
441432

442-
if (::arrow::util::ArrowLog::IsLevelEnabled(
443-
::arrow::util::ArrowLogLevel::ARROW_DEBUG)) {
444-
ARROW_LOG(DEBUG) << "ExternalDBPAEncryptorAdapterFactory::GetEncryptor - "
445-
"selected configuration_properties for EXTERNAL_DBPA_V1:";
446-
if (connection_config_for_algorithm.empty()) {
447-
ARROW_LOG(DEBUG) << " <empty map>";
448-
} else {
449-
for (const auto& [k, v] : connection_config_for_algorithm) {
450-
ARROW_LOG(DEBUG) << " [" << k << "]: [" << v << "]";
451-
}
452-
}
453-
}
454-
455433
std::string key_id;
456434
try {
457435
auto key_metadata =
@@ -681,19 +659,6 @@ std::unique_ptr<DecryptorInterface> ExternalDBPADecryptorAdapterFactory::GetDecr
681659
auto connection_config_for_algorithm = configuration_properties.at(algorithm);
682660
auto key_value_metadata = column_chunk_metadata->key_value_metadata();
683661

684-
if (::arrow::util::ArrowLog::IsLevelEnabled(
685-
::arrow::util::ArrowLogLevel::ARROW_DEBUG)) {
686-
ARROW_LOG(DEBUG) << "ExternalDBPADecryptorAdapterFactory::GetDecryptor - "
687-
"selected configuration_properties for EXTERNAL_DBPA_V1:";
688-
if (connection_config_for_algorithm.empty()) {
689-
ARROW_LOG(DEBUG) << " <empty map>";
690-
} else {
691-
for (const auto& [k, v] : connection_config_for_algorithm) {
692-
ARROW_LOG(DEBUG) << " [" << k << "]: [" << v << "]";
693-
}
694-
}
695-
}
696-
697662
std::string key_id;
698663
try {
699664
auto key_metadata = KeyMetadata::Parse(crypto_metadata->key_metadata());

cpp/src/parquet/meson.build

Lines changed: 15 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,6 @@ if not thrift_dep.found()
6565
{
6666
'BUILD_COMPILER': 'OFF',
6767
'BUILD_EXAMPLES': 'OFF',
68-
'BUILD_TESTING': 'OFF',
69-
'BUILD_TESTS': 'OFF',
7068
'BUILD_TUTORIALS': 'OFF',
7169
'CMAKE_UNITY_BUILD': 'OFF',
7270
'WITH_AS3': 'OFF',
@@ -129,7 +127,10 @@ if openssl_dep.found()
129127
# External DBPA integration and its header-only deps are only relevant when
130128
# encryption support is enabled (i.e., OpenSSL is available).
131129
if needs_parquet_encryption or get_option('parquet_require_encryption').auto()
132-
tcb_span_dep = dependency('tcb_span', fallback: ['tcb-span', 'tcb_span_dep'])
130+
tcb_span_dep = dependency(
131+
'tcb_span',
132+
fallback: ['tcb-span', 'tcb_span_dep'],
133+
)
133134
magic_enum_dep = dependency(
134135
'magic_enum_header_only',
135136
fallback: ['magic-enum', 'magic_enum_dep'],
@@ -139,38 +140,17 @@ if openssl_dep.found()
139140
#
140141
# We do not build DBPS agents as part of Arrow's Meson build. We only need the
141142
# DBPS interface headers (dbpa_interface.h, enums.h, etc.) for compilation.
142-
#
143-
# Resolution order:
144-
# - If user provides a local include dir (Meson option or env var), use it.
145-
# - Otherwise, fall back to Meson's wrap subproject `dbps_agent`, which downloads
146-
# and unpacks DBPS sources, and exposes only the interface headers via a tiny
147-
# header-only Meson wrapper (cpp/subprojects/packagefiles/dbps_agent).
148-
fs = import('fs')
149-
dbps_inc = get_option('parquet_dbps_interface_include_dir')
150-
if dbps_inc != ''
151-
if not fs.exists(dbps_inc / 'dbpa_interface.h')
152-
error('parquet_dbps_interface_include_dir/PARQUET_DBPS_INTERFACE_INCLUDE_DIR was set, but dbpa_interface.h was not found in: ' + dbps_inc)
153-
endif
154-
dbps_interface_dep = declare_dependency(compile_args: ['-I' + dbps_inc])
155-
else
156-
dbps_sp = subproject('dbps_agent')
157-
dbps_interface_dep = dbps_sp.get_variable('dbps_interface_dep')
158-
endif
143+
# These are obtained via Meson's wrap subproject `dbps_agent`, which downloads
144+
# and unpacks DBPS sources, and exposes only the interface headers via a tiny
145+
# header-only Meson wrapper (cpp/subprojects/packagefiles/dbps_agent).
146+
dbps_sp = subproject('dbps_agent')
147+
dbps_interface_dep = dbps_sp.get_variable('dbps_interface_dep')
159148

160149
parquet_deps += [dbps_interface_dep, tcb_span_dep, magic_enum_dep]
161150

162-
if get_option('parquet_build_dbps_libs').enabled()
163-
warning(
164-
'Meson does not build DBPS shared libraries. Provide your own agent shared library and set configuration_properties["agent_library_path"], or build DBPS via its CMake build separately.',
165-
)
166-
endif
167-
168151
# Build the in-tree DBPA test agent shared library used by external encryption tests.
169152
# CMake builds this as `DBPATestAgent` when ARROW_TESTING is enabled; Meson needs an
170153
# equivalent target so tests can dlopen() `libDBPATestAgent.so`.
171-
#
172-
# Keep it Meson-native (no CMake), and place the output next to parquet test
173-
# executables so `TestUtils::GetTestLibraryPath()` can find it via the executable dir.
174154
if needs_testing
175155
dbpa_test_agent_lib = shared_library(
176156
'DBPATestAgent',
@@ -190,23 +170,11 @@ else
190170
parquet_srcs += files('encryption/aes_encryption_nossl.cc')
191171
endif
192172

193-
194-
# Parquet's CMake build uses explicit export macros and (on ELF) a version script
195-
# to control symbol visibility. Meson doesn't currently replicate that machinery.
196-
# With Meson's default hidden visibility, some non-exported-but-test-used symbols
197-
# (e.g. EncodingProperties, IsParquetCipherSupported) are not linkable from test
198-
# executables. When building tests/benchmarks, relax visibility to avoid link
199-
# failures in Meson CI.
200-
parquet_symbol_visibility = 'inlineshidden'
201-
if needs_testing
202-
parquet_symbol_visibility = 'default'
203-
endif
204-
205173
parquet_lib = library(
206174
'arrow-parquet',
207175
sources: parquet_srcs,
208176
dependencies: parquet_deps,
209-
gnu_symbol_visibility: parquet_symbol_visibility,
177+
gnu_symbol_visibility: 'inlineshidden',
210178
)
211179

212180
parquet_dep = declare_dependency(
@@ -332,7 +300,10 @@ parquet_tests = {
332300
),
333301
},
334302
'file_deserialize_test': {
335-
'sources': files('file_deserialize_test.cc', 'encryption/external/test_utils.cc'),
303+
'sources': files(
304+
'encryption/external/test_utils.cc',
305+
'file_deserialize_test.cc',
306+
),
336307
},
337308
'schema_test': {'sources': files('schema_test.cc')},
338309
}
@@ -402,18 +373,11 @@ foreach key, val : parquet_tests
402373
# Make DBPATestAgent lookup deterministic under Meson. Some CI setups may
403374
# not allow /proc/self/exe resolution or run tests with unexpected cwd.
404375
'PARQUET_TEST_LIBRARY_CWD': meson.current_build_dir(),
405-
# Reuse the standard env var used by Python tooling (`base_app.py`).
406-
# Prefer this in C++ as well.
407376
'DBPA_LIBRARY_PATH': dbpa_test_agent_path,
408377
# Hardcode DBPA logging for Meson test runs to aid CI diagnostics.
409378
'PARQUET_DBPA_LOG_LEVEL': 'INFO',
410379
}
411-
test(
412-
test_name,
413-
exc,
414-
depends: dbpa_test_agent_lib,
415-
env: test_env,
416-
)
380+
test(test_name, exc, depends: dbpa_test_agent_lib, env: test_env)
417381
endforeach
418382

419383
parquet_benchmarks = {

0 commit comments

Comments
 (0)