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

Run valgrind on a bot #13217

Closed
brson opened this issue Mar 30, 2014 · 8 comments
Closed

Run valgrind on a bot #13217

brson opened this issue Mar 30, 2014 · 8 comments
Assignees
@brson
Labels
P-medium Medium priority
Milestone
1.0

Comments

@brson
Copy link
Contributor

brson commented Mar 30, 2014

We are not running valgrind anywhere right now, even though the linux dist builder says it's enabling valgrind, valgrind is not installed on that machine.

The makefiles probably should not silently disable valgrind like they appear to be doing.
@alexcrichton
Copy link
Member

cc #8267
cc #8389

@alexcrichton alexcrichton mentioned this issue Jun 5, 2014
Closed
@brson
Copy link
Contributor Author

brson commented Aug 28, 2014

Nominating because I will be super embarrassed if we release 1.0 with memory leaks.

@brson brson added this to the 1.0 milestone Sep 4, 2014
@brson brson self-assigned this Sep 4, 2014
@brson brson added the P-high label Sep 4, 2014
@brson
Copy link
Contributor Author

brson commented Sep 4, 2014

P-high, 1.0

@brson
Copy link
Contributor Author

brson commented Sep 12, 2014

We are actually running valgrind on the snapshot bots: the basic problem here is that we have a broad suppression for most of memcheck because of #5856.

@alexcrichton
Copy link
Member

I actually just turned valgrind off on all the bots. I recently created some new snapshot bots, and those have a very recent valgrind installed, and they uncovered a huge number of errors, most of which I don't think are relevant. I uncovered a few errors here and there, but the volume was too high to deal with and we need a new snapshot, so valgrind is actually turned off now.

@brson brson mentioned this issue Sep 17, 2014
Closed
65 tasks
@bstrie
Copy link
Contributor

bstrie commented Dec 3, 2014

@alexcrichton given that this is a blocker for 1.0, I'd like to know what our plans are for resolving this.

@bstrie bstrie mentioned this issue Dec 3, 2014
Closed
@alexcrichton
Copy link
Member

With the addition of run-pass-valgrind tests I suspect that we may actually punt this from the 1.0 milestone in time as we have a place to put valgrind-critical tests without requiring the entire test suite passes under valgrind (which may be flaky). Which is to say we still lack a concrete plan to resolve this issue.

@alexcrichton
Copy link
Member

Thinking back on this, I"m going to close this as fixed as we can selectively add tests to run-pass-valgrind over time.

@steveklabnik steveklabnik mentioned this issue Jan 21, 2015
Closed
flip1995 pushed a commit to flip1995/rust that referenced this issue Aug 8, 2024
@bors
Auto merge of rust-lang#13217 - dtolnay-contrib:toolsutil, r=flip1995
e611c8e 
Check exit status of subcommands spawned by rustc_tools_util

The git commands `git rev-parse --short HEAD` and `git log -1 --date=short --pretty=format:%cd` that clippy runs from its build script might fail with **"fatal: not a git repository (or any of the parent directories): .git"** if clippy is being built from a source tarball rather than a git repository. That message is written by git to stderr, and nothing is written to stdout.

For `clippy-driver --version` this PR wouldn't make a difference because it treats empty stdout and failed spawns (`git` is not installed) identically:

https://github.com/rust-lang/rust-clippy/blob/7ac242c3d0d3ee867a6c9cdcbdec986c408ac36f/rustc_tools_util/src/lib.rs#L35-L42

But other users of `rustc_tools_util` should be able to expect that the distinction between Some and None is meaningful. They shouldn't need extra code to handle None vs Some-and-empty vs Some-and-nonempty.

---

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brson brson

Labels
P-medium Medium priority
Projects
None yet
Milestone
1.0
Development

No branches or pull requests
3 participants
@alexcrichton @brson @bstrie