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.

Sign up for GitHub

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

Jump to bottom

Set RUST_SAVE_ANALYSIS_CONFIG env variable for run-pass tests #106789

Closed
wants to merge 1 commit into from
Closed

Set RUST_SAVE_ANALYSIS_CONFIG env variable for run-pass tests #106789

wants to merge 1 commit into from

Conversation

azadnn
Copy link

@azadnn azadnn commented Jan 12, 2023 •
edited by Mark-Simulacrum
Loading

This fixes #96928 by setting the RUST_SAVE_ANALYSIS_CONFIG environment variable for run-pass tests in order to change the location where the result of the analysis is saved.

@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

@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. labels Jan 12, 2023
@azadnn
Copy link
Author

azadnn commented Jan 12, 2023

r? @jyn514

@rustbot rustbot assigned jyn514 and unassigned Mark-Simulacrum Jan 12, 2023
@jyn514
Copy link
Member

jyn514 commented Jan 12, 2023

I don't have time for reviews for the next few months, sorry.

@jyn514 jyn514 assigned Mark-Simulacrum and unassigned jyn514 Jan 12, 2023
@Noratrieb
Copy link
Member

I'm not sure how useful this is, given that save-analysis will be deleted in about a month, sorry.

@azadnn
Copy link
Author

azadnn commented Jan 13, 2023

I'm not sure how useful this is, given that save-analysis will be deleted in about a month, sorry.

Newbie here so not quite sure I understand the comment right. If you mean save-analysis folder gets deleted after a month, then that's fine. #96928 is about an untracked save-analysis-temp in the root folder which does not get deleted so this PR aims to redirect the saved analysis for run-pass tests to save-analysis folder.
But if you mean rustc-save-analysis is to be deprecated in about a month's time, then this PR is unnecessary.

@Noratrieb
Copy link
Member

Yes, the latter, the entire feature will be removed. See #101841

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Mark-Simulacrum
Copy link
Member

I'm not opposed to merging this (even a month of a fix is still plausibly good) but it looks like CI is failing, so that'll need to get fixed first.

@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 Jan 14, 2023
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 15, 2023
@azadnn
Copy link
Author

azadnn commented Jan 15, 2023

@rustbot label -S-waiting-on-author +S-waiting-on-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 Jan 15, 2023
@Mark-Simulacrum
Copy link
Member

Please also squash commits. Thanks!

@azadnn azadnn force-pushed the save-analysis-for-runpass-tests branch 2 times, most recently from 86c70b7 to d8b033c Compare January 21, 2023 17:58
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 29, 2023

📌 Commit d8b033c 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 Jan 29, 2023
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jan 29, 2023
@compiler-errors
Rollup merge of rust-lang#106789 - azadnn:save-analysis-for-runpass-t…
b2032e4 
…ests, r=Mark-Simulacrum

Set RUST_SAVE_ANALYSIS_CONFIG env variable for run-pass tests

This fixes rust-lang#96928 by setting the RUST_SAVE_ANALYSIS_CONFIG environment variable for run-pass tests in order to change the location where the result of the analysis is saved.
@compiler-errors
Copy link
Member

@bors r- failed in CI: #107456 (comment)

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 30, 2023
@rustbot rustbot added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Jan 30, 2023
@azadnn azadnn force-pushed the save-analysis-for-runpass-tests branch from c49ffda to 0b9209d Compare January 30, 2023 14:32
@azadnn
Copy link
Author

azadnn commented Jan 30, 2023

@rustbot label -S-waiting-on-author +S-waiting-on-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 Jan 30, 2023
@azadnn
Copy link
Author

azadnn commented Feb 6, 2023

PR failed in CI. This is likely due to the use of \ in Windows OS paths. The resolution of that is to replace \ in the path with \\ as done above. This fix was tested on a windows build at https://github.com/rust-lang/rust/actions/runs/4044234468/jobs/6954129844

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Feb 6, 2023

📌 Commit 0b9209d 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 Feb 6, 2023
@bors
Copy link
Contributor

bors commented Feb 6, 2023

⌛ Testing commit 0b9209d with merge 52dd90118147576a653a46c872099a9a59ff00b5...

@bors
Copy link
Contributor

bors commented Feb 6, 2023

💔 Test failed - checks-actions

@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 Feb 6, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@azadnn
Copy link
Author

azadnn commented Feb 7, 2023

I have no idea what is wrong. Is it an issue with LLVM or an issue with the PR commit

@azadnn azadnn force-pushed the save-analysis-for-runpass-tests branch from 761f243 to b20a57b Compare February 12, 2023 10:32
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@azadnn azadnn force-pushed the save-analysis-for-runpass-tests branch from 54d65b1 to fd5c506 Compare February 12, 2023 17:30
@rust-log-analyzer
Copy link
Collaborator

The job dist-x86_64-mingw failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
Updating files:  99% (39357/39754)
Updating files:  99% (39573/39754)
Updating files: 100% (39754/39754)
Updating files: 100% (39754/39754), done.
Note: switching to 'refs/remotes/pull/106789/merge'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

---
  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at b427793b Merge fd5c506764ea1a6eb3bebd61b55787069679a519 into 00cf19a75a7055171a4ffc8cc557ff63953c9754
[command]"C:\Program Files\Git\bin\git.exe" log -1 --format='%H'
'b427793bca4d0230538a3a56da2d7636c981fc6e'
'b427793bca4d0230538a3a56da2d7636c981fc6e'
##[group]Run echo "[CI_PR_NUMBER=$num]"
echo "[CI_PR_NUMBER=$num]"
env:
  CI_JOB_NAME: dist-x86_64-mingw
  SCCACHE_BUCKET: rust-lang-ci-sccache2
  TOOLSTATE_REPO: https://github.com/rust-lang-nursery/rust-toolstate

@Mark-Simulacrum
Copy link
Member

#101841 should merge this week at this point, so I'm guessing trying to push this through is probably not worth the hassle given how much trouble we've had with it.

I'm going to go ahead and close but if you disagree I'm up for reopening.

@erikdesjardins erikdesjardins mentioned this pull request Feb 19, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mark-Simulacrum Mark-Simulacrum Awaiting requested review from Mark-Simulacrum

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Compiletest should set RUST_SAVE_ANALYSIS_CONFIG for run-pass tests
9 participants
@azadnn @rustbot @jyn514 @Noratrieb @rust-log-analyzer @Mark-Simulacrum @bors @compiler-errors @AmSdGo