check if it's rust-lang/rust CI job in `llvm::is_ci_llvm_modified` #130383

onur-ozkan · 2024-09-15T04:48:23Z

Changes llvm::is_ci_llvm_modified to only work on rust-lang/rust managed CI.

Signed-off-by: onur-ozkan <work@onurozkan.dev>

rustbot · 2024-09-15T04:48:32Z

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 · 2024-09-15T04:48:34Z

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

workingjubilee

thank you! I hope this fixes my workflow but we'll see~

onur-ozkan · 2024-09-15T06:01:08Z

src/bootstrap/src/core/build_steps/llvm.rs

-    CiEnv::is_ci() && config.rust_info.is_managed_git_subrepository() && {
+    CiEnv::is_ci() && config.in_tree_llvm_info.is_managed_git_subrepository() && {


An alternative solution for the backtrace CI issue would be to forcing CI to fetch the LLVM submodule at the beginning of this function.

This feels wrong to me - the checking here runs against the rust-lang/rust checkout, right, not the submodule? So whether or not it's available we should be able to determine if things are checked out...

At the very least it seems like this is worth a comment.

(I guess it's also not clear that is_managed_git_subrepository is ever going to be false in CI -- that seems like a strange condition).

(I guess it's also not clear that is_managed_git_subrepository is ever going to be false in CI -- that seems like a strange condition).

If you mean config.rust_info.is_managed_git_subrepository(), it can be false for tarball sources. But for config.in_tree_llvm_info.is_managed_git_subrepository(), it can be false when submodules aren't fetched and llvm.download-ci-llvm is true.

For us, nothing changes because we always fetch submodules in our CI. This was more to help external CIs (like backtrace one — you can check their problem here) since they might not fetch all submodules and use download-ci-llvm=true.

I tried to make this clear with some comments. Let me know if it needs more updates.

Hm, I wonder if we should modify CiEnv::is_ci to only treat rust-lang/rust as a true CI env -- or at least provide a way to opt-out. I'm not sure that the various tweaks we do for our CI make sense to be done elsewhere, and none of them should be needed to get a build working (e.g., in backtrace's case, it sounds like they probably just want to be a regular consumer as-if they're a human on a laptop?)

I would prefer to add an additional function to check if it's a rust-lang/rust-managed CI instead of modifying Ci::is_ci to keep is_ci logic the same for other downstream CI pipelines.

Mark-Simulacrum

I'm a little confused -- it seems like this should be a no-op outside of CI (and so presumably fairly uninteresting)? Maybe we need to change some non-CI function too?

src/bootstrap/src/lib.rs

Mark-Simulacrum · 2024-09-22T21:38:25Z

src/bootstrap/src/core/build_steps/llvm.rs

-    CiEnv::is_ci() && config.rust_info.is_managed_git_subrepository() && {
+    CiEnv::is_ci() && config.in_tree_llvm_info.is_managed_git_subrepository() && {


This feels wrong to me - the checking here runs against the rust-lang/rust checkout, right, not the submodule? So whether or not it's available we should be able to determine if things are checked out...

At the very least it seems like this is worth a comment.

(I guess it's also not clear that is_managed_git_subrepository is ever going to be false in CI -- that seems like a strange condition).

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2024-09-25T16:52:26Z

@rustbot ready

onur-ozkan · 2024-09-25T16:57:01Z

src/tools/build_helper/src/ci.rs

+            // If both are present, we can assume it's an upstream CI job
+            // as they are always set unconditionally.
+            && std::env::var_os("CI_JOB_NAME").is_some()
+            && std::env::var_os("TOOLSTATE_REPO").is_some()


I could use git config --get remote.origin.url instead of this if someone can confirm whether we run CI on any repository other than rust-lang/rust.

Mark-Simulacrum

r=me, ideally with PR title/description adjusted

src/tools/build_helper/src/ci.rs

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2024-09-29T10:35:21Z

@bors r=Mark-Simulacrum

bors · 2024-09-29T10:35:23Z

📌 Commit 5840d87 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

…llaumeGomez Rollup of 5 pull requests Successful merges: - rust-lang#130383 (check if it's rust-lang/rust CI job in `llvm::is_ci_llvm_modified`) - rust-lang#130416 (resolve rust-lang#130122: reword 'sort-by' edge-conditions documentation) - rust-lang#130537 (rustdoc: add doc comment to DocVisitor) - rust-lang#130743 (Clarifications for set_nonblocking methods) - rust-lang#131010 (extend comment in global_llvm_features regarding target-cpu=native) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#130383 - onur-ozkan:ignore-llvm-changes-on-ci-llvm-true, r=Mark-Simulacrum check if it's rust-lang/rust CI job in `llvm::is_ci_llvm_modified` Changes `llvm::is_ci_llvm_modified` to only work on rust-lang/rust managed CI.

Feed GitInfos from Config to Build

90745bd

Signed-off-by: onur-ozkan <work@onurozkan.dev>

rustbot assigned Mark-Simulacrum Sep 15, 2024

rustbot added 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 Sep 15, 2024

workingjubilee approved these changes Sep 15, 2024

View reviewed changes

onur-ozkan commented Sep 15, 2024

View reviewed changes

Mark-Simulacrum reviewed Sep 22, 2024

View reviewed changes

onur-ozkan force-pushed the ignore-llvm-changes-on-ci-llvm-true branch 3 times, most recently from f076532 to 08d1450 Compare September 23, 2024 04:40

check if the LLVM submodule is fetched in is_ci_llvm_modified

6658c8e

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the ignore-llvm-changes-on-ci-llvm-true branch from 08d1450 to 6658c8e Compare September 23, 2024 04:42

onur-ozkan 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 Sep 25, 2024

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 Sep 25, 2024

onur-ozkan commented Sep 25, 2024

View reviewed changes

Mark-Simulacrum reviewed Sep 28, 2024

View reviewed changes

src/tools/build_helper/src/ci.rs Outdated Show resolved Hide resolved

onur-ozkan force-pushed the ignore-llvm-changes-on-ci-llvm-true branch from 99b95b5 to f4d3209 Compare September 29, 2024 04:10

onur-ozkan added 2 commits September 29, 2024 07:19

create CiEnv::is_rust_lang_managed_ci_job

2d85139

Signed-off-by: onur-ozkan <work@onurozkan.dev>

in llvm::is_ci_llvm_modified, check if it's rust-lang/rust CI job

5840d87

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the ignore-llvm-changes-on-ci-llvm-true branch from f4d3209 to 5840d87 Compare September 29, 2024 04:20

onur-ozkan changed the title ~~ignore LLVM sha if submodule isn't present~~ check if it's rust-lang/rust CI job in llvm::is_ci_llvm_modified Sep 29, 2024

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 Sep 29, 2024

GuillaumeGomez mentioned this pull request Sep 29, 2024

Rollup of 5 pull requests #131012

Merged

bors merged commit f39101a into rust-lang:master Sep 29, 2024
6 checks passed

rustbot added this to the 1.83.0 milestone Sep 29, 2024

onur-ozkan deleted the ignore-llvm-changes-on-ci-llvm-true branch September 29, 2024 19:28

onur-ozkan mentioned this pull request Oct 5, 2024

download_ci_llvm test failure when updating LLVM #131303

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

check if it's rust-lang/rust CI job in `llvm::is_ci_llvm_modified` #130383

check if it's rust-lang/rust CI job in `llvm::is_ci_llvm_modified` #130383

onur-ozkan commented Sep 15, 2024 •

edited

Loading

rustbot commented Sep 15, 2024

rustbot commented Sep 15, 2024

workingjubilee left a comment

onur-ozkan Sep 15, 2024

Mark-Simulacrum Sep 22, 2024

onur-ozkan Sep 23, 2024

Mark-Simulacrum Sep 25, 2024

onur-ozkan Sep 25, 2024

Mark-Simulacrum left a comment

Mark-Simulacrum Sep 22, 2024

onur-ozkan commented Sep 25, 2024

onur-ozkan Sep 25, 2024

Mark-Simulacrum left a comment

onur-ozkan commented Sep 29, 2024

bors commented Sep 29, 2024

		CiEnv::is_ci() && config.rust_info.is_managed_git_subrepository() && {
		CiEnv::is_ci() && config.in_tree_llvm_info.is_managed_git_subrepository() && {

check if it's rust-lang/rust CI job in llvm::is_ci_llvm_modified #130383

check if it's rust-lang/rust CI job in llvm::is_ci_llvm_modified #130383

Conversation

onur-ozkan commented Sep 15, 2024 • edited Loading

rustbot commented Sep 15, 2024

rustbot commented Sep 15, 2024

workingjubilee left a comment

Choose a reason for hiding this comment

onur-ozkan Sep 15, 2024

Choose a reason for hiding this comment

Mark-Simulacrum Sep 22, 2024

Choose a reason for hiding this comment

onur-ozkan Sep 23, 2024

Choose a reason for hiding this comment

Mark-Simulacrum Sep 25, 2024

Choose a reason for hiding this comment

onur-ozkan Sep 25, 2024

Choose a reason for hiding this comment

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

Mark-Simulacrum Sep 22, 2024

Choose a reason for hiding this comment

onur-ozkan commented Sep 25, 2024

onur-ozkan Sep 25, 2024

Choose a reason for hiding this comment

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

onur-ozkan commented Sep 29, 2024

bors commented Sep 29, 2024

check if it's rust-lang/rust CI job in `llvm::is_ci_llvm_modified` #130383

check if it's rust-lang/rust CI job in `llvm::is_ci_llvm_modified` #130383

onur-ozkan commented Sep 15, 2024 •

edited

Loading