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.

Sign up for GitHub

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

Jump to bottom

fix: make sure the const-extern-fn feature is enabled #4134

Merged
merged 4 commits into from
Nov 25, 2024
Merged

fix: make sure the const-extern-fn feature is enabled #4134

merged 4 commits into from
Nov 25, 2024

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Nov 22, 2024 •
edited
Loading

This was dropped in fa554bc on main and 674cc1f on libc-0.2 ("Drop the libc_const_extern_fn conditional"). Unfortunately, this meant that functions were incorrectly never getting marked const, which showed up with CMSG_SPACE #4115.

Instead of the fix here, I attempted to just use cfg(not(libc_ctest)) instead of a Cargo feature to enable const extern functions; however, this seemed extremely problematic with ctest for some reason 2. Instead, leave the feature as-is and just make it enabled by default.

Fixes: #4115

@rustbot
Copy link
Collaborator

rustbot commented Nov 22, 2024

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@tgross35 tgross35 force-pushed the fix-constness branch 3 times, most recently from b0b3526 to f62b452 Compare November 22, 2024 09:51
@tgross35
Copy link
Contributor Author

This makes no sense, ctest2 seems to be parsing the cfg-if backwards. I have to make the const version available under cfg(not(libc_ctest)) to get ctest to not complain, but doing that makes the function not const by default. Changing it to cfg(libc_ctest) makes ctest look at the other branch of cfg_if and fail because it sees const extern "C".

@tgross35 tgross35 changed the title Fix constness Fix constness of CMSG_SPACE Nov 23, 2024
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 23, 2024
@tgross35
fix: make sure the const-extern-fn feature is enabled
3716ba6 
This was dropped in fa554bc on `main` and 674cc1f on `libc-0.2`
("Drop the libc_const_extern_fn conditional"). Unfortunately, this meant
that functions were incorrectly never getting marked `const`.

Instead of the fix here, I attempted to just use `cfg(not(libc_ctest))`
instead of a Cargo feature to enable `const` extern functions; however,
this seemed extremely problematic with `ctest` for some reason [1].
Instead, leave the feature as-is and just make it enabled by default.

Fixes: rust-lang#4115

[1]: rust-lang#4134
@tgross35 tgross35 changed the title Fix constness of CMSG_SPACE fix: make sure the const-extern-fn feature is enabled Nov 23, 2024
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 23, 2024
@tgross35
fix: make sure the const-extern-fn feature is enabled
9f23a63 
This was dropped in fa554bc on `main` and 674cc1f on `libc-0.2`
("Drop the libc_const_extern_fn conditional"). Unfortunately, this meant
that functions were incorrectly never getting marked `const`, which
showed up with `CMSG_SPACE` [1].

Instead of the fix here, I attempted to just use `cfg(not(libc_ctest))`
instead of a Cargo feature to enable `const` extern functions; however,
this seemed extremely problematic with `ctest` for some reason [2].
Instead, leave the feature as-is and just make it enabled by default.

Fixes: rust-lang#4115 [1]

[2]: rust-lang#4134
@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Nov 23, 2024
@tgross35 tgross35 marked this pull request as ready for review November 23, 2024 01:58
@tgross35 tgross35 enabled auto-merge November 23, 2024 01:58
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 23, 2024
@tgross35
ci: Reduce redundancy in run.sh and run tests in the libc crate
026a64c 
We haven't been running any tests associated with the `libc` crate, make
sure we do this here. Also simplify some redundant things in the script.

