Last Comment Bug 549958 - Hourly/Nightly builds should have some way to see which {comm-central|mobile-browser|camino} changeset was used, too
: Hourly/Nightly builds should have some way to see which {comm-central|mobile-...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla2.0b12
Assigned To: Smokey Ardisson (offline for a while; not following bugs - do not email)
:
: Gregory Szorc [:gps]
Mentors:
Depends on: 474610 562114 631006
Blocks: 630487 563208 630176
  Show dependency treegraph
 
Reported: 2010-03-03 10:45 PST by Serge Gautherie (:sgautherie)
Modified: 2011-03-04 10:15 PST (History)
9 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.17-fixed
.19-fixed


Attachments
Thought, very rough poc hack (1.17 KB, patch)
2010-04-13 18:49 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
no flags Details | Diff | Splinter Review
Implements comment 6 + comment 8 (3.44 KB, patch)
2010-04-29 19:40 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
ted: review+
benjamin: approval2.0+
Details | Diff | Splinter Review
Roll-up patch for branches (3.43 KB, patch)
2011-02-24 11:51 PST, Smokey Ardisson (offline for a while; not following bugs - do not email)
dveditz: approval1.9.2.17+
dveditz: approval1.9.1.19+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2010-03-03 10:45:23 PST
Bug 474610 comment 17:
{
Justin Wood (irc: Callek)      2010-02-27 21:30:11 PST

I suspect we may want a slightly different solution for c-c here (package a
"list" of changesets perhaps)
}

***

Current code is at:
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/mozapps/installer/packager.mk#440
{
436 make-package: stage-package $(PACKAGE_XULRUNNER)
440         @echo "$(BUILDID) $(MOZ_SOURCE_STAMP)" > $(DIST)/$(PKG_PATH)/$(PKG_BASENAME).txt
}
Comment 1 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-04-13 18:49:51 PDT
Created attachment 438923 [details] [diff] [review]
Thought, very rough poc hack

Here's an idea that I had for how to attack this (it appears to work, but I haven't checked all my Ps and Qs to make sure it will always do what I think it does):

1) In packager.mk, change the make-package target to depend on a new target, make-sourcestamp-txt (with a better name, of course).

2) make-sourcestamp-txt now calls the rule Ted added in bug 474610.  make-sourcestamp-txt is also a :: target.  Assuming all my Ps and Qs are in order, this is essentially a no-op for single-repo products (Firefox, XR).

Then, every product that has its own repo can add a make-sourcestamp-txt:: rule to its installer/Makefile.in (or equivalent), which gets called along with the rule in packager.mk.

So, for instance, Camino (camino/installer/Makefile.in) could have a target/rule that looks like this:

make-sourcestamp-txt::
 	@echo "$(CM_SOURCE_STAMP)" >> $(DIST)/$(PKG_PATH)/$(PKG_BASENAME).txt

Or, if we wanted, we could modify (override) the output to echo the repo name

make-sourcestamp-txt::
 	@echo "$(BUILDID) $(MOZ_SOURCE_STAMP) $(MOZ_SOURCE_REPO)" > $(DIST)/$(PKG_PATH)/$(PKG_BASENAME).txt
 	@echo "$(CM_SOURCE_STAMP) $(CM_SOURCE_REPO" >> $(DIST)/$(PKG_PATH)/$(PKG_BASENAME).txt

This setup (the pair of make-sourcestamp-txt:: targets, in Core and in the app) is both flexible so that apps can do what they need to make their sourcestamp file useful and understandable, but also at the same time preserves the benefit of the shared packager.mk rule for single-repo apps or multi-repo apps that might not want to mess with changing this now.
Comment 2 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-04-13 18:58:54 PDT
I know you filed this specifically about c-c apps, but since this bug lives in Core:Build Config and not Mailnews Core:Build Config and the problem applies to everyone with app-specific repos, let's broaden the summary to include other relevant repos.  (Not sure who to CC here from fennec, though.)
Comment 3 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-04-13 19:01:41 PDT
(In reply to comment #1)
> 2) make-sourcestamp-txt now calls the rule Ted added in bug 474610. 
> make-sourcestamp-txt is also a :: target.  Assuming all my Ps and Qs are in
> order, this is essentially a no-op for single-repo products (Firefox, XR).

"this" = "splitting the @echo rule into its own target and having make-package depend on the new target"
Comment 4 Mark Banner (:standard8, afk until Dec) 2010-04-14 00:52:02 PDT
I like the general idea of this. What I'd really like to see us be able to do is to produce a file like:

mozilla-central 1234567890
comm-central 1357924680
(and maybe other revisions as appropriate).

Hence we'd actually be able to get exactly which revisions were used (frequently, having the m-c revision as well as the c-c helps us a lot).

