Skip to content

Add a tidy check for GCC submodule version #137683

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

Merged
merged 2 commits into from
Apr 25, 2025

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Feb 26, 2025

To make sure that it stays in sync with the required GCC version of the GCC codegen backend.

The check should succeed on CI, although it will fail after #137660, until the next GCC sync.

r? @GuillaumeGomez

@rustbot rustbot added A-tidy Area: The tidy tool 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) labels Feb 26, 2025
@Kobzol Kobzol mentioned this pull request Feb 26, 2025
@GuillaumeGomez
Copy link
Member

This is great, thanks a lot! r=me once CI is happy.

@Kobzol
Copy link
Contributor Author

Kobzol commented Feb 26, 2025

This races with #137660 though, so maybe I'll wait until that one is merged and the next cg_gcc sync happens (because that PR desyncs the versions).

@GuillaumeGomez
Copy link
Member

Yeah, let's maybe wait for the other PR to be merged so then we can land this one and eventually update the SHA as needed.

@Kobzol Kobzol force-pushed the tidy-gcc-submodule branch from 7823503 to c62a5da Compare March 5, 2025 07:47
@rust-log-analyzer

This comment has been minimized.

@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 5, 2025

Yeah, the lint seems to be working. Now we need to wait for a GCC submodule (or cg_gcc) update.

@antoyo
Copy link
Contributor

antoyo commented Mar 5, 2025

Thanks.
That might take some time before I finish the next sync.

@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 5, 2025

Ok, let me know once that happens :) This lint is only useful during synces anyway, so it shouldn't be needed before the next one :)

Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

tACK. Some nits.

Comment on lines +9 to +12
let cg_gcc_version = std::fs::read_to_string(&cg_gcc_version_path)
.expect(&format!("Cannot read GCC version from {}", cg_gcc_version_path.display()))
.trim()
.to_string();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let cg_gcc_version = std::fs::read_to_string(&cg_gcc_version_path)
.expect(&format!("Cannot read GCC version from {}", cg_gcc_version_path.display()))
.trim()
.to_string();
let cg_gcc_version = std::fs::read_to_string(&cg_gcc_version_path)
.expect(&format!("Cannot read GCC version from {}", cg_gcc_version_path.display()));
let cg_gcc_version = cg_gcc_version.trim();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The to_string allocation doesn't matter here, so I would rather just keep the original.

Comment on lines +29 to +37
let git_output = String::from_utf8_lossy(&git_output.stdout)
.trim()
.split_whitespace()
.next()
.unwrap_or_default()
.to_string();

// The SHA can start with + if the submodule is modified or - if it is not checked out.
let gcc_submodule_sha = git_output.trim_start_matches(&['+', '-']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let git_output = String::from_utf8_lossy(&git_output.stdout)
.trim()
.split_whitespace()
.next()
.unwrap_or_default()
.to_string();
// The SHA can start with + if the submodule is modified or - if it is not checked out.
let gcc_submodule_sha = git_output.trim_start_matches(&['+', '-']);
let git_output = String::from_utf8_lossy(&git_output.stdout);
let gcc_submodule_sha = git_output
.trim()
.split_whitespace()
.next()
.and_then(|sha| sha.strip_prefix(['+','-']))
.unwrap_or_default();

@antoyo
Copy link
Contributor

antoyo commented Apr 24, 2025

Ok, let me know once that happens :) This lint is only useful during synces anyway, so it shouldn't be needed before the next one :)

A sync was done in this PR.

@Kobzol Kobzol force-pushed the tidy-gcc-submodule branch from c62a5da to 796a9ee Compare April 25, 2025 07:28
@rustbot
Copy link
Collaborator

rustbot commented Apr 25, 2025

⚠️ Warning ⚠️

  • Some commits in this PR modify submodules.

@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 25, 2025

I updated the GCC submodule, because it wasn't updated in the sync PR (luckily we will now have this tidy check to detect that 😆). Once CI is green, I'll approve on @GuillaumeGomez's behalf.

@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 25, 2025

@bors r=GuillaumeGomez

@bors
Copy link
Collaborator

bors commented Apr 25, 2025

📌 Commit 796a9ee has been approved by GuillaumeGomez

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 Apr 25, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 25, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#137683 (Add a tidy check for GCC submodule version)
 - rust-lang#138968 (Update the index of Result to make the summary more comprehensive)
 - rust-lang#139572 (docs(std): mention const blocks in const keyword doc page)
 - rust-lang#140152 (Unify the format of rustc cli flags)
 - rust-lang#140193 (fix ICE in `#[naked]` attribute validation)
 - rust-lang#140205 (Tidying up UI tests [2/N])
 - rust-lang#140284 (remove expect() in `unnecessary_transmutes`)
 - rust-lang#140290 (rustdoc: fix typo change from equivelent to equivalent)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4bbd21b into rust-lang:master Apr 25, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 25, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 25, 2025
Rollup merge of rust-lang#137683 - Kobzol:tidy-gcc-submodule, r=GuillaumeGomez

Add a tidy check for GCC submodule version

To make sure that it stays in sync with the required GCC version of the GCC codegen backend.

The check should succeed on CI, although it will fail after rust-lang#137660, until the next GCC sync.

r? `@GuillaumeGomez`
@Kobzol Kobzol deleted the tidy-gcc-submodule branch April 26, 2025 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tidy Area: The tidy tool 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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants