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

Check all files in src/test for borrowck_graphviz_postflow #65630

Merged
merged 2 commits into from
Oct 21, 2019

Conversation

ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Oct 20, 2019

This attribute causes DOT files to be generated in the top-level directory. It is intended to be used only temporarily and should never appear on master. This also tells git to ignore DOT files in the root or the mir_dump directory, which -Z dump-mir uses by default.

This will prevent #65071 from occurring again. It needs to be merged after #65629, otherwise tidy will start failing.

r? @Mark-Simulacrum

This attribute causes DOT files to be generated in the top-level
directory. It is intended to be used only temporarily and should never
appear on master.

This will prevent rust-lang#65071 from occurring again.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 20, 2019
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-10-20T18:06:04.0858664Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-10-20T18:06:04.1020033Z ##[command]git config gc.auto 0
2019-10-20T18:06:04.1099826Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-10-20T18:06:04.1148869Z ##[command]git config --get-all http.proxy
2019-10-20T18:06:04.1305096Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/65630/merge:refs/remotes/pull/65630/merge
---
2019-10-20T18:12:28.5049834Z    Compiling serde_json v1.0.40
2019-10-20T18:12:30.3351734Z    Compiling tidy v0.1.0 (/checkout/src/tools/tidy)
2019-10-20T18:12:41.7830511Z     Finished release [optimized] target(s) in 1m 29s
2019-10-20T18:12:41.7913749Z tidy check
2019-10-20T18:12:42.8833897Z tidy error: /checkout/src/test/ui/consts/const-eval/issue-65394.rs:5: `borrowck_graphviz_postflow` attribute in test
2019-10-20T18:12:44.2095976Z some tidy checks failed
2019-10-20T18:12:44.2099861Z Found 482 error codes
2019-10-20T18:12:44.2100326Z Found 0 error codes with no tests
2019-10-20T18:12:44.2101008Z Done!
2019-10-20T18:12:44.2101008Z Done!
2019-10-20T18:12:44.2101279Z 
2019-10-20T18:12:44.2101483Z 
2019-10-20T18:12:44.2102722Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor"
2019-10-20T18:12:44.2105033Z 
2019-10-20T18:12:44.2105287Z 
2019-10-20T18:12:44.2105567Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2019-10-20T18:12:44.2105761Z Build completed unsuccessfully in 0:01:32
2019-10-20T18:12:44.2105761Z Build completed unsuccessfully in 0:01:32
2019-10-20T18:12:44.2157023Z == clock drift check ==
2019-10-20T18:12:44.2172212Z   local time: Sun Oct 20 18:12:44 UTC 2019
2019-10-20T18:12:44.3816839Z   network time: Sun, 20 Oct 2019 18:12:44 GMT
2019-10-20T18:12:44.3817905Z == end clock drift check ==
2019-10-20T18:12:45.7020683Z 
2019-10-20T18:12:45.7129000Z ##[error]Bash exited with code '1'.
2019-10-20T18:12:45.7162568Z ##[section]Starting: Checkout
2019-10-20T18:12:45.7164165Z ==============================================================================
2019-10-20T18:12:45.7164210Z Task         : Get sources
2019-10-20T18:12:45.7164248Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

r=me when the other PR is merged

@ecstatic-morse

This comment has been minimized.

@bors

This comment has been minimized.

@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 21, 2019
@bors

This comment has been minimized.

@ecstatic-morse

This comment has been minimized.

@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 Oct 21, 2019
@ecstatic-morse
Copy link
Contributor Author

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Oct 21, 2019

📌 Commit efcae57 has been approved by Mark-Simulacrum

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 21, 2019
@bors
Copy link
Contributor

bors commented Oct 21, 2019

⌛ Testing commit efcae57 with merge 1ba7b4e...

bors added a commit that referenced this pull request Oct 21, 2019
Check all files in `src/test` for `borrowck_graphviz_postflow`

This attribute causes DOT files to be generated in the top-level directory. It is intended to be used only temporarily and should never appear on master. This also tells git to ignore DOT files in the root or the  `mir_dump` directory, which `-Z dump-mir` uses by default.

This will prevent #65071 from occurring again. It needs to be merged after #65629, otherwise `tidy` will start failing.

r? @Mark-Simulacrum
@bors
Copy link
Contributor

bors commented Oct 21, 2019

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing 1ba7b4e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 21, 2019
@bors bors merged commit efcae57 into rust-lang:master Oct 21, 2019
Comment on lines +55 to +57
# Generated when dumping Graphviz output for debugging:
/mir_dump/
/*.dot
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be reverted. In fact I just removed these not too long ago.^^

I do sometimes create mir_dump folders during debugging, and those are junk that I want to clean up. They should not be automatically ignored by git. If you are okay with junk in your working dir, that's okay -- git has .git/info/exclude for you to locally configure stuff you want ignored. But please don't interfere with other people who want to keep their working dir clean.

Copy link
Member

Choose a reason for hiding this comment

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

In particular see the comment at the top of this file:

# This file should only ignore things that are generated during a build,
# generated by common IDEs, and optional files controlled by the user
# that affect the build (such as config.toml).

@RalfJung
Copy link
Member

RalfJung commented Oct 21, 2019

I recently saw some *.dot files in my rustc root dir after running x.py test... is that bug fixed by this PR? Just running tests regularly should not leave any junk outside of the build dir.

EDIT: Looks like it does, thanks! Also I think #65071 shows that we do not want these gitignore changes; they just make it harder to notice when a test misbehaves and leaves files behind where it shouldn't.

Centril added a commit to Centril/rust that referenced this pull request Oct 21, 2019
…um,Centril

keep the root dir clean from debugging

We landed this before with rust-lang#63307 but recently in rust-lang#65630 the IMO bad ignore crept back in.

If you regularly do graphviz-based debugging and you are fine leaving junk in the rustc root dir, please configure your local `.git/info/exclude`. But most people working on rustc don't work with graphciz all that often (I for once never did), and not everyone likes to have stray generated files in their source dirs.

Also Cc rust-lang#63373 rust-lang#53768 @ecstatic-morse @Mark-Simulacrum
Centril added a commit to Centril/rust that referenced this pull request Oct 21, 2019
…um,Centril

keep the root dir clean from debugging

We landed this before with rust-lang#63307 but recently in rust-lang#65630 the IMO bad ignore crept back in.

If you regularly do graphviz-based debugging and you are fine leaving junk in the rustc root dir, please configure your local `.git/info/exclude`. But most people working on rustc don't work with graphciz all that often (I for once never did), and not everyone likes to have stray generated files in their source dirs.

Also Cc rust-lang#63373 rust-lang#53768 @ecstatic-morse @Mark-Simulacrum
@ecstatic-morse ecstatic-morse deleted the graphviz-tidy branch October 6, 2020 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants