Closed Bug 1057600 Opened 5 years ago Closed 5 years ago

Build revisions missing when MOZILLA_OFFICIAL=1 is not set in mozconfig

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox38 fixed)

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: kevink9876543, Assigned: kevink9876543)

Details

Attachments

(1 file, 2 obsolete files)

User Agent: Opera/9.80 (Macintosh; Intel Mac OS X 10.6.8; U; en) Presto/2.9.168 Version/11.52

Steps to reproduce:

Environment: self-built SeaMonkey 2.31 nightly, built for Linux i686 on Ubuntu 12.04 32-bit where the Mercurial is a standard source install of Mercurial 2.9.2 set up with Ubuntu default configuration files and the following user hgrc:

[diff]
git = True

[extensions]
color = 
strip = 

[trusted]
groups = vboxsf

[ui]
username = (local_hg_username)

all other build deps came from Ubuntu packages AFAIK

STR:
new profile
Go to about:buildconfig


Actual results:

No sign of what changesets the application was built from.


Expected results:

Should show a link to the m-c changeset the application was built from.
OS: Mac OS X → Linux
Official Linux i686 nightlies are back now, and official build 20140901003005 doesn't seem affected by this bug...
Hit this bug self building SeaMonkey 2.32.1 release for Mac OS X, with the following mozconfig:


mk_add_options MOZ_OBJDIR=/srv/seamonkey/comm-release/objdir

# Big Hack that unsets CC / CXX so that mozconfig.common doesn't get
# mixed up with host/target CPUs when trying to work out how to do the
# universal build. When we redo the build system (bug 648979) this will
# go away.
if test -e "$topsrcdir/suite/config/version.txt"; then
  unset CC
  unset CXX
fi

. $topsrcdir/build/macosx/universal/mozconfig

ac_add_options --disable-eme
ac_add_options --enable-application=suite

export CFLAGS="-gdwarf-2"
export CXXFLAGS="-gdwarf-2"

# For NSS symbols
export MOZ_DEBUG_SYMBOLS=1
ac_add_options --enable-debug-symbols="-gdwarf-2"

# needed to build on my Mac
ac_add_options --without-ccache
ac_add_options --with-macos-sdk=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.7.sdk

# Enable parallel compiling
mk_add_options MOZ_MAKE_FLAGS="-j4"



I'm using the packaged version of my build, packaged with

cd objdir;make -C ./x86_64 package