(backport <rust-lang#4134>)
(cherry picked from commit 3c7be7c)
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 23, 2024
@tgross35
fix: make sure the const-extern-fn feature is enabled
6acd276 
This was dropped in fa554bc on `main` and 674cc1f on `libc-0.2`
("Drop the libc_const_extern_fn conditional"). Unfortunately, this meant
that functions were incorrectly never getting marked `const`, which
showed up with `CMSG_SPACE` [1].

Instead of the fix here, I attempted to just use `cfg(not(libc_ctest))`
instead of a Cargo feature to enable `const` extern functions; however,
this seemed extremely problematic with `ctest` for some reason [2].
Instead, leave the feature as-is and just make it enabled by default.

Fixes: rust-lang#4115 [1]

[2]: rust-lang#4134

(backport <rust-lang#4134>)
(cherry picked from commit 9f23a63)
@tgross35 tgross35 mentioned this pull request Nov 23, 2024
Merged
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 23, 2024
@tgross35
andoid: Don't run unit tests
7475047 
For some reason, running unit tests in CI gives the following on
Android:

```text
2024-11-23T02:17:49.1406378Z      Running unittests src/lib.rs (target/i686-linux-android/debug/deps/libc-79430fca1af8b7ec)
2024-11-23T02:17:49.1432206Z * daemon not running; starting now at tcp:5037
2024-11-23T02:17:52.1460169Z * daemon started successfully
2024-11-23T02:18:00.1454112Z adb: error: failed to copy '/checkout/target/i686-linux-android/debug/deps/libc-79430fca1af8b7ec' to '/data/local/tmp/libc-79430fca1af8b7ec': remote secure_mkdirs failed: Read-only file system
2024-11-23T02:18:00.1457084Z /checkout/target/i686-linux-android/debug/deps/libc-79430fca1af8b7ec: 1 file pushed, 0 skipped. 19.3 MB/s (5418952 bytes in 0.267s)
2024-11-23T02:18:00.3757282Z thread 'main' panicked at /tmp/runtest.rs:26:5:
2024-11-23T02:18:00.3758028Z assertion failed: status.success()
2024-11-23T02:18:00.3758589Z stack backtrace:
2024-11-23T02:18:00.3810307Z    0: rust_begin_unwind
2024-11-23T02:18:00.3811230Z              at /rustc/a47555110cf09b3ed59811d9b02235443e76a595/library/std/src/panicking.rs:665:5
2024-11-23T02:18:00.3812225Z    1: core::panicking::panic_fmt
2024-11-23T02:18:00.3813089Z              at /rustc/a47555110cf09b3ed59811d9b02235443e76a595/library/core/src/panicking.rs:76:14
2024-11-23T02:18:00.3814034Z    2: core::panicking::panic
2024-11-23T02:18:00.3814860Z              at /rustc/a47555110cf09b3ed59811d9b02235443e76a595/library/core/src/panicking.rs:148:5
2024-11-23T02:18:00.3815756Z    3: runtest::main
2024-11-23T02:18:00.3816545Z    4: core::ops::function::FnOnce::call_once
2024-11-23T02:18:00.3817848Z note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
2024-11-23T02:18:00.3822763Z error: test failed, to rerun pass `--lib`
```

For now, don't run unit tests.

(backport <rust-lang#4134>)
(cherry picked from commit b3a0067)
@tgross35
ci: Reduce redundancy in run.sh and run tests in the libc crate
e0c4e5b 
We haven't been running any tests associated with the `libc` crate, make
sure we do this here. Also simplify some redundant things in the script.
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 25, 2024
@tgross35
fix: make sure the const-extern-fn feature is enabled
39b5211 
This was dropped in fa554bc on `main` and 674cc1f on `libc-0.2`
("Drop the libc_const_extern_fn conditional"). Unfortunately, this meant
that functions were incorrectly never getting marked `const`, which
showed up with `CMSG_SPACE` [1].

Instead of the fix here, I attempted to just use `cfg(not(libc_ctest))`
instead of a Cargo feature to enable `const` extern functions; however,
this seemed extremely problematic with `ctest` for some reason [2].
Instead, leave the feature as-is and just make it enabled by default.

Fixes: rust-lang#4115 [1]

[2]: rust-lang#4134
@rustbot rustbot added the A-CI Area: CI-related items label Nov 25, 2024
@tgross35
Add a test that the const extern fn macro works
0304ed5 
By default, any functions defined in this macro (such as `CMSG_SPACE`)
should be `const`.
@tgross35
ci: Skip unit tests on some platforms
f5fdb56 
For some reason, running unit tests in CI gives the following on
Android:

```text
2024-11-23T02:17:49.1406378Z      Running unittests src/lib.rs (target/i686-linux-android/debug/deps/libc-79430fca1af8b7ec)
2024-11-23T02:17:49.1432206Z * daemon not running; starting now at tcp:5037
2024-11-23T02:17:52.1460169Z * daemon started successfully
2024-11-23T02:18:00.1454112Z adb: error: failed to copy '/checkout/target/i686-linux-android/debug/deps/libc-79430fca1af8b7ec' to '/data/local/tmp/libc-79430fca1af8b7ec': remote secure_mkdirs failed: Read-only file system
2024-11-23T02:18:00.1457084Z /checkout/target/i686-linux-android/debug/deps/libc-79430fca1af8b7ec: 1 file pushed, 0 skipped. 19.3 MB/s (5418952 bytes in 0.267s)
2024-11-23T02:18:00.3757282Z thread 'main' panicked at /tmp/runtest.rs:26:5:
2024-11-23T02:18:00.3758028Z assertion failed: status.success()
2024-11-23T02:18:00.3758589Z stack backtrace:
2024-11-23T02:18:00.3810307Z    0: rust_begin_unwind
2024-11-23T02:18:00.3811230Z              at /rustc/a47555110cf09b3ed59811d9b02235443e76a595/library/std/src/panicking.rs:665:5
2024-11-23T02:18:00.3812225Z    1: core::panicking::panic_fmt
2024-11-23T02:18:00.3813089Z              at /rustc/a47555110cf09b3ed59811d9b02235443e76a595/library/core/src/panicking.rs:76:14
2024-11-23T02:18:00.3814034Z    2: core::panicking::panic
2024-11-23T02:18:00.3814860Z              at /rustc/a47555110cf09b3ed59811d9b02235443e76a595/library/core/src/panicking.rs:148:5
2024-11-23T02:18:00.3815756Z    3: runtest::main
2024-11-23T02:18:00.3816545Z    4: core::ops::function::FnOnce::call_once
2024-11-23T02:18:00.3817848Z note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
2024-11-23T02:18:00.3822763Z error: test failed, to rerun pass `--lib`
```
And on s390x:

```
 + grep -Ev ^\[
KASLR disabled: CPU has no PRNG
/prog: /lib/s390x-linux-gnu/libm.so.6: version `GLIBC_2.38' not found (required by /prog)
+ grep -E (PASSED)|(test result: ok) output
/prog: /lib/s390x-linux-gnu/libc.so.6: version `GLIBC_2.39' not found (required by /prog)
error: test failed, to rerun pass `--lib`
```

For now, don't run unit tests on these platforms.
@tgross35
fix: make sure the const-extern-fn feature is enabled
19e9e6a 
This was dropped in fa554bc on `main` and 674cc1f on `libc-0.2`
("Drop the libc_const_extern_fn conditional"). Unfortunately, this meant
that functions were incorrectly never getting marked `const`, which
showed up with `CMSG_SPACE` [1].

Instead of the fix here, I attempted to just use `cfg(not(libc_ctest))`
instead of a Cargo feature to enable `const` extern functions; however,
this seemed extremely problematic with `ctest` for some reason [2].
Instead, leave the feature as-is and just make it enabled by default.

Fixes: rust-lang#4115 [1]

[2]: rust-lang#4134
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 25, 2024
@tgross35
ci: Reduce redundancy in run.sh and run tests in the libc crate
805ccb6 
We haven't been running any tests associated with the `libc` crate, make
sure we do this here. Also simplify some redundant things in the script.

(backport <rust-lang#4134>)
(cherry picked from commit e0c4e5b)
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 25, 2024
@tgross35
Add a test that the const extern fn macro works
35fe9c7 
By default, any functions defined in this macro (such as `CMSG_SPACE`)
should be `const`.

(backport <rust-lang#4134>)
(cherry picked from commit 0304ed5)

No change in the cherry pick since the function came from libc-0.2
originally.
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 25, 2024
@tgross35
ci: Skip unit tests on some platforms
c93e86c 
For some reason, running unit tests in CI gives the following on
Android:

```text
2024-11-23T02:17:49.1406378Z      Running unittests src/lib.rs (target/i686-linux-android/debug/deps/libc-79430fca1af8b7ec)
2024-11-23T02:17:49.1432206Z * daemon not running; starting now at tcp:5037
2024-11-23T02:17:52.1460169Z * daemon started successfully
2024-11-23T02:18:00.1454112Z adb: error: failed to copy '/checkout/target/i686-linux-android/debug/deps/libc-79430fca1af8b7ec' to '/data/local/tmp/libc-79430fca1af8b7ec': remote secure_mkdirs failed: Read-only file system
2024-11-23T02:18:00.1457084Z /checkout/target/i686-linux-android/debug/deps/libc-79430fca1af8b7ec: 1 file pushed, 0 skipped. 19.3 MB/s (5418952 bytes in 0.267s)
2024-11-23T02:18:00.3757282Z thread 'main' panicked at /tmp/runtest.rs:26:5:
2024-11-23T02:18:00.3758028Z assertion failed: status.success()
2024-11-23T02:18:00.3758589Z stack backtrace:
2024-11-23T02:18:00.3810307Z    0: rust_begin_unwind
2024-11-23T02:18:00.3811230Z              at /rustc/a47555110cf09b3ed59811d9b02235443e76a595/library/std/src/panicking.rs:665:5
2024-11-23T02:18:00.3812225Z    1: core::panicking::panic_fmt
2024-11-23T02:18:00.3813089Z              at /rustc/a47555110cf09b3ed59811d9b02235443e76a595/library/core/src/panicking.rs:76:14
2024-11-23T02:18:00.3814034Z    2: core::panicking::panic
2024-11-23T02:18:00.3814860Z              at /rustc/a47555110cf09b3ed59811d9b02235443e76a595/library/core/src/panicking.rs:148:5
2024-11-23T02:18:00.3815756Z    3: runtest::main
2024-11-23T02:18:00.3816545Z    4: core::ops::function::FnOnce::call_once
2024-11-23T02:18:00.3817848Z note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
2024-11-23T02:18:00.3822763Z error: test failed, to rerun pass `--lib`
```
And on s390x:

```
 + grep -Ev ^\[
KASLR disabled: CPU has no PRNG
/prog: /lib/s390x-linux-gnu/libm.so.6: version `GLIBC_2.38' not found (required by /prog)
+ grep -E (PASSED)|(test result: ok) output
/prog: /lib/s390x-linux-gnu/libc.so.6: version `GLIBC_2.39' not found (required by /prog)
error: test failed, to rerun pass `--lib`
```

For now, don't run unit tests on these platforms.

(backport <rust-lang#4134>)
(cherry picked from commit f5fdb56)
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 25, 2024
@tgross35
fix: make sure the const-extern-fn feature is enabled
c2bded3 
This was dropped in fa554bc on `main` and 674cc1f on `libc-0.2`
("Drop the libc_const_extern_fn conditional"). Unfortunately, this meant
that functions were incorrectly never getting marked `const`, which
showed up with `CMSG_SPACE` [1].

Instead of the fix here, I attempted to just use `cfg(not(libc_ctest))`
instead of a Cargo feature to enable `const` extern functions; however,
this seemed extremely problematic with `ctest` for some reason [2].
Instead, leave the feature as-is and just make it enabled by default.

Fixes: rust-lang#4115 [1]

[2]: rust-lang#4134

(backport <rust-lang#4134>)
(cherry picked from commit 19e9e6a)
@tgross35 tgross35 added this pull request to the merge queue Nov 25, 2024
Merged via the queue into rust-lang:main with commit 79afdce Nov 25, 2024
45 checks passed
@tgross35 tgross35 deleted the fix-constness branch November 25, 2024 11:27
@asomers
Copy link
Contributor

asomers commented Nov 25, 2024

This works for me. Thanks!

@tgross35
Copy link
Contributor Author

I was going to ask before doing a release, thanks for confirming :)

@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Nov 25, 2024
@andrewdavidmackenzie andrewdavidmackenzie mentioned this pull request Nov 26, 2024
Closed
@tamird tamird mentioned this pull request Nov 26, 2024
Closed
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 26, 2024
@tgross35
fix: Ensure that const extern fn is always enabled
6d00234 
In [1] this conditional was dropped in favor of a Cargo feature, which
was turned on by default in [2].

Fixes: rust-lang#4149

[1]: rust-lang#4105
[2]: rust-lang#4134
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 26, 2024
@tgross35
fix: Ensure that const extern fn is always enabled
093cf26 
In [1] this conditional was dropped in favor of a Cargo feature, which
was turned on by default in [2].

Fixes: rust-lang#4149

[1]: rust-lang#4105
[2]: rust-lang#4134
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 26, 2024
@tgross35
fix: Ensure that const extern fn is always enabled
b827a0c 
In [1] this conditional was dropped in favor of a Cargo feature, which
was turned on by default in [2]. However, this did not help the case
where `--no-default-features` is passed.

Unfortunately we still can't drop this config entirely since `ctest`
cannot parse the syntax, so change back to useing a `cfg` to control
constness rather than a Cargo feature.

Fixes: rust-lang#4149

[1]: rust-lang#4105
[2]: rust-lang#4134
@tgross35 tgross35 mentioned this pull request Nov 26, 2024
Merged
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 26, 2024
@tgross35
fix: Ensure that const extern fn is always enabled
e18ee8c 
In [1] this conditional was dropped in favor of a Cargo feature, which
was turned on by default in [2]. However, this did not help the case
where `--no-default-features` is passed.

Unfortunately we still can't drop this config entirely since `ctest`
cannot parse the syntax, so change back to useing a `cfg` to control
constness rather than a Cargo feature.

Additionally, remove a portion of the macro's comment that is no longer
relevant.

Fixes: rust-lang#4149

[1]: rust-lang#4105
[2]: rust-lang#4134
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 26, 2024
@tgross35
fix: Ensure that const extern fn is always enabled
ff801a6 
In [1] this conditional was dropped in favor of a Cargo feature, which
was turned on by default in [2]. However, this did not help the case
where `--no-default-features` is passed.

Unfortunately we still can't drop this config entirely since `ctest`
cannot parse the syntax, so change back to useing a `cfg` to control
constness rather than a Cargo feature.

Additionally, remove a portion of the macro's comment that is no longer
relevant.

Fixes: rust-lang#4149

[1]: rust-lang#4105
[2]: rust-lang#4134

(backport <rust-lang#4151>)
(cherry picked from commit e18ee8c)
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 26, 2024
@tgross35
fix: Ensure that const extern fn is always enabled
5d2b17f 
In [1] this conditional was dropped in favor of a Cargo feature, which
was turned on by default in [2]. However, this did not help the case
where `--no-default-features` is passed.

Unfortunately we still can't drop this config entirely since `ctest`
cannot parse the syntax, so change back to useing a `cfg` to control
constness rather than a Cargo feature.

Additionally, remove a portion of the macro's comment that is no longer
relevant.

Fixes: rust-lang#4149

[1]: rust-lang#4105
[2]: rust-lang#4134

(backport <rust-lang#4151>)
(cherry picked from commit e18ee8c)
@shadow3aaa shadow3aaa mentioned this pull request Dec 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@JohnTitor JohnTitor

Labels
A-CI Area: CI-related items S-waiting-on-review stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CMSG_SPACE is no longer a const function
4 participants
@tgross35 @rustbot @asomers @JohnTitor