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

NetBSD support #84

Merged
merged 4 commits into from
Oct 8, 2023
Merged

NetBSD support #84

merged 4 commits into from
Oct 8, 2023

Conversation

0323pin
Copy link
Contributor

@0323pin 0323pin commented Sep 18, 2023

A step closer to fix #83

@Byron don't know if it's feasible with a partial fix but, the remaining issue needs to be fixed in libc crate.

Tasks

  • CI setup to test NetBSD, see this configuration as potential guide
  • use statvfs calls
  • workaround for lack of libc::getmntinfo or libc::get_mount_points

Warning

CI doesn't currently test that target, and it being green falsely suggests that this PR works as is.

@Byron Byron marked this pull request as draft September 18, 2023 06:30
@Byron Byron closed this Sep 18, 2023
@Byron Byron changed the title one step closer NetBSD support Sep 18, 2023
@Byron Byron reopened this Sep 18, 2023
@Byron
Copy link
Owner

Byron commented Sep 18, 2023

Please note that I have modified this PR quite a bit to be more descriptive and convey all the information I have about the NetBSD target.
It's also expected to hang around for a while, and those who would like to contribute to it could probably create their own PR on top of this one to fix one or all of the remaining tasks. Thank you.

@0323pin
Copy link
Contributor Author

0323pin commented Sep 18, 2023

@Byron Thanks for you support.
Some good news, I've managed to successfully compile trash-rs now.

===> Extracting for trash-rs-aee3dceac5575e4a2a23633ec5f3da5da79d9e89
=> Extracting local cargo crates
===> Patching for trash-rs-aee3dceac5575e4a2a23633ec5f3da5da79d9e89
=> Applying pkgsrc patches for trash-rs-aee3dceac5575e4a2a23633ec5f3da5da79d9e89
=> Verifying /usr/pkgsrc/wip/trash-rs/patches/patch-..vendor_libc-0.2.148_src_unix_bsd_netbsdlike_netbsd_mod.rs
=> Applying pkgsrc patch /usr/pkgsrc/wip/trash-rs/patches/patch-..vendor_libc-0.2.148_src_unix_bsd_netbsdlike_netbsd_mod.rs
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|$NetBSD$
|
|Test fix.
|
|--- ../vendor/libc-0.2.148/src/unix/bsd/netbsdlike/netbsd/mod.rs.orig	2006-07-24 01:21:28.000000000 +0000
|+++ ../vendor/libc-0.2.148/src/unix/bsd/netbsdlike/netbsd/mod.rs
--------------------------
Patching file ../vendor/libc-0.2.148/src/unix/bsd/netbsdlike/netbsd/mod.rs using Plan A...
Hunk #1 succeeded at 1853.
Hunk #2 succeeded at 3158.
done
===> Creating toolchain wrappers for trash-rs-aee3dceac5575e4a2a23633ec5f3da5da79d9e89
===> Configuring for trash-rs-aee3dceac5575e4a2a23633ec5f3da5da79d9e89
=> Checking for portability problems in extracted files
===> Building for trash-rs-aee3dceac5575e4a2a23633ec5f3da5da79d9e89
   Compiling tinyvec_macros v0.1.1
   Compiling autocfg v1.1.0
   Compiling unicode-bidi v0.3.13
   Compiling percent-encoding v2.3.0
   Compiling tinyvec v1.6.0
   Compiling libc v0.2.148
   Compiling num-traits v0.2.16
   Compiling form_urlencoded v1.2.0
   Compiling iana-time-zone v0.1.57
   Compiling unicode-normalization v0.1.22
   Compiling idna v0.4.0
   Compiling scopeguard v1.2.0
   Compiling log v0.4.20
   Compiling chrono v0.4.31
   Compiling url v2.4.1
   Compiling once_cell v1.18.0
   Compiling trash v3.0.6 (/usr/pkgsrc/wip/trash-rs/work/trash-rs-aee3dceac5575e4a2a23633ec5f3da5da79d9e89)
    Finished release [optimized] target(s) in 23.00s

This is unknown territory for me, so I've asked for a review on my patch to libc/src/unix/bsd/netbsdlike/netbsd/mod.rs