(I don't know what to do to run the non packaged version on a Mac).


Am I just missing a required step or mozconfig option to get build revisions?
OS: Linux → All
Hardware: x86 → All
OK, so I compared my mozconfig to the official mozconfig - and build revisions show up after only adding the following lines to my mozconfig:

# TESTING - let's see if this gets build revisions in about:buildconfig
export MOZILLA_OFFICIAL=1

I'm not happy about using that option because this is *not* an official build, and setting MOZILLA_OFFICIAL=1 enables Breakpad as well which IIUC in this case would just lead to the crash-stats server getting useless reports.


If that behavior is by design, I'd like to turn this into an RFE: please always include build revisions on all systems where it's possible to do so, regardless of any mozconfig options; or, if that's not reasonable, please add an option to get build revisions in the build without the other implications of MOZILLA_OFFICIAL.  Missing build revisions partially breaks the Nightly Tester Tools add-on, and it makes it MUCH harder to use archived custom builds for finding regression ranges because I have no other way to keep track of what I built from that's as reliable (or as convenient).
Summary: about:buildconfig doesn't show build revisions → Build revisions missing when MOZILLA_OFFICIAL=1 is not set in mozconfig
... and lxr search on comm-release for MOZILLA_OFFICIAL turned up the following:

http://lxr.mozilla.org/comm-release/source/mozilla/build/Makefile.in#36
http://lxr.mozilla.org/comm-release/source/mozilla/toolkit/content/Makefile.in#19
http://lxr.mozilla.org/comm-release/source/mozilla/toolkit/mozapps/installer/package-name.mk#151
http://lxr.mozilla.org/comm-release/source/mozilla/toolkit/xre/Makefile.in#27

(comm-central search currently turns up the same lines in the same files.)

Looking at the other search results, I am even more sure now that I don't want to use MOZILLA_OFFICIAL in my custom builds.


I'll experiment with the code, see if just getting rid of those ifdefs does the trick; if I can make this work in both comm-release and c-c I'd like to submit the patch for review because I just can't see any disadvantage to having build revisions in custom / unofficial builds.

(Since all that code is in mozilla/, should this bug be moved out of SeaMonkey?)
You should talk to the guys in #build first irc://moznet/build
Attached patch Horrible hack that works (obsolete) — Splinter Review
Apologies for the delay, building seems to take at least 3 hours where before it was about one hour...


From talking to nalexander and tbsaunde on #build, seems MOZILLA_OFFICIAL is required for including build revisions because people building from git or tarballs were annoyed by all the spam of failing hg commands.  So here's a patch which allows build revisions to be included in builds without the other implications of MOZILLA_OFFICIAL, but keeps it disabled by default.
There should be no difference if using MOZILLA_OFFICIAL.
Otherwise, the required mozconfig line would be

export MOZ_INCLUDE_SOURCE_INFO=1


I say it's a horrible hack because I couldn't find a nice way to say "or" in Makefile.in ifdefs so I ended up duplicating a lot of code ugh :P

I suppose another way to accomplish this would be make MOZILLA_OFFICIAL automatically set MOZ_INCLUDE_SOURCE_INFO and then just change the existing ifdefs, but being unfamiliar with autoconf language, I'm not sure how to do that (looking at mozilla/configure.in, it looks like it might be possible though, just not sure which syntax(es) would apply here).  If someone can give me pointer on that (and if that's even a good idea), please let me know and I'll fix up the patch to be more elegant before requesting review.

(For the purpose of requesting review, who's glandium?)
Attached patch more elegant patch (obsolete) — Splinter Review
(In reply to kevink9876543 from comment #6)
> being unfamiliar with autoconf language
Ah, the lack of easy docs for newbies is not only because it seems people aren't "supposed to" directly write .in files, but also because it's basically just shell scripting with a few extras.  Fortunately I already know how to write ksh scripts...

Anyway, here's a better patch.  This goes the route described above (setting MOZILLA_OFFICIAL automatically makes set MOZ_INCLUDE_SOURCE_INFO).


(To note, I referenced revision history of some of the files I've edited here, and from that this bug looks caused by bug 1043390.)
Attachment #8564439 - Attachment is obsolete: true
Attachment #8565719 - Flags: review?(mh+mozilla)
Comment on attachment 8565719 [details] [diff] [review]
more elegant patch

Review of attachment 8565719 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +8687,5 @@
>  AC_SUBST(MOZILLA_OFFICIAL)
>  
> +# Build revisions should always be present in official builds
> +if test "$MOZILLA_OFFICIAL"; then
> +    export MOZ_INCLUDE_SOURCE_INFO=1

no need to export here, just set the variable.

::: toolkit/xre/moz.build
@@ +144,5 @@
>  for var in ('APP_VERSION', 'APP_ID'):
>      DEFINES[var] = CONFIG['MOZ_%s' % var]
>  
> +if CONFIG['MOZ_INCLUDE_SOURCE_INFO']:
> +    DEFINES['MOZ_INCLUDE_SOURCE_INFO'] = True

This part is not necessary.
Attachment #8565719 - Flags: review?(mh+mozilla) → review+
Fixed, thanks.

I don't have commit access to m-c so here's the patch with review comments addressed.
Attachment #8565719 - Attachment is obsolete: true
Attachment #8565841 - Flags: review?(mh+mozilla)
Product: SeaMonkey → Core
Comment on attachment 8565841 [details] [diff] [review]
addressed review comments

Review of attachment 8565841 [details] [diff] [review]:
-----------------------------------------------------------------

confirmed this patch addresses the review comments from glandium's previous r+
Attachment #8565841 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/4ba0059e7220
Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.