fix: Revert vita's c_char back to i8 by pheki · Pull Request #4258 · rust-lang/libc

pheki · 2025-02-05T03:39:56Z

Description

Hi,

#4199 changed the definition of c_char from i8 to u8 for most ARM targets. While that would usually be correct, VITASDK uses signed chars by default.

This will surely conflict with #4256, but I can rebase it after that one is merged. I opened that one first because in the off-chance it was just accepted, this fix would only be needed on rustc. I'll also create a rustc PR.

Sources

https://github.com/vitasdk/buildscripts/blob/master/patches/gcc/0001-gcc-10.patch#L33-L34

Checklist

Relevant tests in libc-test/semver have been updated (API didn't change)
No placeholder or unstable values like *LAST or *MAX are
included (see #3131)
Tested locally (cd libc-test && cargo test --target mytarget);
especially relevant for platforms that may not be checked in CI

rustbot · 2025-02-05T03:40:01Z

r? @tgross35

rustbot has assigned @tgross35.
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 · 2025-02-05T04:34:28Z

Thanks for the PR, but could you do the rustc change first? That's the better place to discuss this change and we can ping a few maintainers, libc will just follow whatever the decision is there.

tgross35 · 2025-02-05T10:47:17Z

Also, does Clang do the right thing? This might need to be an upstream issue as well.

pheki · 2025-02-05T15:45:18Z

Thanks for the PR, but could you do the rustc change first? That's the better place to discuss this change and we can ping a few maintainers, libc will just follow whatever the decision is there.

Sure can, will do that.

Also, does Clang do the right thing? This might need to be an upstream issue as well.

It does not, but I'm not too worried as for now Clang is not really supported by the vita programming community / VITASDK. The pre-compiled libraries are all compiled using vita's gcc and we set TARGET_CC and TARGET_CXX in cargo vita for build scripts using cc. Regardless I'll take a look into fixing Clang once the Rust side is fixed.

cuviper · 2025-02-12T18:15:53Z

Thanks for the PR, but could you do the rustc change first? That's the better place to discuss this change and we can ping a few maintainers, libc will just follow whatever the decision is there.

We're in agreement in rust-lang/rust#136667, so I think you can proceed here.

tgross35 · 2025-02-14T20:19:41Z

As you mentioned above, this just needs a rebase

@rustbot author

pheki · 2025-02-16T03:22:33Z

Is it possible to retry the job in some way? The failure seems unrelated, the test for aarch64-linux-android hung

@rustbot ready

tgross35 · 2025-02-17T20:46:11Z

Yeah, that isn't this PR's fault, restarted the job.

(backport <rust-lang#4258>) (cherry picked from commit e5a669b)

…viper Revert vita's c_char back to i8 # Description Hi! rust-lang#132975 changed the definition of `c_char` from i8 to u8 for most ARM targets. While that would usually be correct, [VITASDK uses signed chars by default](https://github.com/vitasdk/buildscripts/blob/master/patches/gcc/0001-gcc-10.patch#L33-L34). The Clang definitions are incorrect because Clang is not (yet?) supported by the vita commmunity / `VITADSK`, On the Rust side, the pre-compiled libraries the user can link to are all compiled using vita's `gcc` and [we set `TARGET_CC` and `TARGET_CXX`](https://github.com/vita-rust/cargo-vita/blob/d564a132cbd43947118c0d6d0ebfbea7d1dd7fa7/src/commands/build.rs#L230) in `cargo vita` for build scripts using `cc`. I'm creating it as a draft PR so that we can discuss it and possibly get it approved here, but wait to merge the [libc side](rust-lang/libc#4258) and get a libc version first, as having the definitions out of sync breaks std. As a nightly-only target it can be confusing/frustrating for new users when the latest nightly, which is the default, is broken.

Rollup merge of rust-lang#136667 - vita-rust:revert-vita-c-char, r=cuviper Revert vita's c_char back to i8 # Description Hi! rust-lang#132975 changed the definition of `c_char` from i8 to u8 for most ARM targets. While that would usually be correct, [VITASDK uses signed chars by default](https://github.com/vitasdk/buildscripts/blob/master/patches/gcc/0001-gcc-10.patch#L33-L34). The Clang definitions are incorrect because Clang is not (yet?) supported by the vita commmunity / `VITADSK`, On the Rust side, the pre-compiled libraries the user can link to are all compiled using vita's `gcc` and [we set `TARGET_CC` and `TARGET_CXX`](https://github.com/vita-rust/cargo-vita/blob/d564a132cbd43947118c0d6d0ebfbea7d1dd7fa7/src/commands/build.rs#L230) in `cargo vita` for build scripts using `cc`. I'm creating it as a draft PR so that we can discuss it and possibly get it approved here, but wait to merge the [libc side](rust-lang/libc#4258) and get a libc version first, as having the definitions out of sync breaks std. As a nightly-only target it can be confusing/frustrating for new users when the latest nightly, which is the default, is broken.

…viper Revert vita's c_char back to i8 # Description Hi! rust-lang#132975 changed the definition of `c_char` from i8 to u8 for most ARM targets. While that would usually be correct, [VITASDK uses signed chars by default](https://github.com/vitasdk/buildscripts/blob/master/patches/gcc/0001-gcc-10.patch#L33-L34). The Clang definitions are incorrect because Clang is not (yet?) supported by the vita commmunity / `VITADSK`, On the Rust side, the pre-compiled libraries the user can link to are all compiled using vita's `gcc` and [we set `TARGET_CC` and `TARGET_CXX`](https://github.com/vita-rust/cargo-vita/blob/d564a132cbd43947118c0d6d0ebfbea7d1dd7fa7/src/commands/build.rs#L230) in `cargo vita` for build scripts using `cc`. I'm creating it as a draft PR so that we can discuss it and possibly get it approved here, but wait to merge the [libc side](rust-lang/libc#4258) and get a libc version first, as having the definitions out of sync breaks std. As a nightly-only target it can be confusing/frustrating for new users when the latest nightly, which is the default, is broken.

rustbot assigned tgross35 Feb 5, 2025

rustbot added the S-waiting-on-review label Feb 5, 2025

pheki mentioned this pull request Feb 7, 2025
Revert vita's c_char back to i8 rust-lang/rust#136667
Merged

tgross35 reviewed Feb 12, 2025
View reviewed changes

Comment threadsrc/lib.rs Outdated

tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Feb 14, 2025

tgross35 approved these changes Feb 14, 2025
View reviewed changes

Comment threadsrc/lib.rs Outdated

tgross35 removed the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Feb 14, 2025

rustbot added S-waiting-on-author and removed S-waiting-on-review labels Feb 14, 2025

fix: Revert vita's c_char to i8
e5a669b

pheki force-pushed the revert-vita-c-char branch from 23056e8 to e5a669bCompare February 15, 2025 03:16

tgross35 approved these changes Feb 15, 2025
View reviewed changes

tgross35 added this pull request to the merge queue Feb 15, 2025

pheki mentioned this pull request Feb 15, 2025
Copy definitions from core::ffi and centralize them #4256
Merged
3 tasks

github-merge-queueBot removed this pull request from the merge queue due to failed status checks Feb 15, 2025

rustbot added S-waiting-on-review and removed S-waiting-on-author labels Feb 16, 2025

tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Feb 17, 2025

tgross35 added this pull request to the merge queue Feb 17, 2025

Merged via the queue into rust-lang:main with commit 780f817Feb 17, 2025

pheki deleted the revert-vita-c-char branch February 17, 2025 21:38

tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Feb 19, 2025
fix: Revert vita's c_char to i8
97d425f
(backport <rust-lang#4258>) (cherry picked from commit e5a669b)

tgross35 mentioned this pull request Feb 19, 2025
[0.2] Backports #4274
Merged

tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Feb 19, 2025
fix: Revert vita's c_char to i8
757dd99
(backport <rust-lang#4258>) (cherry picked from commit e5a669b)

tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Feb 22, 2025
fix: Revert vita's c_char to i8
5ea53bc
(backport <rust-lang#4258>) (cherry picked from commit e5a669b)

tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Feb 22, 2025
fix: Revert vita's c_char to i8
3e2fb17
(backport <rust-lang#4258>) (cherry picked from commit e5a669b)

github-actionsBot mentioned this pull request Feb 22, 2025
chore: release v0.2.170 #4219
Merged

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 Feb 22, 2025

Navigation Menu

Uh oh!

fix: Revert vita's c_char back to i8 - #4258

fix: Revert vita's c_char back to i8#4258
tgross35 merged 1 commit into
rust-lang:mainfrom
vita-rust:revert-vita-c-char

pheki commented Feb 5, 2025•
edited
Loading

Uh oh!

rustbot commented Feb 5, 2025

Uh oh!

tgross35 commented Feb 5, 2025

Uh oh!

tgross35 commented Feb 5, 2025

Uh oh!

pheki commented Feb 5, 2025

Uh oh!

cuviper commented Feb 12, 2025

Uh oh!

Uh oh!

Uh oh!

tgross35 commented Feb 14, 2025•
edited
Loading

Uh oh!

Uh oh!

pheki commented Feb 16, 2025

Uh oh!

tgross35 commented Feb 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

pheki commented Feb 5, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Sources

Checklist

Uh oh!

rustbot commented Feb 5, 2025

Uh oh!

tgross35 commented Feb 5, 2025

Uh oh!

tgross35 commented Feb 5, 2025

Uh oh!

pheki commented Feb 5, 2025

Uh oh!

cuviper commented Feb 12, 2025

Uh oh!

Uh oh!

Uh oh!

tgross35 commented Feb 14, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

pheki commented Feb 16, 2025

Uh oh!

tgross35 commented Feb 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pheki commented Feb 5, 2025•
edited
Loading

tgross35 commented Feb 14, 2025•
edited
Loading