293455 - Remove -DBUILD_ID=?????????? from command line of gcc

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Artur Polaczynski]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050508
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050508

I use ccache to speed-up building proces, but if i use rm -rf
obj-i686-pc-linux-gnu i get recompile all the tree

This beacuse -DBUILD_ID is in command line of gcc 
Only small number of source files rally get this info


proposition:                                                                    
- change to and use nsBuildID.h                   
- make new build_id.h and use this when needed
- modify make sytem to include this when neded
any other ideas ???


greping tree
Include nsBuildID.h and have using BUILD_ID:
 browser/app/nsBrowserApp.cpp
 calendar/sunbird/app/nsCalendarApp.cpp
 toolkit/xre/nsAppRunner.cpp
 xpinstall/src/nsJSInstall.cpp

This files don't Include nsBuildLD.h:
 xulrunner/app/nsXULRunnerApp.cpp

I only check this on mozilla current tree (not ff and tb)


Reproducible: Always

Steps to Reproduce:

Comment 1

[Benjamin Smedberg]

Darin, this sounds like a good reason not to globally define BUILD_ID. We can
either add it to XULPPFLAGS instead of DEFINES in rules.mk (assuming that we use
XULPPFLAGS consisently), or we can go back to adding it only to the makefiles
that actually need it.

Comment 2

[Artur Polaczynski]

in this files we can use NS_BUILD_ID instead BUILD_ID
 browser/app/nsBrowserApp.cpp
 calendar/sunbird/app/nsCalendarApp.cpp
 toolkit/xre/nsAppRunner.cpp
 xpinstall/src/nsJSInstall.cpp


only this one xulrunner/app/nsXULRunnerApp.cpp not include nsBuildID.h 
option: 
 include nsBuildID.h and s/BUILD_ID/NS_BUILD_ID/g or 
 fix  xulrunner/app/makefile

Comment 3

[Darin Fisher]

OK, I'll work on a patch.

-> Core: Build Config

Comment 4

[Artur Polaczynski]

Attachment: removing use of BUILD_ID in mozilla trunk

removing use of BUILD_ID in mozilla trunk + callendar extension
How about ff and tb?

If you like it change flags r and sr 

(My first patch ;-)

Comment 5

[Darin Fisher]

Comment on attachment 183253 [details] [diff] [review]
removing use of BUILD_ID in mozilla trunk

This patch is a good start, but it doesn't solve the reported bug.  You need to
remove the lines at the bottom of mozilla/config/config.mk

Comment 6

[Darin Fisher]

... and after removing those lines from config.mk, you need to go and fix
anything that is broken by their absence.  I recommend using lxr.mozilla.org to
search for all occurances of BUILD_ID in the tree.

Comment 7

[Artur Polaczynski]

Attachment: removing use of BUILD_ID in mozilla trunk ver 2

removing use of BUILD_ID in mozilla trunk + callendar extension + XulRunner
aditionaly i change config/config.mk (Removing from DEFINES [only])

I check lxr module SeaMonkey and mozilla (Its the same? for me) (I use
SeaMonkey to compile my tree)

building proces on recompiling drop from 2h to 15min (same tree using ccache +
rm -f obj-i686-pc-linux-gnu-art)

set r?/+ and sr?/+ if you like it ;-)

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

it's not the same... /mozilla/ also includes camino, among other stuff.

Comment 9

[Artur Polaczynski]

(In reply to comment #8)
> it's not the same... /mozilla/ also includes camino, among other stuff.

in Mozilla tree 
I found one more change (But i don't know how to get this)

http://lxr.mozilla.org/mozilla/source/mail/app/nsMailApp.cpp#51

NS_STRINGIFY(BUILD_ID), 
must be changed to 
NS_STRINGIFY(NS_BUILD_ID), 

and my patches can be apliayed in SeaMonkey and Mozilla 

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

cvs -d :pserver:[e-mail]:/cvsroot co
mozilla/mail/app/nsMailApp.cpp

Comment 11

[Darin Fisher]

please post a final patch including changes to mozilla/mail.  thanks!

Comment 12

[Artur Polaczynski]

Attachment: removing use of BUILD_ID in mozilla trunk ver 3 final

Comment 13

[Artur Polaczynski]

Attachment: removing use of BUILD_ID in mozilla trunk ver 4 final

some changes in calendar commponent: fixed

Comment 14

[Darin Fisher]

It looks like you can remove more from the end of config.mk.  There's no reason
to even generate BUILD_ID, so it should just be removed.

Comment 15

[Artur Polaczynski]

(In reply to comment #14)
> It looks like you can remove more from the end of config.mk.  There's no reason
> to even generate BUILD_ID, so it should just be removed.

Some scripts and makefiles using this, Whay i don't remove

(old grep maybe changed)
browser/app/profile/firefox.js:57:pref("app.build_id", @BUILD_ID@);             
extensions/layout-debug/application.ini:6:BuildID=@BUILD_ID@                    
xulrunner/examples/simple/simple.xulapp:5:BuildID=BUILD_ID                      
extensions/xmlterm/scripts/Makefile.in:82:XPIFILE = xmlterm$(XMLTERM_VERS)-$(PLA
TFORM_STR)-$(BUILD_MSTONE)-$(BUILD_ID).xpi

Its posible to commit this ver 4 to tree and don't close this bug 
I try to remove this BUILD_ID and fix that skripts later ;-P

Comment 16

[Darin Fisher]

OK, that sounds reasonable.

Comment 17

[Artur Polaczynski]

i remove BUILD_ID from config.mk , compile and all is working
but I test only SeaMonkey

but i found 
http://lxr.mozilla.org/seamonkey/source/toolkit/locales/en-US/chrome/mozapps/update/updates.properties#19
http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/update/src/nsUpdateService.js.in#1438
http://lxr.mozilla.org/seamonkey/source/extensions/layout-debug/application.ini#6
http://lxr.mozilla.org/seamonkey/source/xulrunner/examples/simple/application.ini#16
and what to do with that @BUILD_ID@ 
any sugestion

Comment 18

[Darin Fisher]

-> default owner

Comment 19

[Benjamin Smedberg]

Comment on attachment 183806 [details] [diff] [review]
removing use of BUILD_ID in mozilla trunk ver 4 final

This patch still breaks http://lxr.mozilla.org/mozilla/source/xulrunner/examples/simple/application.ini#16 which relies on DEFINES.

Comment 20

[jag (Peter Annema)]

Don't worry about toolkit/mozapps/update/src/nsUpdateService.js.in, it replaces stuff like BUILD_ID at runtime and won't be affected by the fix for this bug.

Here are the ones you're not handling:

/browser/app/application.ini, line 43 -- BuildID=@BUILD_ID@
/extensions/layout-debug/application.ini, line 6 -- BuildID=@BUILD_ID@
/xulrunner/examples/simple/application.ini, line 16 -- BuildID=@BUILD_ID@

Simplest might be to add this line to their respective Makefile.in right before or after the DIST_FILES line:

XULAPP_DEFINES = -DBUILD_ID=$(BUILD_ID)

On a side note, could we make it such that BUILD_ID only gets set if the build_number file actually exists? I'd like to get rid of all those "cat: ../../config/build_number: No such file or directory" in clobber build log files during the clobber phase and right up to |make export| gets run in config/.

Or move setting it into those few places that need it (and make it silently fail during make clean/clobber/distclean).

Comment 21

[(not currently active) Ted Mielczarek]

This is probably easier since bug 383167 whacked a lot of that.

Comment 22

[Benjamin Smedberg]

I'm pretty sure this was FIXED by bug 383167