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.

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

GH-44515: [D] Add Initial Support #44536

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

kassane
Copy link

@kassane kassane commented Oct 26, 2024

Rationale for this change

Add Dlang support. Initial implementation based on c_glib, using gir-to-d generator.

Tools used

  • ldc2 (based on LLVM backend + dmd-frontend) toolchain - works in macOS ARM64, too.
  • dub (pkg-manager, like cargo [rust])
  • gtkd-developers/gir-to-d

What changes are included in this PR?

Add D implementation only. No modify any other language support.

Are these changes tested?

Unit tests will also be added during development.

Are there any user-facing changes?

It adds a new programming language alternative for attracting more users (the small D community).

References

cc: @kou

Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@kassane kassane changed the title [D] Initial Support GH:44515 - [D] Add initial Support Oct 26, 2024
@kassane kassane changed the title GH:44515 - [D] Add initial Support GH:44515 [D] Add Initial Support Oct 26, 2024
@kou kou changed the title GH:44515 [D] Add Initial Support GH-44515: [D] Add Initial Support Oct 26, 2024
Copy link

⚠️ GitHub issue #44515 has been automatically assigned in GitHub to PR creator.

@kou
Copy link
Member

kou commented Oct 26, 2024

Could you remove auto generated files?
Can we add a shell script that generates files and fix them (by sed or something)?

@kassane
Copy link
Author

kassane commented Oct 26, 2024

Could you remove auto generated files?

Ok.

Can we add a shell script that generates files and fix them (by sed or something)?

It's possible, but is it viable?
Because there are so many lines and the fixes vary ( looking at previous commits).

Note that it is possible to use meson to configure the build of D projects (officially supported). However, it may be interesting to make build.d with D (std/phobos2) features to solve.

@kassane
Copy link
Author

kassane commented Oct 26, 2024

Now, base build.d done. (try run dub build to test)

  • cpp:cmake (config, build): 🆗 - the flags can be revised later!
  • c_glib:meson (setup, build): 🆗 - the flags can be revised later!

Right now, I will review the issue of gir-to-d. If needed, it requires an improved fork to fix the D bindings generation.
Note: Still need to fix D's CI later.

@kou
Copy link
Member

kou commented Oct 26, 2024

I'm not familiar with D but why do we need to create a D program just build the D bindings?

