Skip to content

build dist for x86_64-pc-solaris and sparcv9-sun-solaris#138699

Merged
bors merged 1 commit into
rust-lang:masterfrom
psumbera:solaris-ci-build3
Jun 4, 2025
Merged

build dist for x86_64-pc-solaris and sparcv9-sun-solaris#138699
bors merged 1 commit into
rust-lang:masterfrom
psumbera:solaris-ci-build3

Conversation

@psumbera

@psumbera psumbera commented Mar 19, 2025 •
edited by marcoieni
Loading

Copy link
Copy Markdown
Contributor

FIx for #126025

Note that this needs Nix with following commit:
nix-rust/nix@0f45593
Unfortunatelly there is no new release.

And for sparcv9 it needs also:
libffi-rs/libffi-rs#103

For now it therefore uses cargo patch infrastructure for these.

try-job: dist-sparcv9-solaris
try-job: dist-x86_64-solaris
try-job: dist-various-2
try-job: dist-x86_64-illumos

@rustbot

rustbot commented Mar 19, 2025

Copy link
Copy Markdown
Collaborator

r? @marcoieni

rustbot has assigned @marcoieni.
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

@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-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. labels Mar 19, 2025
@rustbot

rustbot commented Mar 19, 2025

Copy link
Copy Markdown
Collaborator

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Veykril
Veykril reviewed Mar 19, 2025
View reviewed changes
Comment thread src/tools/rust-analyzer/Cargo.toml Outdated
@rust-log-analyzer

This comment has been minimized.

Sign in to view
jieyouxu
jieyouxu reviewed Mar 19, 2025
View reviewed changes
Comment thread Cargo.toml Outdated
@psumbera psumbera force-pushed the solaris-ci-build3 branch from 3b9fbda to 0deec39 Compare March 19, 2025 11:00
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jieyouxu

jieyouxu commented Mar 20, 2025

Copy link
Copy Markdown
Member

(That being said, thank you for investigating fixes for the Solaris CI)

@psumbera psumbera mentioned this pull request Mar 21, 2025
Merged
@bors

bors commented Apr 15, 2025

Copy link
Copy Markdown
Collaborator

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

@psumbera psumbera force-pushed the solaris-ci-build3 branch from 0deec39 to f556156 Compare April 16, 2025 10:15
@marcoieni

marcoieni commented Apr 16, 2025

Copy link
Copy Markdown
Member

Is this ready for review? 👀

Veykril
Veykril reviewed Apr 16, 2025
View reviewed changes
Comment thread src/tools/rust-analyzer/Cargo.lock
@psumbera psumbera mentioned this pull request Apr 17, 2025
Merged
@psumbera psumbera force-pushed the solaris-ci-build3 branch from f556156 to 7152b36 Compare April 18, 2025 05:49
@jieyouxu jieyouxu removed the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Apr 22, 2025
marcoieni
marcoieni reviewed Apr 22, 2025
View reviewed changes
Comment thread src/ci/docker/scripts/solaris-toolchain.sh Outdated
BINUTILS_URL="https://ftp.gnu.org/gnu/binutils/$BINUTILS_TAR"


