kou commented on code in PR #41886:
URL: https://github.com/apache/arrow/pull/41886#discussion_r1623285096
##########
csharp/generate_gobject_bindings.sh:
##########
@@ -0,0 +1,55 @@
+#!/bin/bash
+
+set -e
+
+if [ -z "${ARROW_HOME}" ]; then
+ echo "ARROW_HOME must be set"
+ exit 1
+fi
+
+csharp_dir=$(dirname $0)
+source_dir="${csharp_dir}/src"
+gir_dir="${csharp_dir}/gir_files"
+
+gircore_gir_dir="${csharp_dir}/tools/gir.core/ext/gir-files"
+arrow_gir_dir="${ARROW_HOME}/share/gir-1.0"
+
+gir_dependencies=(GObject-2.0 GLib-2.0 Gio-2.0 GModule-2.0)
+arrow_gobject_libs=(Arrow-1.0 ArrowDataset-1.0)
+namespaces=(Apache.Arrow.GLibBindings Apache.Arrow.Dataset.GLibBindings)
+
+os_names=(linux macos windows)
+for os_name in ${os_names[@]}; do
Review Comment:
In general, we should use `"${ARRAY[@]}"` style to avoid unexpected.
```suggestion
for os_name in "${os_names[@]}"; do
```
e.g.:
```shell
a=(aaa "bbb ccc" ddd)
echo "quoted"
for x in "${a[@]}"; do
echo "<${x}>"
done
echo "not quoted"
for x in ${a[@]}; do
echo "<${x}>"
done
```
```console
$ bash /tmp/xxx.sh
quoted
<aaa>
<bbb ccc>
<ddd>
not quoted
<aaa>
<bbb>
<ccc>
<ddd>
```
##########
csharp/glib_generated_fixes.patch:
##########
@@ -0,0 +1,2285 @@
+diff --git
a/csharp/src/Apache.Arrow.Dataset.GLibBindings-1.0/Public/DatasetFactory.Methods.Generated.cs
b/csharp/src/Apache.Arrow.Dataset.GLibBindings-1.0/Public/DatasetFactory.Methods.Generated.cs
+index dfaac7a27..6068d4530 100644
+---
a/csharp/src/Apache.Arrow.Dataset.GLibBindings-1.0/Public/DatasetFactory.Methods.Generated.cs
++++
b/csharp/src/Apache.Arrow.Dataset.GLibBindings-1.0/Public/DatasetFactory.Methods.Generated.cs
+@@ -14,7 +14,7 @@ public partial class DatasetFactory
+ {
+
+ [Version("5.0.0")]
+-public Apache.Arrow.Dataset.GLibBindings.Dataset?
Finish(Apache.Arrow.Dataset.GLibBindings.FinishOptions? options)
++public virtual Apache.Arrow.Dataset.GLibBindings.Dataset?
Finish(Apache.Arrow.Dataset.GLibBindings.FinishOptions? options)
+ {
+
+ var resultFinish =
Apache.Arrow.Dataset.GLibBindings.Internal.DatasetFactory.Finish(this.Handle,
options?.Handle ?? IntPtr.Zero, out GLib.Internal.ErrorOwnedHandle error);
+diff --git
a/csharp/src/Apache.Arrow.Dataset.GLibBindings-1.0/Public/FileSystemDatasetFactory.Methods.Generated.cs
b/csharp/src/Apache.Arrow.Dataset.GLibBindings-1.0/Public/FileSystemDatasetFactory.Methods.Generated.cs
+index 809181969..6c0c5855a 100644
+---
a/csharp/src/Apache.Arrow.Dataset.GLibBindings-1.0/Public/FileSystemDatasetFactory.Methods.Generated.cs
++++
b/csharp/src/Apache.Arrow.Dataset.GLibBindings-1.0/Public/FileSystemDatasetFactory.Methods.Generated.cs
+@@ -25,7 +25,7 @@ if(!error.IsInvalid)
+ }
+
+ [Version("5.0.0")]
+-public Apache.Arrow.Dataset.GLibBindings.FileSystemDataset?
Finish(Apache.Arrow.Dataset.GLibBindings.FinishOptions? options)
++public override Apache.Arrow.Dataset.GLibBindings.FileSystemDataset?
Finish(Apache.Arrow.Dataset.GLibBindings.FinishOptions? options)
+ {
+
+ var resultFinish =
Apache.Arrow.Dataset.GLibBindings.Internal.FileSystemDatasetFactory.Finish(this.Handle,
options?.Handle ?? IntPtr.Zero, out GLib.Internal.ErrorOwnedHandle error);
+diff --git
a/csharp/src/Apache.Arrow.GLibBindings-1.0/Public/AggregateNodeOptions.Constructors.Generated.cs
b/csharp/src/Apache.Arrow.GLibBindings-1.0/Public/AggregateNodeOptions.Constructors.Generated.cs
+index 313ec0ef1..caa4d5ab6 100644
+---
a/csharp/src/Apache.Arrow.GLibBindings-1.0/Public/AggregateNodeOptions.Constructors.Generated.cs
++++
b/csharp/src/Apache.Arrow.GLibBindings-1.0/Public/AggregateNodeOptions.Constructors.Generated.cs
+@@ -16,10 +16,14 @@ public partial class AggregateNodeOptions
+ [Version("6.0.0")]
+ public static AggregateNodeOptions? New(GLib.List aggregations, string[]?
keys, nuint nKeys)
+ {
+-
++
++ if (keys == null)
++ {
++ throw new ArgumentNullException(nameof(keys));
++ }
Review Comment:
Why do we need this?
This is annotated as `nullable`:
https://github.com/apache/arrow/blob/255dbf990c3d3e5fb1270a2a11efe0af2be195ab/c_glib/arrow-glib/compute.cpp#L1380
If this is a current Gir.Core limitation, could you report this to Gir.Core?
##########
csharp/generate_gobject_bindings.sh:
##########
@@ -0,0 +1,55 @@
+#!/bin/bash
+
+set -e
+
+if [ -z "${ARROW_HOME}" ]; then
+ echo "ARROW_HOME must be set"
+ exit 1
+fi
+
+csharp_dir=$(dirname $0)
+source_dir="${csharp_dir}/src"
+gir_dir="${csharp_dir}/gir_files"
+
+gircore_gir_dir="${csharp_dir}/tools/gir.core/ext/gir-files"
+arrow_gir_dir="${ARROW_HOME}/share/gir-1.0"
+
+gir_dependencies=(GObject-2.0 GLib-2.0 Gio-2.0 GModule-2.0)
+arrow_gobject_libs=(Arrow-1.0 ArrowDataset-1.0)
+namespaces=(Apache.Arrow.GLibBindings Apache.Arrow.Dataset.GLibBindings)
+
+os_names=(linux macos windows)
+for os_name in ${os_names[@]}; do
+ mkdir -p "${gir_dir}/${os_name}"
+ for gir_dep in ${gir_dependencies[@]}; do
+ cp "${gircore_gir_dir}/${os_name}/${gir_dep}.gir"
"${gir_dir}/${os_name}"
+ done
+
+done
+
+# TODO: Make this select the appropriate platform and merge all platforms in
CI build
+os_name="linux"
+for index in ${!arrow_gobject_libs[*]}; do
Review Comment:
```suggestion
for index in "${!arrow_gobject_libs[@]}"; do
```
##########
csharp/generate_gobject_bindings.sh:
##########
@@ -0,0 +1,55 @@
+#!/bin/bash
+
+set -e
+
+if [ -z "${ARROW_HOME}" ]; then
+ echo "ARROW_HOME must be set"
+ exit 1
+fi
+
+csharp_dir=$(dirname $0)
+source_dir="${csharp_dir}/src"
+gir_dir="${csharp_dir}/gir_files"
+
+gircore_gir_dir="${csharp_dir}/tools/gir.core/ext/gir-files"
+arrow_gir_dir="${ARROW_HOME}/share/gir-1.0"
+
+gir_dependencies=(GObject-2.0 GLib-2.0 Gio-2.0 GModule-2.0)
+arrow_gobject_libs=(Arrow-1.0 ArrowDataset-1.0)
+namespaces=(Apache.Arrow.GLibBindings Apache.Arrow.Dataset.GLibBindings)
+
+os_names=(linux macos windows)
+for os_name in ${os_names[@]}; do
+ mkdir -p "${gir_dir}/${os_name}"
+ for gir_dep in ${gir_dependencies[@]}; do
Review Comment:
```suggestion
for gir_dep in "${gir_dependencies[@]}"; do
```
##########
csharp/src/Apache.Arrow.Dataset.GLibBindings-1.0/Module.cs:
##########
@@ -0,0 +1,22 @@
+namespace Apache.Arrow.Dataset.GLibBindings;
+
+public static class Module
+{
+ public static void Initialize()
+ {
+ if (_isInitialized)
+ {
+ return;
+ }
+
+ GObject.Module.Initialize();
Review Comment:
Can we remove this because `Apache.Arrow.GLibBindings.Module.Initialize()`
calls this?
##########
csharp/generate_gobject_bindings.sh:
##########
@@ -0,0 +1,55 @@
+#!/bin/bash
+
+set -e
+
+if [ -z "${ARROW_HOME}" ]; then
+ echo "ARROW_HOME must be set"
+ exit 1
+fi
+
+csharp_dir=$(dirname $0)
+source_dir="${csharp_dir}/src"
+gir_dir="${csharp_dir}/gir_files"
+
+gircore_gir_dir="${csharp_dir}/tools/gir.core/ext/gir-files"
+arrow_gir_dir="${ARROW_HOME}/share/gir-1.0"
+
+gir_dependencies=(GObject-2.0 GLib-2.0 Gio-2.0 GModule-2.0)
+arrow_gobject_libs=(Arrow-1.0 ArrowDataset-1.0)
+namespaces=(Apache.Arrow.GLibBindings Apache.Arrow.Dataset.GLibBindings)
+
+os_names=(linux macos windows)
+for os_name in ${os_names[@]}; do
+ mkdir -p "${gir_dir}/${os_name}"
+ for gir_dep in ${gir_dependencies[@]}; do
+ cp "${gircore_gir_dir}/${os_name}/${gir_dep}.gir"
"${gir_dir}/${os_name}"
+ done
+
+done
+
+# TODO: Make this select the appropriate platform and merge all platforms in
CI build
+os_name="linux"
+for index in ${!arrow_gobject_libs[*]}; do
+ arrow_lib=${arrow_gobject_libs[$index]}
+ cp "${arrow_gir_dir}/${arrow_lib}.gir" "${gir_dir}/${os_name}"
+ namespace=${namespaces[$index]}
+ sed -i.bak "s/<namespace name=\"\([^\"]*\)\"/<namespace
name=\"${namespace}\"/" "${gir_dir}/${os_name}/${arrow_lib}.gir"
+ rm -r "${gir_dir}/${os_name}/${arrow_lib}.gir.bak"
Review Comment:
```suggestion
namespace=${namespaces[$index]}
sed -e "s/<namespace name=\"\([^\"]*\)\"/<namespace
name=\"${namespace}\"/" \
"${arrow_gir_dir}/${arrow_lib}.gir" > "${gir_dir}/${os_name}"
```
--
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]
