Skip to content

Add xattr-based attribute support to LocalFileSystem#630

Open
Barre wants to merge 1 commit intoapache:mainfrom
Barre:feat/local-xattr-attributes
Open

Add xattr-based attribute support to LocalFileSystem#630
Barre wants to merge 1 commit intoapache:mainfrom
Barre:feat/local-xattr-attributes

Conversation

@Barre
Copy link
Copy Markdown

@Barre Barre commented Jan 30, 2026

Closes #331

Store object attributes as extended attributes on the local filesystem, enabling put_opts and put_multipart_opts to persist attributes. Attributes are set on the staging file before atomic rename to preserve consistency.

  • Add xattr dependency (gated by fs feature)
  • Map standard attributes to user.* xattr namespace
  • Read attributes back in get_opts

Rationale for this change

The LocalFileSystem currently does not support attributes.

What changes are included in this PR?

The LocalFileSystem now supports attributes.

Are there any user-facing changes?

The LocalFileSystem now supports attributes.

@crepererum
Copy link
Copy Markdown
Contributor

I think this needs fixes / code adjustments for WASM & Windows.

@Barre Barre force-pushed the feat/local-xattr-attributes branch from 9869ef3 to 6aaf5cf Compare February 15, 2026 16:48
@Barre
Copy link
Copy Markdown
Author

Barre commented Feb 15, 2026

I think this needs fixes / code adjustments for WASM & Windows.

My most recent changes should do the trick.

@crepererum
Copy link
Copy Markdown
Contributor

Needs a rebase, otherwise looks good to me. @tustvold any objections?

@tustvold
Copy link
Copy Markdown
Contributor

LGTM, just a few comments

  • I think the namespace for the attributes should be user.object_store or possibly user.os
  • I think we should probably feature flag this, just in case there are compatibility issues
  • I do wonder what the performance implications of this are

@Barre Barre force-pushed the feat/local-xattr-attributes branch from 6aaf5cf to 0fe78e7 Compare April 1, 2026 14:44
@Barre
Copy link
Copy Markdown
Author

Barre commented Apr 1, 2026

LGTM, just a few comments

  • I think the namespace for the attributes should be user.object_store or possibly user.os
  • I think we should probably feature flag this, just in case there are compatibility issues
  • I do wonder what the performance implications of this are

Should be ok now.

wasm-bindgen-futures = "0.4.18"
futures-channel = {version = "0.3", features = ["sink"]}

[features]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this missing the xattr feature?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, I thought you didn't want it by default.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, but you still need to define the feature. An optional dependency doesn't automatically define a feature, no?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

$ cargo metadata --format-version 1 --no-deps | jq '.packages[0].features'

[...]
  "xattr": [
    "dep:xattr"
  ]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Weird, but since it passes the test this is obviously working 🤷

@Barre Barre force-pushed the feat/local-xattr-attributes branch 2 times, most recently from 625c402 to 1141ff0 Compare April 2, 2026 11:10
Store object attributes as extended attributes on the local filesystem,
enabling put_opts and put_multipart_opts to persist attributes. Attributes
are set on the staging file before atomic rename to preserve consistency.

- Add xattr dependency (gated by fs feature)
- Map standard attributes to user.* xattr namespace
- Read attributes back in get_opts
@Barre Barre force-pushed the feat/local-xattr-attributes branch from 1141ff0 to 129d021 Compare April 2, 2026 11:31
@Barre
Copy link
Copy Markdown
Author

Barre commented Apr 2, 2026

(Ran cargo fmt on last force-push).

@crepererum
Copy link
Copy Markdown
Contributor

I think we need to do something about Windows. So I think selecting this feature on Windows should probably lead to compile_error! and the feature shouldn't be part of default = [...] (since we don't support it on all platforms).

Or you find a way to set file attributes on Windows (see for example this discussion), but I think this might be a bit out of scope for this PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow attributes in local filesystem (or impove the error msg)

3 participants