You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "tao12345666333 (via GitHub)" <gi...@apache.org> on 2025/12/11 08:58:14 UTC
[PR] refactor: Split service fs out of core [opendal]
tao12345666333 opened a new pull request, #6970:
URL: https://github.com/apache/opendal/pull/6970
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes https://github.com/apache/opendal/issues/6893
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking-changes` label.
-->
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #6970:
URL: https://github.com/apache/opendal/pull/6970#discussion_r2610121345
##########
core/src/lib.rs:
##########
@@ -24,6 +24,29 @@
pub use opendal_core::*;
+#[cfg(feature = "services-aliyun-drive")]
+extern crate opendal_service_aliyun_drive;
+#[cfg(feature = "services-azblob")]
+extern crate opendal_service_azblob;
+#[cfg(feature = "services-azdls")]
+extern crate opendal_service_azdls;
+#[cfg(feature = "services-azfile")]
+extern crate opendal_service_azfile;
+#[cfg(feature = "services-fs")]
+extern crate opendal_service_fs;
+#[cfg(feature = "services-ghac")]
+extern crate opendal_service_ghac;
+#[cfg(feature = "services-hdfs-native")]
+extern crate opendal_service_hdfs_native;
+#[cfg(feature = "services-moka")]
+extern crate opendal_service_moka;
+#[cfg(feature = "services-mysql")]
+extern crate opendal_service_mysql;
+#[cfg(feature = "services-s3")]
+extern crate opendal_service_s3;
+#[cfg(feature = "services-vercel-blob")]
+extern crate opendal_service_vercel_blob;
+
Review Comment:
I don't think is the correct fix.
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #6970:
URL: https://github.com/apache/opendal/pull/6970#issuecomment-3641676949
Ok, it's actually a Rust bug:
- https://github.com/mmastrac/rust-ctor/issues/280
- https://github.com/rust-lang/rust/issues/99721
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #6970:
URL: https://github.com/apache/opendal/pull/6970#discussion_r2618378903
##########
core/src/lib.rs:
##########
@@ -24,6 +24,46 @@
pub use opendal_core::*;
+// Force linking of external service crates to ensure their #[ctor] registration functions run.
Review Comment:
Sure. https://ampcode.com/threads/T-019b20ea-d705-759f-bd0d-d943661e0cc5
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #6970:
URL: https://github.com/apache/opendal/pull/6970#discussion_r2616284999
##########
core/src/lib.rs:
##########
@@ -24,6 +24,46 @@
pub use opendal_core::*;
+// Force linking of external service crates to ensure their #[ctor] registration functions run.
Review Comment:
Can you please remove those changes?
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #6970:
URL: https://github.com/apache/opendal/pull/6970#discussion_r2610148575
##########
core/src/lib.rs:
##########
@@ -24,6 +24,29 @@
pub use opendal_core::*;
+#[cfg(feature = "services-aliyun-drive")]
+extern crate opendal_service_aliyun_drive;
+#[cfg(feature = "services-azblob")]
+extern crate opendal_service_azblob;
+#[cfg(feature = "services-azdls")]
+extern crate opendal_service_azdls;
+#[cfg(feature = "services-azfile")]
+extern crate opendal_service_azfile;
+#[cfg(feature = "services-fs")]
+extern crate opendal_service_fs;
+#[cfg(feature = "services-ghac")]
+extern crate opendal_service_ghac;
+#[cfg(feature = "services-hdfs-native")]
+extern crate opendal_service_hdfs_native;
+#[cfg(feature = "services-moka")]
+extern crate opendal_service_moka;
+#[cfg(feature = "services-mysql")]
+extern crate opendal_service_mysql;
+#[cfg(feature = "services-s3")]
+extern crate opendal_service_s3;
+#[cfg(feature = "services-vercel-blob")]
+extern crate opendal_service_vercel_blob;
+
Review Comment:
emm, seems `services-fs` have already been registered. Need another over the test failure.
##########
core/src/lib.rs:
##########
@@ -24,6 +24,29 @@
pub use opendal_core::*;
+#[cfg(feature = "services-aliyun-drive")]
+extern crate opendal_service_aliyun_drive;
+#[cfg(feature = "services-azblob")]
+extern crate opendal_service_azblob;
+#[cfg(feature = "services-azdls")]
+extern crate opendal_service_azdls;
+#[cfg(feature = "services-azfile")]
+extern crate opendal_service_azfile;
+#[cfg(feature = "services-fs")]
+extern crate opendal_service_fs;
+#[cfg(feature = "services-ghac")]
+extern crate opendal_service_ghac;
+#[cfg(feature = "services-hdfs-native")]
+extern crate opendal_service_hdfs_native;
+#[cfg(feature = "services-moka")]
+extern crate opendal_service_moka;
+#[cfg(feature = "services-mysql")]
+extern crate opendal_service_mysql;
+#[cfg(feature = "services-s3")]
+extern crate opendal_service_s3;
+#[cfg(feature = "services-vercel-blob")]
+extern crate opendal_service_vercel_blob;
+
Review Comment:
emm, seems `services-fs` have already been registered. Need another look over the test failure.
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #6970:
URL: https://github.com/apache/opendal/pull/6970#issuecomment-3645915833
I think we are good to go after merging with main.
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #6970:
URL: https://github.com/apache/opendal/pull/6970#issuecomment-3640983672
Code looks good overall, but some place inside `core` seems to refer to `fs` service, so need another loop
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #6970:
URL: https://github.com/apache/opendal/pull/6970#discussion_r2610126584
##########
core/src/lib.rs:
##########
@@ -24,6 +24,29 @@
pub use opendal_core::*;
+#[cfg(feature = "services-aliyun-drive")]
+extern crate opendal_service_aliyun_drive;
+#[cfg(feature = "services-azblob")]
+extern crate opendal_service_azblob;
+#[cfg(feature = "services-azdls")]
+extern crate opendal_service_azdls;
+#[cfg(feature = "services-azfile")]
+extern crate opendal_service_azfile;
+#[cfg(feature = "services-fs")]
+extern crate opendal_service_fs;
+#[cfg(feature = "services-ghac")]
+extern crate opendal_service_ghac;
+#[cfg(feature = "services-hdfs-native")]
+extern crate opendal_service_hdfs_native;
+#[cfg(feature = "services-moka")]
+extern crate opendal_service_moka;
+#[cfg(feature = "services-mysql")]
+extern crate opendal_service_mysql;
+#[cfg(feature = "services-s3")]
+extern crate opendal_service_s3;
+#[cfg(feature = "services-vercel-blob")]
+extern crate opendal_service_vercel_blob;
+
Review Comment:
`fs` used to be enabled default in opendal but now it's now, so we should add it here
https://github.com/apache/opendal/blob/661e65bb97e56354e7986cdd75b3e01b595d6377/bindings/ocaml/Cargo.toml#L36-L54
##########
core/src/lib.rs:
##########
@@ -24,6 +24,29 @@
pub use opendal_core::*;
+#[cfg(feature = "services-aliyun-drive")]
+extern crate opendal_service_aliyun_drive;
+#[cfg(feature = "services-azblob")]
+extern crate opendal_service_azblob;
+#[cfg(feature = "services-azdls")]
+extern crate opendal_service_azdls;
+#[cfg(feature = "services-azfile")]
+extern crate opendal_service_azfile;
+#[cfg(feature = "services-fs")]
+extern crate opendal_service_fs;
+#[cfg(feature = "services-ghac")]
+extern crate opendal_service_ghac;
+#[cfg(feature = "services-hdfs-native")]
+extern crate opendal_service_hdfs_native;
+#[cfg(feature = "services-moka")]
+extern crate opendal_service_moka;
+#[cfg(feature = "services-mysql")]
+extern crate opendal_service_mysql;
+#[cfg(feature = "services-s3")]
+extern crate opendal_service_s3;
+#[cfg(feature = "services-vercel-blob")]
+extern crate opendal_service_vercel_blob;
+
Review Comment:
`fs` used to be enabled default in opendal but now it's not, so we should add it here
https://github.com/apache/opendal/blob/661e65bb97e56354e7986cdd75b3e01b595d6377/bindings/ocaml/Cargo.toml#L36-L54
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #6970:
URL: https://github.com/apache/opendal/pull/6970#issuecomment-3641456202
I double-checked again. I believe the changes in this PR are correct. However, for reasons we don't know (maybe `ctor` doesn't work in OCaml?), it doesn't work as expected.
Can you change OCaml to test against memory instead? We should just change https://github.com/apache/opendal/blob/661e65bb97e56354e7986cdd75b3e01b595d6377/bindings/ocaml/test/test.ml#L29
Some test cases might fail too, so we need to update them accordingly.
--
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
Re: [PR] refactor: Split service fs out of core [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #6970:
URL: https://github.com/apache/opendal/pull/6970
--
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