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

libc crate tests do not run (or even compile) on linux targets in CI #4149

Closed
tamird opened this issue Nov 26, 2024 · 5 comments · Fixed by #4151
Closed

libc crate tests do not run (or even compile) on linux targets in CI #4149

tamird opened this issue Nov 26, 2024 · 5 comments · Fixed by #4151
Labels
C-bug Category: bug

Comments

@tamird
Copy link
Contributor

tamird commented Nov 26, 2024

#1536 added the ability to mark functions as const, along with a test in https://github.com/rust-lang/libc/blob/42e5708d3c0cfcf68f6ffea45eed7e1732dbd7ec/tests/const_fn.rs. Unfortunately this file is completely dead due to the combination of #[cfg(target_os = "linux")] in it and the behavior introduced in 3eb4a48 which skips building the libc crate when docker: true, which is the case for all linuxes.

This all culminated in the fact that #4105 broke const functions and #4134 failed to fix it in the --no-default-features case. This remains broken.

@tgross35

EDIT: all releases after 0.2.163 which do not contain a fix for this should be yanked.

@tamird tamird added the C-bug Category: bug label Nov 26, 2024
vadorovsky added a commit to vadorovsky/aya that referenced this issue Nov 26, 2024
`0.2.164` and `0.2.165` are affected by rust-lang/libc#4149. Pin the
last working version until a proper fix is released.
vadorovsky added a commit to vadorovsky/aya that referenced this issue Nov 26, 2024
`0.2.164` and `0.2.165` are affected by rust-lang/libc#4149. Pin the
last working version until a proper fix is released.
tamird pushed a commit to aya-rs/aya that referenced this issue Nov 26, 2024
`0.2.164` and `0.2.165` are affected by rust-lang/libc#4149. Pin the
last working version until a proper fix is released.
@tgross35
Copy link
Contributor

Your --no-default-features configuration is not covered, but isn't this test being run by

libc/ci/run.sh

Lines 83 to 92 in 42e5708

cmd="cargo test --target $target ${LIBC_CI_ZBUILD_STD+"-Zbuild-std"}"
# Run tests in the `libc` crate
case "$target" in
# FIXME(android): unit tests fail to start on Android
# FIXME(s390x): unit tests fail to locate glibc
*android*) ;;
*s390x*) ;;
*) $cmd
esac
?

@tamird
Copy link
Contributor Author

tamird commented Nov 26, 2024

No, it is not, because of exec:

libc/ci/run.sh

Line 80 in 42e5708

exec grep -E "^(PASSED)|(test result: ok)" "${CARGO_TARGET_DIR}/out.log"

Were it not for that exec then --no-default-features would be covered:

libc/ci/run.sh

Line 123 in 42e5708

$cmd --no-default-features

@tgross35
Copy link
Contributor

tgross35 commented Nov 26, 2024

The branch with exec would only be executed when QEMU is set, which shouldn't be the case for anything x86 (though a command followed by exit is better style than relying on exec, and that grep seems fragile). I also don't think QEMU is set for any of the qemu-system tests, since those were failing when I added the test branch.

$cmd is not the same at lines 91 and 123, it gains --manifest-path libc-test/Cargo.toml in between. This should probably just be --all on platforms where unit tests can run successfully.

@tamird
Copy link
Contributor Author

tamird commented Nov 26, 2024

The branch with exec would only be executed when QEMU is set, which shouldn't be the case for anything x86 (though a command followed by exit is better style than relying on exec, and that grep seems fragile). I also don't think QEMU is set for any of the qemu-system tests, since those were failing when I added the test branch.

Yes, I think you're right.

$cmd is not the same at lines 91 and 123, it gains --manifest-path libc-test/Cargo.toml in between. This should probably just be --all on platforms where unit tests can run successfully.

Right, I paged this out. Either that or we shouldn't have tests in the libc crate. The situation today is quite inconsistent. Note that --all is a deprecated alias for --workspace.

tgross35 added a commit to tgross35/rust-libc that referenced this issue Nov 26, 2024
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 issue Nov 26, 2024
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 issue Nov 26, 2024
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 added a commit to tgross35/rust-libc that referenced this issue Nov 26, 2024
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 issue Nov 26, 2024
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 issue Nov 26, 2024
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
Copy link
Contributor

Released in 0.2.166 https://github.com/rust-lang/libc/releases/tag/0.2.166

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
2 participants