731795 - MOZ_SOURCESTAMP_FILE shouldn't depend on MOZ_PKG_PRETTYNAMES

Status: RESOLVED FIXED

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

Description

[Rail Aliiev [:rail]]

if you use MOZ_PKG_PRETTYNAMES=1, "make package" generates a sourcestamp file with pretty names (only for Mac), but the output goes to a different file. I don't think that we want this:

$ wget -O- -q http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/11.0b5-candidates/build1/mac/en-US/Firefox       
20120228210006 11.0b5.txt
http://hg.mozilla.org/releases/mozilla-beta/rev/8c9e4873d419 11.0b5.txt

Comment 1

[Rail Aliiev [:rail]]

Attachment: ignore pretty names

Try run: https://tbpl.mozilla.org/?tree=Try&rev=923a9135681e

Comment 2

[Rail Aliiev [:rail]]

Comment on attachment 601774 [details] [diff] [review]
ignore pretty names

Try build looks good: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-923a9135681e/

Comment 3

[Rail Aliiev [:rail]]

Comment on attachment 601774 [details] [diff] [review]
ignore pretty names

http://hg.mozilla.org/integration/mozilla-inbound/rev/14b18863a61c

Comment 4

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/14b18863a61c

Comment 5

[Rail Aliiev [:rail]]

Comment on attachment 601774 [details] [diff] [review]
ignore pretty names

This patch can be reproduced only in release builds and doesn't affect any CI builds. Worked fine in m-c and staging release builds.

Regression caused by (bug #): the current bug
Testing completed (on m-c, etc.): tested in dev environment by running staging releases 
Risk to taking this patch (and alternatives if risky): very low
String changes made by this patch: none

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 601774 [details] [diff] [review]
ignore pretty names

[Triage comment]
Approved based on dev testing and low risk.

Comment 7

[Rail Aliiev [:rail]]

Comment on attachment 601774 [details] [diff] [review]
ignore pretty names

http://hg.mozilla.org/releases/mozilla-beta/rev/e6540031ea08

Comment 8

[Rail Aliiev [:rail]]

Worked fine in 12.0b1 build2:

$ wget -q -O- http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/12.0b1-candidates/build2/mac/en-US/firefox-12.0b1.txt
20120314195616
http://hg.mozilla.org/releases/mozilla-beta/rev/4027017bbaba

Comment 9

[Rail Aliiev [:rail]]

Comment on attachment 601774 [details] [diff] [review]
ignore pretty names

I would like to land this patch on m-r and esr10 branches. 12.0b1 release passed without any problems. The risk is very low since the patch contains only cosmetic changes.

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 601774 [details] [diff] [review]
ignore pretty names

[Triage Comment]
wfm. low risk, go for it.

Comment 11

[Rail Aliiev [:rail]]

Comment on attachment 601774 [details] [diff] [review]
ignore pretty names

http://hg.mozilla.org/releases/mozilla-esr10/rev/249bf7547c84
http://hg.mozilla.org/releases/mozilla-release/rev/134642a3f460

Comment 12

[Rail Aliiev [:rail]]

Attachment: xulrunner related part

This is a followup patch, which can be descried as "nobody loves xulrunner!". :)

It is similar to attachment 602897 [details] [diff] [review] from bug 732963: MOZ_SOURCESTAMP_FILE should contain platform name in its name since we upload all file in one directory.

Comment 13

[Rail Aliiev [:rail]]

Try (https://tbpl.mozilla.org/?tree=Try&rev=69c7ac90311a) and staging tests pass.

Comment 14

[Rail Aliiev [:rail]]

Comment on attachment 608485 [details] [diff] [review]
xulrunner related part

http://hg.mozilla.org/integration/mozilla-inbound/rev/b25f7ea34460

Comment 15

[Rail Aliiev [:rail]]

Comment on attachment 608485 [details] [diff] [review]
xulrunner related part

Merged from m-i: http://hg.mozilla.org/mozilla-central/rev/b25f7ea34460

Comment 17

[Rail Aliiev [:rail]]

Comment on attachment 608485 [details] [diff] [review]
xulrunner related part

I would land this trivial patch to m-a, m-b, m-r and esr10

Regression caused by (bug #): the current bug, xulrunner overwrites sourcestamp files
User impact if declined: N/A, the files affected are not used by end users
Testing completed (on m-c, etc.): ran a couple of staging releases
Risk to taking this patch (and alternatives if risky): very low
String changes made by this patch: None

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 608485 [details] [diff] [review]
xulrunner related part

Thanks for the thorough risk assessment :)

Comment 19

[Rail Aliiev [:rail]]

Comment on attachment 608485 [details] [diff] [review]
xulrunner related part

http://hg.mozilla.org/releases/mozilla-aurora/rev/f7896dabf8a9
http://hg.mozilla.org/releases/mozilla-beta/rev/17cfd97cb859
http://hg.mozilla.org/releases/mozilla-release/rev/6504535d557b
http://hg.mozilla.org/releases/mozilla-esr10/rev/e23ec0355bf4

Comment 20

[Rail Aliiev [:rail]]

All done here. Looks fine in 12.0b4

Comment 21

[u279076]

Rail, is there anything QA can do to verify this fix? Apart from checking for the changes in the source code of course.

Comment 22

[Rail Aliiev [:rail]]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #21)
> Rail, is there anything QA can do to verify this fix? Apart from checking
> for the changes in the source code of course.

I don't think that you need to verify something here:

1) I verified that we don't generate pretty named source stamp files on ftp anymore
2) End users has nothing to do with this files

Thanks!

Comment 23

[u279076]

Okay, thanks Rail. Marking qa- to remove from my queries.