I've not looked at where this is used to know what a format change like this might break.
Comment 5 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-04-14 09:41:22 PDT
(In reply to comment #4)
> I've not looked at where this is used to know what a format change like this
> might break.

I don't think it's being used elsewhere yet, though I haven't attempted to parse the automation code to verify that.  Bug 548549 does want to do something with it, though.

Maybe we should try to standardize on a format like:

$BuildID
$PlatformRepoName $PlatformChangest
$AppRepoName1     $AppChangeset1
$AppRepoName2     $AppChangeset2
$AppRepoName3     $AppChangeset3
…

where the first two lines are required (and produced by packager.mk) and any additional lines are optional (and are produced by the target's pair in the app's installer Makefile)?
Comment 6 Ted Mielczarek [:ted.mielczarek] 2010-04-26 08:34:18 PDT
I kind of like the simplicity of the current Firefox .txt files, but I'd settle for:
$BUILDID
$REPO/rev/$CHANGESET
...

so like:
20100426010203
http://hg.mozilla.org/mozilla-central/rev/abcd1234
http://hg.mozilla.org/comm-central/rev/fedc4321
Comment 7 Ted Mielczarek [:ted.mielczarek] 2010-04-26 08:34:33 PDT
Also your implementation suggestion sounds sane.
Comment 8 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-04-26 13:21:34 PDT
Ted, thanks for the feedback :)  I had also started thinking that maybe the rev-URL would be slightly more useful; it's certainly easier to generate.

I've already renamed "make-sourcestamp-txt" to "make-sourcestamp-file" locally; maybe it should even be "make-sourcestampfile" in keeping with the other make-foo targets (though that gets to be a bit long).

The other thing I think we should do is make "$(DIST)/$(PKG_PATH)/$(PKG_BASENAME).txt" its own variable, so that the half-dozen-or-so places across the trees that are going to be dealing with this file won't have to all be modified if anyone ever change the file's filename (there was some talk of that in another bug at one point).

I'll see about getting a working patch up this week.
Comment 9 Ted Mielczarek [:ted.mielczarek] 2010-04-27 06:16:49 PDT
Apparently Socorro is now scraping this data:
http://groups.google.com/group/mozilla.dev.planning/msg/ddf8c7234a4b74a7

So we'll have to be careful about changing the format of the file that Firefox uploads. The best plan would probably be to file a Socorro bug to change the scraping script it's using to handle both the old format and the new format, and get that landed and in production before landing any changes here.
Comment 10 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-04-27 11:26:33 PDT
Filed bug 562114 on comment 9; thanks for the heads-up on that, too!
Comment 11 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-04-29 19:40:49 PDT
Created attachment 442577 [details] [diff] [review]
Implements comment 6 + comment 8

Here's a patch that implements what we've discussed.  

