Skip to content

Conversation

@devnexen
Copy link
Contributor

@devnexen devnexen commented Mar 5, 2023

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2023

r? @JohnTitor

(rustbot has picked a reviewer for you, use r? to override)

@devnexen devnexen marked this pull request as ready for review March 5, 2023 10:34
@JohnTitor
Copy link
Member

Note that there're some overrides with #2406.

@bors
Copy link
Contributor

bors commented Mar 8, 2023

☔ The latest upstream changes (presumably #3037) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented May 6, 2023

☔ The latest upstream changes (presumably #2999) made this pull request unmergeable. Please resolve the merge conflicts.

@workingjubilee
Copy link
Member

@devnexen Please resolve the merge conflict.

@devnexen devnexen force-pushed the remove_fbsd11_support branch from f01623a to 9e7d63b Compare July 26, 2023 20:42
build.rs Outdated
Some(13) if libc_ci => set_cfg("freebsd13"),
Some(14) if libc_ci => set_cfg("freebsd14"),
Some(_) | None => set_cfg("freebsd11"),
Some(12) if libc_ci => println!("cargo:rustc-cfg=freebsd12"),
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a merge conflict. You should use set_cfg instead of println!

// APIs that were changed after FreeBSD 11

// The type of `nlink_t` changed from `u16` to `u64` in FreeBSD 12:
pub type nlink_t = u16;
Copy link
Contributor

Choose a reason for hiding this comment

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

All of the stuff that was in this file should now be moved into freebsdlike/freebsd/mod.rs For example, move all three nlink_t definitions from freebsd{12,13,14}.rs into that file.

@devnexen devnexen force-pushed the remove_fbsd11_support branch 3 times, most recently from ce5b55f to 72de88e Compare July 26, 2023 22:46
@workingjubilee
Copy link
Member

workingjubilee commented Jul 28, 2023

I started a conversation on Zulip to see how people feel about this.

@tgross35
Copy link
Contributor

This is basically 100% conflicted, but @devnexen would you be up for rebasing this?

EOL was 5 years ago so I can't imagine anyone would object at this point.

@devnexen devnexen marked this pull request as draft August 10, 2025 06:07
@devnexen devnexen force-pushed the remove_fbsd11_support branch from 72de88e to e3d1192 Compare August 10, 2025 06:07
@devnexen devnexen force-pushed the remove_fbsd11_support branch 2 times, most recently from 3fe771e to 662e1b4 Compare August 10, 2025 06:24
@rustbot rustbot added the O-x86 label Aug 10, 2025
@devnexen devnexen force-pushed the remove_fbsd11_support branch 3 times, most recently from 1c0de03 to 5eb24f0 Compare August 10, 2025 07:02
};

match which_freebsd {
x if x < 10 => panic!("FreeBSD older than 10 is not supported"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you update this comment?

@tgross35
Copy link
Contributor

I haven't looked through this in detail yet but the changes look correct. @asomers would you be able to review?

@devnexen devnexen force-pushed the remove_fbsd11_support branch from 5eb24f0 to 2da0438 Compare August 11, 2025 04:44
@asomers
Copy link
Contributor

asomers commented Aug 13, 2025

I haven't looked through this in detail yet but the changes look correct. @asomers would you be able to review?

In theory I agree with this. I'll try to review it next Saturday. We'll want to run a few dozen crates' test suites with this change, too.

Copy link
Contributor

@asomers asomers left a comment

Choose a reason for hiding this comment

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

These are the only two problems I've found in my testing so far. But that begs the question: how did you test these changes?

all(target_os = "freebsd", any(freebsd11, freebsd10)),
link_name = "kevent@FBSD_1.0"
)]
pub fn kevent(
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you really mean to remove this function? I think you should just remove the link_name instead. The function is definitely needed.


pub type nlink_t = u64;
pub type dev_t = u64;
pub type ino_t = c_ulong;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be u64. Otherwise stat is broken on x86.

Suggested change
pub type ino_t = c_ulong;
pub type ino_t = u64;

@devnexen devnexen force-pushed the remove_fbsd11_support branch 3 times, most recently from 2c962b8 to cb008d7 Compare August 17, 2025 17:10
@devnexen devnexen requested a review from asomers August 30, 2025 06:33
@asomers
Copy link
Contributor

asomers commented Sep 7, 2025

Your latest changes fix the two bugs that I found. What about you? How did you test these changes?

@devnexen
Copy link
Contributor Author

devnexen commented Sep 7, 2025

I test within a VM the vast majority of time, sometimes when I can t I look at freebsd sources and count on CI to see. Not ideal I know.

@asomers
Copy link
Contributor

asomers commented Sep 7, 2025

But what did you do within that VM? What projects did you test? How did you patch them to use libc main branch?

@devnexen
Copy link
Contributor Author

devnexen commented Sep 7, 2025

Usually, I just do cargo test --manifest-path libc-test/Cargo.toml. I did not try to use in any project.

@devnexen devnexen force-pushed the remove_fbsd11_support branch from cb008d7 to 3f8c507 Compare September 7, 2025 20:34
@asomers
Copy link
Contributor

asomers commented Sep 7, 2025

Usually, I just do cargo test --manifest-path libc-test/Cargo.toml. I did not try to use in any project.

For a change like this, that really isn't sufficient. You need to run the test suites of a selection of other crates, patched to use this branch of libc.

@devnexen
Copy link
Contributor Author

devnexen commented Sep 7, 2025

Usually, I just do cargo test --manifest-path libc-test/Cargo.toml. I did not try to use in any project.

For a change like this, that really isn't sufficient. You need to run the test suites of a selection of other crates, patched to use this branch of libc.

Ok I ll try

@devnexen
Copy link
Contributor Author

devnexen commented Sep 8, 2025

@asomers, here a test with nix crate

[dcarlier@ ~/Contribs/nix]$ cargo test --all-features
    Updating git repository `https://github.com/devnexen/libc`
     Locking 1 package to latest compatible version
      Adding libc v1.0.0-alpha.1 (https://github.com/devnexen/libc?branch=remove_fbsd11_support#3f8c5074)
   Compiling libc v0.2.172
   Compiling nix v0.30.1 (/home/dcarlier/Contribs/nix)
   Compiling getrandom v0.3.3
   Compiling errno v0.3.13
   Compiling parking_lot_core v0.9.11
   Compiling rustix v1.0.8
   Compiling rand_core v0.9.3
   Compiling rand_chacha v0.9.0
   Compiling parking_lot v0.12.4
   Compiling rand v0.9.2
   Compiling sysctl v0.4.6
   Compiling tempfile v3.21.0
error[E0308]: mismatched types
  --> src/sys/select.rs:25:53
   |
25 |         usize::try_from(fd).map_or(false, |fd| fd < FD_SETSIZE),
   |                                                --   ^^^^^^^^^^ expected `usize`, found `i32`
   |                                                |
   |                                                expected because this is `usize`
   |
help: you can convert an `i32` to a `usize` and panic if the converted value doesn't fit
   |
25 |         usize::try_from(fd).map_or(false, |fd| fd < FD_SETSIZE.try_into().unwrap()),
   |                                                               ++++++++++++++++++++

error[E0308]: mismatched types
    --> src/sys/select.rs:113:65
     |
 113 |             range: 0..highest.map(|h| h as usize + 1).unwrap_or(FD_SETSIZE),
     |                                                       --------- ^^^^^^^^^^ expected `usize`, found `i32`
     |                                                       |
     |                                                       arguments to this method are incorrect
     |
help: the return type of this call is `i32` due to the type of the argument passed
    --> src/sys/select.rs:113:23
     |
 113 |             range: 0..highest.map(|h| h as usize + 1).unwrap_or(FD_SETSIZE),
     |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^----------^
     |                                                                 |
     |                                                                 this argument influences the return type of `unwrap_or`
note: method defined here
    --> /home/dcarlier/.rustup/toolchains/nightly-x86_64-unknown-freebsd/lib/rustlib/src/rust/library/core/src/option.rs:1031:18
     |
1031 |     pub const fn unwrap_or(self, default: T) -> T
     |                  ^^^^^^^^^
help: you can convert an `i32` to a `usize` and panic if the converted value doesn't fit
     |
 113 |             range: 0..highest.map(|h| h as usize + 1).unwrap_or(FD_SETSIZE.try_into().unwrap()),
     |                                                                           ++++++++++++++++++++

error[E0308]: mismatched types
    --> src/unistd.rs:1100:41
     |
1100 |     unsafe { libc::execv(path.as_ptr(), args_p.as_ptr()) };
     |              -----------                ^^^^^^^^^^^^^^^ types differ in mutability
     |              |
     |              arguments to this function are incorrect
     |
     = note: expected raw pointer `*const *mut i8`
                found raw pointer `*const *const i8`
note: function defined here
    --> /home/dcarlier/.cargo/git/checkouts/libc-42f01ba267c5d37b/3f8c507/src/unix/mod.rs:1033:12
     |
1033 |     pub fn execv(prog: *const c_char, argv: *const *mut c_char) -> c_int;
     |            ^^^^^

error[E0308]: arguments to this function are incorrect
    --> src/unistd.rs:1126:14
     |
1126 |     unsafe { libc::execve(path.as_ptr(), args_p.as_ptr(), env_p.as_ptr()) };
     |              ^^^^^^^^^^^^
     |
note: types differ in mutability
    --> src/unistd.rs:1126:42
     |
1126 |     unsafe { libc::execve(path.as_ptr(), args_p.as_ptr(), env_p.as_ptr()) };
     |                                          ^^^^^^^^^^^^^^^
     = note: expected raw pointer `*const *mut i8`
                found raw pointer `*const *const i8`
note: types differ in mutability
    --> src/unistd.rs:1126:59
     |
1126 |     unsafe { libc::execve(path.as_ptr(), args_p.as_ptr(), env_p.as_ptr()) };
     |                                                           ^^^^^^^^^^^^^^
     = note: expected raw pointer `*const *mut i8`
                found raw pointer `*const *const i8`
note: function defined here
    --> /home/dcarlier/.cargo/git/checkouts/libc-42f01ba267c5d37b/3f8c507/src/unix/mod.rs:1034:12
     |
1034 |     pub fn execve(prog: *const c_char, argv: *const *mut c_char, envp: *const *mut c_char)
     |            ^^^^^^

error[E0308]: mismatched types
    --> src/unistd.rs:1147:46
     |
1147 |     unsafe { libc::execvp(filename.as_ptr(), args_p.as_ptr()) };
     |              ------------                    ^^^^^^^^^^^^^^^ types differ in mutability
     |              |
     |              arguments to this function are incorrect
     |
     = note: expected raw pointer `*const *mut i8`
                found raw pointer `*const *const i8`
note: function defined here
    --> /home/dcarlier/.cargo/git/checkouts/libc-42f01ba267c5d37b/3f8c507/src/unix/mod.rs:1036:12
     |
1036 |     pub fn execvp(c: *const c_char, argv: *const *mut c_char) -> c_int;
     |            ^^^^^^

error[E0308]: arguments to this function are incorrect
    --> src/unistd.rs:1197:14
     |
1197 |     unsafe { libc::fexecve(fd.as_fd().as_raw_fd(), args_p.as_ptr(), env_p.as_ptr()) };
     |              ^^^^^^^^^^^^^
     |
note: types differ in mutability
    --> src/unistd.rs:1197:52
     |
1197 |     unsafe { libc::fexecve(fd.as_fd().as_raw_fd(), args_p.as_ptr(), env_p.as_ptr()) };
     |                                                    ^^^^^^^^^^^^^^^
     = note: expected raw pointer `*const *mut i8`
                found raw pointer `*const *const i8`
note: types differ in mutability
    --> src/unistd.rs:1197:69
     |
1197 |     unsafe { libc::fexecve(fd.as_fd().as_raw_fd(), args_p.as_ptr(), env_p.as_ptr()) };
     |                                                                     ^^^^^^^^^^^^^^
     = note: expected raw pointer `*const *mut i8`
                found raw pointer `*const *const i8`
note: function defined here
    --> /home/dcarlier/.cargo/git/checkouts/libc-42f01ba267c5d37b/3f8c507/src/unix/bsd/freebsdlike/mod.rs:1530:12
     |
1530 |     pub fn fexecve(fd: c_int, argv: *const *mut c_char, envp: *const *mut c_char) -> c_int;
     |            ^^^^^^^

error[E0308]: mismatched types
   --> src/macros.rs:72:35
    |
 72 |                       const $Flag = libc::$Flag $(as $cast)*;
    |                                     ^^^^^^^^^^^ expected `i32`, found `i16`
    |
   ::: src/spawn.rs:203:1
    |
203 | / libc_bitflags!(
204 | |     /// Process attributes to be changed in the new process image when invoking [`posix_spawn`]
205 | |     /// or [`posix_spawnp`]. See
206 | |     /// [posix_spawn](https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_spawn.html).
...   |
227 | | );
    | | -
    | | |
    | |_arguments to this function are incorrect
    |   in this macro invocation
    |
note: associated function defined here
   --> src/spawn.rs:203:1
    |
203 | / libc_bitflags!(
204 | |     /// Process attributes to be changed in the new process image when invoking [`posix_spawn`]
205 | |     /// or [`posix_spawnp`]. See
206 | |     /// [posix_spawn](https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_spawn.html).
...   |
227 | | );
    | |_^
    = note: this error originates in the macro `libc_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0308`.
error: could not compile `nix` (lib) due to 10 previous errors

@tgross35
Copy link
Contributor

I'm a bit confused: where did the breakage in the above example come from here? The default FreeBSD version didn't change, so I'm not sure where the errors are coming from. AIUI this just deletes the FreeBSD 11 module then commonizes 12+.

@devnexen
Copy link
Contributor Author

devnexen commented Oct 29, 2025

I do not think it s related to this PR in particular but might have more to do with the breaking changes of the future libc major version.

@tgross35
Copy link
Contributor

Ah, I forgot about that bit. @asomers what were your concerns with breakage? AIUI there should be no delta for anybody not using freebsd11.

If you just want to verify there isn't any accidental breakage then it's possible to run cargo rustdoc -- --output-format json -Zunstable-options before and after this patch (setting RUST_LIBC_UNSTABLE_FREEBSD_VERSION as needed) and saving off the file, then run cargo semver-checks --baseline-rustdoc ... --current-rustdoc ....

Copy link
Contributor

@asomers asomers left a comment

Choose a reason for hiding this comment

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

@tgross35 the breakage shown in the Nix crate came from switching libc-0.2 to libc-1.0. But it's important for the submitter to check that nix doesn't break any worse on libc-1.0 with this PR than without it.
And I agree that there shouldn't be any breakage as a result of removing support for the FreeBSD 10 and 11 ABIs from libc. However, there may be as a result of mistakes in the PR. The fact that the submitter didn't catch the accidental removal of kevent or changing the type of ino_t suggests that he didn't test it very closely.

)]
pub fn rand() -> c_int;
#[cfg_attr(
all(target_os = "freebsd", any(freebsd12, freebsd11, freebsd10)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you removing this link name? FreeBSD 12 still needs it.

pub fn abs(i: c_int) -> c_int;
pub fn labs(i: c_long) -> c_long;
#[cfg_attr(
all(target_os = "freebsd", any(freebsd12, freebsd11, freebsd10)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, too. Why remove the link name?

)]
#[cfg_attr(
all(target_os = "freebsd", any(freebsd12, freebsd11, freebsd10)),
link_name = "wait4@FBSD_1.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Like the others, I think that removing this link_name was unintended. However, it is coincidentally the right thing to do. It never should've been there in the first place. wait4 hasn't ever been modified since FreeBSD first adopted ELF symbol versions.

@devnexen devnexen force-pushed the remove_fbsd11_support branch from 3f8c507 to 72a72a7 Compare October 29, 2025 22:04
@rustbot
Copy link
Collaborator

rustbot commented Oct 29, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@devnexen devnexen force-pushed the remove_fbsd11_support branch 4 times, most recently from 952d8d8 to d7e87b8 Compare October 29, 2025 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants