383775 - Make tinderbox work with MozillaBuild

Status: RESOLVED FIXED

(Webtools Graveyard :: Tinderbox, defect)

Description

[Benjamin Smedberg]

Attachment: Make tinderbox client work with mozillabuild, rev. 1

This patch is what I needed to make tinderbox work with MozillaBuild. Quick, let's check it in now while the tree's closed!

Comment 1

[J. Paul Reed [:preed]]

Comment on attachment 267740 [details] [diff] [review]
Make tinderbox client work with mozillabuild, rev. 1

>Index: build-seamonkey-util.pl
>===================================================================
>RCS file: /cvsroot/mozilla/tools/tinderbox/build-seamonkey-util.pl,v
>retrieving revision 1.358
>diff -u -4 -r1.358 build-seamonkey-util.pl
>--- build-seamonkey-util.pl	25 May 2007 13:29:10 -0000	1.358
>+++ build-seamonkey-util.pl	8 Jun 2007 19:20:04 -0000
>@@ -324,8 +324,11 @@
>     if ($Settings::OS =~ /^WIN/) {
>         $host =~ tr/A-Z/a-z/;
>         $Settings::TieStderr = "" if $Settings::OS eq 'WIN98';
>     }
>+    if ($Settings::OS =~ /^MINGW/) {
>+      $Settings::OS = 'WINNT';
>+    }

Would it make sense to stick this block above the Windows block, so $host gets munged like it would under windows?



>-	      $ssh_opts = "-i $ENV{HOME}/.ssh/aus";
>-	      $scp_opts = $ssh_opts;
>+             $ssh_opts = "-i $ENV{'HOME'}/.ssh/aus";
>+             $scp_opts = $ssh_opts;

Why change the quoting around home? Is this a mingw requirements?

>-    push(@cmds,"rsync -av -e \"ssh $ssh_opts\" $upload_directory/ $Settings::ssh_user\@$ssh_server:$remote_path/$short_ud/");
>+    push(@cmds,"scp -r -p $scp_opts $upload_directory/. $Settings::ssh_user\@$ssh_server:$remote_path/$short_ud/");

Is there no rsync in Mingw?

>+sub unix2dos {
>+  my ($path) = @_;
>+  local($/, *FH);
>+  open(FH, "+<$path");
>+  binmode(FH);
>+  my $data = <FH>;
>+  $data =~ s/(?<!\r)\n/\r\n/g;
>+  seek(FH, 0, 0);
>+  print FH $data;
>+  close FH;
>+}

I'd like to see some error handling here... it can even by like the rest of tbox's error handling, i.e. "do_something() or die("aww!)," but it'd be good to check the open at teh very least.)

