330374 - 7 changes for Tinderbox

Status: RESOLVED FIXED

(Webtools Graveyard :: Tinderbox, defect)

Description

[Mark Mentovai]

These were changes originally included in a patch in bug 329801.  Now, they get their own bug.

- add ReleaseWithBuildNumber (default on), which uploads to yyyy-mm-dd-hh
  directories using the build id instead of the upload time (the build id may
  be earlier and makes it difficult to correlate build ids with files on the
  ftp server)
- allow Camino to build like Firefox and Thunderbird and the other grown-ups,
  now that it, too, is all grown up
- allow tests to find the right profile when the app name is DeerPark
- fix Tdhtml to use its own timeout and not the timeout for Tp
- add an HourlyOnly option to always suppress official builds
- change the notification address to build-announce@m.o
- when uploading, mkdir -p the full path needed

Comment 1

[Mark Mentovai]

Attachment: Lucky 7

FC_* changes not included.  Talkback Makefiles will be updated for Camino instead.

Comment 2

[J. Paul Reed [:preed]]

Comment on attachment 214939 [details] [diff] [review]
Lucky 7

Is there a reason to do $ReleaseWithBuildNumber? Shouldn't this just be the default?

Comment 3

[Mark Mentovai]

(In reply to comment #2)
> Is there a reason to do $ReleaseWithBuildNumber? Shouldn't this just be the
> default?

I guess it doesn't have to be configurable.  I wasn't sure if all of the clients that upgraded would want to move to the new behavior immediately or not.

Comment 4

[Mark Mentovai]

Comment on attachment 214939 [details] [diff] [review]
Lucky 7

+$mac_bundle_path = "/browser/app";

This should still be commented out:

+#$mac_bundle_path = "/browser/app";

We don't want to provide a default for that, there's an assertion in post-mozilla-rel that bails out if it's undefined, to force the user to config it manually.

Comment 5

[J. Paul Reed [:preed]]

(In reply to comment #3)
> I guess it doesn't have to be configurable.  I wasn't sure if all of the
> clients that upgraded would want to move to the new behavior immediately or
> not.

I consider that to be a bug, so go ahead and make it a non-option.

The only concern I have, then, is what to do if we can't get a build number (i.e. if get_buildid fails for some reason; can we put an assert or something in there?)

Other than that, the rest of the changes looked fine.

I did have one question: what's the difference between $HourlyOnly and $ReleaseToDated?

Also, a couple of other minor nits: you've got "if ($Settings::BinaryName =~ /^Camino/) {" in some places, but if ($Settings::BinaryName eq 'Camino') {" in others. Is there a reason for the difference?

And when chaging the email addr to build-announce, can you use single quotation marks, so the address is readable (then you don't have to escape the '@').

Comment 6

[Mark Mentovai]

> I did have one question: what's the difference between $HourlyOnly and
> $ReleaseToDated?

$HourlyOnly suppresses $cachebuild.  With $HourlyOnly set, you're keeping yourself out of official-release business.  You can't get that behavior by tuning $ReleaseToLatest and $ReleaseToDated: eventually, $cachebuild will go true, and you'll clobber and potentially make talkback and aus and l10n and other stuff you wouldn't get in an ordinary hourly.  It's possible to turn most of that stuff off in the config, but I wanted a single big switch that would keep me out of business I didn't want to be in for the Fx and Tb test builds.

> Also, a couple of other minor nits: you've got "if ($Settings::BinaryName =~
> /^Camino/) {" in some places, but if ($Settings::BinaryName eq 'Camino') {" in
> others. Is there a reason for the difference?

My personal preference is for |eq|, but I generally follow local convention.  That's what I was doing here.  Since the conventions vary throughout the tinderbox scripts, so did what I had to follow.

Comment 7

[J. Paul Reed [:preed]]

(In reply to comment #6)

> $HourlyOnly suppresses $cachebuild.  With $HourlyOnly set, you're keeping
> yourself out of official-release business.  You can't get that behavior by
> tuning $ReleaseToLatest and $ReleaseToDated: eventually, $cachebuild will go
> true, and you'll clobber and potentially make talkback and aus and l10n and
> other stuff you wouldn't get in an ordinary hourly.  It's possible to turn most
> of that stuff off in the config, but I wanted a single big switch that would
> keep me out of business I didn't want to be in for the Fx and Tb test builds.

Alright; that sounds reasonable. However, it's not clear from that variable name that that's what that is.

Is there a clearer name we could use? Maybe $OfficialBuildMachinery = 0; or something?

> My personal preference is for |eq|, but I generally follow local convention. 
> That's what I was doing here.  Since the conventions vary throughout the
> tinderbox scripts, so did what I had to follow.

Understood. I prefer |eq| too, and think it's a better convention than pattern matching (for this specific instance). Feel free to change it to |eq|.

If you blow away the $ReleaseWithBuildNumber "option," r=preed.

Comment 8

[Mark Mentovai]

Attachment: Lucky 7.1

Addresses all comments.  When the build ID can't be retrieved, the tinderbox will catch fire.

Comment 9

[J. Paul Reed [:preed]]

Comment on attachment 214956 [details] [diff] [review]
Lucky 7.1

Ship it!

Comment 10

[Mark Mentovai]

Checked in, including a typo fix that slipped into 7.1.  I HATE this keyboard.  Too bad my good keyboard is so LOUD.