This repository has been archived by the owner. It is now read-only.

valgrind: patch for xcode-only #13548

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
8 participants
@samueljohn
Contributor

samueljohn commented Jul 23, 2012

Patch needed to build on Xcode-only systems. The patch is not applied if the CLT are installed.

Fixes #13042.

UPDATE: Valgrind does not (yet) support Mountain Lion 10.8.

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Jul 23, 2012

Contributor

I am not a valgrind expert, I have to admit, but I can successfully build valgrind now.

Contributor

samueljohn commented Jul 23, 2012

I am not a valgrind expert, I have to admit, but I can successfully build valgrind now.

@adamv

This comment has been minimized.

Show comment Hide comment
@adamv

adamv Jul 23, 2012

Contributor

What is the source of the patch? Has it been reported upstream? The patch will need some documentation.

Contributor

adamv commented Jul 23, 2012

What is the source of the patch? Has it been reported upstream? The patch will need some documentation.

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Jul 23, 2012

Contributor

@adamv I reported this in February to the valgrind tracker. No response so far. I even attached the git diff. The core devs don't seem to have access to Macs.

The patch is done by me. It's really simple in the sense that I have looked at their Makefiles and fixed the hard coded pathes. The patch is only applied on xcode-only systems.

Given that the valgrind devs have not responded yet, I thought we might want to patch this for homebrew until they care about.

Contributor

samueljohn commented Jul 23, 2012

@adamv I reported this in February to the valgrind tracker. No response so far. I even attached the git diff. The core devs don't seem to have access to Macs.

The patch is done by me. It's really simple in the sense that I have looked at their Makefiles and fixed the hard coded pathes. The patch is only applied on xcode-only systems.

Given that the valgrind devs have not responded yet, I thought we might want to patch this for homebrew until they care about.

@2bits

This comment has been minimized.

Show comment Hide comment
@2bits

2bits Jul 23, 2012

Contributor

These:

  ENV.remove_from_cflags '-mmacosx-version-min=10.7'
  ENV.remove 'CXXFLAGS', '-mmacosx-version-min=10.7'

Should probably by

  ENV.remove 'CFLAGS', " -mmacosx-version-min=#{MACOS_VERSION}"
  ENV.remove 'CXXFLAGS', " -mmacosx-version-min=#{MACOS_VERSION}"

So that we correctly support 10.6 and 10.8 also.

Contributor

2bits commented Jul 23, 2012

These:

  ENV.remove_from_cflags '-mmacosx-version-min=10.7'
  ENV.remove 'CXXFLAGS', '-mmacosx-version-min=10.7'

Should probably by

  ENV.remove 'CFLAGS', " -mmacosx-version-min=#{MACOS_VERSION}"
  ENV.remove 'CXXFLAGS', " -mmacosx-version-min=#{MACOS_VERSION}"

So that we correctly support 10.6 and 10.8 also.

@adamv

This comment has been minimized.

Show comment Hide comment
@adamv

adamv Jul 23, 2012

Contributor

Note that links to upstream bugs ought to appear in comments in the source, since this serves as documentation.

Contributor

adamv commented Jul 23, 2012

Note that links to upstream bugs ought to appear in comments in the source, since this serves as documentation.

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Jul 23, 2012

Contributor

@2bits good catch!
@adamv sorry, I forgot that.

Contributor

samueljohn commented Jul 23, 2012

@2bits good catch!
@adamv sorry, I forgot that.

@axpence

This comment has been minimized.

Show comment Hide comment
@axpence

axpence Jul 24, 2012

==> Build Environment
HOMEBREW_VERSION: 0.9.2
HEAD: (none)
CPU: dual-core 64-bit penryn
OS X: 10.7.3-x86_64
Xcode: 4.3.3
CLT: 4.3.0.0.1.1249367152
X11: 2.6.3 @ /usr/X11
CC: /usr/bin/clang
CXX: /usr/bin/clang++ => /usr/bin/clang
LD: /usr/bin/clang
CFLAGS: -Os -w -pipe -march=native -Qunused-arguments -mmacosx-version-min=10.7
CXXFLAGS: -Os -w -pipe -march=native -Qunused-arguments -mmacosx-version-min=10.7
CPPFLAGS: -isystem /usr/local/include
LDFLAGS: -L/usr/local/lib
MACOSX_DEPLOYMENT_TARGET: 10.7
MAKEFLAGS: -j2
Error: Failed executing: make (valgrind.rb:26)
These existing issues may help you:
mxcl#13548
Otherwise, this may help you fix or report the issue:
https://github.com/mxcl/homebrew/wiki/bug-fixing-checklist
Az-macbook:lab6 joshuadager$

