Closed Bug 431270 Opened 16 years ago Closed 16 years ago

Change build process to generate unique BuildIDs

Categories

(Release Engineering :: General, defect, P3)

Product:
Release Engineering
Component:
General
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: joduinn, Assigned: joduinn)

References

Details

Attachments

(4 files, 4 obsolete files)

make-platformini.py - It gives the buildID all the way to the second
870 bytes, patch
ted
: review+
Details | Diff | Splinter Review
application.ini - You can see the result of using the previous patch
2.01 KB, text/plain
Details
[checked in] hg version of make-platformini.py & config/version_win.pl to generate BuildID with up to the second resolution
1.36 KB, patch
Details | Diff | Splinter Review
[checked in] transfer.py needs to handle MMSS
698 bytes, patch
bhearsum
: review+
Details | Diff | Splinter Review 
Our buildID is currently of the format: "2008032619". This represents "YYYYMMDDhh" of when the build reached a particular step part-way through the build process.

1) As our incremental builds take under an hour, its possible to get two builds produced one after the other with the same buildID.

2) Currently, the buildid is generated someway into the build, and is based on the current time when that part of the build process is reached. It sometimes, but not always, represents the hour in which the build started. This means that as builds on each O.S. runs at different speeds, its possible to start the linux+mac+win32 builds all at the same time, and end up with these builds containing different buildIDs, even though it might be expected they have the same buildID.


Can we make the following changes:
1) change the BuildID to detailed enough resolution so that BuildID collisions like this can be avoided? If BuildID needs to be based on time, maybe going to YYYYMMDDhhmmss might be enough? Or instead of using more accurate time, maybe we could use some repo-specific revision# or changeSet# to guarantee no collisions?

2) change when BuildID is generated? Instead of calculating BuildID using current time when part-way through a build, how about calculating BuildID at the start of the build process? ...or calculating BuildID part-way through the build using a pre-determined value, so the speed of the build does not impact the value of the BuildID. It seems important to me that builds starting at the same time, using the same source repo cutoff, should have the same BuildID. 

Extra Wish list: 
- it would be nice if the BuildID trended the same direction, so that newer BuildIDs were larger then older BuildIDs. I dont think BuildIDs need to be sequential, but trending the same direction seems useful when plotting on graph server or looking on ftp.m.o for the latest build.

- it would be nice if the BuildID could be easily correlated to what changes are included in a given build, when looking on bonsai / lxr. This would simplify work for people trying to figure out what changes are in a given build.
For the "different buildID's for the same start" issue, here are some details on what the tinderbox based systems are doing:

* a unix-style timestamp is collected when the cycle starts, and passed on to the tinderbox server in the build-start email
* if it's a clobber build, then the source and objdir will be deleted. Elapsed time varies on platform, with Mac fastest & Windows slowest (I/O dependent)
* the timestamp is used to checkout the source code from CVS, via MOZ_CO_DATE, so the source pulled would be the same if two disparate machines started at the same time. Elapsed time for a checkout also depends on I/O
* configure is called and then the build commences
* on trunk, it looks like the BuildID is generated by make-platformini.py, when building in mozilla/config. This is very early in the build, after configuring the main build and nspr, and is vulnerable to the total time since the cycle started
* IIRC, the Mac universal builds use the buildID from the ppc build, which is done first, via some trickery
* just noticed that for a Windows PGO build it appears that the buildID is updated for the second (optimizing) pass, and the second value used. I guess this is because it occurs in the same object directory