download_file() {

@marcoieni marcoieni Apr 22, 2025 •
edited
Loading

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 saw this function is duplicated from illuminos-toolchain.sh
Can you move it to a shared script file or similar and source the file? so that we avoid having duplicate code.

Same is true for other parts of the scripts which are very similar

@psumbera psumbera Apr 24, 2025

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.

I saw this function is duplicated from illuminos-toolchain.sh Can you move it to a shared script file or similar and source the file? so that we avoid having duplicate code.

I have moved it into shared.sh.

Same is true for other parts of the scripts which are very similar

There are no other common functions. While the structure looks similiar I don't think it's good to connect these two more.

@marcoieni

marcoieni commented Apr 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Sorry for the wait, I'm having a look now. I'm familiar with the Rust CI in general but not with solaris specifically, so please don't assume I'm familiar with the scripts you edited 👍

From what I can see you are extracting the build of x86_64-solaris and sparcv9-solaris from the dist-various-2 job.
My first question is: why? What's the advantage with respect to keeping these jobs in the dist-various-2 job?

The second question is: the dockerfile of dist-various-2 uses ubuntu:22.04. Why are you using ubuntu:18.04 for the new dockerfiles? Is it possible to use newer ubuntu images?
Same for older software like python2. Previously python2 wasn't needed in dist-various-2, why do we need it now?

Also can you explain how this PR fixes the issue you linked?

@psumbera psumbera force-pushed the solaris-ci-build3 branch from 7152b36 to d5925a8 Compare April 24, 2025 07:10
@psumbera

psumbera commented Apr 24, 2025

Copy link
Copy Markdown
Contributor Author

From what I can see you are extracting the build of x86_64-solaris and sparcv9-solaris from the dist-various-2 job. My first question is: why? What's the advantage with respect to keeping these jobs in the dist-various-2 job?

dist-various-2 was generating just standard library for Solaris. Now also host tools are generated. So I think it's correct to move it outside of dist-various-2 job and have separate job like Illumos or BSDs.

The second question is: the dockerfile of dist-various-2 uses ubuntu:22.04. Why are you using ubuntu:18.04 for the new dockerfiles? Is it possible to use newer ubuntu images? Same for older software like python2. Previously python2 wasn't needed in dist-various-2, why do we need it now?

I have updated change to use ubuntu:22.04 and python3.

Also can you explain how this PR fixes the issue you linked?

I was asked in #126025 to do clean up. I decided to clean it up by compleltely removing all Solaris related stuff from dist-various-2 and have bright new build job for Solaris :-)

marcoieni
marcoieni reviewed Apr 24, 2025
View reviewed changes

@marcoieni marcoieni 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.

A few minor comments.
@Kobzol can you have a look at this PR as well?

Comment thread src/ci/docker/host-x86_64/dist-sparcv9-solaris/Dockerfile Outdated
Comment thread src/ci/docker/host-x86_64/dist-sparcv9-solaris/Dockerfile Outdated
Comment thread src/ci/docker/scripts/solaris-toolchain.sh Outdated
Comment thread src/ci/docker/scripts/solaris-toolchain.sh Outdated
Comment thread src/ci/docker/scripts/solaris-toolchain.sh Outdated
Comment thread src/ci/docker/scripts/solaris-toolchain.sh Outdated
Comment thread src/ci/docker/scripts/solaris-toolchain.sh Outdated
Comment thread src/ci/docker/scripts/solaris-toolchain.sh Outdated
Comment thread src/ci/github-actions/jobs.yml
Comment thread src/ci/docker/scripts/solaris-toolchain.sh Outdated
@jieyouxu

jieyouxu commented Jun 2, 2025

Copy link
Copy Markdown
Member

@bors2 try

@rust-bors

rust-bors Bot commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

⌛ Trying commit 0f1579b with merge 1a278b8…

@rust-bors

rust-bors Bot commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 1a278b8 (1a278b8e761394e5409f5ebe9b4ab99f94cfcfb7)

@marcoieni

marcoieni commented Jun 3, 2025

Copy link
Copy Markdown
Member

This looks good. I'm trying the other jobs edited by this pr to make sure nothing broke and if the try jobs work I'll approve.

@bors2 try

@rust-bors

rust-bors Bot commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

⌛ Trying commit 0f1579b with merge da4fc05…

@rust-bors

rust-bors Bot commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: da4fc05 (da4fc05f7b652b51d3bfdd1538406ad5a6204a7b)

@marcoieni

marcoieni commented Jun 3, 2025

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jun 3, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 0f1579b has been approved by marcoieni

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 Jun 3, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 4, 2025
Merged
@bors bors merged commit 8ae7ca0 into rust-lang:master Jun 4, 2025
@rustbot rustbot added this to the 1.89.0 milestone Jun 4, 2025
@psumbera

psumbera commented Jun 6, 2025

Copy link
Copy Markdown
Contributor Author

Seems that only llvm-tools are newly delivered. Where are others?
https://rust-lang.github.io/rustup-components-history/x86_64-pc-solaris.html

@nikic

nikic commented Jun 6, 2025

Copy link
Copy Markdown
Contributor

@psumbera Missing changes to build-manifest: https://github.com/rust-lang/rust/blob/master/src/tools/build-manifest/src/main.rs

This was referenced Jun 6, 2025
Merged
Merged
@psumbera psumbera mentioned this pull request Jul 2, 2025
Closed
@ehuss ehuss mentioned this pull request May 16, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Veykril Veykril Veykril left review comments
@marcoieni marcoieni marcoieni left review comments
@jieyouxu jieyouxu jieyouxu left review comments

Assignees

@marcoieni marcoieni

Labels

A-testsuite Area: The testsuite used to check the correctness of rustc 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.89.0

Development

Successfully merging this pull request may close these issues.

9 participants

@psumbera @rustbot @rust-log-analyzer @jieyouxu @bors @marcoieni @nikic @Kobzol @Veykril