Second step in putting Valgrind/TSan binaries into SVN.

Second step in putting Valgrind/TSan binaries into SVN.

This CL affects neither bots nor developers (yet).

The next step would be putting the binaries into SVN
into deps/third_party/valgrind/binaries.

Then - implement pulling these binaries via some script
(e.g. get_valgrind.sh) and running theminstead or using /usr/valgrind-XXX.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=36404

[M-A Ruel, 2010-01-13 16:26:28]

How large is the directory?

[Timur Iskhodzhanov, 2010-01-13 16:37:20]

On 2010/01/13 16:26:28, Marc-Antoine Ruel wrote:
> How large is the directory?

$ du -s -h deps/third_party/valgrind/binaries/*
26M	deps/third_party/valgrind/binaries/mac
8.6M	deps/third_party/valgrind/binaries/x64
4.4M	deps/third_party/valgrind/binaries/x86

$ du -s -h deps/third_party/valgrind/
39M	deps/third_party/valgrind/

Please note that it will only be checked out if valgrind is needed
AND only the platform-specific binaries will be checked out when needed
(e.g. mac binaries won't be checked out on Linux)

Mac is larger than other platforms because it also contains gdb-1344.
However, the gdb part is very unlikely to change.
Without gdb binaries/mac are only 8.8Mb.

[dank, 2010-01-13 16:37:35]

You probably want an initial checkin that is only the
files being copied unchanged.  That makes the history a 
tad cleaner.

Have you verified it's easy to get the scripts/patches
and not the binaries?  Perhaps the scripts and patches
should be in a src subdirectory to make that easier.

It is incorrect to say that we expect developers to not
run the build script.  Only people using
linux or mac versions compatible with the checked in
binaries will be able to avoid that.  People with sufficiently
different versions of linux or mac may well have to run
the script.

[dank, 2010-01-13 16:44:18]

Also, the curmudgeon in me suggests checking the binaries
in one platform at a time to avoid overloading the server,
but if it works all in one go, that's probably fine.

[Timur Iskhodzhanov, 2010-01-13 18:50:18]

On 2010/01/13 16:37:35, dank wrote:
> You probably want an initial checkin that is only the
> files being copied unchanged.  That makes the history a 
> tad cleaner.
Yes, you're right - updated the changelist

> Have you verified it's easy to get the scripts/patches
> and not the binaries?  Perhaps the scripts and patches
> should be in a src subdirectory to make that easier.
I guess it would be more difficult to check out deps/third_party/valgrind and
src/tools/valgrind in this case.

You'll need either a client with both deps/ and src/
(which is bigger than just deps/third_party/valrgind)
OR you'll need two separate clients which would make
patching {both of them at the same time to keep script
and binaries in sync} more difficult.

Is 40Mb such a problem for a small fraction of developers?..
Anyway, we can do it later it if appears to be a problem.

Also, my vision of the "valgrind update" procedure I'd like to achieve is the
following:
a) you checkout deps/third_party/valgrind
b) you edit the build-xyz.sh file and run it
c) as a result, the binaries in your client are updated
d) you do "svn commit" -> both binaries and the script are updated and in-sync
I'll discuss it with you later - during the review of subsequent changelists.
 
> It is incorrect to say that we expect developers to not
> run the build script.  Only people using
> linux or mac versions compatible with the checked in
> binaries will be able to avoid that.  People with sufficiently
> different versions of linux or mac may well have to run
> the script.
You're right - updated the comment!

> Also, the curmudgeon in me suggests checking the binaries
> in one platform at a time to avoid overloading the server,
> but if it works all in one go, that's probably fine.
I'll try to check them in all at once - in the next CL.

[dank, 2010-01-13 18:57:33]

On Wed, Jan 13, 2010 at 10:50 AM, <timurrrr@chromium.org> wrote:
>> Have you verified it's easy to get the scripts/patches
>> and not the binaries?  Perhaps the scripts and patches
>> should be in a src subdirectory to make that easier.
>
> I guess it would be more difficult to check out deps/third_party/valgrind and
> src/tools/valgrind in this case.

I think you misunderstand.  I just meant move the scripts
down a level.  This is very easy, and makes it easy to
avoid checking out the binaries if they're not needed.

> Is 40Mb such a problem for a small fraction of developers?..

40MB, not 40Mb, and yes, bloat hurts users with slower connections.

> Anyway, we can do it later it if appears to be a problem.

It's very easy to do now.

> Also, my vision of the "valgrind update" procedure I'd like to achieve is the
> following:
> a) you checkout deps/third_party/valgrind
> b) you edit the build-xyz.sh file and run it
> c) as a result, the binaries in your client are updated
> d) you do "svn commit" -> both binaries and the script are updated and in-sync
> I'll discuss it with you later - during the review of subsequent changelists.

Yes, though you would need to do the build on several machines,
not just one, so there will be multiple commits.  May as well
make the source checkin separate.

[Timur Iskhodzhanov, 2010-01-13 19:54:56]

On 2010/01/13 18:57:33, dank wrote:
> On Wed, Jan 13, 2010 at 10:50 AM, <mailto:timurrrr@chromium.org> wrote:
> >> Have you verified it's easy to get the scripts/patches
> >> and not the binaries? &nbsp;Perhaps the scripts and patches
> >> should be in a src subdirectory to make that easier.
> >
> > I guess it would be more difficult to check out deps/third_party/valgrind
and
> > src/tools/valgrind in this case.
> 
> I think you misunderstand.  I just meant move the scripts
> down a level.  This is very easy, and makes it easy to
> avoid checking out the binaries if they're not needed.
Yeah, I didn't think about 'src' as 'deps/third_party/valgrind/src/'.
As we've discussed in chat, I put everything into
'deps/third_party/valgrind/scripts/'.
PTAL

> > Is 40Mb such a problem for a small fraction of developers?..
> 
> 40MB, not 40Mb, and yes, bloat hurts users with slower connections.
> 
> > Anyway, we can do it later it if appears to be a problem.
> 
> It's very easy to do now.
> 
> > Also, my vision of the "valgrind update" procedure I'd like to achieve is
the
> > following:
> > a) you checkout deps/third_party/valgrind
> > b) you edit the build-xyz.sh file and run it
> > c) as a result, the binaries in your client are updated
> > d) you do "svn commit" -> both binaries and the script are updated and
in-sync
> > I'll discuss it with you later - during the review of subsequent
changelists.
> 
> Yes, though you would need to do the build on several machines,
> not just one, so there will be multiple commits.  May as well
> make the source checkin separate.
That's true, but not always :-)
You can build the binaries for all the platforms from two machines - a 64-bit
Linux and a Mac.
You can checkout deps/third_party/valgrind/ to a network share and then build
all the binaries.

[Timur Iskhodzhanov, 2010-01-13 23:41:00]

Updated a couple of things;
Now you can build "local" target platform on any machine.

This would be more friendly to developers with different architectures - they'll
be able to simply
a) mkdir src/tools/valgrind/binaries
b) checkout deps/3rdp/valgrind/scripts _into_ src/tools/valgrind
c) run build-xyz.sh and 
d) (after my following patches) run Valgrind automatically by using
chrome_tests.sh

[Timur Iskhodzhanov, 2010-01-14 15:41:36]

1) I've split the patch into two and commited
http://codereview.chromium.org/550036

2) As we discussed on chat with Dan yesterday, now the build script is ok to
build "local" platform and it builds binaries for all the platforms buildable on
the builder's PC. AND it doesn't require to specify the target platform. Yeah.

On 2010/01/13 23:41:00, Timur Iskhodzhanov wrote:
> Updated a couple of things;
> Now you can build "local" target platform on any machine.
> 
> This would be more friendly to developers with different architectures -
they'll
> be able to simply
> a) mkdir src/tools/valgrind/binaries
> b) checkout deps/3rdp/valgrind/scripts _into_ src/tools/valgrind
> c) run build-xyz.sh and 
> d) (after my following patches) run Valgrind automatically by using
> chrome_tests.sh

[dank, 2010-01-15 17:14:32]

Looking good, I just have four questions.

http://codereview.chromium.org/536043/diff/7051/7052
File deps/third_party/valgrind/scripts/build-valgrind-for-chromium.sh (right):

http://codereview.chromium.org/536043/diff/7051/7052#newcode13
deps/third_party/valgrind/scripts/build-valgrind-for-chromium.sh:13: # {{{1
What are these mysterious {{{ and }}} comments throughout?

http://codereview.chromium.org/536043/diff/7051/7052#newcode26
deps/third_party/valgrind/scripts/build-valgrind-for-chromium.sh:26: # Clean
checkout our untar
what does checkout have to do with untar?  I confused.

http://codereview.chromium.org/536043/diff/7051/7052#newcode129
deps/third_party/valgrind/scripts/build-valgrind-for-chromium.sh:129: then
Why did you replace true with --help?  That's a significantly weaker test.

[Timur Iskhodzhanov, 2010-01-15 17:22:46]

four?

http://codereview.chromium.org/536043/diff/7051/7052
File deps/third_party/valgrind/scripts/build-valgrind-for-chromium.sh (right):

http://codereview.chromium.org/536043/diff/7051/7052#newcode13
deps/third_party/valgrind/scripts/build-valgrind-for-chromium.sh:13: # {{{1
On 2010/01/15 17:14:33, dank wrote:
> What are these mysterious {{{ and }}} comments throughout?
These are vim folds, they make the script look much more compact and readable if
you use vim.
If you want, I can delete these {{{1 }}} braces

http://codereview.chromium.org/536043/diff/7051/7052#newcode26
deps/third_party/valgrind/scripts/build-valgrind-for-chromium.sh:26: # Clean
checkout our untar
On 2010/01/15 17:14:33, dank wrote:
> what does checkout have to do with untar?  I confused.
Huh, that was just moved from line 50 of the original file...
Deleted the line

http://codereview.chromium.org/536043/diff/7051/7052#newcode129
deps/third_party/valgrind/scripts/build-valgrind-for-chromium.sh:129: then
On 2010/01/15 17:14:33, dank wrote:
> Why did you replace true with --help?  That's a significantly weaker test.
Yes but "true" on 64-bit Linux doesn't pass on 32-bit binaries... :-)
Can you please suggest a better test? I tried to use "arch" magic, but my
linux-fu was not high enough to succeed.

[dank, 2010-01-15 17:33:12]

I guess the vim folds can stay, though they look kind
of confusing to the uninitiated. 

It's worth keeping the sanity check, I think.
Try replacing 'true' with one you compile yourself, that
should give you the needed control?

[dank, 2010-01-15 20:56:34]

I think the strip and "remove unused binaries" is
new, right?   Please split that into a second changelist.
Otherwise LGTM!