Skip to content

ci: Add support for dist-loongarch64-linux#110519

Merged
bors merged 1 commit into
rust-lang:masterfrom
heiher:ci
May 23, 2023
Merged

ci: Add support for dist-loongarch64-linux#110519
bors merged 1 commit into
rust-lang:masterfrom
heiher:ci

Conversation

@heiher

@heiher heiher commented Apr 19, 2023

Copy link
Copy Markdown
Contributor

We are preparing to promote loongarch64-unknown-linux-gnu to Tier 2, and one of the tasks is to add CI support. We are currently in the process of upgrading the dependencies for the build tools, and before this is completed, we would like to request comments. Thanks

Progress

Tier 2 with host tools MCP: rust-lang/compiler-team#518

@rustbot

rustbot commented Apr 19, 2023

Copy link
Copy Markdown
Collaborator

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Apr 19, 2023
@rust-log-analyzer

This comment has been minimized.

@heiher heiher marked this pull request as ready for review April 26, 2023 03:36
@heiher

heiher commented Apr 26, 2023

Copy link
Copy Markdown
Contributor Author

@bors try

@bors

bors commented Apr 26, 2023

Copy link
Copy Markdown
Collaborator

@heiher: 🔑 Insufficient privileges: not in try users

@heiher

heiher commented Apr 29, 2023

Copy link
Copy Markdown
Contributor Author

@Mark-Simulacrum Ready for review, ci test result: https://github.com/loongarch-rs/rust/actions/runs/4804319116

@cuviper

cuviper commented May 2, 2023

Copy link
Copy Markdown
Member

Note that try only runs dist-x86_64-linux unless you (temporarily) edit the CI config for that job.

My crosstool refactoring in #110865 was just approved, so it would be great if you followed suit here. I see that you're using a git snapshot for loongarch support, but apart from that you should be able to work like the others.

@heiher

heiher commented May 3, 2023

Copy link
Copy Markdown
Contributor Author

Hello @cuviper, Thanks for your comments.

Note that try only runs dist-x86_64-linux unless you (temporarily) edit the CI config for that job.

You're right, I did make a temporary change to the CI configuration for that job.

My crosstool refactoring in #110865 was just approved, so it would be great if you followed suit here. I see that you're using a git snapshot for loongarch support, but apart from that you should be able to work like the others.

Great job! Thank you for pushing the update for crosstool-ng. However, support for LoongArch is expected to be available only in the next release version (1.26). Therefore, it may be a good idea for us to use the git snapshot version for now and make the change once version 1.26 is released. This way, we shouldn't miss out on Rust 1.71.

I am grateful for your insightful comments and suggestions. ❤️

@heiher heiher marked this pull request as draft May 3, 2023 01:19
@cuviper

cuviper commented May 3, 2023

Copy link
Copy Markdown
Member

@heiher Yes, it's fine that you need to use a pre-release version of crosstool-ng. I meant that you should try to follow the rest of the refactoring that I implemented: committing a defconfig rather than a full config, and using the new common scripts/crosstool-ng-build.sh instead of individual scripts like your build-toolchains.sh.

@bors

bors commented May 3, 2023

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #110865) made this pull request unmergeable. Please resolve the merge conflicts.

@heiher

heiher commented May 4, 2023

Copy link
Copy Markdown
Contributor Author

@cuviper It has been rebased to the mainline and passed the tests. Does it look good to you please?

@heiher heiher marked this pull request as ready for review May 4, 2023 00:02

@xen0n xen0n left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is much easier to review now, thanks for the rebasing! Only one question from me below.

Comment thread src/ci/docker/README.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we able to raise this to GCC 13.1.0 now that it's released?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Certainly, although GCC 13 has been released, the upgrade for crosstool-ng is still in progress. We would like to catch up with the Rust 1.71 release, and since LoongArch CI is not dependent on GCC 13, I propose that we upgrade after crosstool-ng is ready, possibly through a new PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If crosstool-ng is ready earlier than expected, I would be happy to include it in this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah yes. I forgot we privately talked about this a while ago.

