Closed
Bug 448155
Opened 16 years ago
Closed 16 years ago
builds should have changeset ID in about:buildconfig when possible
Categories
(Toolkit Graveyard :: Build Config, defect)
Toolkit Graveyard
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dbaron, Assigned: ted)
References
()
Details
Attachments
(3 files, 1 obsolete file)
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
We should really find a way to have builds -- at least the nightlies and hourlies and probably releases that we produce -- have the mercurial changeset ID in about:buildconfig so testers can have a better idea what they're testing.
Steps to reproduce: enter about:buildconfig in the URL bar
Expected results: mercurial changeset ID present
Actual results: mercurial changeset ID not present
(It's possible we might want this ifdef MOZILLA_OFFICIAL or something like that -- assuming that we use that for all nightlies/hourlies.)
Reporter | ||
Updated•16 years ago
|
Version: 1.8 Branch → Trunk
Assignee | ||
Comment 1•16 years ago
|
||
Armen has a bug on putting the changeset ID into application.ini, for l10n repackaging.
Comment 2•16 years ago
|
||
I'd like to see the changeset ID in about:buildconfig, as well. It seems like the natural place to look for it.
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → ted.mielczarek
Assignee | ||
Comment 3•16 years ago
|
||
This patch changes the way we create buildconfig.html. Instead of treating it like a Makefile and letting configure do the substitution, we run it through the preprocessor when it's put into a jar file. I had to add two variables that wouldn't otherwise be persisted this far to autoconf.mk, and add all the things it expects to DEFINES in the makefile, but otherwise it's pretty straightforward. Since this is a HTML page anyway, I made it create a link to the source changeset in hgweb.
Attachment #339596 -
Flags: review?(benjamin)
Assignee | ||
Comment 4•16 years ago
|
||
Forgot to remove buildconfig.dtd (a remnant of a previous approach), and fixed it so that repo URLs without a trailing slash will work. I'm just stripping off the trailing slash, and re-adding a slash in the html file.
Attachment #339596 -
Attachment is obsolete: true
Attachment #339771 -
Flags: review?(benjamin)
Attachment #339596 -
Flags: review?(benjamin)
Comment 5•16 years ago
|
||
Comment on attachment 339771 [details] [diff] [review]
fix some minor issues
>diff --git a/toolkit/content/Makefile.in b/toolkit/content/Makefile.in
>-DEFINES += -DMOZ_APP_VERSION=$(MOZ_APP_VERSION)
>+DEFINES += \
>+ -DMOZ_APP_VERSION=$(MOZ_APP_VERSION) \
>+ -Dtarget="$(target)" \
>+ -Dac_configure_args="$(ac_configure_args)" \
>+ -DCC="$(CC)" \
>+ -DCC_VERSION="$(CC_VERSION)" \
>+ -DCFLAGS="$(CFLAGS)" \
>+ -DCXX="$(CXX)" \
>+ -DCXX_VERSION="$(CXX_VERSION)" \
>+ -DCXXFLAGS="$(CXXFLAGS)" \
>+ -DCPPFLAGS="$(CPPFLAGS)" \
>+ $(NULL)
I'm a little afraid of this requiring more shell escaping: I don't think it's unheard-of to do
CC='gcc "-g3"'
which would break your quoting. Perhaps a small makefile function could be used to escape these:
shellescape="$(subst ",\",$(subst \,\\,$(1)))"
-DCC=$(call shellescape,$(CC))
Attachment #339771 -
Flags: review?(benjamin) → review-
Assignee | ||
Comment 6•16 years ago
|
||
That syntax just seems to confuse configure or gcc:
checking for gcc... gcc "-ggdb3"
checking whether the C compiler (gcc "-ggdb3" ) works... no
configure: error: installation or configuration problem: C compiler cannot create executables.
Is there some other similar syntax you're worried about?
Comment 7•16 years ago
|
||
Comment on attachment 339771 [details] [diff] [review]
fix some minor issues
ok
Attachment #339771 -
Flags: review- → review+
Assignee | ||
Comment 8•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 9•16 years ago
|
||
This change breaks building in scratchbox. The scratchbox hg doesn't support 'identifty -i'. The makefile should probably check the exit code of hg and do something appropriate if it fails.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 10•16 years ago
|
||
Come on, can you at least be more specific about the failure message or failure mode?
Assignee | ||
Comment 11•16 years ago
|
||
We discussed this in #developers, and I said I'd be ok with using `hg identify | cut -f1 -d' '`, since that appears to work in scratchbox.
The error code shouldn't matter, since the command redirects stderr to /dev/null, so it should just wind up empty. Unfortunately, hg appears to put its help text on stdout for some reason.
Comment 12•16 years ago
|
||
(In reply to comment #10)
> Come on, can you at least be more specific about the failure message or failure
> mode?
hg outputs its help text to stdout which gets set as CHANGESET causing failures in Preprocess.py later on.
(In reply to comment #11)
> We discussed this in #developers, and I said I'd be ok with using `hg identify
> | cut -f1 -d' '`, since that appears to work in scratchbox.
>
> The error code shouldn't matter, since the command redirects stderr to
> /dev/null, so it should just wind up empty. Unfortunately, hg appears to put
> its help text on stdout for some reason.
Doesn't it make more sense to check whether the command succeeded then whether it wrote any text to stdout like the code currently does?
Comment 13•16 years ago
|
||
The root cause being:
[sbox-CHINOOK-ARMEL-2007: ~] > hg --version
Mercurial Distributed SCM (version 0.9.1)
Comment 14•16 years ago
|
||
FTR, the linux slaves are otherwise using Mercurial 0.9.5 for the mozilla-central and tracemonkey builds.
Here's the fixup patch; was going to land this, but tree is closed. Tested this on win32, works there as well.
Comment 16•16 years ago
|
||
(In reply to comment #10)
> Come on, can you at least be more specific about the failure message or failure
> mode?
Within scratchbox (which uses hg v0.9.1), there is no support for "hg identify -i...", so attempts to use "-i" will fail out, and generate help-usage text. No error is caught, so that help-usage text is then appended to -DSOURCE_CHANGESET, causing the build to fail with invalid parameters.
Even without rest of build complexity, it can be reproduced by doing:
scratchbox>hg identify -i .
hg identify: option -i not recognized
hg identify
print information about the working copy
Print a short summary of the current state of the repo.
This summary identifies the repository state using one or two parent
hash identifiers, followed by a "+" if there are uncommitted changes
in the working directory, followed by a list of tags for this revision.
aliases: id
However, the following works just fine:
scratchbox> hg identify | cut -f1 -d' '
667a63ed2a63
scratchbox>
Also same thing in browser/app/Makefile.in... checked in to unbreak mobile builds.
Pushed:
19526[tip] ccdb84fa5b6a 2008-09-22 17:30 -0700 vladimir
b=448155; fixup patch for hg identify -i not working in scratchbox to fix mobile build bustage; r=me
Comment 18•16 years ago
|
||
Wow, 0.9.1? We're going to have to upgrade that before we do names release branches!
And pushed again, this time with "cd $(srcdir) ; hg identify" instead of "hg identify $(srcdir)". And yes, hg will need to be upgraded inside scratchbox for sure.
Assignee | ||
Comment 20•16 years ago
|
||
vlad: you shouldn't have to touch browser/app/Makefile, since AFAIK fennec doesn't build that.
Comment 21•16 years ago
|
||
Vlad: thanks for landing that, the builds now gets past those two failures.
Ted: sadly, even after Vlad's bustage fix, the builds still fail. A few lines further down, it turns out that "hg -R" doesnt work either. :-(
toolkit/content/Makefile.in, line#70
SOURCE_REPO := $(shell hg -R $(topsrcdir) showconfig paths.default 2>/dev/null | sed -e "s/^ssh:/http:/" -e "s/\/$(_dollar)//" )
browser/app/Makefile.in, line#80
SOURCE_REPO := $(shell hg -R $(topsrcdir) showconfig paths.default 2>/dev/null | sed s/^ssh:/http:/)
These "hg -R" both fail out, throwing a hg usage error message, this means SOURCE_REPO contains invalid text, and subsequent build steps fail.
Help?
Blocks: 430200
Assignee | ||
Comment 22•16 years ago
|
||
Here's a possible bustage fix. I pastebin'ed this to stuart, but I'm not sure he got to try it. Someone want to give this a whirl in scratchbox?
Assignee | ||
Comment 23•16 years ago
|
||
Comment on attachment 339957 [details] [diff] [review]
[checked in] potential bustage fix
Pushed (without that extraneous if):
http://hg.mozilla.org/mozilla-central/rev/31f1081d9681
Attachment #339957 -
Attachment description: potential bustage fix → [checked in] potential bustage fix
Assignee | ||
Updated•16 years ago
|
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
If we still fail, we should probably just upgrade hg and back out those two changes -- we know we'll need an upgrade at some point anyway, as soon as we do a branch.
Updated•6 years ago
|
Product: Toolkit → Toolkit Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•