Skip to content

Fuchsia CI zircon lib improvement #99833

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
Merged
merged 1 commit into from
Aug 7, 2022
Merged

Fuchsia CI zircon lib improvement #99833

merged 1 commit into from
Aug 7, 2022

Conversation

@andrewpollack
Copy link
Member

Removing Zircon build process, instead pulling sysroot and related libs directly from Fuchsia SDK

cc. @tmandry @djkoloski

@rustbot rustbot added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Jul 27, 2022
@rust-highfive
Copy link
Contributor

r? @pietroalbini

(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 Jul 27, 2022
tmandry
tmandry requested changes Jul 28, 2022
View reviewed changes
@tmandry
Copy link
Member

tmandry commented Jul 28, 2022

Also, can you squash your fixup commits?

@andrewpollack andrewpollack force-pushed the fuchsia-rust-ci-zircon-lib-improvement branch from ac4e3e0 to 9eeb2fe Compare July 28, 2022 18:48
@andrewpollack andrewpollack requested a review from tmandry July 28, 2022 19:40
tmandry
tmandry reviewed Jul 28, 2022
View reviewed changes
tmandry
tmandry approved these changes Jul 28, 2022
View reviewed changes
Copy link
Member

@tmandry tmandry left a comment

Choose a reason for hiding this comment

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

Looks good to me after the last change I requested. @pietroalbini want to give this a once over?

@andrewpollack andrewpollack force-pushed the fuchsia-rust-ci-zircon-lib-improvement branch from 9eeb2fe to 6e03ed6 Compare July 28, 2022 20:25
@tmandry tmandry mentioned this pull request Jul 29, 2022
Merged
@tmandry
Copy link
Member

tmandry commented Jul 29, 2022

I may have heard that Pietro was on vacation,

r? @rust-lang/infra

@jyn514
Copy link
Member

jyn514 commented Jul 29, 2022

r? rust-lang/infra-ci

(you can see the list of teams highfive accepts at https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust.json)

@andrewpollack andrewpollack force-pushed the fuchsia-rust-ci-zircon-lib-improvement branch from 6e03ed6 to 976da4e Compare July 29, 2022 17:56
@andrewpollack
Copy link
Member Author

Unable to mark a re-request review; thank you for the feedback @petrhosek ! Please let me know if any other additions would be good to add

@andrewpollack andrewpollack force-pushed the fuchsia-rust-ci-zircon-lib-improvement branch 4 times, most recently from 242c03f to d98b6d2 Compare July 29, 2022 21:07
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

@bors
Copy link
Collaborator

bors commented Jul 30, 2022

📌 Commit d98b6d2d9935eb0fbf78045263c6991b8a5f759e has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@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 Jul 30, 2022
@bors
Copy link
Collaborator

bors commented Aug 2, 2022

⌛ Testing commit d98b6d2d9935eb0fbf78045263c6991b8a5f759e with merge 95ffcfe4ed162665fb8191b6fa69e98f47a47492...

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 7, 2022

📌 Commit c913a06937f2952a7c80b142d363e01146a4c2f7 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@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 Aug 7, 2022
@bors
Copy link
Collaborator

bors commented Aug 7, 2022

⌛ Testing commit c913a06937f2952a7c80b142d363e01146a4c2f7 with merge fcb586e9fb67607b468f89e441a036d48938997c...

@bors
Copy link
Collaborator

bors commented Aug 7, 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 Aug 7, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Mark-Simulacrum Mark-Simulacrum 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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 7, 2022
@andrewpollack
Shifting CI to pull Zircon libraries directly from Fuchsia SDK
fa4ce7a 
PR feedback

PR Followups

Updating clang download

Updating clang download

Restructuring env used

Restructuring env used

Adding chmod

Adding chmod

Adding chmod
@andrewpollack andrewpollack force-pushed the fuchsia-rust-ci-zircon-lib-improvement branch from c913a06 to fa4ce7a Compare August 7, 2022 15:19
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 7, 2022

📌 Commit fa4ce7a has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@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 Aug 7, 2022
@bors
Copy link
Collaborator

bors commented Aug 7, 2022

⌛ Testing commit fa4ce7a with merge 9ee22ff...

@bors
Copy link
Collaborator

bors commented Aug 7, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 9ee22ff to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 7, 2022
@bors bors merged commit 9ee22ff into rust-lang:master Aug 7, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 7, 2022
@andrewpollack
Copy link
Member Author

Thank you @Mark-Simulacrum for helping on this! I really appreciate it 🙏

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9ee22ff): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)		 2.8% 3.3% 2
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) 2.8% 3.3% 2

Cycles

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: 🎉 relevant improvement found
mean1 max count2
Regressions 😿
(primary)		 2.5% 2.5% 1
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 -2.3% -2.3% 1
All 😿🎉 (primary) 2.5% 2.5% 1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2

  2. number of relevant changes ↩ ↩2

@andrewpollack andrewpollack mentioned this pull request Aug 9, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 13, 2022
@bors
Auto merge of rust-lang#100341 - andrewpollack:fuchsia-llvm-libunwind…
e8dc8bc 
…, r=tmandry

Use llvm-libunwind="in-tree" for Fuchsia targets

With updates to Fuchsia CI's Zircon libraries rust-lang#99833, we can introduce `llvm-libunwind="in-tree"` for Fuchsia targets. This PR restores functionality removed from rust-lang#93604 (comment).

cc `@tmandry` `@djkoloski`
@Kobzol Kobzol mentioned this pull request Mar 18, 2024
Merged
@workingjubilee workingjubilee added the O-fuchsia Operating system: Fuchsia label May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tmandry tmandry tmandry approved these changes

+1 more reviewer

@petrhosek petrhosek petrhosek left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels

merged-by-bors This PR was explicitly merged by bors. O-fuchsia Operating system: Fuchsia S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.65.0

Development

Successfully merging this pull request may close these issues.