axpence commented Jul 24, 2012

==> Build Environment
HOMEBREW_VERSION: 0.9.2
HEAD: (none)
CPU: dual-core 64-bit penryn
OS X: 10.7.3-x86_64
Xcode: 4.3.3
CLT: 4.3.0.0.1.1249367152
X11: 2.6.3 @ /usr/X11
CC: /usr/bin/clang
CXX: /usr/bin/clang++ => /usr/bin/clang
LD: /usr/bin/clang
CFLAGS: -Os -w -pipe -march=native -Qunused-arguments -mmacosx-version-min=10.7
CXXFLAGS: -Os -w -pipe -march=native -Qunused-arguments -mmacosx-version-min=10.7
CPPFLAGS: -isystem /usr/local/include
LDFLAGS: -L/usr/local/lib
MACOSX_DEPLOYMENT_TARGET: 10.7
MAKEFLAGS: -j2
Error: Failed executing: make (valgrind.rb:26)
These existing issues may help you:
mxcl#13548
Otherwise, this may help you fix or report the issue:
https://github.com/mxcl/homebrew/wiki/bug-fixing-checklist
Az-macbook:lab6 joshuadager$

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Jul 24, 2012

Contributor

@axpence ???
Please read the bug-fixing-checklist first. Did you use the code suggested here in this pull request?

Contributor

samueljohn commented Jul 24, 2012

@axpence ???
Please read the bug-fixing-checklist first. Did you use the code suggested here in this pull request?

@axpence

This comment has been minimized.

Show comment Hide comment
@axpence

axpence Jul 24, 2012

I read through it, sorry, I just saw the line that said "please don't paste from the console log.."
My apologies.
I'm not sure I know how to use the code suggested here for this pull request.

axpence commented Jul 24, 2012

I read through it, sorry, I just saw the line that said "please don't paste from the console log.."
My apologies.
I'm not sure I know how to use the code suggested here for this pull request.

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Jul 24, 2012

Contributor

@adamv I added the source comment about the upstream ticket.

@axpence Ok, no problem. I see that the debug output did not list all the currently open valgrind issues, so it led you here. Please have a look at the issue #13042 and post a link to a gist of your output of brew install valgrind -v.
Even if you have installed the Command Line Tools, this pull request here might help you as discussed in that issue.

If you want to try out my commit here (which I'd appreciate, since it's always good to get feedback, capture mistakes early on, or get confirmation that it works), please first install git by brew install git and assure your homebrew is up to date. Then, go to cd $(brew --prefix) and start a new branch in order to keep your main homebrew clean: brew checkout -b valgrind_13548 (the name is just arbitrary). Now get this pull request by brew pull 13548. I hope at this point brew install valgrind -v works. If not report it. If it works, report here, too :-)

To switch back to your main (clean) homebrew do git checkout master in your brew --prefix directory.

Contributor

samueljohn commented Jul 24, 2012

@adamv I added the source comment about the upstream ticket.

@axpence Ok, no problem. I see that the debug output did not list all the currently open valgrind issues, so it led you here. Please have a look at the issue #13042 and post a link to a gist of your output of brew install valgrind -v.
Even if you have installed the Command Line Tools, this pull request here might help you as discussed in that issue.

If you want to try out my commit here (which I'd appreciate, since it's always good to get feedback, capture mistakes early on, or get confirmation that it works), please first install git by brew install git and assure your homebrew is up to date. Then, go to cd $(brew --prefix) and start a new branch in order to keep your main homebrew clean: brew checkout -b valgrind_13548 (the name is just arbitrary). Now get this pull request by brew pull 13548. I hope at this point brew install valgrind -v works. If not report it. If it works, report here, too :-)

To switch back to your main (clean) homebrew do git checkout master in your brew --prefix directory.

@axpence

This comment has been minimized.

Show comment Hide comment
@axpence

axpence Jul 24, 2012

-v failed without your commit, here is my gist:
git://gist.github.com/3170994.git

