Summary: | Please review patches for dev-util/valgrind-3.6.1 adding OSX support | ||
---|---|---|---|
Product: | Gentoo Linux | Reporter: | Fabian Groffen <grobian> |
Component: | New packages | Assignee: | Anthony Basile <blueness> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | johan, prefix |
Priority: | Normal | ||
Version: | unspecified | ||
Hardware: | AMD64 | ||
OS: | OS X | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Bug Depends on: | |||
Bug Blocks: | 315803 | ||
Attachments: |
Fix for the 3.7.0 non-exec patch to un-break compilation on Darwin
Darwin support for the 3.7.0 ebuild |
Description
Fabian Groffen
2011-03-22 18:53:19 UTC
I assume this is in an effort to get portage into MacOSX? Two points: 1) I'd like to see more work done on getting non-exec-stack working on darwin. It the right thing to do even from the point of view of porting this. I have access to MaxOSX but I don't have time to pursue this. 2) Whats ${ED} in the line pax-mark m "${ED}"/usr/$(get_libdir)/valgrind/*-*-linux ? Why the change? (In reply to comment #1) > I assume this is in an effort to get portage into MacOSX? Two points: Not quite, it is an effort to merge the ebuild for Gentoo Prefix back to gentoo-x86. > > 1) I'd like to see more work done on getting non-exec-stack working on darwin. > It the right thing to do even from the point of view of porting this. I have > access to MaxOSX but I don't have time to pursue this. > > 2) Whats ${ED} in the line > > pax-mark m "${ED}"/usr/$(get_libdir)/valgrind/*-*-linux > > ? Why the change? ED is the offset in Gentoo Prefix. Essentially, ED=${D}/${EPREFIX} http://www.gentoo.org/proj/en/gentoo-alt/prefix/techdocs.xml#doc_chap2 (In reply to comment #1) > I assume this is in an effort to get portage into MacOSX? Two points: > > 1) I'd like to see more work done on getting non-exec-stack working on darwin. > It the right thing to do even from the point of view of porting this. I have > access to MaxOSX but I don't have time to pursue this. As far as I understand from the Apple people, all stacks are already read-write-noexecute. Created attachment 306465 [details, diff]
Fix for the 3.7.0 non-exec patch to un-break compilation on Darwin
Created attachment 306467 [details, diff]
Darwin support for the 3.7.0 ebuild
Please reconsider adding support for Prefix + Darwin to valgrind.
The changes are mostly the same as for 3.6, but this time instead of disabling the non-execstack patch on Darwin, I've fixed it so it doesn't break the compilation by requiring non-existing targets.
With these changes, valgrind produces some useful output on Darwin 9 (tested).
(In reply to comment #5) > Created attachment 306467 [details, diff] [details, diff] > Darwin support for the 3.7.0 ebuild > > Please reconsider adding support for Prefix + Darwin to valgrind. > > The changes are mostly the same as for 3.6, but this time instead of > disabling the non-execstack patch on Darwin, I've fixed it so it doesn't > break the compilation by requiring non-existing targets. > > With these changes, valgrind produces some useful output on Darwin 9 > (tested). Have you tested that both of these patches don't break valgrind on the other arches, mostly x86 and amd64. I don't see that they should, they seem innocent enough, but just in case. My only hesitation with this is that valgrind is undoubtedly the most fragil package I maintain. Let me know and I'll apply to the 3.7.0 ~arch ebuilds. Sorry for the delay. Yes it works. I didn't observe any problems with my patch on amd64. Okay this is in with valgrind-3.7.0-r4.ebuild. I isolated the change in a rev bump. I also added KEYWORDS="~x86-macos ~x64-macos" since I assume you tested on those arches. If there's any problem please reopen. I'd like to have this bug reopened, because the patches eventually applied seem incomplete. In particular, I'd like the patch from comment #4 to be applied, and also: @@ -84,7 +84,7 @@ } src_install() { - emake DESTDIR="${ED}" install + emake DESTDIR="${D}" install dodoc AUTHORS FAQ.txt NEWS README* pax-mark m "${ED}"/usr/$(get_libdir)/valgrind/*-*-linux to fix a double prefix issue while installing. Passed test on x64-macos and amd64-linux. ebuild is indeed broken for Prefix, regardless Darwin. @blueness, I took the liberty to fix these issues. You added files/valgrind-3.7.0-non-exec-darwin.patch but never applied it. Since it was a patch to your exec patch, I applied it to the patch as valgrind-3.7.0-non-exec-stack-v2.patch and used that in -r4 instead. (In reply to comment #11) > @blueness, I took the liberty to fix these issues. You added > files/valgrind-3.7.0-non-exec-darwin.patch but never applied it. Since it > was a patch to your exec patch, I applied it to the patch as > valgrind-3.7.0-non-exec-stack-v2.patch and used that in -r4 instead. I looked at the changes and that's fine. Now I see what happened. Normally I test but I don't have prefix set up anywhere. |