Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(407)

Issue 2526883002: Add valgrind.gni listing all the files. (Closed)

Created:
4 years ago by ehmaldonado_chromium
Modified:
3 years, 2 months ago
CC:
chromium-reviews
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Add valgrind.gni listing all the files. This will be used to include them as run-time dependencies when the tests are run with memcheck on swarming. BUG=chromium:497757

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+421 lines, -0 lines) Patch
A valgrind.gni View 1 chunk +421 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (6 generated)
ehmaldonado_chromium
Is this the right place? Do you know who can review this?
4 years ago (2016-11-23 17:27:00 UTC) #4
ehmaldonado_chromium
agable: Are you the right person? Is this the right place to do it?
4 years ago (2016-11-23 18:27:23 UTC) #7
kjellander_chromium
I would have expected something like valgrind/binaries/ in the path. Did you put this in ...
4 years ago (2016-11-23 20:02:40 UTC) #8
agable
I know this much about .gni files: --->||<--- So... I'm not the right person for ...
4 years ago (2016-11-29 00:33:12 UTC) #9
Dirk Pranke
This seems like the wrong place for this. Why isn't this in the webrtc repo?
4 years ago (2016-11-29 01:26:17 UTC) #11
kjellander_chromium
On 2016/11/29 01:26:17, Dirk Pranke wrote: > This seems like the wrong place for this. ...
4 years ago (2016-11-29 07:36:55 UTC) #12
Dirk Pranke
4 years ago (2016-12-01 00:20:20 UTC) #13
On 2016/11/29 07:36:55, kjellander_chromium wrote:
> On 2016/11/29 01:26:17, Dirk Pranke wrote:
> > This seems like the wrong place for this. Why isn't this in the webrtc repo?
> 
> Assuming this is created in
> https://chromium.googlesource.com/chromium/deps/valgrind/binaries/ (it's not
> clear to me in Rietveld what the "chromium_deps" Project means), isn't it the
> right spot for this file? Sure, we can keep it in WebRTC repo only (we already
> have a version there), but if we put it close the the actual files, it could
be
> reused if Chromium or other sub-projects decides to use memcheck. In WebRTC we
> still find it useful as it detects leaks that the Sanitizers can't. It also
> effectively demonstrates poorly written tests since it's so slow. With
Swarming
> it also gets a welcome speed bump, making it far more useful than before.

Part of the problem I had was that I actually didn't know where this file was
being created :).

I don't know if anyone else is still using valgrind/memcheck. As a result, it's
hard to judge whether this
file as-is is useful in other projects and whether we should locate it in the
valgrind repo.

But, since I don't know of others using valgrind, I also don't have a strong
leaning here. If this works for
you that's fine be me.

Powered by Google App Engine
This is Rietveld 408576698