I tried doing brew install git, but I got a similar error:
Error: Failed executing: make prefix=/usr/local/Cellar/git/1.7.11.3 CC=/usr/bin/clang CFLAGS=-Os\ -w\ -pipe\ -march=native\ -Qunused-arguments\ -mmacosx-version-min=10.7 LDFLAGS=-L/usr/local/lib install (git.rb:49)

Also I tried to checkout your file by doing brew checkout -b valgrind_13548 and i got the following error:
Az-macbook:lab6 joshuadager$ brew checkout -b valgrind_13548
/usr/local/Library/Homebrew/global.rb:65: warning: Insecure world writable dir /usr/local/bin in PATH, mode 040777
/usr/local/Library/Homebrew/global.rb:95: warning: Insecure world writable dir /usr/local/bin in PATH, mode 040777
Error: Unknown command: checkout

I just learned what homebrew is a few days ago. Guess I'm a n00b. :)
thanks for the help

axpence commented Jul 24, 2012

-v failed without your commit, here is my gist:
git://gist.github.com/3170994.git

I tried doing brew install git, but I got a similar error:
Error: Failed executing: make prefix=/usr/local/Cellar/git/1.7.11.3 CC=/usr/bin/clang CFLAGS=-Os\ -w\ -pipe\ -march=native\ -Qunused-arguments\ -mmacosx-version-min=10.7 LDFLAGS=-L/usr/local/lib install (git.rb:49)

Also I tried to checkout your file by doing brew checkout -b valgrind_13548 and i got the following error:
Az-macbook:lab6 joshuadager$ brew checkout -b valgrind_13548
/usr/local/Library/Homebrew/global.rb:65: warning: Insecure world writable dir /usr/local/bin in PATH, mode 040777
/usr/local/Library/Homebrew/global.rb:95: warning: Insecure world writable dir /usr/local/bin in PATH, mode 040777
Error: Unknown command: checkout

I just learned what homebrew is a few days ago. Guess I'm a n00b. :)
thanks for the help

@2bits

This comment has been minimized.

Show comment Hide comment
@2bits

2bits Jul 24, 2012

Contributor

He's probably asleep by now. He meant git checkout -b valgrindTest or some other name. brew doesn't have that command.

The fact that you get a failure installing git is sign there are some global problems. We'll try to help you fix those. You should read the Wiki Bug Fixing Checklist when you get a chance. Open a new issue for your git install and post what the wiki asks for. Thanks.

Contributor

2bits commented Jul 24, 2012

He's probably asleep by now. He meant git checkout -b valgrindTest or some other name. brew doesn't have that command.

The fact that you get a failure installing git is sign there are some global problems. We'll try to help you fix those. You should read the Wiki Bug Fixing Checklist when you get a chance. Open a new issue for your git install and post what the wiki asks for. Thanks.

@sansuiso

This comment has been minimized.

Show comment Hide comment
@sansuiso

sansuiso Jul 25, 2012

Hello,
I tried the 13548 pull request, as described above. Valgrind now builds fine for me (10.7.4, latest stable Xcode).
Thanks for the patch !

Hello,
I tried the 13548 pull request, as described above. Valgrind now builds fine for me (10.7.4, latest stable Xcode).
Thanks for the patch !

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Jul 25, 2012

Contributor

@axpence could you gist us your output of brew doctor and the other things described in the wiki (link in a previous post above). @2bits is right, I mixed git and brew. I always do.

Contributor

samueljohn commented Jul 25, 2012

@axpence could you gist us your output of brew doctor and the other things described in the wiki (link in a previous post above). @2bits is right, I mixed git and brew. I always do.

@axpence

This comment has been minimized.

Show comment Hide comment
@axpence

axpence Jul 26, 2012

brew doctor gist:

https://gist.github.com/3180639