I'd be curious if there is some difference between the 1.8 branch and trunk (1.9), and where any change would be targeted at. (A build system change should be done in Core::Build Config).
(In reply to comment #1)
> * IIRC, the Mac universal builds use the buildID from the ppc build, which is
> done first, via some trickery
> * just noticed that for a Windows PGO build it appears that the buildID is
> updated for the second (optimizing) pass, and the second value used. I guess
> this is because it occurs in the same object directory

Hm, hadn't considered the PGO issue. We could feasibly make it use the same trickery as the universal build. In fact, with no code changes in the build system, we could set the build id to whatever we wanted (the build start timestamp, if you chose) by passing MOZ_BUILD_DATE=whatever on the make commandline, like:
make -f client.mk build MOZ_BUILD_DATE=2008042904

See http://lxr.mozilla.org/mozilla/source/config/Makefile.in#111

For 1.9 we cannot change the buildid with extra information safely: buildids show up in the product UA string which is exposed to web content and can cause all sort of unexpected problems if the format changes so late in the cycle.

For mercurial repos I think we should just use the hex ID of the changeset: e.g.
06a3d0cd6aca

The only problem with this is if you had two builds with no intervening checkins, they would have the same BuildID.

But the problem we always have with buildid changes is that they aren't designed, and everybody has a different expectation about whether the buildid is actually supposed to encode any useful information.
The fact that the build id is currently a date is very useful to humans, even if the date is slightly questionable. Fixing the date to be more consistent would be nice, but swapping it out for a changeset id would have a negative impact, I think. Making that changeset ID available somewhere in the product would be good, since it would allow you to pinpoint the exact source that your product was built from.

This is specifically a problem for talos.  Currently, talos timestamps test results with test time for its reports to the graph server.  This means that the results end up graphed an hour or two (or more) after the build that they are testing was produced - making it hard to track regressions or determine a bonsai range.

We have some open bugs about changing talos from using test time to build time.  It is simple to pull the buildid but since it isn't granular enough to be unique I end up with collisions in the graph server from a single machine testing two different builds that ended up with the same buildid.

I can understand not wanting to touch the current buildid because it is entrenched in the system in various ways, but our testing infrastructure would really benefit from the addition of a build time stamp in the application.ini that would be unique and meaningful in terms of change tracking.

The changeset used for hg that has no meaning in terms of a time stamp is a whole 'nother problem for running tests and reporting data.  Our infrastructure for testing and reporting was built upon the assumption that you would graph results in a sequential manner.  We'd be stuck with using test time as the timestamp which again, raises problems in figuring out regression ranges.  If we want to go in this direction we'll have to put significant effort in a graph server redesign to remove timestamps as the main means of determining browser contents.

It's not hard to add extra information to application.ini for talos.

We could even add the specific checkout-by-date string we use for pulling, so that Talos knows what exact sources were pulled (and similarly, the mercurial changeset ID). But I don't think we should mess with the buildid format if that's all we need.

I'll file a separate bug about getting build-graph to display the mercurial changeset ID of graph points, since it's pretty clear we're going to want that.
That sounds fair - leave the buildid alone and provide the information needed by the testing infrastructure in another variable.  I heartily approve this plan.
(In reply to comment #1)
> For the "different buildID's for the same start" issue, here are some details
> on what the tinderbox based systems are doing:
[snip]
> * on trunk, it looks like the BuildID is generated by make-platformini.py, when
> building in mozilla/config. This is very early in the build, after configuring
> the main build and nspr, and is vulnerable to the total time since the cycle
> started
> * IIRC, the Mac universal builds use the buildID from the ppc build, which is
> done first, via some trickery
> * just noticed that for a Windows PGO build it appears that the buildID is
> updated for the second (optimizing) pass, and the second value used. I guess
> this is because it occurs in the same object directory

(In reply to comment #2)
> (In reply to comment #1)
> Hm, hadn't considered the PGO issue. We could feasibly make it use the same
> trickery as the universal build. In fact, with no code changes in the build
> system, we could set the build id to whatever we wanted (the build start
> timestamp, if you chose) by passing MOZ_BUILD_DATE=whatever on the make
> commandline, like:
> make -f client.mk build MOZ_BUILD_DATE=2008042904
> 
> See http://lxr.mozilla.org/mozilla/source/config/Makefile.in#111
> 

Does this mean that a change to Makefile.in and to make-platformini.py would solve the "builds started at same time get different buildID" situation?
I believe that if we changed the way we called the build target (in the buildbot harness), we could ensure that Build IDs always reflected the checkout time.

(In reply to comment #3)
> For mercurial repos I think we should just use the hex ID of the changeset:
> e.g.
> 06a3d0cd6aca
> 
> The only problem with this is if you had two builds with no intervening
> checkins, they would have the same BuildID.
> 

(In reply to comment #4)
> The fact that the build id is currently a date is very useful to humans, even
> if the date is slightly questionable. Fixing the date to be more consistent
> would be nice, but swapping it out for a changeset id would have a negative
> impact, I think. Making that changeset ID available somewhere in the product
> would be good, since it would allow you to pinpoint the exact source that your
> product was built from.
> 

1) I agree with Ted. Human readable dates are important for developers trying to quickly understand if their change landed last night caused a regression. If we use changesets, we'll need to somehow, somewhere, work backwards to generate the timestamps corresponding to those changesetIDs so we can graph them in a human understandable form. 


2) I think we need a solution that also works for CVS, not just mercurial, given how long we will need to support 2.x, 3.x releases after the release of 3.next/moz2. 

3) To me, it feels that sequencing is important. I dont think we have to 
enforce strict linear 1,2,3 sequence, but knowing that larger buildIDs mean newer builds seems very useful to me, and I dont believe we get that if we use mercurial changesets.

4) We need to consider situations where the code does not change, but we rebuild. One example would be redoing win32 builds with PGO enabled for recent beta. The source did not change, but we needed a different buildID to distinguish between the wrong (nonPGO) build and the right (PGO-enabled) build. If we used changesetIDs, these two builds would have the same BuildID.


Therefore I dont think mercurial changesetIDs make good BuildIDs.


(Separate to all this discussion, if you want to put mercurial changesetIDs into application.ini, for other unrelated reasons, thats totally fine by me.)
> 1) I agree with Ted. Human readable dates are important for developers trying
> to quickly understand if their change landed last night caused a regression. If
> we use changesets, we'll need to somehow, somewhere, work backwards to generate
> the timestamps corresponding to those changesetIDs so we can graph them in a
> human understandable form. 

What does the buildid "mean" in that case? This build started at time YYYYMMDDHHMM? That's useful information, and suits the requirements that buildids be roughly sequential. That doesn't really tell you much of anything about what set of sources were used to produce that build...

For "graphs", do you mean Talos graphs? For Talos, I think it's much more important for *users* that we match build results against source information than the build date... so the buildid is mostly useless: we either want the CVS checkout-by-date stamp, or the mercurial changeset ID. Of course the talos machinery internally may also want an ever-increasing unique stamp to make database operations practical.

joduinn, what resolution are you looking for in this bug?

A: changing the current buildid to have extra resolution. WONTFIX for 1.9, but we could do that for moz2
B: Add a separate, more-unique buildid with minutes (and other info?)
C: unify the build id so that three builds kicked off at the same time have the same buildid: this sounds like marginal value to me, but shouldn't be hard to do with the existing unify-builddate machinery
I like the idea of using changeset id for moz2 and beyond. If not as the actual buildid, as an extra data-point we can use to say exactly what we're using and testing.
(In reply to comment #9)
> I believe that if we changed the way we called the build target (in the
> buildbot harness), we could ensure that Build IDs always reflected the checkout
> time.

Filed bug#431905 to track this "have all builds generate with same BuildID" issue. This is orthogonal to the rest of the discussion in this bug about changing BuildID granularity/format.

It seems that we all have different ideas of what exactly is being tracked by BuildID, so let me backup a little. 

I'm interested in tracking build-start-time. This is separate to (or maybe in addition to?) source-code-cutoff-time. 


Main points:

1) build-start-time and source-code-cutoff-time
Using build-start-time is the only way I know to identify what specific build a user has. Using source-code-cutoff-time would not be sufficient if we recompile with no source changes. For example, FF3.0beta4 win32 builds were built without PGO, and then the *same* source rebuilt *with* PGO. Using build-start-time would clearly identify whether the user had a non-PGO or PGO build. Using source-code-cutoff-time would give us the same BuildID for both the non-PGO and PGO build. I therefore believe that the user-visible About box should display build-start-time, not source-code-cutoff-time. 


2) Uniqueness:
Currently, BuildID does not uniquely identify the running product, even though that is the widespread assumption. We use BuildID in the About box, the UA string, ftp directory names, update snippets, etc because we assume BuildID uniquely identifies a given set of bits.