--- ../vendor/libc-0.2.148/src/unix/bsd/netbsdlike/netbsd/mod.rs.orig	2006-07-24 01:21:28.000000000 +0000
+++ ../vendor/libc-0.2.148/src/unix/bsd/netbsdlike/netbsd/mod.rs
@@ -1853,6 +1853,11 @@ pub const MNT_SOFTDEP: ::c_int = 0x80000
 pub const MNT_POSIX1EACLS: ::c_int = 0x00000800;
 pub const MNT_ACLS: ::c_int = MNT_POSIX1EACLS;
 
+// For use with vfs_fsync and getfsstat
+pub const MNT_WAIT: ::c_int = 1;
+pub const MNT_NOWAIT: ::c_int = 2;
+pub const MNT_LAZY: ::c_int = 3;
+
 //<sys/timex.h>
 pub const NTP_API: ::c_int = 4;
 pub const MAXPHASE: ::c_long = 500000000;
@@ -3153,6 +3158,34 @@ extern "C" {
     pub fn kinfo_getvmmap(pid: ::pid_t, cntp: *mut ::size_t) -> *mut kinfo_vmentry;
 }
 
+#[link(name = "execinfo")]
+extern "C" {
+    pub fn backtrace(addrlist: *mut *mut ::c_void, len: ::size_t) -> ::size_t;
+    pub fn backtrace_symbols(addrlist: *const *mut ::c_void, len: ::size_t) -> *mut *mut ::c_char;
+    pub fn backtrace_symbols_fd(
+        addrlist: *const *mut ::c_void,
+        len: ::size_t,
+        fd: ::c_int,
+    ) -> ::c_int;
+    pub fn backtrace_symbols_fmt(
+        addrlist: *const *mut ::c_void,
+        len: ::size_t,
+        fmt: *const ::c_char,
+    ) -> *mut *mut ::c_char;
+}
+
+cfg_if! {
+    if #[cfg(libc_union)] {
+        extern {
+            // these functions use statfs which uses the union mount_info:
+            pub fn statvfs(path: *const ::c_char, buf: *mut statvfs) -> ::c_int;
+            pub fn fstatvfs(fd: ::c_int, buf: *mut statvfs) -> ::c_int;
+            pub fn getmntinfo(mntbufp: *mut *mut ::statvfs, flags: ::c_int) -> ::c_int;
+            pub fn getfsstat(buf: *mut statvfs, bufsize: ::size_t, flags: ::c_int) -> ::c_int;
+        }
+    }
+}
+
 cfg_if! {
     if #[cfg(target_arch = "aarch64")] {
         mod aarch64;

This was referenced Sep 18, 2023
@0323pin
Copy link
Contributor Author

0323pin commented Sep 23, 2023

@Byron PR submitted to libc, rust-lang/libc#3359

When both PRs are merged, we can enable the trash feature on the packages that are currently built with it disabled.

@Byron
Copy link
Owner

Byron commented Sep 24, 2023

Thanks for the update, and for seeing it through! It's much appreciated.

@0323pin
Copy link
Contributor Author

0323pin commented Sep 25, 2023

Not a git wizard, have no clue on how to squash non-consecutive commits so, rust-lang/libc#3361

Regarding the CI, both NetBSD and OpenBSD have been added to cirrus-ci, https://cirrus-ci.com/build/6221284932583424

getmntinfo is available on DragoFly BSD also, see: https://leaf.dragonflybsd.org/cgi/web-man?command=getmntinfo&section=ANY
So. perhaps someone using DragonFly BSD could do a similar PR to add those functions to rust-lang/libc

@0323pin
Copy link
Contributor Author

0323pin commented Sep 26, 2023

@Byron I'm clearly missing something 😞 rust-lang/libc#3361 (comment)

@Byron
Copy link
Owner

Byron commented Sep 26, 2023

My best guess is that on linux, this function isn't used because it's not actually accessible from the crate-root anymore, maybe due to compile-time conditionals. It's definitely not easy to deal with software that builds on so many platforms.

@0323pin
Copy link
Contributor Author

0323pin commented Sep 26, 2023

Thanks!
But no, the CI is running on Ubuntu, yes. But, the failure is there on NetBSD.

I'm getting the same but, as a warning, not an error:

~> cargo build --release
    Updating crates.io index
   Compiling libc v0.2.148 (/home/pin/Downloads/libc)
warning: function `statvfs` is never used
    --> src/unix/bsd/netbsdlike/netbsd/mod.rs:3188:20
     |
3188 |             pub fn statvfs(path: *const ::c_char, buf: *mut statvfs) -> ::c_int;
     |                    ^^^^^^^
     |
     = note: `#[warn(dead_code)]` on by default

warning: function `fstatvfs` is never used
    --> src/unix/bsd/netbsdlike/netbsd/mod.rs:3189:20
     |
3189 |             pub fn fstatvfs(fd: ::c_int, buf: *mut statvfs) -> ::c_int;
     |                    ^^^^^^^^

warning: `libc` (lib) generated 2 warnings
    Finished release [optimized] target(s) in 7.51s

So yes, I'm defining two functions that are never used. Still, it solves my issue compiling trash-rs and the move to trash functionality is working. I would really like to get this fixed but, it will take a bit longer.

@0323pin
Copy link
Contributor Author

0323pin commented Sep 26, 2023

@Byron 😄

~> cargo build --release
    Updating crates.io index
   Compiling libc v0.2.148 (/home/pin/Downloads/libc)
    Finished release [optimized] target(s) in 5.88s

bors added a commit to rust-lang/libc that referenced this pull request Sep 29, 2023
backtrace definitions and support for getmntinfo and getvfsstat

After the failures of merging #3361 as a follow-up to #3359 I've spent sometime making sure everything is indeed supported and defined.

```
~> grep -r "MNT_WAIT" /usr/include/sys/fstypes.h; grep -r "MNT_NOWAIT" /usr/include/sys/fstypes.h; grep -r "MNT_LAZY" /usr/include/sys/fstypes.h
#define	MNT_WAIT	1	/* synchronously wait for I/O to complete */
#define	MNT_NOWAIT	2	/* start all I/O, but do not wait for it */
#define	MNT_LAZY 	3	/* push data not written by filesystem syncer */
```

```
~> grep -r "getmntinfo" /usr/include/sys/; grep -r "getvfsstat" /usr/include/sys/
/usr/include/sys/statvfs.h:int	getmntinfo(struct statvfs **, int)
/usr/include/sys/statvfs.h:    __RENAME(__getmntinfo90);
/usr/include/sys/fstypes.h: * waitfor flags to vfs_sync() and getvfsstat()
/usr/include/sys/statvfs.h:int	getvfsstat(struct statvfs *, size_t, int)
/usr/include/sys/statvfs.h:    __RENAME(__getvfsstat90);
/usr/include/sys/syscall.h:/* syscall: "compat_90_getvfsstat" ret: "int" args: "struct statvfs90 *" "size_t" "int" */
/usr/include/sys/syscall.h:#define	SYS_compat_90_getvfsstat	356
/usr/include/sys/syscall.h:/* syscall: "__getvfsstat90" ret: "int" args: "struct statvfs *" "size_t" "int" */
/usr/include/sys/syscall.h:#define	SYS___getvfsstat90	483
/usr/include/sys/syscallargs.h:struct compat_90_sys_getvfsstat_args {
/usr/include/sys/syscallargs.h:check_syscall_args(compat_90_sys_getvfsstat)
/usr/include/sys/syscallargs.h:struct sys___getvfsstat90_args {
/usr/include/sys/syscallargs.h:check_syscall_args(sys___getvfsstat90)
/usr/include/sys/syscallargs.h:int	compat_90_sys_getvfsstat(struct lwp *, const struct compat_90_sys_getvfsstat_args *, register_t *);
/usr/include/sys/syscallargs.h:int	sys___getvfsstat90(struct lwp *, const struct sys___getvfsstat90_args *, register_t *);
```

Also, I've made sure the code compiles with rustc 1.72.0 without warning or errors.

```
~> cargo build --release
    Updating crates.io index
   Compiling libc v0.2.148 (/home/pin/Git/libc)
    Finished release [optimized] target(s) in 8.87s
```

Moreover, the _move to trash_ functionality has again been verified using [simp](https://github.com/Kl4rry/simp) compiled with this commit as patch, in combination with the patch submited to [trash-rs](Byron/trash-rs#84).

`@JohnTitor` Please give bors another try
@0323pin
Copy link
Contributor Author

0323pin commented Sep 29, 2023

@Byron rust-lang/libc PR has been merged, rust-lang/libc#3368 (comment)

@Byron
Copy link
Owner

Byron commented Sep 30, 2023

Thanks for the update.

@0323pin
Copy link
Contributor Author

0323pin commented Sep 30, 2023

@Byron I think it's unreasonable to wait with this PR until a fix for DragonFly comes-up.

Looking at the definitions that libc has for the DragonFly platform and at the manpages for the mount command, it's rather clear that a reasonable number is missing.

Now, I don't use DragonFly and doing things in the blind, without a test environment is tough and, most probably not advisable, as it could make things worst.

To fix this we need someone with insights into DragonFly which, may or, may not show-up. I don't see why we need to wait for this, another PR to fix DragonFly could come at a later point.

@Byron
Copy link
Owner

Byron commented Oct 1, 2023

Thanks again!

I think once there is anything I should do, you'd have to spell it out for me. All I have is to look at the tasks in the PR description - from what I can gather, we'd now wait until libc with a fix is available? Maybe you can update them so they are in-line with reality - this looks a bit like I created them.

@0323pin
Copy link
Contributor Author

0323pin commented Oct 1, 2023

I think once there is anything I should do, you'd have to spell it out for me.

😆 Will do.

we'd now wait until libc with a fix is available?

Correct. Once libc > 0.2.148 is out, this PR can be merged and NetBSD will be fixed.

As for DragonFly, someone with a native environment needs to add the missing bits to the libc crate.
This is not something that should be fixed in trash-rs, the reason it fails to build is because those functions are not defined in libc.

Maybe you can update them so they are in-line with reality - this looks a bit like I created them.

Nah! You didn't create them at all. For NetBSD, there were missing pieces both on trash-rs and libc. For DragonFly, these are missing only on libc.

Hope it's clear and sorry if I wasn't before. I'll ping you once a new release of the libc crate is out, so you can merge this PR. Agreed?

@Byron
Copy link
Owner

Byron commented Oct 1, 2023

Thanks for the clarification, sounds good!

@wravoc
Copy link

wravoc commented Oct 1, 2023

@Byron @0323pin Ok, thanks for keeping Dragonfly and I in the loop. Unfortunately this is out of my lane, but I did ping the DF dev team just now about this issue.

@0323pin
Copy link
Contributor Author

0323pin commented Oct 7, 2023

@Byron libc-0.2.149 has been released

You can now merge this PR.

Needless to say, we (NetBSD) still need a new release of trash-rs for it to work.
Thank you!

@Byron
Copy link
Owner

Byron commented Oct 7, 2023

Thanks for the update. Can you mark the required libc version in the cargo manifest and tick all the checkboxes above if they are fulfilled sufficiently? The PR is still in draft mode as well.

@0323pin
Copy link
Contributor Author

0323pin commented Oct 7, 2023

Can you mark the required libc version in the cargo manifest

Fixed:

~> cargo build --release
    Updating crates.io index
   Compiling autocfg v1.1.0
   Compiling tinyvec_macros v0.1.1
   Compiling unicode-bidi v0.3.13
   Compiling percent-encoding v2.3.0
   Compiling tinyvec v1.6.0
   Compiling libc v0.2.149
   Compiling num-traits v0.2.16
   Compiling form_urlencoded v1.2.0
   Compiling unicode-normalization v0.1.22
   Compiling iana-time-zone v0.1.57
   Compiling idna v0.4.0
   Compiling scopeguard v1.2.0
   Compiling log v0.4.20
   Compiling chrono v0.4.31
   Compiling url v2.4.1
   Compiling once_cell v1.18.0
   Compiling trash v3.0.6 (/home/pin/Downloads/trash-rs)
    Finished release [optimized] target(s) in 37.07s

tick all the checkboxes above if they are fulfilled sufficiently?

DragonFly issue needs fixing in libc. As for the CI, I don't know anything about this but, ok I'll tick the boxes.

@0323pin 0323pin marked this pull request as ready for review October 7, 2023 21:44
Test if it makes the CI happy
Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks for all the effort that went into this!

I will create a new release soon.

@Byron Byron merged commit 24e0cb6 into Byron:master Oct 8, 2023
3 checks passed
@Byron
Copy link
Owner

Byron commented Oct 8, 2023

The improvement is now available in the new release.

@0323pin
Copy link
Contributor Author

0323pin commented Oct 8, 2023

Thank you!

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.

NetBSD support
3 participants