514466 - uploadsymbols target fails when for certain filenames

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P2)

Description

[[e-mail] (:bhearsum)]

Upload symbols has two problems with files with more than one space in name. Firstly, the hash calculation, http://hg.mozilla.org/mozilla-central/file/tip/toolkit/crashreporter/tools/upload_symbols.sh#l51, assumes the output looks like 
  SHA1(dist/namoroko-3.6a1-crashreporter-symbols.zip)=1210b518ae71241c853b7d9cf795f2e0ce90c855

and gets confused if there are spaces in the filename. Secondly, the quoting in the scp upload, http://hg.mozilla.org/mozilla-central/file/tip/toolkit/crashreporter/tools/upload_symbols.sh#l54 doesn't cope with multiple spaces. Mysteriously, it does cope with single spaces, eg Firefox 3.5.2.crashreporter-symbols.zip.

This needs to be fixed before 3.6b1

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

For the hash we could just switch to using the PID of the make process. That should be sufficiently unique to avoid the simultaneous upload by two hosts, which occurs only infrequently.

Don't think we hit this with 3.5b1, did something change to break 3.6b1 ? The switch to official branding may mean we're OK until 3.7a1.

Comment 2

[[e-mail] (:bhearsum)]

(In reply to comment #1)
> For the hash we could just switch to using the PID of the make process. That
> should be sufficiently unique to avoid the simultaneous upload by two hosts,
> which occurs only infrequently.
> 
> Don't think we hit this with 3.5b1, did something change to break 3.6b1 ? The
> switch to official branding may mean we're OK until 3.7a1.

J'accuse bug 504953

Comment 3

[Chris Cooper [:coop] (he/him)]

Attachment: [checked in] Fix hash parsing, escape spaces prior to upload

For the new hash parsing, I simply throw away everything before the equal sign and then remove any extraneous spaces. Assigning via backticks seems to remove those spaces automatically, but I want to be sure.

AFAICT, the accepted way to transfer files with spaces is to escape the spaces with backslashes and then double-quote the entire target path.

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 401076 [details] [diff] [review]
[checked in] Fix hash parsing, escape spaces prior to upload

looks good

Comment 5

[Chris Cooper [:coop] (he/him)]

Comment on attachment 401076 [details] [diff] [review]
[checked in] Fix hash parsing, escape spaces prior to upload

This is not shipping code, but does get run by the build system for every build that is created.

Checked in on m-c: http://hg.mozilla.org/mozilla-central/rev/b4ef29fbae96

Comment 6

[Chris Cooper [:coop] (he/him)]

Comment on attachment 401076 [details] [diff] [review]
[checked in] Fix hash parsing, escape spaces prior to upload

FYI, we only need the archive space-escape changes on 1.9.1. We don't append the hash on that branch, and there's really no point in adding it now.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 401076 [details] [diff] [review]
[checked in] Fix hash parsing, escape spaces prior to upload

Approved for 1.9.1.4, a=dveditz

Comment 8

[Chris Cooper [:coop] (he/him)]

Comment on attachment 401076 [details] [diff] [review]
[checked in] Fix hash parsing, escape spaces prior to upload

Further testing in staging reveals that we don't need this on 1.9.1 at all. Sorry for the trouble, dveditz.

Comment 9

[Chris Cooper [:coop] (he/him)]

Comment on attachment 401076 [details] [diff] [review]
[checked in] Fix hash parsing, escape spaces prior to upload

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/642dcbef2f4a

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

When doing a test 3.6b1 release, we get symbols uploaded but then fail to unpack the archive:

make uploadsymbols
/bin/sh /builds/slave/macosx_build/build/toolkit/crashreporter/tools/upload_symbols.sh "./dist/Firefox 3.6 Beta 1.crashreporter-symbols.zip"
Transferring symbols... ./dist/Firefox 3.6 Beta 1.crashreporter-symbols.zip

Firefox 3.6 Beta 1.crashreporter-symbols.zip    0%    0     0.0KB/s   --:-- ETA
[snip]
Firefox 3.6 Beta 1.crashreporter-symbols.zip  100%  199MB  11.0MB/s   00:18    
Unpacking symbols on remote host...
unzip:  cannot find or open 2ddef3e494fa711081717ee323a6c8dbac14640c-Firefox\ 3.6\ Beta\ 1.crashreporter-symbols.zip, 2ddef3e494fa711081717ee323a6c8dbac14640c-Firefox\ 3.6\ Beta\ 1.crashreporter-symbols.zip.zip or 2ddef3e494fa711081717ee323a6c8dbac14640c-Firefox\ 3.6\ Beta\ 1.crashreporter-symbols.zip.ZIP.
make: *** [uploadsymbols] Error 9

I think we should just drop the spaces in the filename, which only occur in release builds where we don't preserve the file anyway (just its contents!).

Comment 11

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: Use a plain name, doesn't have to be end-user friendly

This makes MOZ_PKG_PRETTYNAMES irrelevant to the symbol and packaged-test archive naming. It resolves the issue I was seeing on mac by creating firefox-3.6b1.crashreporter-symbols.zip. The file hash prevents collisions should more than one platform upload at the same time. 

Think I have to ask review from Ted on this, destined for 1.9.2 ahead of 3.6b1.

Comment 12

[(not currently active) Ted Mielczarek]

Comment on attachment 404209 [details] [diff] [review]
Use a plain name, doesn't have to be end-user friendly

This won't work. We need the symbol and test packages to have the same basename as the build for the builders that run packaged tests (and bug 457753 will run those on releases).

Incidentally, I filed bug 519679 yesterday about probably this same issue because catlee hit it. I suggested there that we just override the destination filename ('archive' in the upload-symbols script), since that in fact doesn't matter.

Comment 14

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: Squash spaces in filename on upload host

Ted's suggestion from bug 519679, using the sed foo that coop already wrote. This works on the big-three platforms on a fake 3.6b1 release.

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 404574 [details] [diff] [review]
Squash spaces in filename on upload host

http://hg.mozilla.org/mozilla-central/rev/8f1ba5c38bb1
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/fab82463c585