To be precise, BuildID identifies in which hour we reached a certain point part-way through the build process, which is *not* unique. 

If we ever generate two builds within the same hour-resolution, we get two builds with the same BuildID. This happens a few times every day with depend/hourly builds. It also happened last night with full/nightly builds. A clobber happened immediately after a nightly build, so we ended up with two nightly mac builds with the same BuildID, landing in the same BuildID-named-directory, which then messed up the nightly updates, requiring manual repair work. For details, see bug#431866.



3) Cleanup infrastructure workarounds
I feel its important to change our current BuildID because in our current situation, we have the unfortunate side effect of different people having to work around the BuildID mismatch in different ways. For example:
- Talos & graph-server use test-run-start-time-per-machine as a unique identifier, because BuildID in its current form is not unique. This has the unfortunate side effect that developers cannot easily tell what changes are in what test run. Or for that matter what build is used in a given test run. Currently, its only possible to see that a test started at a certain time, without knowing which build is being tested at that time.
- Getting talos showing up on tinderbox means we then have to fake times even further, so that Talos test runs line up with the corresponding builds on tinderbox waterfall. 
- Getting buildbot to mimic tinderbox waterfall means more date/time fudging. 
- Active open bugs #419487, #400107, #410869 are trying to minimise side effects of these workarounds, but not fixing the root cause of the problem.



