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)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dbaron, Assigned: ted)

References

()

Details

Attachments

(3 files, 1 obsolete file)

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.)
Version: 1.8 Branch → Trunk
Armen has a bug on putting the changeset ID into application.ini, for l10n repackaging.
I'd like to see the changeset ID in about:buildconfig, as well. It seems like the natural place to look for it.
Assignee: nobody → ted.mielczarek
Attached patch add a source link to about:buildconfig (obsolete) (deleted) — Splinter Review
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)
Blocks: 456360
Attached patch fix some minor issues (deleted) — Splinter Review
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 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-
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 on attachment 339771 [details] [diff] [review] fix some minor issues ok
Attachment #339771 - Flags: review- → review+
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
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 → ---
Come on, can you at least be more specific about the failure message or failure mode?
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.
(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?
The root cause being: [sbox-CHINOOK-ARMEL-2007: ~] > hg --version Mercurial Distributed SCM (version 0.9.1)
FTR, the linux slaves are otherwise using Mercurial 0.9.5 for the mozilla-central and tracemonkey builds.
Attached patch fixup patch (deleted) — Splinter Review
Here's the fixup patch; was going to land this, but tree is closed. Tested this on win32, works there as well.
(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
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.
vlad: you shouldn't have to touch browser/app/Makefile, since AFAIK fennec doesn't build that.
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
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?
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
Status: REOPENED → RESOLVED
Closed: 16 years ago16 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.
Depends on: 456726
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: