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

Update libc to 0.2.121 #94052

Closed
wants to merge 1 commit into from
Closed

Conversation

glaubitz
Copy link
Contributor

@glaubitz glaubitz commented Feb 16, 2022

Updating libc to 0.2.119 adds platform support for m68k-unknown-linux-gnu.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 16, 2022
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 16, 2022

📌 Commit cc9fecc has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 16, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 16, 2022
…crum

Update libc to 0.2.118

Updating libc to 0.2.118 adds platform support for m68k-unknown-linux-gnu.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 19, 2022
…crum

Update libc to 0.2.118

Updating libc to 0.2.118 adds platform support for m68k-unknown-linux-gnu.
@matthiaskrgr
Copy link
Member

@bors r-
failed in a rollup: #94151 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 19, 2022
@glaubitz
Copy link
Contributor Author

@bors r- failed in a rollup: #94151 (comment)

Just rebased and repushed. Could you approve the PR again? Thanks!

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 22, 2022

📌 Commit d98a052 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 22, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 22, 2022
…crum

Update libc to 0.2.118

Updating libc to 0.2.118 adds platform support for m68k-unknown-linux-gnu.
@matthiaskrgr
Copy link
Member

Looks like there is still a problem: #94251 (comment)
@bors r- rollup=never

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 22, 2022
@glaubitz
Copy link
Contributor Author

Looks like there is still a problem: #94251 (comment) @bors r- rollup=never

That looks more like a missing build dependency on the CI host.

The library packages libsendfile and liblgrp need to be installed.

@glaubitz
Copy link
Contributor Author

Both libsendfile and liblgrp seem to be exclusive Solaris libraries. So, I'm not sure how to download them on the Ubuntu host for cross-compilation.

@psumbera Do you have any suggestion how to fix the CI build for Solaris x86_64?

@Dylan-DPC
Copy link
Member

failed in rollup

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 20, 2022
@klensy
Copy link
Contributor

klensy commented Mar 20, 2022

failed in rollup

@bors r-

rollup=never ?

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 20, 2022
…lacrum

solaris build environment should include libsendfile/liblgrp

As of version 0.2.120 of the libc crate, the solaris target now requires
some additional libraries to be present in the sysroot.  Note that the
solaris target doesn't really build against files from Solaris, but
rather against some files from DilOS (a platform similar to both Solaris
and illumos).  Pull in the extra libraries and their compilation links
from that apt repository.

This aims to assist with rust-lang#94052.
@MabezDev
Copy link
Contributor

MabezDev commented Apr 4, 2022

Could we try and merge this again?

@Dylan-DPC
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 4, 2022

📌 Commit e69e652 has been approved by Dylan-DPC

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 4, 2022
@bors
Copy link
Contributor

bors commented Apr 4, 2022

⌛ Testing commit e69e652 with merge f1e86a7708a6670a98f2efb3d63a9759ef4b286c...

@bors
Copy link
Contributor

bors commented Apr 4, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 4, 2022
@rust-log-analyzer
Copy link
Collaborator

The job dist-x86_64-illumos failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] addr2line test:false 0.399
[RUSTC-TIMING] core test:false 22.842
[RUSTC-TIMING] gimli test:false 4.364
[RUSTC-TIMING] object test:false 4.478
error[E0615]: attempted to take value of method `si_addr` on type `siginfo_t`
  --> library/std/src/sys/unix/stack_overflow.rs:70:17
   |
70 |         (*info).si_addr as usize
   |
help: use parentheses to call the method
   |
   |
70 |         (*info).si_addr() as usize

For more information about this error, try `rustc --explain E0615`.
[RUSTC-TIMING] std test:false 2.225
error: could not compile `std` due to previous error

@glaubitz
Copy link
Contributor Author

glaubitz commented Apr 4, 2022 via email

@pfmooney
Copy link
Contributor

pfmooney commented Apr 4, 2022

I guess we should disable the Illumos CI for the time being.

I'd like to help solve the issue to avoid that outcome, please.

As of 0.2.121, all of the UNIX-ish platforms in libc now have an si_addr() accessor. I've drafted pfmooney/rust@f9e8d05, which should hopefully solve the illumos issue, as well as consolidating the logic for the other platforms.

I haven't been able to build it yet, as I'm resurrecting my rust bootstrap build environment.

(edit: Updated reference to drafted fix)

@glaubitz
Copy link
Contributor Author

glaubitz commented Apr 5, 2022

I guess we should disable the Illumos CI for the time being.

I'd like to help solve the issue to avoid that outcome, please.

I'm not opposed to fixing the underlying problem, of course.

As of 0.2.121, all of the UNIX-ish platforms in libc now have an si_addr() accessor. I've drafted pfmooney/rust@f9e8d05, which should hopefully solve the illumos issue, as well as consolidating the logic for the other platforms.

Correct if I'm wrong but didn't Illumos use the si_addr() accessor before!?

I haven't been able to build it yet, as I'm resurrecting my rust bootstrap build environment.

Why not just create a PR with the libc update included and see if both changes together build?

@pfmooney
Copy link
Contributor

pfmooney commented Apr 5, 2022

Correct if I'm wrong but didn't Illumos use the si_addr() accessor before!?

It was using the si_addr public field, rather than the si_addr() accessor method. When exposing other parts of the siginfo_t on illumos, it was updated to expose si_addr(), which all of the other UNIX-y platforms had added. This also did away with the fiction of the si_addr field on illumos, since it's not a field on that struct, but rather a deeper member of siginfo data union.

Why not just create a PR with the libc update included and see if both changes together build?

I've since done that locally, building for both Linux and illumos as small test cases. I wasn't sure how you wanted to proceed with this PR, either including the aforementioned patch to use si_addr(), or respinning as a whole new PR. Please let me know what would be best.

@glaubitz
Copy link
Contributor Author

glaubitz commented Apr 5, 2022

Why not just create a PR with the libc update included and see if both changes together build?

I've since done that locally, building for both Linux and illumos as small test cases. I wasn't sure how you wanted to proceed with this PR, either including the aforementioned patch to use si_addr(), or respinning as a whole new PR. Please let me know what would be best.

Feel free to just pick my patch and create a new PR. Or just, if you already have done so, use your own patch to update libc.

I don't insist on this PR coming from myself ;). If we can get libc updated, I'm more than happy.

@pfmooney
Copy link
Contributor

pfmooney commented Apr 5, 2022

Feel free to just pick my patch and create a new PR. Or just, if you already have done so, use your own patch to update libc.

I don't insist on this PR coming from myself ;). If we can get libc updated, I'm more than happy.

Cool, thanks. I've opened #95688 with the combined changes.

@glaubitz
Copy link
Contributor Author

glaubitz commented Apr 5, 2022

I would have split it into two commits though.

@pfmooney
Copy link
Contributor

pfmooney commented Apr 5, 2022

I would have split it into two commits though.

Generally speaking, I agree.

In this case, the intermediate commits would fail to build by themselves, since the si_addr() accessor was added in the newer libc. It makes sense as a single unit, I think.

@glaubitz
Copy link
Contributor Author

glaubitz commented Apr 5, 2022

I would have split it into two commits though.

Generally speaking, I agree.

In this case, the intermediate commits would fail to build by themselves, since the si_addr() accessor was added in the newer libc. It makes sense as a single unit, I think.

Hmm, fair enough. Let's hope the CI succeeds so we can finally get this fixed.

@glaubitz
Copy link
Contributor Author

glaubitz commented Apr 5, 2022

@Mark-Simulacrum Can you approve #95688 instead? I'm closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.