kou commented on code in PR #41599:
URL: https://github.com/apache/arrow/pull/41599#discussion_r1600815849
##########
c_glib/arrow-glib/codec.hpp:
##########
@@ -22,13 +22,20 @@
#include <arrow/util/compression.h>
#include <arrow-glib/codec.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/expression.hpp:
##########
@@ -22,8 +22,12 @@
#include <arrow/compute/expression.h>
#include <arrow-glib/expression.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/output-stream.hpp:
##########
@@ -24,24 +24,34 @@
#include <arrow/io/memory.h>
#include <arrow-glib/output-stream.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
ci/scripts/c_glib_build.sh:
##########
@@ -28,17 +28,38 @@ build_root=${2}
: ${BUILD_DOCS_C_GLIB:=OFF}
with_doc=$([ "${BUILD_DOCS_C_GLIB}" == "ON" ] && echo "true" || echo "false")
-export PKG_CONFIG_PATH=${ARROW_HOME}/lib/pkgconfig
+
+if [ -n "${MSYSTEM:-}" ]; then
+ # Fix ARROW_HOME when running under MSYS2
+ export ARROW_HOME="$(cygpath --unix "${ARROW_HOME}")"
+fi
+
+meson_pkg_config_path="${ARROW_HOME}/lib/pkgconfig:${ARROW_HOME}/bin/pkgconfig"
export CFLAGS="-DARROW_NO_DEPRECATED_API"
export CXXFLAGS="-DARROW_NO_DEPRECATED_API"
mkdir -p ${build_dir}
+if [ -n "${VCPKG_ROOT:-}" ]; then
+ vcpkg_install_root="${build_root}/vcpkg_installed"
+ $VCPKG_ROOT/vcpkg install --x-manifest-root=${source_dir}
--x-install-root=${vcpkg_install_root}
+ export
PKG_CONFIG="${vcpkg_install_root}/x64-windows/tools/pkgconf/pkgconf.exe"
+
meson_pkg_config_path="${vcpkg_install_root}/x64-windows/lib/pkgconfig:${meson_pkg_config_path}"
+fi
+
+if [ -n "${VCToolsInstallDir:-}" -a -n "${MSYSTEM:-}" ]; then
+ # Meson finds the gnu link.exe instead of MSVC link.exe when running in
MSYS2/git bash,
+ # so we need to make sure the MSCV link.exe is first in $PATH
+ export PATH="$(cygpath --unix
"${VCToolsInstallDir}")/bin/HostX64/x64:${PATH}"
Review Comment:
```suggestion
# Meson finds the gnu link.exe instead of MSVC link.exe when running in
MSYS2/git bash,
# so we need to make sure the MSCV link.exe is first in $PATH
export PATH="$(cygpath --unix
"${VCToolsInstallDir}")/bin/HostX64/x64:${PATH}"
```
##########
c_glib/arrow-glib/error.h:
##########
@@ -20,6 +20,7 @@
#pragma once
#include <glib-object.h>
+#include <arrow-glib/version.h>
Review Comment:
```suggestion
#include <arrow-glib/version.h>
```
##########
c_glib/arrow-glib/error.hpp:
##########
@@ -22,19 +22,28 @@
#include <arrow/api.h>
#include <arrow-glib/error.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/buffer.h:
##########
@@ -20,58 +20,90 @@
#pragma once
#include <glib-object.h>
+#include <arrow-glib/version.h>
Review Comment:
```suggestion
#include <arrow-glib/version.h>
```
##########
c_glib/arrow-glib/chunked-array.hpp:
##########
@@ -22,11 +22,17 @@
#include <arrow/api.h>
#include <arrow-glib/chunked-array.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/input-stream.hpp:
##########
@@ -25,35 +25,50 @@
#include <arrow/io/memory.h>
#include <arrow-glib/input-stream.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/record-batch.hpp:
##########
@@ -22,14 +22,20 @@
#include <arrow/api.h>
#include <arrow-glib/record-batch.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/table.hpp:
##########
@@ -23,11 +23,16 @@
#include <arrow/ipc/api.h>
#include <arrow-glib/table.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/file-system.hpp:
##########
@@ -22,29 +22,37 @@
#include <arrow/filesystem/api.h>
#include <arrow-glib/file-system.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/memory-pool.h:
##########
@@ -20,23 +20,32 @@
#pragma once
#include <glib-object.h>
+#include <arrow-glib/version.h>
Review Comment:
```suggestion
#include <arrow-glib/version.h>
```
##########
c_glib/arrow-glib/compute-definition.h:
##########
@@ -20,10 +20,12 @@
#pragma once
#include <glib-object.h>
+#include <arrow-glib/version.h>
Review Comment:
```suggestion
#include <arrow-glib/version.h>
```
##########
c_glib/arrow-glib/reader.hpp:
##########
@@ -26,43 +26,63 @@
#include <arrow/json/api.h>
#include <arrow-glib/reader.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/schema.hpp:
##########
@@ -22,8 +22,12 @@
#include <arrow/api.h>
#include <arrow-glib/schema.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
c_glib/arrow-glib/basic-array-definition.h:
##########
@@ -20,17 +20,20 @@
#pragma once
#include <glib-object.h>
+#include <arrow-glib/version.h>
Review Comment:
```suggestion
#include <arrow-glib/version.h>
```
##########
c_glib/arrow-glib/basic-array.hpp:
##########
@@ -22,23 +22,34 @@
#include <arrow/api.h>
#include <arrow-glib/basic-array.h>
+#include <arrow-glib/visibility.h>
Review Comment:
I think that this is needless.
```suggestion
```
##########
ci/scripts/c_glib_build.sh:
##########
@@ -28,17 +28,38 @@ build_root=${2}
: ${BUILD_DOCS_C_GLIB:=OFF}
with_doc=$([ "${BUILD_DOCS_C_GLIB}" == "ON" ] && echo "true" || echo "false")
-export PKG_CONFIG_PATH=${ARROW_HOME}/lib/pkgconfig
+
+if [ -n "${MSYSTEM:-}" ]; then
+ # Fix ARROW_HOME when running under MSYS2
+ export ARROW_HOME="$(cygpath --unix "${ARROW_HOME}")"
+fi
+
+meson_pkg_config_path="${ARROW_HOME}/lib/pkgconfig:${ARROW_HOME}/bin/pkgconfig"
Review Comment:
Do we need `:${ARROW_HOME}/bin/pkgconfig`?
##########
ci/scripts/c_glib_build.sh:
##########
@@ -28,17 +28,38 @@ build_root=${2}
: ${BUILD_DOCS_C_GLIB:=OFF}
with_doc=$([ "${BUILD_DOCS_C_GLIB}" == "ON" ] && echo "true" || echo "false")
-export PKG_CONFIG_PATH=${ARROW_HOME}/lib/pkgconfig
+
+if [ -n "${MSYSTEM:-}" ]; then
+ # Fix ARROW_HOME when running under MSYS2
+ export ARROW_HOME="$(cygpath --unix "${ARROW_HOME}")"
+fi
+
+meson_pkg_config_path="${ARROW_HOME}/lib/pkgconfig:${ARROW_HOME}/bin/pkgconfig"
export CFLAGS="-DARROW_NO_DEPRECATED_API"
export CXXFLAGS="-DARROW_NO_DEPRECATED_API"
mkdir -p ${build_dir}
+if [ -n "${VCPKG_ROOT:-}" ]; then
+ vcpkg_install_root="${build_root}/vcpkg_installed"
+ $VCPKG_ROOT/vcpkg install --x-manifest-root=${source_dir}
--x-install-root=${vcpkg_install_root}
+ export
PKG_CONFIG="${vcpkg_install_root}/x64-windows/tools/pkgconf/pkgconf.exe"
+
meson_pkg_config_path="${vcpkg_install_root}/x64-windows/lib/pkgconfig:${meson_pkg_config_path}"
Review Comment:
```suggestion
vcpkg_install_root="${build_root}/vcpkg_installed"
$VCPKG_ROOT/vcpkg install --x-manifest-root=${source_dir}
--x-install-root=${vcpkg_install_root}
export
PKG_CONFIG="${vcpkg_install_root}/x64-windows/tools/pkgconf/pkgconf.exe"
meson_pkg_config_path="${vcpkg_install_root}/x64-windows/lib/pkgconfig:${meson_pkg_config_path}"
```
##########
ci/scripts/c_glib_build.sh:
##########
@@ -28,17 +28,38 @@ build_root=${2}
: ${BUILD_DOCS_C_GLIB:=OFF}
with_doc=$([ "${BUILD_DOCS_C_GLIB}" == "ON" ] && echo "true" || echo "false")
-export PKG_CONFIG_PATH=${ARROW_HOME}/lib/pkgconfig
+
+if [ -n "${MSYSTEM:-}" ]; then
+ # Fix ARROW_HOME when running under MSYS2
+ export ARROW_HOME="$(cygpath --unix "${ARROW_HOME}")"
Review Comment:
```suggestion
# Fix ARROW_HOME when running under MSYS2
export ARROW_HOME="$(cygpath --unix "${ARROW_HOME}")"
```
--
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]