still lost :( tear

thanks for all your help friends.

On Wed, Jul 25, 2012 at 11:51 AM, Samuel John <
reply@reply.github.com

wrote:

@axpence could you gist us your output of brew doctor and the other
things described in the wiki (link in a previous post above). @2bits is
right, I mixed git and brew. I always do.


Reply to this email directly or view it on GitHub:
mxcl#13548 (comment)

axpence commented Jul 26, 2012

brew doctor gist:

https://gist.github.com/3180639

still lost :( tear

thanks for all your help friends.

On Wed, Jul 25, 2012 at 11:51 AM, Samuel John <
reply@reply.github.com

wrote:

@axpence could you gist us your output of brew doctor and the other
things described in the wiki (link in a previous post above). @2bits is
right, I mixed git and brew. I always do.


Reply to this email directly or view it on GitHub:
mxcl#13548 (comment)

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Jul 26, 2012

Contributor

@axpence you should definitely try to move those offending (unbrewed) libs away (copy them). Software may easily pick up these during linking. Do you have a clue which software put them there? Perhaps you have installed some standalone compiler or whatever?

Contributor

samueljohn commented Jul 26, 2012

@axpence you should definitely try to move those offending (unbrewed) libs away (copy them). Software may easily pick up these during linking. Do you have a clue which software put them there? Perhaps you have installed some standalone compiler or whatever?

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Jul 26, 2012

Contributor

As if all this trouble would not be enough, I found that valgrind does not want to be installed on 10.8.
#13609

Contributor

samueljohn commented Jul 26, 2012

As if all this trouble would not be enough, I found that valgrind does not want to be installed on 10.8.
#13609

@axpence

This comment has been minimized.

Show comment Hide comment
@axpence

axpence Jul 26, 2012

Still no progress for me. Best of luck

On Thu, Jul 26, 2012 at 2:04 AM, Samuel John <
reply@reply.github.com

wrote:

As if all this trouble would not be enough, I found that valgrind does not
want to be installed on 10.8.
#13609


Reply to this email directly or view it on GitHub:
mxcl#13548 (comment)

axpence commented Jul 26, 2012

Still no progress for me. Best of luck

On Thu, Jul 26, 2012 at 2:04 AM, Samuel John <
reply@reply.github.com

wrote:

As if all this trouble would not be enough, I found that valgrind does not
want to be installed on 10.8.
#13609


Reply to this email directly or view it on GitHub:
mxcl#13548 (comment)

@mkscrg

This comment has been minimized.

Show comment Hide comment
@mkscrg

mkscrg Jul 27, 2012

This patch works for me, on 10.7 with XCode 4.4 (4F250).

mkscrg commented Jul 27, 2012

This patch works for me, on 10.7 with XCode 4.4 (4F250).

@chrisdevereux

This comment has been minimized.

Show comment Hide comment
@chrisdevereux

chrisdevereux Aug 2, 2012

Patch fixes the issues for me too (10.7 / XCode 4.4)

Patch fixes the issues for me too (10.7 / XCode 4.4)

@jacknagel

View changes

Library/Formula/valgrind.rb
+ def patches
+ # For Xcode-only systems, we have to patch hard-coded paths, use xcrun and
+ # add missing CFLAGS. See: https://bugs.kde.org/show_bug.cgi?id=295084
+ DATA unless MacOS.clt_installed?

This comment has been minimized.

Show comment Hide comment
@jacknagel

jacknagel Aug 3, 2012

Contributor

This is now MacOS::CLT.installed?

@jacknagel

jacknagel Aug 3, 2012

Contributor

This is now MacOS::CLT.installed?

@jacknagel

View changes

Library/Formula/valgrind.rb
def install
+ ENV.remove_from_cflags "-mmacosx-version-min=#{MACOS_VERSION}"
+ ENV.remove 'CXXFLAGS', "-mmacosx-version-min=#{MACOS_VERSION}"

This comment has been minimized.

Show comment Hide comment
@jacknagel

jacknagel Aug 3, 2012

Contributor

remove_from_cflags should touch CXXFLAGS automatically

@jacknagel

jacknagel Aug 3, 2012

Contributor

remove_from_cflags should touch CXXFLAGS automatically

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Aug 6, 2012

Contributor

@jacknagel updated according to your review.

As said before, on 10.8, valgrind's configure tells:

checking for the kernel version... unsupported (12.0.0)
configure: error: Valgrind works on Darwin 10.x and 11.x (Mac OS X 10.6/7)

but that is upstream and we have to wait for a new version.

Contributor

samueljohn commented Aug 6, 2012

@jacknagel updated according to your review.

As said before, on 10.8, valgrind's configure tells:

checking for the kernel version... unsupported (12.0.0)
configure: error: Valgrind works on Darwin 10.x and 11.x (Mac OS X 10.6/7)

but that is upstream and we have to wait for a new version.

@samueljohn

This comment has been minimized.

Show comment Hide comment
@samueljohn

samueljohn Aug 15, 2012

Contributor

I will open a new PR on top of #14119.

Contributor

samueljohn commented Aug 15, 2012

I will open a new PR on top of #14119.

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 16, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.