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

Enable "std" feature on linux-raw-sys when "std" feature is enabled #945

Open
notgull opened this issue Nov 26, 2023 · 15 comments
Open

Enable "std" feature on linux-raw-sys when "std" feature is enabled #945

notgull opened this issue Nov 26, 2023 · 15 comments

Comments

@notgull
Copy link
Contributor

notgull commented Nov 26, 2023

As "std" changes API details from linux-raw-sys that is exposed in the public API, this would be a breaking change.

cc #753

@sdroege
Copy link

sdroege commented Dec 9, 2023

This currently means that xattr (1.1.0) and rustix can't be used in the same crate. xattr enables the std feature of linux-raw-sys, and rustix then fails compiling if also the net feature of rustix is enabled.

error[E0308]: mismatched types
  --> src/backend/linux_raw/net/addr.rs:39:32
   |
39 |             unix.sun_path[i] = *b;
   |             ----------------   ^^ expected `i8`, found `u8`
   |             |
   |             expected due to the type of this binding

error[E0308]: mismatched types
    --> src/backend/linux_raw/net/addr.rs:52:32
     |
52   |             id.copy_from_slice(name);
     |                --------------- ^^^^ expected `&[i8]`, found `&[u8]`
     |                |
     |                arguments to this method are incorrect
     |
     = note: expected reference `&[i8]`
                found reference `&[u8]`
[...]

gstreamer-github pushed a commit to sdroege/gst-plugin-rs that referenced this issue Dec 9, 2023
Keep dash-mpd at 0.14.5 and xattr at 1.0.1 because otherwise compilation
fails, see:
  - Stebalien/xattr#44
  - bytecodealliance/rustix#945

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1404>
Stebalien added a commit to Stebalien/xattr that referenced this issue Dec 9, 2023
This reverts rustix support until enabling the "std" feature no longer
causes downstream breakage.

See:

- bytecodealliance/rustix#945
- #44
@edulix
Copy link

edulix commented Dec 9, 2023

I'm having this same issue. Our build suddently is broken:

error[E0308]: mismatched types
  --> /home/vscode/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustix-0.38.27/src/backend/linux_raw/net/addr.rs:39:32
   |
39 |             unix.sun_path[i] = *b;
   |             ----------------   ^^ expected `i8`, found `u8`
   |             |
   |             expected due to the type of this binding

error[E0308]: mismatched types
  --> /home/vscode/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustix-0.38.27/src/backend/linux_raw/net/addr.rs:52:32
   |
52 |             id.copy_from_slice(name);
   |                --------------- ^^^^ expected `&[i8]`, found `&[u8]`
   |                |
   |                arguments to this method are incorrect
   |
   = note: expected reference `&[i8]`
              found reference `&[u8]`
note: method defined here
  --> /rustc/9d871b0617a4b3d6610b7cee0ab5310dcb542c62/library/core/src/slice/mod.rs:3331:12

@edulix
Copy link

edulix commented Dec 9, 2023

BTW Possible temporal quick and dirty fix: depend on rustix = "0.38.26" and it works. It got broken in a patch release..

@Stebalien
Copy link
Contributor

xattr 1.1.1 has been released with a fix (reverts the change). I've yanked 1.1.0 now to prevent his from spreading.

@Stebalien
Copy link
Contributor

(unless your issue is unrelated to xattr)

@sunfishcode
Copy link
Member

I've now submitted #971 with a fix for this.

@sunfishcode
Copy link
Member

#971 is now released in rustix 0.38.28.

@Tuupertunut
Copy link

I'm still getting compilation errors when adding xattr crate alongside rustix. It seems to change the definition of c_char from u8 to i8. Was this supposed to be fixed already?

@Stebalien
Copy link
Contributor

@Tuupertunut what platform, what xattr version, and what rustix version?

@Tuupertunut
Copy link

Tuupertunut commented Jan 11, 2025

Platform: stable-x86_64-unknown-linux-gnu, rustc 1.83.0

Dependencies before adding xattr:

[dependencies]
fuse_mt = "0.6.1"
fuser = "0.13.0"
ctrlc = { version = "3.4.5", features = ["termination"] }
rustix = { version = "0.38.43", features = ["fs"] }

[dev-dependencies]
tempfile = "3.15.0"

rustix::fs::listxattr() takes in &mut [linux_raw_sys::ctypes::c_char] = &mut [u8]

After adding xattr:

[dependencies]
fuse_mt = "0.6.1"
fuser = "0.13.0"
ctrlc = { version = "3.4.5", features = ["termination"] }
rustix = { version = "0.38.43", features = ["fs"] }
xattr = "1.4.0"

[dev-dependencies]
tempfile = "3.15.0"

rustix::fs::listxattr() takes in &mut [core::ffi::c_char] = &mut [i8]

@Stebalien
Copy link
Contributor

Stebalien commented Jan 11, 2025 via email

@Stebalien
Copy link
Contributor

Does Stebalien/xattr#70 work? You can patch it in with (untested, but it should work...):

[patch.crates-io]
xattr = { git = "https://github.com/Stebalien/xattr", branch = "steb/use-rustix-c-char" }

@Tuupertunut
Copy link

Can you post the compilation error?

The compilation error is simply a type mismatch

error[E0308]: mismatched types
   --> src/overlayfs.rs:803:56
    |
803 |                     rustix::fs::llistxattr(layer_path, &mut buffer).err_to_errno()?;
    |                     ----------------------             ^^^^^^^^^^^ expected `&mut [i8]`, found `&mut Vec<u8>`
    |                     |
    |                     arguments to this function are incorrect
    |
    = note: expected mutable reference `&mut [i8]`
               found mutable reference `&mut Vec<u8>`

Does Stebalien/xattr#70 work?

No, it shows the same compilation error. Thank you for your help in debugging this issue.

Now I'm just wondering two things:

  • Why is xattr enabling the "std" feature of linux-raw-sys (here)? This seems to cause the problem for me as rustix does not have the "std" feature of linux-raw-sys enabled, but when I add xattr as dependency, it turns that feature on and the definition of c_char changes for existing rustix code.

  • Why is rustix using c_char to represent characters in listxattr() when in Rust CString is usually considered to consist of u8s? (In fact that's the definition of Rust CString) Could it be changed to take in &mut [u8]? This has been requested also in Painful [i8] in rustix::fs::listxattr() #1218.

@Stebalien
Copy link
Contributor

Everything breaks without the rustix "std" feature (wrong error types, issues with string conversions, etc.). xattr itself requires the standard library so there's no real benefit to supporting no-std (and it's not supposed to have any other knock-on effects).

It sounds like this bug wasn't actually fixed. xattr shouldn't actually depend on linux-raw-sys and, as you say, shouldn't enable the "std" feature there. But, if I don't enable that feature, rustix becomes incompatible with stdlib CStr, so that's a non-starter.

@Stebalien
Copy link
Contributor

Ok, I still think this issue should be fixed (enabling "std" in rustix should enable "std" in linux-raw-sys), but the real issue here is #1261.

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

No branches or pull requests

6 participants