Skip to content

Start update to musl v1.2.3 #4443

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Start update to musl v1.2.3 #4443

wants to merge 10 commits into from

Conversation

xbjfk
Copy link
Contributor

@xbjfk xbjfk commented May 3, 2025

Description

This is effectively a partial continuation of #3068 (no time64 yet), however done behind a flag so people can test changes in 0.2. Some changes are not behind the musl_v1_2_3 config flag, and these are:

  • The change to RLIM_NLIMITS. This one is definitely safe, as it has had a deprecation warning saying it could change any time with OS version for >5 years
  • The change to O_LARGEFILE is likely safe.
  • Same with the change to the tcp_info struct, as these only add fields while keeping ABI. However, if manually initializing all fields this could cause breakage.

Let me know if any of m assumptions are incorrect and I am happy to gate those changes behind the cfg as well.

I've also added #4437, as this seems like a good chance to rename the field while the cfg option is in place.

Sources

O_LARGEFILE change: bminor/musl@b8b729b
RLIM_NLIMITS change: bminor/musl@2507e7f
utmpx change: bminor/musl@1e7f0fc
tcp_info changes: bminor/musl@d4f2981, bminor/musl@5e0c9f2

Checklist

  • (rest of) time64 API - this can be done in another PR after this one is merged
  • CPU_SETSIZE change (not urgent as changed after 1.2.3)
  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see rust-lang/libc#3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot label +stable-nominated

@rustbot rustbot added O-arm O-linux O-mips O-musl O-riscv O-unix O-x86 S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels May 3, 2025
@rustbot

This comment has been minimized.

This was referenced May 3, 2025
@xbjfk
Copy link
Contributor Author

xbjfk commented May 3, 2025

hmm, looks like I need to allow deprecation for CI to pass - will allow deprecated tomorrow.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this! A handful of small comments, most importantly we don't want to stop testing the current musl version because that needs to continue working until we do the final version bump. Also looks like there is a CI failure.

@tgross35
Copy link
Contributor

tgross35 commented May 4, 2025

⚠️ Warning ⚠️

* There are issue links (such as `#123`) in the commit messages of the following commits.
  _Please remove them as they will spam the issue with references to the commit._
  
  * [5f03e36](https://github.com/rust-lang/libc/commit/5f03e36a51907a467127c9a722039842ff1fd066)

Disregard this, that's not a good warning

@rustbot rustbot added the A-CI Area: CI-related items label May 4, 2025
tgross35 added a commit to tgross35/rust-libc that referenced this pull request May 4, 2025
This was enabled in order to encourage canonical links. Unfortunately it
seems to also create warnings for crosslinking issues [1], which is
something we definitely do want contributors to do (especially
considering how commits get cherry picked around in this repo, there may
not be a PR to easily refer to).

Disable `issue-links` for now to avoid this. This also means we lose
canonical link warnings, but there wasn't really any specific reason we
needed that anyway.

Cc: rust-lang/triagebot#1965
[1]: rust-lang#4443 (comment)
@tgross35
Copy link
Contributor

tgross35 commented May 5, 2025

This looks pretty good as a first step to me, I think we can merge it once you get CI passing.

@rustbot author

@rustbot
Copy link
Collaborator

rustbot commented May 5, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@xbjfk xbjfk force-pushed the musl-1-2-3 branch 2 times, most recently from 7a67414 to 60e55da Compare May 5, 2025 08:05
xbjfk and others added 2 commits May 5, 2025 08:26
This will be chosen based on the RUST_LIBC_UNSTABLE_MUSL_V1_2_3
variable.

Co-authored-by: Daniel Frampton <[email protected]>
This commit gates various workarounds of older musl versions behind the
RUST_LIBC_UNSTABLE_MUSL_V1_2_3 variable.
@xbjfk xbjfk force-pushed the musl-1-2-3 branch 2 times, most recently from 8596b7d to a5dcb9b Compare May 5, 2025 08:41
xbjfk added 2 commits May 5, 2025 09:17
This was accidentally set to 0 in upstream, but fixed in commit b8b729b.
If running with prior versions without that commit, this commit
effectively backports it.
This feature, controlled by the environment variable
RUST_LIBC_UNSTABLE_MUSL_V1_2_3 will control whether breaking changes up
to musl v1.2.3 will be reflected
@xbjfk xbjfk force-pushed the musl-1-2-3 branch 2 times, most recently from 690ea06 to 60f7b3d Compare May 5, 2025 10:02
xbjfk and others added 6 commits May 5, 2025 10:02
This ut_session has changed from long to int + padding in newer
versions. This was already reflected on loongarch64 and ohos - this
commit adds this change, and re-enables the test when musl_v1_2_3 is
set.

Co-authored-by: Ariadne Conill <[email protected]>
This reflects the upstream commits, 5e0c9f2 and d4f2981, which reflect
changes in linux 5.4 and 5.5 respectively

As mentioned in the comments, this is possible now as the CI musl version
has updated and the headers are newer.
This reflects upstream commit 2507e7f. This should be safe to change as
this has been marked deprecated to warn people it will change across OS
versions since 0.2.64 (>5 years ago)
This isn't strictly related to musl 1.2.3, however now presents a good
time to change it, before the 1.0 release.
The architecture-specific definitions was removed in upstream commit
319b2d0, changing the type to the generic definition of int.
This silences shellcheck warnings
@xbjfk
Copy link
Contributor Author

xbjfk commented May 5, 2025

Thanks for the review! Looks like the CI passes, except for freebsd (which seems to have had failures previously)

I've started working on the time64 changes, expect a PR for those in a few days if all goes to plan :)

@rustbot ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI Area: CI-related items O-arm O-linux O-mips O-musl O-riscv O-unix O-x86 S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants