builds should have changeset ID in about:buildconfig when possible

RESOLVED FIXED

Status

()

Toolkit
Build Config
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: dbaron, Assigned: ted)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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

10 years ago
Version: 1.8 Branch → Trunk
(Assignee)

Comment 1

10 years ago
Armen has a bug on putting the changeset ID into application.ini, for l10n repackaging. 

Comment 2

10 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

10 years ago
Assignee: nobody → ted.mielczarek
(Assignee)

Comment 3

10 years ago
Created attachment 339596 [details] [diff] [review]
add a source link to about:buildconfig

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)

Updated

10 years ago
Blocks: 456360
(Assignee)

Comment 4

10 years ago
Created attachment 339771 [details] [diff] [review]
fix some minor issues

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

10 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

10 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

10 years ago
Comment on attachment 339771 [details] [diff] [review]
fix some minor issues

ok
Attachment #339771 - Flags: review- → review+
(Assignee)

Comment 8

10 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/dedc9cf1052c
Status: NEW → RESOLVED
Last Resolved: 10 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 → ---

Comment 10

10 years ago
Come on, can you at least be more specific about the failure message or failure mode?
(Assignee)

Comment 11

10 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.
(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.
Created attachment 339881 [details] [diff] [review]
fixup patch

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

Comment 18

10 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

10 years ago
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
(Assignee)

Comment 22

10 years ago
Created attachment 339957 [details] [diff] [review]
[checked in] potential bustage fix

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

10 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

10 years ago
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 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.
(Assignee)

Updated

9 years ago
Depends on: 456726
You need to log in before you can comment on or make changes to this bug.