>-  if ($cachebuild && $Settings::crashreporter_buildsymbols) {
>+  if ($cachebuild && $Settings::airbag_pushsymbols) {

Did you mean to change the names here? If so, you'll need to change the names in tinder-defaults.pl too, and we'll have to patch things for the running tboxen.

(To be clear, this last point is the only reason I'm r-ing this; the rest were just questions, although if we could get rsync back, that'd be nice; I'm worried about the semantics of cp/scp not being the same as rsync, and biting us in some new and exciting way... but maybe I'm just having a case of too much "bit once, twice shy."

Comment 2

[Benjamin Smedberg]

There is no rsync in mozillabuild and there is no prospect of getting one: rsync makes use of forking and inherited filehandles extensively, which is basically impossible without the cygwin emulation layer.

No, I did not mean to change s/crashreporter/airbag/ that was a mistake.

I changed the quoting of $env{HOME} because that syntax causes strict errors: we should always quote literal values passed as dictionary lookups.

I'll have a new patch up shortly.

Comment 3

[Benjamin Smedberg]

Attachment: Make tinderbox client work with mozillabuild, rev. 1.1

Updated per queries

Comment 4

[J. Paul Reed [:preed]]

Comment on attachment 268392 [details] [diff] [review]
Make tinderbox client work with mozillabuild, rev. 1.1

(In reply to comment #2)

> I changed the quoting of $env{HOME} because that syntax causes strict errors:
> we should always quote literal values passed as dictionary lookups.

I read the diff wrong; quoting makes me happy. But most of tbox doesn't do it... I'm surprised that strict woudln't comoplain elsewhere.

>-  if ($cachebuild && $Settings::crashreporter_buildsymbols) {
>+  if ($cachebuild && $Settings::crashreporter__pushsymbols) {
>     TinderUtils::run_shell_command("make -C $objdir buildsymbols");
>-  }
>-  if ($cachebuild && $Settings::crashreporter_pushsymbols) {
>     TinderUtils::run_shell_command("make -C $objdir uploadsymbols");
>   }

Two things here:

Typo on the double underscore?

Also, I think I was the one that originally asked ted to separate out building of symbols vs. pushing them. Is there a reason to revert that change, or does buildsymbols now also push them as well? If not, I'd like to keep the separation (just because we've got a bunch of tinderboxen using those variables, and I'd rather ignore having to go change them or dealing with failures on boxen that have _buildsysmbols set, but not _pushsymbols for some reason, and that might fail now).

Also, we'll really have to test the rsync replacements... still a bit worried about that, but I understand the motivation to move away. Wish mingw had an rsync port.

Otherwise, looks good.

Comment 5

[Benjamin Smedberg]

Fixed on trunk. cf did a quick check of the nightlies and said they appeared to be in the correct locations and have the correct permissions and all that good stuff, so I'm crossing my fingers that this actually didn't break anything. Woot, a first!

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

Not so fast! ;-)

pacifica-vm isn't publishing Firefox Moz1.8 hourlies, which impacts the perf test box bl-bldxp01. It started the nightly build before the changes landed, so it may be a more general problem.

No clues that I can see in the build log:

ssh -2 -l cltbld stage.mozilla.org mkdir -p /home/ftp/pub/firefox/tinderbox-builds/pacifica-vm-mozilla1.8

scp -r -p -oProtocol=2 /cygdrive/c/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/pacifica-vm-mozilla1.8/packages/. [e-mail]:/home/ftp/pub/firefox/tinderbox-builds/pacifica-vm-mozilla1.8/

ssh -2 -l cltbld stage.mozilla.org chmod -R 775 /home/ftp/pub/firefox/tinderbox-builds/pacifica-vm-mozilla1.8

What is the . on the end of the source path for the scp command for ? There are up to date hourlies of sensible size in that dir.

sha1sums on stage and the tbox are different, so it doesn't look like a failure to update the timestamp.

[Possibly also moz180-win32-tbox (Moz1.8.0 & Moz1.8.0-l10n) and cerberus (Trunk-l10n, Moz1.8-l10n), but it's hard to be patient while they take forever to cycle.]

Comment 7

[Benjamin Smedberg]

scp has this nice/weird convention like so:
scp -r somedir/ remote:/otherdir
you end up with remote:/otherdir/somedir/stuff

but if you append the dot:
scp -r somedir/. remote:/otherdir
you end up with remote:/othedir/stuff

Which is what we want in this case.

Comment 8

[Nick Thomas [:nthomas] (UTC+12)]

Interesting. It seems that the older version of ssh/scp on pacifica-vm doesn't support the . notation, as neither nightlies or hourlies are pushed to the FTP server (which has unfortunate side-effects on nightly updates). 

I have made a local change which replaces the . with *, which pushes the files we really need but not the xforms.xpi within windows-xpi the directory. Suggestions for a better fix welcome.

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: Special case Cygwin for the push to FTP server

This implements Benjamin's suggestion for working around the old scp on pacifica-vm, which is to go back to using rsync on Cygwin.

Comment 11

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #10)
> Created an attachment (id=270916) [details]
> Special case Cygwin for the push to FTP server

Landed, with spot checks that it's working properly for 1.8 branch/trunk tinderboxes for Firefox/Thunderbird.

Comment 12

[[e-mail] (:bhearsum)]

Mass re-assign of MozillaBuild bugs into mozilla.org:MozillaBuild

Comment 13

[[e-mail] (:bhearsum)]

Whoops. Didn't mean to move this one.