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

ci update freebsd version proposal, freebsd 12 being eol #120869

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

devnexen
Copy link
Contributor

@devnexen devnexen commented Feb 10, 2024

raising to the lowest still active supported freebsd version.
From 13.1 (already eol too), freebsd introduces a cpu affinity layer
with linux. It also introduces a api compatible copy_file_range which
can be used like its linux's counterpart.
The former is essential to build #120589, therefore breaks the backward
compatibility with the previous FreeBSD releases.

Blocked on #130465

@rustbot
Copy link
Collaborator

rustbot commented Feb 10, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Feb 10, 2024
@Noratrieb
Copy link
Member

Does this mean that the minimum supported version is raised? Or is freebsd backwards-compatible so that builds against a new version still run on the old versiom? (A thing that's, for example, not true for linux glibc).
Can you edit the description to describe the impact of this change this way?

@Mark-Simulacrum Mark-Simulacrum added the relnotes Marks issues that should be documented in the release notes of the next release. label Feb 11, 2024
@Mark-Simulacrum
Copy link
Member

@rustbot author

Please update the PR description with details that @Nilstrieb asked for. It would probably also be a good idea to reflect in the platform support page (https://github.com/rust-lang/rust/blob/master/src/doc/rustc/src/platform-support.md) the target details if this does raise the minimum, so that users have something to reference for future Rust versions.

@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 Feb 11, 2024
@devnexen
Copy link
Contributor Author

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 11, 2024
@Noratrieb
Copy link
Member

What is the current minimum freebsd version supported by rustc? The platform support doc page doesn't document this.

@devnexen
Copy link
Contributor Author

I would say freebsd 12.

@Noratrieb
Copy link
Member

Thank you for adding it! Not in this PR, but it would be nice if you could add a new target documentation page in platform-support in src/doc/rustc for freebsd, adding yourself as the target maintainer (if you're interested) :).

I just looked it up, the last FreeBSD bump was #97944. It didn't include a blog post (which we do for bumps of more popular targets), so that seems fine. It superseded #89083 which contains more information.

Looks like we use FreeBSD artifacts on our CI mirror, so @Mark-Simulacrum (or someone else with permissions) will have to upload them first before this can be merged.

@Mark-Simulacrum
Copy link
Member

I've put the artifacts in our mirror (2024-02-18-freebsd-13.2-i386-base.txz and 2024-02-18-freebsd-13.2-amd64-base.txz). Please update the download file to match those, and then we can move ahead.

@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 Feb 18, 2024
@devnexen devnexen force-pushed the update_fbsd_ci branch 2 times, most recently from c7285bd to 42d5b87 Compare February 18, 2024 17:44
@devnexen
Copy link
Contributor Author

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 19, 2024
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Feb 24, 2024

📌 Commit 42d5b87 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Feb 24, 2024

🌲 The tree is currently closed for pull requests below priority 50. This pull request will be tested once the tree is reopened.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 24, 2024
@devnexen devnexen mentioned this pull request Sep 17, 2024
@devnexen
Copy link
Contributor Author

Issue opened here.

raising to the lowest still active supported freebsd version.
From 13.1 (already eol too), freebsd introduces a cpu affinity layer
with linux. It also introduces a api compatible copy_file_range which
can be used like its linux's counterpart.
The former is essential to build rust-lang#120589, therefore breaks the backward
compatibility with the previous FreeBSD releases.
@devnexen
Copy link
Contributor Author

@Mark-Simulacrum would it be possible to give another try ? since I ve updated the Docker container version, it should come with a newer clang version.

@beetrees
Copy link
Contributor

Per

@Mark-Simulacrum would it be possible to give another try ? since I ve updated the Docker container version, it should come with a newer clang version.

and

I easily miss pings so would recommend setting to waiting-on-review as Ralf did if you need reviewer attention.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Oct 20, 2024
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 20, 2024

📌 Commit 65f05af 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 Oct 20, 2024
@bors
Copy link
Contributor

bors commented Oct 21, 2024

⌛ Testing commit 65f05af with merge fb32dd4...

@bors
Copy link
Contributor

bors commented Oct 21, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 21, 2024
@bors bors merged commit fb32dd4 into rust-lang:master Oct 21, 2024
7 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 21, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fb32dd4): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -3.1%, secondary -2.7%)

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)
- - 0
Improvements ✅
(primary)
-3.1% [-3.1%, -3.1%] 1
Improvements ✅
(secondary)
-2.7% [-4.1%, -1.6%] 3
All ❌✅ (primary) -3.1% [-3.1%, -3.1%] 1

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: 781.834s -> 781.459s (-0.05%)
Artifact size: 333.76 MiB -> 333.77 MiB (0.00%)

@taiki-e
Copy link
Member

taiki-e commented Oct 25, 2024

It seems that Cargo distributed by rustup is causing segmentation fault after this change...

`powerpc64-unknown-freebsd` | ✓ | ✓ | PPC64 FreeBSD (ELFv1 and ELFv2)
`powerpc64le-unknown-freebsd` | | | PPC64LE FreeBSD
`powerpc-unknown-freebsd` | | | PowerPC FreeBSD
`powerpc64-unknown-freebsd` | ✓ | ✓ | PPC64 FreeBSD (ELFv1 and ELFv2, version 13.2)
Copy link
Member

@taiki-e taiki-e Oct 23, 2024

Choose a reason for hiding this comment

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

PPC64 FreeBSD (ELFv1 and ELFv2, version 13.2)

It seems odd that ELFv1 and 13.N coexist.

https://www.freebsd.org/releases/13.0R/relnotes/

powerpc64 switched to ELFv2 ABI at the same time it switched to LLVM. This brings us to a parity with modern Linux distributions. This also makes the binaries from previous FreeBSD versions incompatible with 13.0-RELEASE. Kernel still supports ELFv1, so jails and chroots using older FreeBSD versions are still compatible. e4399d169acc

Well, it is also odd that this target claims ELFv2 support since our ABI code does not use ELFv2 on this target.

let abi = if cx.target_spec().env == "musl" {
ELFv2
} else if cx.target_spec().os == "aix" {
AIX
} else {
match cx.data_layout().endian {
Endian::Big => ELFv1,
Endian::Little => ELFv2,
}
};

Copy link
Member

@taiki-e taiki-e Oct 25, 2024

Choose a reason for hiding this comment

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

Filed #132150 for this.

@beetrees
Copy link
Contributor

It seems that Cargo distributed by rustup is causing segmentation fault after this change...

Pinging target maintainers from #129220: @asomers @MikaelUrankar

@asomers
Copy link
Contributor

asomers commented Oct 26, 2024

@beetrees I don't think this PR is responsible for that failure. See #132185 .

@taiki-e
Copy link
Member

taiki-e commented Oct 26, 2024

#132185

I've managed to bisect the problem. It was introduced by rust-lang/rust@6d88158 . I don't know why yet.

Hmm. AFAIK the segfault has been occurring consistently since nightly-2024-10-22 and that commit was merged in 2024-10-20 (#131772 (comment)), so it is odd that the problem is not occurring in nightly-2024-10-21, which supposedly contains that commit...

@asomers
Copy link
Contributor

asomers commented Oct 26, 2024

I don't know, @taiki-e . But I can reproduce the problem using a stage0 toolchain built at 6d88158 , but not with one built at 6d88158^ .

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. relnotes Marks issues that should be documented in the release notes of the next release. 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
Development

Successfully merging this pull request may close these issues.