You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "Xuanwo (via GitHub)" <gi...@apache.org> on 2023/06/18 12:26:24 UTC
[GitHub] [incubator-opendal] Xuanwo opened a new pull request, #2488: feat: Add golang binding for OpenDAL
Xuanwo opened a new pull request, #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488
A PoC of opendal golang binding just to show how bad a golang binding could be.
PLEASE DON'T MERGE.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#discussion_r1233289105
##########
bindings/go/go.mod:
##########
@@ -0,0 +1,7 @@
+module opendal.apache.org/go
+
+go 1.20
Review Comment:
This pull request will not be merged in its current form. Please prioritize other more important issues. Thank you.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] hwdef commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "hwdef (via GitHub)" <gi...@apache.org>.
hwdef commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1725382935
Can I first confirm that go binding is indeed required? Because I see that even without go binding, there are not many related requirements
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Ji-Xinyou commented on a diff in pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou commented on code in PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#discussion_r1233273496
##########
bindings/go/init.go:
##########
@@ -0,0 +1,21 @@
+package opendal
+
+import (
+ "github.com/ebitengine/purego"
+)
+
+var OPENDAL_LIB uintptr
+
+var opendal_operator_options_new func() uintptr
Review Comment:
I can try to fix that now
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#discussion_r1233288938
##########
bindings/go/go.mod:
##########
@@ -0,0 +1,7 @@
+module opendal.apache.org/go
+
+go 1.20
+
+require github.com/ebitengine/purego v0.4.0-alpha.4
Review Comment:
This pull request will not be merged in its current form. Please prioritize other more important issues. Thank you.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] jiaoew1991 commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "jiaoew1991 (via GitHub)" <gi...@apache.org>.
jiaoew1991 commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1728924777
we need a go bindings tooðŸ˜
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] jiaoew1991 commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "jiaoew1991 (via GitHub)" <gi...@apache.org>.
jiaoew1991 commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1738322023
The kafka-go project provides dynamic compilation commands to find libraries through the pkg-config command. I think it is not necessary to provide binaries in the code. It would be sufficient to provide users with instructions on how to compile libopendal_c and generate the corresponding pkg-config file during the compilation process. 🤔
https://github.com/confluentinc/confluent-kafka-go/blob/dc61dda93a2546dbb3f16280b476f4c2c20b08a5/kafka/build_dynamic.go#L1-L10C62
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] hwdef commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "hwdef (via GitHub)" <gi...@apache.org>.
hwdef commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1725034967
Any progress? Need help?
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] ImSingee commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "ImSingee (via GitHub)" <gi...@apache.org>.
ImSingee commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1597247257
I think we can provide an interface instead of the struct.
This also provides an extra benefit: we can return a (maybe auto-generated) type which always return an error.
```go
type Operator interface {
DoSomething() error
}
type errOperator struct {}
func (o errOperator) DoSomething() error {
return fmt.Errorf("not supported")
}
func NewOperator() Operator {
if opendal not exist {
return errOperator{}
}
return ...
}
```
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] jiaoew1991 commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "jiaoew1991 (via GitHub)" <gi...@apache.org>.
jiaoew1991 commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1728950082
> > we need a go bindings tooðŸ˜
>
> Even with CGO?
yes, it is acceptable. A unified API to access all kinds of Object store is very useful.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Xuanwo commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1725279751
> Any progress? Need help?
There has been no progress thus far. Additionally, due to the ASF limitation, we are unable to develop an opendal-go under this project (since we will need to build opendal as a binary in project like [go-duckdb](https://github.com/marcboeker/go-duckdb)). Therefore, we should initiate it externally.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Xuanwo commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1737143580
> May I ask why you need to split a separate repo?
ASF does not permit projects to include binaries such as `libopendal.so` or `libopendal.a` in the code. However, CGO may require them for linking purposes. Is there a method to construct `libopendal` using cgo?
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#discussion_r1233273244
##########
bindings/go/init.go:
##########
@@ -0,0 +1,21 @@
+package opendal
+
+import (
+ "github.com/ebitengine/purego"
+)
+
+var OPENDAL_LIB uintptr
+
+var opendal_operator_options_new func() uintptr
Review Comment:
cc @Ji-Xinyou, I will meet `segmentation violation` if I'm trying to pass the returning value of `opendal_operator_options_new` into `opendal_operator_new`. Maybe it's related to https://github.com/apache/incubator-opendal/issues/2462?
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] gaby commented on a diff in pull request #2488: feat: Add golang binding for OpenDAL
Posted by "gaby (via GitHub)" <gi...@apache.org>.
gaby commented on code in PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#discussion_r1233287488
##########
bindings/go/go.mod:
##########
@@ -0,0 +1,7 @@
+module opendal.apache.org/go
+
+go 1.20
+
+require github.com/ebitengine/purego v0.4.0-alpha.4
Review Comment:
This should be https://github.com/ebitengine/purego/releases/tag/v0.3.2
Instead of using 0.4.0 which is an alpha release
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] jiaoew1991 commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "jiaoew1991 (via GitHub)" <gi...@apache.org>.
jiaoew1991 commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1737050189
> > Any progress? Need help?
>
> There has been no progress thus far. Additionally, due to the ASF limitation, we are unable to develop an opendal-go under this project (since we will need to build opendal as a binary in project like [go-duckdb](https://github.com/marcboeker/go-duckdb)). Therefore, we should initiate it externally.
May I ask why you need to split a separate repo? Is it because Go relies on commit IDs or tags for dependencies? If that's the case, we can refer to the Arrow project, where tags include paths to subdirectories. 🤔
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Xuanwo commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1728932088
> we need a go bindings tooðŸ˜
Even with CGO?
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] jiaoew1991 commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "jiaoew1991 (via GitHub)" <gi...@apache.org>.
jiaoew1991 commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1738716026
> The kafka-go project provides dynamic compilation commands to find libraries through the pkg-config command. I think it is not necessary to provide binaries in the code. It would be sufficient to provide users with instructions on how to compile libopendal_c and generate the corresponding pkg-config file during the compilation process. 🤔
>
> https://github.com/confluentinc/confluent-kafka-go/blob/dc61dda93a2546dbb3f16280b476f4c2c20b08a5/kafka/build_dynamic.go#L1-L10C62
I wrote a demo for this proposal https://github.com/apache/incubator-opendal/pull/3204
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Xuanwo commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1725397844
> Can I first confirm that go binding is indeed required? Because I see that even without go binding, there are not many related requirements
No strong requirement yet.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] gaby commented on a diff in pull request #2488: feat: Add golang binding for OpenDAL
Posted by "gaby (via GitHub)" <gi...@apache.org>.
gaby commented on code in PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#discussion_r1233286776
##########
bindings/go/go.mod:
##########
@@ -0,0 +1,7 @@
+module opendal.apache.org/go
+
+go 1.20
Review Comment:
Go 1.19 is still supported
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Xuanwo commented on pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2488:
URL: https://github.com/apache/incubator-opendal/pull/2488#issuecomment-1634619875
Golang binding should not be implemented in this repo, we will start a new repo instead.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-opendal] Xuanwo closed pull request #2488: feat: Add golang binding for OpenDAL
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo closed pull request #2488: feat: Add golang binding for OpenDAL
URL: https://github.com/apache/incubator-opendal/pull/2488
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org