@cuviper cuviper left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm going to leave the formal review to Mark. I'm not sure if the target should be built in CI at all until it has approval per the target tier policy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't need GDB, so all 4 of these can be removed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On all other targets, I left out versions for things like ISL and ZSTD so they could just use their defaults. Are there particular needs here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't really need logging, but I guess it doesn't hurt either way.

Please do add the mirror config like the other targets though:

CT_USE_MIRROR=y
CT_MIRROR_BASE_URL="https://ci-mirrors.rust-lang.org/rustc"

(We don't have everything mirrored, but AIUI this will let our mirror take precedence when we do add things.)

@heiher

heiher commented May 5, 2023

Copy link
Copy Markdown
Contributor Author

I'm going to leave the formal review to Mark. I'm not sure if the target should be built in CI at all until it has approval per the target tier policy.

Thanks for your feedback. We are moving forward, and there is a resolved MCP that for LoongArch to Tier2: rust-lang/compiler-team#518

@heiher

heiher commented May 6, 2023

Copy link
Copy Markdown
Contributor Author

@rustbot author

cargo build breaks by libc constants are missing.

@rustbot rustbot 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 May 6, 2023
@Mark-Simulacrum

Copy link
Copy Markdown
Member

Can you temporarily enable the builder here for try builds or in PR CI? That will let us test how long this takes (after Docker is cached) and make a decision in T-infra on whether we currently have capacity to add the builder.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be pinned to a fixed commit, to make sure we don't pick up a broken commit.

@heiher

heiher commented May 7, 2023

Copy link
Copy Markdown
Contributor Author

@Mark-Simulacrum Certainly, Thank you.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum

Copy link
Copy Markdown
Member

Infra discussed and agreed to move forward with adding the builder.

@bors r+

@bors

bors commented May 22, 2023

Copy link
Copy Markdown
Collaborator

📌 Commit 5f173e9 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 May 22, 2023
@bors

bors commented May 22, 2023

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 5f173e9 with merge 4f80088941583158b9f62e8d6a60b94a087329d9...

@bors

bors commented May 23, 2023

Copy link
Copy Markdown
Collaborator

💥 Test timed out

@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 May 23, 2023
@xen0n

xen0n commented May 23, 2023

Copy link
Copy Markdown
Contributor

The dist-x86_64-apple build seems to be flaking out.

@rust-log-analyzer

Copy link
Copy Markdown
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@Dylan-DPC Dylan-DPC 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 May 23, 2023
@heiher

heiher commented May 23, 2023

Copy link
Copy Markdown
Contributor Author

@bors retry

This PR doesn't seem bad, retry again.

@bors

bors commented May 23, 2023

Copy link
Copy Markdown
Collaborator

@heiher: 🔑 Insufficient privileges: not in try users

@workingjubilee

Copy link
Copy Markdown
Member

@bors retry

@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 May 23, 2023
@bors

bors commented May 23, 2023

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 5f173e9 with merge cda5bec...

patch \
pkg-config \
python3 \
rsync \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rsync used somewhere? Can't see any matches here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is used by crosstool-ng git version.

@bors

bors commented May 23, 2023

Copy link
Copy Markdown
Collaborator

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 23, 2023
@bors bors merged commit cda5bec into rust-lang:master May 23, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 23, 2023
@heiher heiher deleted the ci branch May 23, 2023 10:33
@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (cda5bec): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.9% [2.8%, 3.0%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.6% [-1.6%, -1.6%] 1
All ❌✅ (primary) - - 0

Cycles

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

Binary size

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

Bootstrap: 644.363s -> 644.28s (-0.01%)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 10, 2023
ci: Upgrade loongarch64-linux-gnu GCC to 13.1.0

This PR upgrades GCC to 13.1.0 for the `loongarch64-unknown-linux-gnu` target. This upgrade was suggested in a previous review discussion: rust-lang#110519 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.