Can we simplify this?

  • Can we detect already installed Apache Arrow GLib by pkgconf --cflags --libs arrow-glib instead of building Apache Arrow C++ and GLib?
  • Can we create a shell script that just runs dub run girtod -- --gir-directory $(pkgconf --variable=girdir arrow-glib) --input $(pkgconf --variable=girdir arrow-glib/Arrow-1.0.gir --output source --user-runtime-linker?

@kassane
Copy link
Author

kassane commented Oct 26, 2024

I'm not familiar with D but why do we need to create a D program just build the D bindings?

Like cargo (Rust), dub only get D files (all *.d in rootpath) by default.

build.d will only be applied for building non-D project.
For this use case, it is assumed that the user does not have Arrow installed.

Can we simplify this?

  • Can we detect already installed Apache Arrow GLib by pkgconf --cflags --libs arrow-glib instead of building Apache Arrow C++ and GLib?

dub.json: libs = [...] use pkg-config by default on posix.

Also is possible add specialized libraries like: libs-posix|libs-windows

  • Can we create a shell script that just runs dub run girtod -- --gir-directory $(pkgconf --variable=girdir arrow-glib) --input $(pkgconf --variable=girdir arrow-glib/Arrow-1.0.gir --output source --user-runtime-linker?

Yeah! dub.json can add multi commands in preBuildCommands. Like dub.json from glibD project.

@kou
Copy link
Member

kou commented Oct 27, 2024

it is assumed that the user does not have Arrow installed.

Can we require users to install Apache Arrow GLib before they install the D bindings?

FYI: We have the APT/Yum repositories for Apache Arrow GLib: https://arrow.apache.org/install/

  • Can we create a shell script that just runs dub run girtod -- --gir-directory $(pkgconf --variable=girdir arrow-glib) --input $(pkgconf --variable=girdir arrow-glib/Arrow-1.0.gir --output source --user-runtime-linker?

Yeah! dub.json can add multi commands in preBuildCommands. Like dub.json from glibD project.

Could you try this approach?

@kassane
Copy link
Author

kassane commented Oct 27, 2024

Could you try this approach?

"preBuildCommands": [
		"dub run girtod -- -i Arrow-1.0.gir -o source --use-runtime-linker",
		"dub run girtod -- -i ArrowDataset-1.0.gir -o source --use-runtime-linker",
		"dub run girtod -- -i ArrowFlight-1.0.gir -o source --use-runtime-linker"
	],
"dependencies": {
		"glibd": {
			"repository": "git+https://github.com/gtkd-developers/GlibD.git", // <= get glibd + gir-to-d, generating gio bindings 
			"version": "1546823185334c4727d378baf890fa13d9fa4cbd" // latest commit
		}
	},

Works: bb07030

dub build-ouput
dub test -f --root=d
             Generating test runner configuration 'arrow-d-test-unittest' for 'unittest' (library).
     Pre-gen Running commands for glibd
    Existing package girtod found locally
0 packages fetched, 1 already present, 0 failed
             Building package girtod in /home/kassane/.dub/packages/girtod/0.23.2/girtod/
     Pre-gen Running commands for girtod
    Starting Performing "debug" build using /home/kassane/zig/ldc2-master/bin/ldc2 for x86_64.
    Building girtod 0.23.2: building configuration [application]
     Linking girtod
     Running ../../../girtod/0.23.2/girtod/girtod -i src -o generated --use-runtime-linker
copying file [src/gtkd] to [generated/gtkd]
    Starting Performing "unittest" build using /home/kassane/zig/ldc2-master/bin/ldc2 for x86_64.
    Building glibd 2.4.3+commit.2.g1546823: building configuration [library]
    Building arrow-d ~master: building configuration [arrow-d-test-unittest]
   Pre-build Running commands
             Building package girtod in /home/kassane/.dub/packages/girtod/0.23.2/girtod/
     Pre-gen Running commands for girtod
    Starting Performing "debug" build using /home/kassane/zig/ldc2-master/bin/ldc2 for x86_64.
    Building girtod 0.23.2: building configuration [application]
     Linking girtod
     Running ../../.dub/packages/girtod/0.23.2/girtod/girtod -i Arrow-1.0.gir -o source --use-runtime-linker
             Building package girtod in /home/kassane/.dub/packages/girtod/0.23.2/girtod/
     Pre-gen Running commands for girtod
    Starting Performing "debug" build using /home/kassane/zig/ldc2-master/bin/ldc2 for x86_64.
    Building girtod 0.23.2: building configuration [application]
     Linking girtod
     Running ../../.dub/packages/girtod/0.23.2/girtod/girtod -i ArrowDataset-1.0.gir -o source --use-runtime-linker
             Building package girtod in /home/kassane/.dub/packages/girtod/0.23.2/girtod/
     Pre-gen Running commands for girtod
    Starting Performing "debug" build using /home/kassane/zig/ldc2-master/bin/ldc2 for x86_64.
    Building girtod 0.23.2: building configuration [application]
     Linking girtod
     Running ../../.dub/packages/girtod/0.23.2/girtod/girtod -i ArrowFlight-1.0.gir -o source --use-runtime-linker

@kassane
Copy link
Author

kassane commented Oct 27, 2024

@kassane

This comment was marked as resolved.

paths:
- '.dockerignore'
- '.github/workflows/d.yml'
- 'ci/docker/*d*'
Copy link
Member

Choose a reason for hiding this comment

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

All .dockerfiles are matched...

Copy link
Author

Choose a reason for hiding this comment

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

@kou,
I have not listed the d/ files in .dockerignore.
One question: is docker_ruby already included in c_glib?

Previously the CI/CD section, I just attempted adapting swift-lang recipe.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- 'ci/docker/*d*'
- 'ci/docker/*-d.dockerfile'

One question: is docker_ruby already included in c_glib?

What is referred by the docker_ruby? https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-ruby.dockerfile ?

.gitignore Outdated Show resolved Hide resolved
ci/docker/ubuntu-d.dockerfile Outdated Show resolved Hide resolved
d/.gitignore Outdated Show resolved Hide resolved
d/.gitignore Show resolved Hide resolved
d/README.md Outdated Show resolved Hide resolved
d/source/APILookup.txt Outdated Show resolved Hide resolved
d/source/APILookupArrowDataset.txt Outdated Show resolved Hide resolved
d/source/APILookupArrowFlight.txt Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting review Awaiting review awaiting changes Awaiting changes labels Oct 30, 2024
* use `gir-to-d` to generate D bindings (need arrow-c-glib)
paths:
- '.dockerignore'
- '.github/workflows/d.yml'
- 'ci/docker/*d*'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- 'ci/docker/*d*'
- 'ci/docker/*-d.dockerfile'

One question: is docker_ruby already included in c_glib?

What is referred by the docker_ruby? https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-ruby.dockerfile ?


jobs:
docker:
name: AMD64 Ubuntu ${{ matrix.ubuntu }} Glib & D
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name: AMD64 Ubuntu ${{ matrix.ubuntu }} Glib & D
name: AMD64 Ubuntu ${{ matrix.ubuntu }} GLib & D

UBUNTU: ${{ matrix.ubuntu }}
steps:
- name: Checkout Arrow
uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.0.0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.0.0
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1

Copy link
Member

Choose a reason for hiding this comment

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

Could you rename this to linux-apt-d.dockerfile and use ubuntu-c-glib as the base image like ubuntu-ruby does?

arrow/docker-compose.yml

Lines 853 to 875 in 33e8cbb

ubuntu-ruby:
# Usage:
# docker compose build ubuntu-cpp
# docker compose build ubuntu-c-glib
# docker compose build ubuntu-ruby
# docker compose run --rm ubuntu-ruby
# Parameters:
# ARCH: amd64, arm64v8, ...
# UBUNTU: 20.04, 22.04
image: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-ruby
build:
context: .
dockerfile: ci/docker/linux-apt-ruby.dockerfile
cache_from:
- ${REPO}:${ARCH}-ubuntu-${UBUNTU}-ruby
args:
base: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-c-glib
shm_size: *shm-size
ulimits: *ulimits
environment:
<<: [*common, *ccache]
volumes: *ubuntu-volumes
command: *ruby-command

}
},
"preBuildCommands": [
"\"$DUB\" run girtod -- -i source -o source --use-runtime-linker"
Copy link
Member

Choose a reason for hiding this comment

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

Can we use out-of-source build?

Copy link
Member

Choose a reason for hiding this comment

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

We can't add license header to this file because this is JSON, right?
Could you add this file to https://github.com/apache/arrow/blob/main/dev/release/rat_exclude_files.txt ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants