trash feature optional?

[0323pin]

Hi,

NetBSD package maintainer here trying to package yazi.
I've added support for NetBSD into shared/src/fs.rs with the following patch,

--- shared/src/fs.rs.orig	2023-09-01 07:52:48.000000000 +0000
+++ shared/src/fs.rs
@@ -99,6 +99,10 @@ pub fn file_mode(mode: u32) -> String {
 
 	#[cfg(target_os = "macos")]
 	let m = mode as u16;
+        #[cfg(target_os = "freebsd")]
+	let m = mode as u16;
+        #[cfg(target_os = "netbsd")]
+	let m = mode;
 	#[cfg(target_os = "linux")]
 	let m = mode;

Unfortunately, the build still fails due to the dependency on trash and unsupported features in NetBSDs Rust-libc.

The error is general and applies to all projects with a dependency on trash, so I wonder if you would be willing to place trash support beyond a feature gate?

Thanks!

[sxyazi]

Hi, I'm willing to help with this issue, but I'm not familiar with NetBSD. I'd like to know what crate/tool is typically used in NetBSD to move files to the trash bin, or if there's a concept of trash bin in NetBSD at all?

[0323pin]

Hi,

Yes, trash bin exists on NetBSD. The issue is that support for it is missing in Rust libc itself.

In other projects, if possible, I just switch off the trash feature. Which, in practice means you just delete the files permanently.

[0323pin]

@sxyazi FI, Byron/trash-rs#83 (comment)

[wravoc]

Not meaning to sidetrack here, but add some support for optional trash. It is the same situation on Dragonfly BSD that any cargo that requires trash will not work.

[sxyazi]

I have already made the trash crate optional in #178

[0323pin]

I have already made the trash crate optional in #178

Thanks, I'll give #178 a go in a few hours. But, we still need to define the target_os in shared/src/fs.rs.orig

[0323pin]

Hmm ... something still missing. I've just tried building from #178 with --no-default-features and the error remains, it shouldn't.

[sxyazi]

The situation with Yazi is a bit complicated.

It uses a workspace where the app serves as the program entry, depending on the core, and core depends on trash.

I'm not sure how to make --no-default-features work for trash. I found rust-lang/cargo#4753, but it seems to be applicable for enabling features rather than disabling them...

[0323pin]

@sxyazi I'm getting close on this one, Byron/trash-rs#84

Next step is to submit a patch to libc 😃

Obviously, it needs to be merged and we will need new releases of both libc and trash.

[sxyazi]

@0323pin Awesome work!

Just reminded by @George-Miao, I've found a new approach to address it. Simply check the target; no need to use conditional features.

I've pushed the new code, and it should be working now!

[0323pin]

Simply check the target; no need to use conditional features.

Yes, that works. Now we don't need to wait for future releases. Obviously, I still need a release of yazi containing these commits.
Thanks also for merging my patch to shared/src/fs.rs.

Only a single warning now 👍

warning: field `is_link` is never read
  --> core/src/files/file.rs:13:13
   |
8  | pub struct File {
   |            ---- field in this struct
...
13 |     pub(super) is_link:   bool,
   |                ^^^^^^^
   |
   = note: `File` has derived impls for the traits `Debug` and `Clone`, but these are intentionally ignored during dead code analysis
   = note: `#[warn(dead_code)]` on by default

warning: `core` (lib) generated 1 warning

Awesome work!

Thanks but, I've just adapted the missing definitions from OpenBSD mod.rs.

Oh ... almost forgot, ... the evidence 😄

[0323pin]

@sxyazi thanks 👍

Would it be possible to cut a release tag or, am I asking too much? If possible (and if msrv < 1.71) I could merge the package today, before we enter commit freeze tomorrow, ahead of the stable branch cut in a week.

This would mean the package would be available for users of the stable branch very soon after the 27th

[sxyazi]

Sorry, Yazi is not ready to release version 0.1.5 yet, but I can create a v0.1.4-netbsd-support tag for this commit if you need.

[0323pin]

That's fine, we can wait, I'm not in a rush.

Also, I can pull the sources from that specific commit and call it v0.1.4 which, sometimes is a bit controversial but, in this case it's explainable.

No worries.

[0323pin]

@sxyazi Merged 😃

After September 27, (give it a week for the servers to build the updates) it should be available from our repositories for users to install.
Users will just need to run pkgin install yazi

[sxyazi]

Oh, thank you!!

Should I add it to the README? And is there an option to install it from Git in NetBSD, like yazi-git?

[0323pin]

Oh, thank you!!

You're welcome, thanks for caring for NetBSD as platform.

Should I add it to the README?

yeah, that would be appreciated. Thanks.

is there an option to install it from Git in NetBSD, like yazi-git?

For pre-compiled binaries, no, only releases. With some exceptions, like in this case. If you build stuff from source (I build all my packages from source), than you can create your own modifications and build whatever you want.

I keep a few git packages myself (alacritty, elvish and marswm) but, those are hosted in the wip repo and are not available as binaries for installation with pkgin.

That said, any user can add the wip repository and build anything locally. Well, not everything in wip will build but, you get the point.

[wravoc]

Great seeing this, well done you two. I learned the other day that NetBSD is the oldest BSD, the Grandfather of BSD and Open Source Operating Systems, yes Linux is a kernel, having originated before most all the others, the first to come out of 386BSD! Keep it up!

[sxyazi]

I've added it to Yazi's new document, which will soon replace the README as the default document.

https://yazi-rs.github.io/docs/usage/installation#netbsd

[0323pin]

@sxyazi Submitted to libc, rust-lang/libc#3359

[0323pin]

@sxyazi Combining libc-0.2.149 and trash-rs-3.1.0 allows the trash feature to build and function on NetBSD. Feel free to enable it now.

[sxyazi]

@0323pin I made a PR #251 for that, please give it a try to see if it works on NetBSD :)

[0323pin]

@sxyazi Yes, it works. Building inside the pkgsrc infrastructure,

===> Building for yazi-0.1.4
   Compiling autocfg v1.1.0
   Compiling proc-macro2 v1.0.69
   Compiling unicode-ident v1.0.12
   Compiling cfg-if v1.0.0
   Compiling libc v0.2.149
   [...]
   Compiling trash v3.1.0
   Compiling adaptor v0.1.0 (/usr/pkgsrc/wip/yazi/work/yazi-abedf44d5b7a0ac330ac564ec231e33f585f96de/adaptor)
   Compiling yazi-prebuild v0.1.0
   Compiling tokio-stream v0.1.14
   Compiling crossbeam-channel v0.5.8
   Compiling core v0.1.0 (/usr/pkgsrc/wip/yazi/work/yazi-abedf44d5b7a0ac330ac564ec231e33f585f96de/core)
   Compiling tracing-appender v0.2.2
   Compiling ansi-to-tui v3.1.0
   Compiling signal-hook-tokio v0.3.1
warning: field `is_link` is never read
  --> core/src/files/file.rs:13:13
   |
8  | pub struct File {
   |            ---- field in this struct
...
13 |     pub(super) is_link:   bool,
   |                ^^^^^^^
   |
   = note: `File` has derived impls for the traits `Debug` and `Clone`, but these are intentionally ignored during dead code analysis
   = note: `#[warn(dead_code)]` on by default

warning: `core` (lib) generated 1 warning
   Compiling app v0.1.0 (/usr/pkgsrc/wip/yazi/work/yazi-abedf44d5b7a0ac330ac564ec231e33f585f96de/app)
   Finished release [optimized] target(s) in 12m 05s

[sxyazi]

Thanks for the testing, ill merge it now!

Edit: Done!

[0323pin]

@sxyazi If a new release is still far away, I can switch the git-tag we are building from. Just let me know 😃

[sxyazi]

@0323pin Yazi isn't fully ready yet, sorry for the long release cycle this time...

[0323pin]

No need to be sorry, I'll just bump the package revision and get this feature enabled. Only thought, I'd ask before doing that.

Thanks.

[github-actions]

I'm going to lock this issue because it has been closed for 30 days. ⏳ This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.