refactor: Split service fs out of core by tao12345666333 · Pull Request #6970 · apache/opendal

tao12345666333 · 2025-12-11T08:58:08Z

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

It's Amp Code in smart mode with Claude 4.5 Opus model. Just one shot.
Full thread https://ampcode.com/threads/T-019b0c93-129d-77d0-a528-471145a5796d

Xuanwo · 2025-12-11T09:15:51Z

Code looks good overall, but some place inside core seems to refer to fs service, so need another loop

Xuanwo · 2025-12-11T10:53:32Z

core/src/lib.rs

+#[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;
+


I don't think is the correct fix.

fs used to be enabled default in opendal but now it's not, so we should add it here

opendal/bindings/ocaml/Cargo.toml

Lines 36 to 54 in 661e65b

opendal = { version = ">=0", path = "../../core", features = [

"blocking",

# These are default features before v0.46. TODO: change to optional features

"services-azblob",

"services-azdls",

"services-cos",

"services-fs",

"services-gcs",

"services-ghac",

"services-http",

"services-ipmfs",

"services-memory",

"services-obs",

"services-oss",

"services-s3",

"services-webdav",

"services-webhdfs",

"services-azfile",

] }

emm, seems services-fs have already been registered. Need another look over the test failure.

Xuanwo · 2025-12-11T11:21:13Z

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. We can debug the ocaml's bug in another new issue instead.

Can you change OCaml to test against memory instead? We should just change

opendal/bindings/ocaml/test/test.ml

Line 29 in 661e65b

test_check_result (Operator.new_operator "fs" cfgs)

Some test cases might fail too, so we need to update them accordingly.

Xuanwo · 2025-12-11T12:23:30Z

Ok, it's actually a Rust bug:

ctor not running for statically linked C library mmastrac/rust-ctor#280
Linking to a static library does not always include exported symbols rust-lang/rust#99721

Xuanwo · 2025-12-12T10:35:25Z

I think we are good to go after merging with main.

Xuanwo

Others LGTM now

core/src/lib.rs

Signed-off-by: Jintao Zhang <[email protected]>

Added services-fs to the dev-dependencies in integrations/object_store/Cargo.toml so that the local_fs behavior test for the object_store integration can use the fs service. Signed-off-by: Jintao Zhang <[email protected]>

Signed-off-by: Jintao Zhang <[email protected]>

tao12345666333 requested a review from Xuanwo as a code owner December 11, 2025 08:58

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" labels Dec 11, 2025

tao12345666333 force-pushed the vibe-fs branch from 3a1e59d to 727fc81 Compare December 11, 2025 10:52

Xuanwo reviewed Dec 11, 2025

View reviewed changes

tao12345666333 force-pushed the vibe-fs branch from 727fc81 to 72d9bad Compare December 11, 2025 11:32

This was referenced Dec 11, 2025

ctor doesn't work for ocaml as staticlib #6975

Open

fix(bindings/ocaml): Disable staticlib to ensure ctor works #6976

Closed

tao12345666333 force-pushed the vibe-fs branch from 72d9bad to ab8ae93 Compare December 12, 2025 23:47

Xuanwo reviewed Dec 13, 2025

View reviewed changes

core/src/lib.rs Outdated Show resolved Hide resolved

tao12345666333 added 4 commits December 15, 2025 15:38

refactor: Split service fs out of core

66ad803

Signed-off-by: Jintao Zhang <[email protected]>

fix local_fs test

5589d8d

Added services-fs to the dev-dependencies in integrations/object_store/Cargo.toml so that the local_fs behavior test for the object_store integration can use the fs service. Signed-off-by: Jintao Zhang <[email protected]>

fix test cases

252cf57

Signed-off-by: Jintao Zhang <[email protected]>

fix: remove extra blank lines in services/mod.rs

bfb7b85

tao12345666333 force-pushed the vibe-fs branch from b31cb46 to bfb7b85 Compare December 15, 2025 07:59

tao12345666333 requested a review from Xuanwo December 15, 2025 09:20

Xuanwo approved these changes Dec 15, 2025

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 15, 2025

Xuanwo merged commit f7ac9c1 into apache:main Dec 15, 2025
332 of 333 checks passed

tao12345666333 deleted the vibe-fs branch December 15, 2025 09:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

refactor: Split service fs out of core#6970

refactor: Split service fs out of core#6970
Xuanwo merged 4 commits intoapache:mainfrom
tao12345666333:vibe-fs

tao12345666333 commented Dec 11, 2025 •

edited

Loading

Uh oh!

Xuanwo commented Dec 11, 2025

Uh oh!

Xuanwo Dec 11, 2025

Uh oh!

Xuanwo Dec 11, 2025 •

edited

Loading

Uh oh!

Xuanwo Dec 11, 2025 •

edited

Loading

Uh oh!

Xuanwo commented Dec 11, 2025 •

edited

Loading

Uh oh!

Xuanwo commented Dec 11, 2025

Uh oh!

Xuanwo commented Dec 12, 2025

Uh oh!

Xuanwo left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	opendal = { version = ">=0", path = "../../core", features = [
	"blocking",
	# These are default features before v0.46. TODO: change to optional features
	"services-azblob",
	"services-azdls",
	"services-cos",
	"services-fs",
	"services-gcs",
	"services-ghac",
	"services-http",
	"services-ipmfs",
	"services-memory",
	"services-obs",
	"services-oss",
	"services-s3",
	"services-webdav",
	"services-webhdfs",
	"services-azfile",
	] }

Comments

Conversation

tao12345666333 commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

Xuanwo commented Dec 11, 2025

Uh oh!

Xuanwo Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Xuanwo Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xuanwo Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xuanwo commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Xuanwo commented Dec 11, 2025

Uh oh!

Xuanwo commented Dec 12, 2025

Uh oh!

Xuanwo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tao12345666333 commented Dec 11, 2025 •

edited

Loading

Xuanwo Dec 11, 2025 •

edited

Loading

Xuanwo Dec 11, 2025 •

edited

Loading

Xuanwo commented Dec 11, 2025 •

edited

Loading