My proposal would be:
- extend the BuildID displayed in the About box, and the UA string, from YYYYMMDDHH to YYYYMMDDHHMMSS.
- the time being represented in the extended BuildID should be the build-start-time, not source-code-cutoff-time.
- fix anywhere in Release Infrastructure that uses BuildID as a "unique identifier" to handle the new BuildID (ftp directory, update snippets,...).



Once we resolve "what" we are doing, we can have a separate followon discussion about "where" we are doing it. Decisions about rollout on each branch needs to be balanced with the repercussions to graph-server, tinderbox and buildbot of not rolling out these changes. 

ps: Its fine with me if we want to *also* add source-code-cutoff-time to the application.ini. If its helpful to developers, lets do it. Whether source-code-cuttoff-time should be in one format for cvs-based releases, and a different format for hg releases is not something I have strong opinions on.

(In reply to comment #14)

> - Active open bug#419487, bug#400107, bug#410869 are trying to minimise side
> effects of these workarounds, but not fixing the root cause of the problem.
Forgot also: bug#291167 
Speaking with joduinn over IRC, we agreed that it's probably too risky to try and cram this into the Firefox 3 release cycle.

Instead, I suggested that we can start playing with this on new trunk (hg) and letting it bake there, and then if that works out, we can back-port to 1.9.0.x or 1.9.1 depending on where we're at with those releases.
Another option: Keep the format the same ("YYYYMMDDHH"), but make HH be in 15 minute increments instead of hour increments. That would give 24x4 = 96 possible values in the hour field. For example, "2008013125" would be 2008 Jan 31 at 6:15. An example of the the greatest value for the field would be "2008013195", which would be 2008 Jan 31 at 23:45.
For l10n repacks, having the source stamp in application.ini would be really helpful. For hg, that's probably a hex key, for cvs, a timestamp. I guess we're free to use different names for the entries to work around ambiguity when we backport to 1.9
(In reply to comment #19)
> For l10n repacks, having the source stamp in application.ini would be really
> helpful. For hg, that's probably a hex key, for cvs, a timestamp. I guess we're
> free to use different names for the entries to work around ambiguity when we
> backport to 1.9
> 
Agreed, checkout time for l10n repacks would be really necessary since before doing a locale's repack we have to checkout firefox's source code.
The way we do repackages (*currently*) is that we wget the latest en-US build and we checkout the latest source code (which is different from the checkout time of the binary). If we had the checkout time in application.ini we could checkout the same firefox/en-US time stamp as when that build happened.

---------
I am working on changing how we do l10n repackages (bug 434878) and part of that is getting the en-US source (bug 398954). I am doing this in buildbot and I can checkout the same source time stamp by setting a build property with "GenerateCvsCoDate"
|armenFactory.addStep(GenerateCvsCoDate)
|armenFactory.addStep(ShellCommand,
|    command = ["cvs","-d",cvsroot,"co","mozilla/client.mk"],
|    workdir = '.')
|armenFactory.addStep(ShellCommand,
|    command = ['make','-f','client.mk','checkout'],
|    workdir = 'mozilla',
|    env={'MOZ_CO_DATE':WithProperties("%(cvscodate)s"),   <-- here
|        'MOZ_CO_PROJECT':'browser'})
|#Other steps to create an en-US build
|armenFactory.addStep(ShellCommand,
|    command = ['make','-f','client.mk','l10n-checkout'],
|    workdir = 'mozilla-l10n',
|    env={'MOZ_CO_DATE':WithProperties("%(cvscodate)s"),   <-- and here
|        'MOZ_CO_PROJECT':'browser'})
|#Rest of steps to get a locale

If this fix works on buildbot I doubt that I would need to have the checkout timestamp in application.ini for "regular repackages", BUT I would TOTALLY need it for cases in which I do have to create an l10n repacakge WITHOUT building first. I would need the checkout time for this.

Now that Firefox3.0 has shipped, and we're running on hg, I'm revisiting this.

1) Armen is experimenting with changing make-platformini.py to handle %Y%m%d%H%M%S in the timestamp. 

2) Follow on from comment#15. Having application.ini contain the CVS-cutoff used or hg changesetID used is already covered in bug#407319.
Component: Release Engineering: Future → Release Engineering
Priority: -- → P3
I have tested that it sets the value all the way to the second in application.ini (to be attached)

