Closed
Bug 1057600
Opened 11 years ago
Closed 10 years ago
Build revisions missing when MOZILLA_OFFICIAL=1 is not set in mozconfig
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox38 fixed)
RESOLVED
FIXED
mozilla38
| Tracking | Status | |
|---|---|---|
| firefox38 | --- | fixed |
People
(Reporter: kevink9876543, Assigned: kevink9876543)
Details
Attachments
(1 file, 2 obsolete files)
|
2.45 KB,
patch
|
Callek
:
review+
|
Details | Diff | Splinter Review |
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.
| Assignee | ||
Updated•11 years ago
|
OS: Mac OS X → Linux
| Assignee | ||
Comment 1•11 years ago
|
||
Official Linux i686 nightlies are back now, and official build 20140901003005 doesn't seem affected by this bug...
| Assignee | ||
Comment 2•10 years ago
|
||
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
| Assignee | ||
Comment 3•10 years ago
|
||
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
| Assignee | ||
Comment 4•10 years ago
|
||
... 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?)
Comment 5•10 years ago
|
||
You should talk to the guys in #build first irc://moznet/build
| Assignee | ||
Comment 6•10 years ago
|
||
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?)
| Assignee | ||
Comment 7•10 years ago
|
||
(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 8•10 years ago
|
||
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+
| Assignee | ||
Comment 9•10 years ago
|
||
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)
Updated•10 years ago
|
Keywords: checkin-needed
Updated•10 years ago
|
Product: SeaMonkey → Core
Comment 10•10 years ago
|
||
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+
Comment 11•10 years ago
|
||
Assignee: nobody → kevink9876543
Keywords: checkin-needed
Comment 12•10 years ago
|
||
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•