Only build B2G Valgrind with B2G_VALGRIND environment variable is on

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: qdot, Assigned: qdot)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Right now we build valgrind for all builds of B2G. We should only have any package built if the B2G_VALGRIND environment variable is on.
Created attachment 753939 [details] [diff] [review]
Patch 1 (v1) - Only build valgrind when B2G_VALGRIND environment variable is on
Attachment #753939 - Flags: review?(mwu)

Comment 2

5 years ago
What repo is this against?
Oh, sorry, http://github.com/mozilla-b2g/valgrind

Comment 4

5 years ago
Comment on attachment 753939 [details] [diff] [review]
Patch 1 (v1) - Only build valgrind when B2G_VALGRIND environment variable is on

I think I would put this ifneq a little closer to https://github.com/mozilla-b2g/valgrind/blob/fxos/Android.mk#L23, but it doesn't matter a whole much.

BTW, another alternative would be to put this on valgrind.mk. Removing from PRODUCT_PACKAGES doesn't stop it from building, but stops it from being installed on the image. I suspect we would rather avoid building at all, so this is better.

Another observation - this valgrind integration might also be useful for Android, so using something less B2G specific than B2G_VALGRIND might be better.
Attachment #753939 - Flags: review?(mwu) → review+
(In reply to Michael Wu [:mwu] from comment #4)
> Comment on attachment 753939 [details] [diff] [review]
> Patch 1 (v1) - Only build valgrind when B2G_VALGRIND environment variable is
> on
> 
> I think I would put this ifneq a little closer to
> https://github.com/mozilla-b2g/valgrind/blob/fxos/Android.mk#L23, but it
> doesn't matter a whole much.
> 
> BTW, another alternative would be to put this on valgrind.mk. Removing from
> PRODUCT_PACKAGES doesn't stop it from building, but stops it from being
> installed on the image. I suspect we would rather avoid building at all, so
> this is better.

We already do that, actually. The valgrind block in gonk-misc/b2g.mk has the same ifneq around it. So currently we already build, but only integrate if it's on. Adding this means we only build and integrate if the environment variable is on.

> Another observation - this valgrind integration might also be useful for
> Android, so using something less B2G specific than B2G_VALGRIND might be
> better.

For fennec you mean? We'd need a few different things for that, as I'm making some assumptions about locations of files in the B2G tree in the current setup.
https://git.mozilla.org/?p=b2g/valgrind.git;a=commit;h=24ef46521f503d5ebe0ebf7bbf66a2dedcd38778
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.