I would also like to test what would happen if I do set the MOZ_BUILD_DATE with my patch
FWIW, changing the format of the BuildID is likely to break things in various places.

Please ignore the SourceStamp line, that is related to bug 407319.

You can see that it goes all the way to the second.

You can also see this paste from the log of the build
| c++ -o nsBrowserApp.o -c -I../../dist/include/system_wrappers -include /Users/mozilla/Projects/default/bug407319/tree/mozilla/config/gcc_hidden.h -DAB_CD=en-US -DAPP_VERSION="3.0.1pre" -DAPP_UA_NAME="Minefield" -DGRE_MILESTONE=1.9.0.1pre -DGRE_BUILDID=20080625005532

Forgive me if I have been working after hours :)
(In reply to comment #23)
> FWIW, changing the format of the BuildID is likely to break things in various
> places.
> 
How can we evaluate this? Anyone wants to add specific examples?

There is one example I know: Axel has some code that out of a BuildID interpolates an approximate checkout time (rounded to minute 0 of the hour) which going all the way to second could break it but I am working on adding source stamp to application.ini which would kill 2 birds at one shot

This bug does not block bug 434878 anymore. It does not affect L10n (unless I am missing anything) since this is the way to go (bug bug 407319) for us.

To test the patch attached might be good to try it in the try server, put the 3 builds through unit test and through graph server

Leaving this bug to discussion - not asking for review until a final decision is made
No longer blocks: 434878
Attachment #326745 - Flags: review?(ted.mielczarek)
Comment on attachment 326745 [details] [diff] [review]
make-platformini.py - It gives the buildID all the way to the second

I'm fine with this patch, but it can't land without fixing a few other places. I'm not going to do an exhaustive search, but bsmedberg mentioned the dependencies of bug 383167, and from a brief survey, I can find that the changes from bug 386740 would need to be patched, and we might want to revisit the discussion in bug 392204 if we're going to lengthen the Build ID further.
Attachment #326745 - Flags: review?(ted.mielczarek) → review-
Attached patch config/version_win.pl (obsolete) — — Splinter Review 
This needs to land at same time as make-platformini.py patch.
Attachment #328385 - Flags: review?(ted.mielczarek)
Comment on attachment 328385 [details] [diff] [review]
config/version_win.pl 

Looks good, r=me on these two patches combined, provided you have signoff from alice and bhearsum that this isn't going to break anything in Talos/buildbot.
Attachment #328385 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 326745 [details] [diff] [review]
make-platformini.py - It gives the buildID all the way to the second

r=me on the combination of these two patches.
Attachment #326745 - Flags: review- → review+
Comment on attachment 326745 [details] [diff] [review]
make-platformini.py - It gives the buildID all the way to the second

re-marking for r? at same time as patch for version_win.pl
Attachment #326745 - Flags: review+ → review?(ted.mielczarek)
Attachment #326745 - Flags: review?(ted.mielczarek) → review+
Another thought: dates in build IDs are good, and monotonicity is good, but yes, more resolution would be great. So how about:

2008070823 (at 23:37)
->
5807082337

I.e. take out unused figures in the year, shift everything up a bit, add minutes in to the spare space, and change the initial 0 to a 5 so that the result is still larger than all previous build IDs. (When we get to 2010, it would change to 6, and so on. The UK number plate system uses a similar scheme.)

That gets you to-the-minute resolution, at least, without changing the build ID format and while preserving monotonicity and human-readability.

Gerv
(In reply to comment #33)
> Another thought: dates in build IDs are good, and monotonicity is good, but
> yes, more resolution would be great. So how about:
> 
> 2008070823 (at 23:37)
> ->
> 5807082337
> 
> I.e. take out unused figures in the year, shift everything up a bit, add
> minutes in to the spare space, and change the initial 0 to a 5 so that the
> result is still larger than all previous build IDs. (When we get to 2010, it
> would change to 6, and so on. The UK number plate system uses a similar
> scheme.)
> 
> That gets you to-the-minute resolution, at least, without changing the build ID
> format and while preserving monotonicity and human-readability.
> 
> Gerv
> 

To be honest, I think this would break more tools and scripts than simply appending it. I'm sure we have things that just grab the first 10 digits and assume that's the buildid. This format would break those, for obvious reasons ;-). Additionally, the format you propose is less human readable IMHO (admittedly, I've never seen a method like this used before so take it FWIW).

I still favour appending MMSS
We definitely have tools that expect the format to (at least start with) YYYYMMDDHH. Also, I happen to like the human-readability (and I think our testers do too) and I think your suggestion sacrifices a bit of that. We've done a survey, and it doesn't look like we'll break any of our known tools. There certainly may be other things that get broken, but those are our unknown unknowns, y'know.


Not having the YYYYMMDDHH format will greatly hinder nighty testers and make it almost impossible for us to pinpoint which builds users are using.  As of right now, we just glance and can tell the exact build within a second or two.  Why would someone what to make it some random format that would have to be decoded each time we examine a build id is beyond me.  KISS is a great explanation to sum up my point.
I'm happy to land this *if* we decide where it needs to land. Did we come to a consensus in #build over this re: 3.0 vs. 3.1?
I wasn't aware we were considering landing it anywhere other than 1.9.1. I don't see any good reason we'd do this on the branch.
Assignee: nobody → armenzg
This is the patch for mozilla-central
Attachment #328601 - Flags: review?(ted.mielczarek)
Comment on attachment 328601 [details] [diff] [review]
make-platformini.py & config/version_win.pl to generate BuildID up to the secong (hg patch)

Looks the same to me. :)
Attachment #328601 - Flags: review?(ted.mielczarek) → review+
John asked me to land this and said that comment #28 and comment #30 had been checked/addressed. There is test bustage currently so I'm out of time to do so.

Looking at attachment 328601 [details] [diff] [review], seems to me that the change to version_win.pl is going to return a float instead of an int, which might make Windows spew. Did you try making a full windows build with this patch in place ?

Also, is the SourceStamp line in attachment 326746 [details] for real or from another patch? It looks a little wonky.


(In reply to comment #41)
> John asked me to land this and said that comment #28 and comment #30 had been
> checked/addressed. There is test bustage currently so I'm out of time to do so.
> 
> Looking at attachment 328601 [details] [diff] [review], seems to me that the change to version_win.pl is
> going to return a float instead of an int, which might make Windows spew. Did
> you try making a full windows build with this patch in place ?
> 
Yes, I did try it in the try server
http://tinderbox.mozilla.org/showbuilds.cgi?tree=MozillaTry&maxdate=1215475439&mindate=1215466817&legend=0&norules=1
The builds:
https://build.mozilla.org/tryserver-builds/2008-07-07_14:41-armenzg@mozilla.com-1215466817/
https://build.mozilla.org/tryserver-builds/2008-07-07_13:24-armenzg@mozilla.com-1215462194/

> Also, is the SourceStamp line in attachment 326746 [details] for real or from another
> patch? It looks a little wonky.
> 
From another patch, forget about the SourceStamp line; it was to show the BuildID to reach up to the second


I tried to look back in my comments and I can't see that I mentioned that I did not want this bug since it doesn't affect in my L10n project but it seems like in Mozilla world - you touch it - you own it :P



The "File version" property does seem to be fine in that build (something like 1.9.0.3110), although it seems like bad form to rely on Microsoft utility to sanitize our input.
This replaces the previous patch, which was already r+'d. Discussion in irc today about the returnValue from daysFromBuildID revealed that one change wasnt needed. Rather then post "as landed" patch after the fact, this is a new patch of the proposed changes.
Attachment #328601 - Attachment is obsolete: true
(In reply to comment #30)
> (From update of attachment 328385 [details] [diff] [review])
> Looks good, r=me on these two patches combined, provided you have signoff from
> alice and bhearsum that this isn't going to break anything in Talos/buildbot.
> 

Sorry it took me so long to respond to this. There's only one place I could find that needs a patch:
http://mxr.mozilla.org/mozilla/source/tools/buildbotcustom/steps/transfer.py#151

I looked around by search for '%Y%m%d' in mozilla/tools/buildbotcustom and mozilla/tools/buildbot-configs

http://mxr.mozilla.org/mozilla/source/testing/performance/talos/PerfConfigurator.py might need to be patched too -- what do you think, Alice?
Talos isn't using the built-in buildid from the browser anywhere.  I do have a patch in the works to handle buildid's of the form YYYYMMDDHHMMSS but there's no rush to push it out.
Does Talos not run PerfConfigurator.py anymore?
Talos still uses PerfConfigurator, but it doesn't make use of the pull-buildid-from-browser option - mostly because there doesn't seem to be a way to pull the buildid from a 1.8 browser.
(In reply to comment #45)
> (In reply to comment #30)
> > (From update of attachment 328385 [details] [diff] [review] [details])
> > Looks good, r=me on these two patches combined, provided you have signoff from
> > alice and bhearsum that this isn't going to break anything in Talos/buildbot.
> Sorry it took me so long to respond to this. There's only one place I could
> find that needs a patch:
> http://mxr.mozilla.org/mozilla/source/tools/buildbotcustom/steps/transfer.py#151
Good catch, bhearsum! I only see transfer.py in cvs, not hg, is that expected? Meanwhile, I'll post a cvs patch for this. 


> http://mxr.mozilla.org/mozilla/source/testing/performance/talos/PerfConfigurator.py
> might need to be patched too -- what do you think, Alice?
(In reply to comment #48)
> Talos still uses PerfConfigurator, but it doesn't make use of the
> pull-buildid-from-browser option - mostly because there doesn't seem to be a
> way to pull the buildid from a 1.8 browser.
ok, thanks for the confirm, Alice.
Attached patch transfer.py needs to handle MMSS (obsolete) — — Splinter Review 
Fix transfer.py to handle extra digits for MMSS. The need for this was spotted by bhearsum in comment#45.
Attachment #329935 - Flags: review?
Because this code is used by branches in both *cvs* and *hg*, we need to handle branches that do use new extended BuildID and also handles branches that continue to use the older shorter BuildID.
Attachment #329935 - Attachment is obsolete: true
Attachment #329943 - Flags: review?
Attachment #329935 - Flags: review?
Comment on attachment 329943 [details] [diff] [review]
[checked in] transfer.py needs to handle MMSS

You need to type my name in the 'Requestee' field when you request review :P
Attachment #329943 - Flags: review? → review?(bhearsum)
Comment on attachment 329943 [details] [diff] [review]
[checked in] transfer.py needs to handle MMSS

Looks fine.
Attachment #329943 - Flags: review?(bhearsum) → review+
(In reply to comment #49)
> (In reply to comment #45)
> > (In reply to comment #30)
> > > (From update of attachment 328385 [details] [diff] [review] [details] [details])
> > > Looks good, r=me on these two patches combined, provided you have signoff from
> > > alice and bhearsum that this isn't going to break anything in Talos/buildbot.
> > Sorry it took me so long to respond to this. There's only one place I could
> > find that needs a patch:
> > http://mxr.mozilla.org/mozilla/source/tools/buildbotcustom/steps/transfer.py#151
> Good catch, bhearsum! I only see transfer.py in cvs, not hg, is that expected?
> Meanwhile, I'll post a cvs patch for this. 
> 
Yes, buildbotcustom lives in CVS.
Assignee: armenzg → joduinn
I'm going to try to land these changes for John today, pending sheriff approval.
Attached patch transfer.py needs to handle MMSS (obsolete) — — Splinter Review 
Doing last minute checks before landing all these patches, I think the previously r+'d attachment#329943 [details] [diff] [review] is not enough, as it would always truncate buildID to 10digits. 

I think we need this instead, which will handle 10 digits in the cvs-trunk scenario, and 14digits in the hg-mozilla-central scenario.

Bhearsum, what do you think?
Attachment #329943 - Attachment is obsolete: true
Attachment #332644 - Flags: review?(bhearsum)
Comment on attachment 332644 [details] [diff] [review]
transfer.py needs to handle MMSS

Er, that's what we need to do here. Despite the poor naming of the method, the string returned here is used to generate dated directory names, such as, 2008/08/07/2008-08-07-23-mozilla-central. This behaviour should not change.
Attachment #332644 - Attachment is obsolete: true
Attachment #332644 - Flags: review?(bhearsum)
Attachment #329943 - Attachment is obsolete: false
Attachment #332644 - Flags: review?(bhearsum)
Comment on attachment 332644 [details] [diff] [review]
transfer.py needs to handle MMSS

see previous comments
Attachment #332644 - Flags: review?(bhearsum)
Attachment #329943 - Attachment description: transfer.py needs to handle MMSS → [checked in] transfer.py needs to handle MMSS
Comment on attachment 329778 [details] [diff] [review]
[checked in] hg version of make-platformini.py & config/version_win.pl to generate BuildID with up to the second resolution

http://hg.mozilla.org/index.cgi/mozilla-central/rev/e9f69e5e5a41
Attachment #329778 - Attachment description: hg version of make-platformini.py & config/version_win.pl to generate BuildID with up to the second resolution → [checked in] hg version of make-platformini.py & config/version_win.pl to generate BuildID with up to the second resolution
Comment on attachment 328385 [details] [diff] [review]
config/version_win.pl 

obsoleted by attachment#329778 [details] [diff] [review]. No need to calculate HH MM SS as part of partial days - we only care about complete days here.
Attachment #328385 - Attachment is obsolete: true
To fix the red on SeaMonkey need to get KaiRo (or suitable substitute) to do a "buildbot reconfig"
(In reply to comment #61)
> To fix the red on SeaMonkey need to get KaiRo (or suitable substitute) to do a
> "buildbot reconfig"

... after doing cvs update in the buildbotcustom checkout, to pick up attachment 329943 [details] [diff] [review].


Yep, we did "cvs update" and "buildbot reconfig" on the Firefox master just after landing this patch. And gozer did the same for Thunderbird master shortly afterwards. Those are all green.

Kairo: Sorry, I should have coordinated this better with you. Doing 
reconfig is sufficient, no need to do restart, so no need for downtime. Please ping me if you've any questions ok?

Anyway we can make this easier to read at a glance rather then taking a while to break the down the numbers?

Now:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080807135249 Firefox/3.0

Proposed:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/2008-08-07 13:52:49 Firefox/3.0

This: 2008-08-07 13:52:49
Vs  : 20080807135249

Proposed just make it so much easier to read instead of a jumble of numbers thrown together.
Even just delimit the date and time with a dot so we get something like:
 - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre)
Gecko/20080807.135249 Firefox/3.0

would be better
(In reply to comment #63)
> Kairo: Sorry, I should have coordinated this better with you. Doing 
> reconfig is sufficient, no need to do restart, so no need for downtime. Please
> ping me if you've any questions ok?

D'Oh. I'm on vacation and can't really do this before Monday, so new SeaMonkey nightlies until Tuesday :(
(In reply to comment #65)
> Even just delimit the date and time with a dot so we get something like:
> Gecko/20080807.135249 Firefox/3.0

OTOH, if you use “T”, it will be a bit closer to RFC 3339 and ISO 8601.
I explicitly did not want to change the format of BuildID any more neccessary, while still solving the "each build has unique BuildID" problem. There was enough concern that any change to BuildID would cause problems. In this specific situation, changing from 10digits to 14digits seemed the least disruptive thing to do... and there was plenty of discussion about even that. 

If folks would like a subsequent change of how BuildID is formatted, lets track that discussion in a separate bug. I've filed bug#449903 and seeded it with the suggestions posted above.
Coop landed these patches mid-Thursday PST. As far as I know, the only remaining open issue is the burning SeaMonkey builds... they need to follow the restart steps in comment#61, comment#62.

I'm leaving this bug open for the weekend to see if anything else breaks; will close if all still good after weekend.
Status: NEW → ASSIGNED
(In reply to comment #68)
> I explicitly did not want to change the format of BuildID any more neccessary,
> while still solving the "each build has unique BuildID" problem. There was
> enough concern that any change to BuildID would cause problems. In this
> specific situation, changing from 10digits to 14digits seemed the least
> disruptive thing to do...

What about 12 digits? I.e. wouldn't just adding minutes have solved the problem? Are there concerns that builds take less than a minute?
cvs updated buildbotcustom on SeaMonkey master, did a reconfig and forced build cycles so we see if it works. Despite me being back from vacation, I'm probably too tired to watch if it works out, but I guess it can't get worse than the current red.
(In reply to comment #71)
> cvs updated buildbotcustom on SeaMonkey master, did a reconfig and forced build
> cycles so we see if it works. Despite me being back from vacation, I'm probably
> too tired to watch if it works out, but I guess it can't get worse than the
> current red.
> 

"build" (hourly?) boxen are now all green. "nightly" boxen haven't run yet. Hope you had a nice holiday. Travel fatigue will hopefully subside after a good night's sleep. :-)
Yes, it's the hourlies, the "tinderbox-builds" directory trees are being filled again, and I'm running a build made a few hours ago:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a2pre) Gecko/20080810131042 SeaMonkey/2.0a1pre

I guess we'll have to wait until the "nightly" boxen decide to cycle before we can close this bug and mark it VERIFIED though?
John: generally we just mark bugs FIXED once all the relevant patches are checked in. If there are problems we can always reopen or someone can file a new bug and mark it blocking this one.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
verified fixed using Build identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080811020618 Minefield/3.1a2pre and Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a2pre) Gecko/20080811053724 Minefield/3.1a2pre.
Status: RESOLVED → VERIFIED
Depends on: 450973
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.