875659 - Only build B2G Valgrind with B2G_VALGRIND environment variable is on

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

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.

Comment 1

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - Only build valgrind when B2G_VALGRIND environment variable is on

Comment 2

[Michael Wu [:mwu]]

What repo is this against?

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Oh, sorry, http://github.com/mozilla-b2g/valgrind

Comment 4

[Michael Wu [:mwu]]

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.

Comment 5

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

(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.

Comment 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

https://git.mozilla.org/?p=b2g/valgrind.git;a=commit;h=24ef46521f503d5ebe0ebf7bbf66a2dedcd38778