The only really new change here is fixing the MOZ_SOURCE_STAMP variable definition to use $(MOZILLA_DIR) instead of $(topsrcdir).  This is done to make sure that packager.mk's make-sourcetamp-file rule always generates the platform changeset, so that the file format will remain consistent about what's on the first two lines (build id, platform changeset) no matter what the repo layout is.  With the current code in the tree, in comm-*, packager.mk's rule is actually generating "BuildID comm-*-changeset" for the file's content (according to SeaMonkey; Tb doesn't seem to be uploading them, for whatever reason), so switching to $(MOZILLA_DIR) should fix that.

I don't understand the .ini files (or maybe how $(topsrcdir) changes meaning in a comm-* build), but somehow the same sourcestamp rule in toolkit/xre/Makefile.in appears to be generating the right sourcestamp for platform.ini using $(topsrcdir), even though it seems like it should be returning the wrong thing there, too.
Comment 12 Serge Gautherie (:sgautherie) 2010-04-29 20:22:49 PDT
(In reply to comment #11)

> The only really new change here is fixing the MOZ_SOURCE_STAMP variable
> definition to use $(MOZILLA_DIR) instead of $(topsrcdir).

You may want to add a comment about that in the file.

> With the current code in the tree, in comm-*, packager.mk's rule is
> actually generating "BuildID comm-*-changeset" for the file's content
> (according to SeaMonkey

(Indeed ;-|)

> I don't understand the .ini files (or maybe how $(topsrcdir) changes meaning in
> a comm-* build), but somehow the same sourcestamp rule in
> toolkit/xre/Makefile.in appears to be generating the right sourcestamp for
> platform.ini using $(topsrcdir), even though it seems like it should be
> returning the wrong thing there, too.

It does because packager.mk is included into c-c makefiles,
whereas toolkit/xre/Makefile.in is run from within m-c.
Comment 13 Ted Mielczarek [:ted.mielczarek] 2010-05-18 10:39:56 PDT
Comment on attachment 442577 [details] [diff] [review]
Implements comment 6 + comment 8

>diff --git a/toolkit/mozapps/installer/package-name.mk b/toolkit/mozapps/installer/package-name.mk
>--- a/toolkit/mozapps/installer/package-name.mk
>+++ b/toolkit/mozapps/installer/package-name.mk
>+# strip a trailing slash from the repo URL because it's not always present,
>+# and we want to construct a working URL in the sourcestamp file.
>+# make+shell+sed = awful
>+_dollar=$$
>+MOZ_SOURCE_REPO := $(shell cd $(MOZILLA_DIR) && hg showconfig paths.default 2>/dev/null | head -n1 | sed -e "s/^ssh:/http:/" -e "s/\/$(_dollar)//" )

Use =, not :=, otherwise this will get evaluated in every makefile that includes this file, instead of just the one place where it's used.

r=me with that fix, although obviously we shouldn't land this until the blocking bug is fixed, so as to not break Socorro.
Comment 14 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-05-18 11:17:36 PDT
(In reply to comment #13)
> Use =, not :=, otherwise this will get evaluated in every makefile that
> includes this file, instead of just the one place where it's used.
> 
> r=me with that fix, although obviously we shouldn't land this until the
> blocking bug is fixed, so as to not break Socorro.

Thanks, Ted; fixed locally in anticipation of that day coming at some point :)
Comment 15 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-09-15 12:38:39 PDT
Comment on attachment 442577 [details] [diff] [review]
Implements comment 6 + comment 8

Now that Socorro 1.8 has finally deployed, requesting approval2.0 on this build-config-only change that allows the sourcestamp info file produced by the build system to be useful for other-than-Firefox apps.  This file does not ship with the application and has no effect on application code.

(We'll want this on the branches, too, but branch-drivers will still probably be happier if it's landed on trunk first.)
Comment 16 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-09-16 11:35:48 PDT
Comment on attachment 442577 [details] [diff] [review]
Implements comment 6 + comment 8

Socorro's being rolled back to 1.7 :(  They hope to go back to 1.8 in a week or two, so I'm not going to cancel the approval request (it may take that long for someone to get to it); I just won't land until Socorro's back to 1.8.  Since this bug isn't a blocker or anything, I hope doing that will not screw up queries; if it does, I guess I'll just forego approval until Socorro's back to 1.8 :(
Comment 17 Smokey Ardisson (offline for a while; not following bugs - do not email) 2011-01-26 19:30:18 PST
And Socorro finally went forward to 1.7.whatever-had-bug-562114-in-it over the weekend, with no signs of being rolled back :)

http://hg.mozilla.org/mozilla-central/rev/0d93633ce739

I'd still like to get this on the branches (like the sourcestamp file introduction was), to help with branch regressions and whatnot for apps there; I'll wait a few days to make sure there are no unexpected consequences on the trunk.
Comment 18 Serge Gautherie (:sgautherie) 2011-01-27 10:57:09 PST
http://hg.mozilla.org/comm-central/rev/ce2bbf8d11fd
Use a locally generated version of source stamp for now, not the exported one.
Comment 19 Smokey Ardisson (offline for a while; not following bugs - do not email) 2011-01-27 12:20:47 PST
I'm confused about that bustage; is it because comm-central was reusing the same MOZ_SOURCE_STAMP variable to mean the comm-central revision rather than the platform one?  (Except for Calendar, which was using MOZILLA_SRCDIR rather than topsrcdir in its rule, so it apparently wanted the platform changeset.)

Sorry about that; I'll be sure to give advanced notice of any checkins on the branch(es) to avoid this again :(
Comment 20 Mark Banner (:standard8, afk until Dec) 2011-01-27 12:59:57 PST
(In reply to comment #19)
> I'm confused about that bustage; is it because comm-central was reusing the
> same MOZ_SOURCE_STAMP variable to mean the comm-central revision rather than
> the platform one?  (Except for Calendar, which was using MOZILLA_SRCDIR rather
> than topsrcdir in its rule, so it apparently wanted the platform changeset.)

So the MOZ_SOURCE_STAMP is exported via the main Makefile. i.e. comm-central/Makefile.in or mozilla/Makefile.in

At that time, the MOZ_SOURCE_STAMP variable was calculated via package-name.mk by the build system that was built.

So for comm-central/Makefile.in the $(topsrcdir) variable would equate to '.', when building the mozilla-central part, mozilla/Makefile.in would equate to mozilla/ in a comm-central set-up.

The patch changed it to $(MOZILLA_SRCDIR) which is always mozilla/ in the comm-central set-up whether or not you're in the mozilla directory.

The bustage fix changes the way we rely on the MOZ_SOURCE_STAMP variable so that we'll always re-generate it and not rely on the exported variable.

> Sorry about that; I'll be sure to give advanced notice of any checkins on the
> branch(es) to avoid this again :(

No problem.
Comment 21 Smokey Ardisson (offline for a while; not following bugs - do not email) 2011-01-28 14:33:16 PST
I posted http://www.ardisson.org/afkar/2011/01/28/sourcestamp-file-format-changed-on-mozilla-central/ to hopefully head off any other lurking issues with tools or code depending on the format of the sourcestamp file.
Comment 22 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2011-02-02 13:46:50 PST
This caused build failure in Firefox4.0beta11 build#1. This change now backed out of relbranch, and we're now starting Firefox4.0beta11 build#2.

bug#631006 filed to track fixing bustage for next time.
Comment 23 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2011-02-02 13:48:42 PST
(In reply to comment #22)
> This caused build failure in Firefox4.0beta11 build#1. This change now backed
> out of relbranch, and we're now starting Firefox4.0beta11 build#2.
> 
> bug#631006 filed to track fixing bustage for next time.

Duh. Meant to leave this bug#549958 closed, because new bug#631006 already filed to track fixup work.
Comment 24 Smokey Ardisson (offline for a while; not following bugs - do not email) 2011-02-24 11:51:15 PST
Created attachment 514861 [details] [diff] [review]
Roll-up patch for branches

This is a roll-up patch for the branches, which is attachment 442577 [details] [diff] [review] plus Gavin's PRETTYNAMES fix in bug 631006.  Fx 4.0 Beta 12 included both and had no sourcestamp file-related build issues, so the combination of the two fixes now has been tested in release situations and should be safe for branches.

Landing this on the branches will allow tools depending on this file (e.g. Socorro) to no longer have to support both old and new sourcestamp file formats and will allow non-Firefox products on the branch to include their repository and changeset info in the sourcestamp file to aid in build identification and regression hunting.
Comment 25 Daniel Veditz [:dveditz] 2011-02-28 10:52:13 PST
Comment on attachment 514861 [details] [diff] [review]
Roll-up patch for branches

Approved for 1.9.2.15 and 1.9.1.18, a=dveditz
Comment 26 Smokey Ardisson (offline for a while; not following bugs - do not email) 2011-02-28 12:15:42 PST
standard8 (and anyone else watching this bug for notice of the branch landings):

I'd like to land this on the branches some late afternoon or evening (GMT-5) this week (but not this afternoon/evening, obviously); can we coordinate so that the comm-* changes are ready for someone to push (or they can even go in beforehand, right, and then I could land once I've heard they're in)?
Comment 27 Mark Banner (:standard8, afk until Dec) 2011-02-28 12:41:05 PST
(In reply to comment #26)
> I'd like to land this on the branches some late afternoon or evening (GMT-5)
> this week (but not this afternoon/evening, obviously); can we coordinate so
> that the comm-* changes are ready for someone to push (or they can even go in
> beforehand, right, and then I could land once I've heard they're in)?

I've just done this on comm-1.9.2:

http://hg.mozilla.org/releases/comm-1.9.2/rev/11a5d8031d8c

with a=me.

(just a straight landing of http://hg.mozilla.org/comm-central/rev/ce2bbf8d11fd).

You'll need one of the SeaMonkey team to do comm-1.9.1 as Thunderbird is no longer active there (EOLed).
Comment 28 Justin Wood (:Callek) 2011-02-28 18:43:23 PST
(In reply to comment #27)
> You'll need one of the SeaMonkey team to do comm-1.9.1 as Thunderbird is no
> longer active there (EOLed).

From a skim I don't see anything necessary for SeaMonkey to cope with this on the 1.9.1 branch, it looks like we used a different method there that is not conflicting.

If anything breaks for us there after this lands on branch, I feel safe to deal with it in a new bug.
Comment 29 Smokey Ardisson (offline for a while; not following bugs - do not email) 2011-03-01 09:29:13 PST
Thanks, gentlemen.  I'll aim for this evening.
Comment 30 Smokey Ardisson (offline for a while; not following bugs - do not email) 2011-03-01 18:51:08 PST
"Roll-up patch for the branches" landed on 

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/b66aad6d3eb4
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/087603a8766a
Comment 31 Daniel Veditz [:dveditz] 2011-03-04 10:15:12 PST
The "3.6.15" we're releasing today does not fix this bug, the release containing this bug fix has been renamed to "3.6.16" and the bugzilla flags will be updated to reflect that soon. Today's release is a re-release of 3.6.14 plus a fix for a bug that prevented many Java applets from starting up.

Note You need to log in before you can comment on or make changes to this bug.