Upload symbols when uploading mojo binaries to the CDN.

Upload symbols when uploading mojo binaries to the CDN.

R=viettrungluu@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/f961e5eb98f79da4550fb257c463f67eb560997a

[viettrungluu, 2015-07-10 18:27:02]

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries.py
File mojo/tools/upload_binaries.py (right):

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:26: '../../third_party/pyelftools'))
os.path.join(os.pardir, os.pardir, "third_party", "pyelftools")

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:88: """Compute a unique signature of a library
file.
And what if it's not a library file, or even an ELF binary?

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:94: text_section =
elf.get_section_by_name('.text')
Apparently, "s are mostly preferred in this file over 's.

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:96: data = file_object.read(min(4096,
text_section['sh_size']))
It seems a little weak (in terms of "uniqueness") to look at only 4096 bytes. I
don't see why you don't just hash all of it. (Looking only at .text is a little
weak, but I guess OK for looking at the stack. But maybe you should hash the
entire stripped library (or the .mojo).

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:100: for i in xrange(0, len(data), len(result)):
... and I don't understand why you wouldn't use a proper hash function.

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:114: os.access(path, os.X_OK) and
This seems to assume that all files in the build dir with the x bit set are
interesting libraries, which seems wrong to me:

* It's not clear to me why we set the x bit for .so's in the first place, so if
we ever changed that this would break.
* This will also include tools, such as md5sum_bin.
* This will even include tools built for the host, such as md5sum_bin_host.

So for .mojo's, we should modify the rule to copy (or maybe symlink) the
unstripped library somewhere, so that we'll have a canonical(ish[*]) set of
things to upload. (We can deal with the shell itself separately.)

[*] Of course, if we're not doing clobber builds, then we could have stale
stuff.

[qsr, 2015-07-10 19:35:27]

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries.py
File mojo/tools/upload_binaries.py (right):

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:88: """Compute a unique signature of a library
file.
On 2015/07/10 18:27:01, viettrungluu wrote:
> And what if it's not a library file, or even an ELF binary?

Was certain it was returning None. Will change.

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:96: data = file_object.read(min(4096,
text_section['sh_size']))
On 2015/07/10 18:27:01, viettrungluu wrote:
> It seems a little weak (in terms of "uniqueness") to look at only 4096 bytes.
I
> don't see why you don't just hash all of it. (Looking only at .text is a
little
> weak, but I guess OK for looking at the stack. But maybe you should hash the
> entire stripped library (or the .mojo).

So this algo is what breakpad use, so I do not see any reason to do anything
else. But the property I really want are:

1) Do not depend on whether the symbol are there or not
2) Computed from a small part of the file. The reason for this is that when
debugging with gdb, I want to be able to compute the hash of a file without
having to transfer it completely over USB because this is slow as hell.

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:100: for i in xrange(0, len(data), len(result)):
On 2015/07/10 18:27:01, viettrungluu wrote:
> ... and I don't understand why you wouldn't use a proper hash function.

 Same thing, this is what breakpad is doing and I do not see any reason to
re-invent my own algo.

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:114: os.access(path, os.X_OK) and
On 2015/07/10 18:27:01, viettrungluu wrote:
> This seems to assume that all files in the build dir with the x bit set are
> interesting libraries, which seems wrong to me:
> 
> * It's not clear to me why we set the x bit for .so's in the first place, so
if
> we ever changed that this would break.
> * This will also include tools, such as md5sum_bin.
> * This will even include tools built for the host, such as md5sum_bin_host.

 Getting some more artifact is not really a problem.

> So for .mojo's, we should modify the rule to copy (or maybe symlink) the
> unstripped library somewhere, so that we'll have a canonical(ish[*]) set of
> things to upload. (We can deal with the shell itself separately.)
> 
> [*] Of course, if we're not doing clobber builds, then we could have stale
> stuff.

 Stale stuff is not a problem either. We would just re-upload old symbols.

 Will change the mojo application target to hard link to a new directory. I
don't want symlinks because stale symlink without their target might break this
script.

[qsr, 2015-07-15 13:49:11]

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries.py
File mojo/tools/upload_binaries.py (right):

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:26: '../../third_party/pyelftools'))
On 2015/07/10 18:27:01, viettrungluu wrote:
> os.path.join(os.pardir, os.pardir, "third_party", "pyelftools")

Done.

https://codereview.chromium.org/1236543003/diff/1/mojo/tools/upload_binaries....
mojo/tools/upload_binaries.py:94: text_section =
elf.get_section_by_name('.text')
On 2015/07/10 18:27:01, viettrungluu wrote:
> Apparently, "s are mostly preferred in this file over 's.

Done.

[viettrungluu, 2015-07-15 18:15:19]

lgtm

[qsr, 2015-07-16 09:43:42]

Committed patchset #2 (id:20001) manually as
f961e5eb98f79da4550fb257c463f67eb560997a (presubmit successful).