Skip to content

configure: Don't succeed if valgrind isn't available. Fixes #18588#24477

Closed
brson wants to merge 2 commits into
rust-lang:masterfrom
brson:valgrindconfig
Closed

configure: Don't succeed if valgrind isn't available. Fixes #18588#24477
brson wants to merge 2 commits into
rust-lang:masterfrom
brson:valgrindconfig

Conversation

@brson

@brson brson commented Apr 15, 2015

Copy link
Copy Markdown
Contributor

Based on @emanueLczirai's patch.

r? @nrc

Comment thread configure Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/it's/its/

@nrc

nrc commented Apr 15, 2015

Copy link
Copy Markdown
Member

r+, I guess with the typo fixed

@brson

brson commented Apr 22, 2015

Copy link
Copy Markdown
Contributor Author

@bors r=nrc rollup

@bors

bors commented Apr 22, 2015

Copy link
Copy Markdown
Collaborator

📌 Commit 31856a5 has been approved by nrc

@Manishearth

Copy link
Copy Markdown
Member

Can valgrind be disabled by default in the configure? IMO ./configure should generally just work on common systems, and valgrind is usually not there out of the box.

@bors

bors commented Apr 23, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 31856a5 with merge e75d39d...

@bors

bors commented Apr 23, 2015

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-win-64-nopt-t

@Manishearth

Copy link
Copy Markdown
Member
configure: error: valgrind not found, but needed. consider adding --disable-valgrind-rpass
program finished with exit code 1
elapsedTime=2.558000

@ghost

ghost commented Apr 24, 2015

Copy link
Copy Markdown

One thing to note is that --disable-valgrind doesn't imply --disable-valgrind-rpass as far as I remember. It seemed to me, it was intended this way from the beginning.
The relevant "discussion" is here: #18588 (comment)

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Apr 24, 2015
@steveklabnik

Copy link
Copy Markdown
Contributor

@bors: r-

@steveklabnik

Copy link
Copy Markdown
Contributor

@brson: now that #24859 is merged, is this PR needed?

@brson

brson commented Jun 12, 2015

Copy link
Copy Markdown
Contributor Author

I think it is not.

@brson brson closed this Jun 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants