392579 - partial mar files not created since move to WINNT 5.2 fxnewref-win32- Dep Nightly

Status: VERIFIED FIXED

(Release Engineering :: General, defect, P1)

Description

[Jim Jeffery not reading bug-mail 1/2/11]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007081704 Minefield/3.0a8pre Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007081704 Minefield/3.0a8pre Firefox/3.0

No .mar files (partial update file) are being created since the move to the new build box WINNT 5.2 fxnewref-win32- Dep Nightly.



Reproducible: Always

Steps to Reproduce:
1. Help '-> Check for updates
2. No updates found
3. look at the nightly http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/ 
site and note that .mar files for Windows is not being created, but are for the other OS's.
Actual Results:  
No .mar file created so nightly testers can update to latest build.

Expected Results:  
.mar file created for partial update

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I see windows mar files for the 17th here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2007-08-17-05-trunk/

and here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/

This does seem bad, though (from http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1187349180.25640.gz&fulltext=1 at the end):

Pushing third-gen update info...
ssh -i /c/Documents and Settings/cltbld/.ssh/aus [e-mail] mkdir -p /opt/aus2/build/0/Firefox/trunk/WINNT_x86-msvc/2007081705/en-US
Warning: Identity file /c/Documents does not exist.
ssh: and: no address associated with hostname.
scp -i /c/Documents and Settings/cltbld/.ssh/aus /e/builds/tinderbox/Fx-Trunk-Newref/WINNT_5.2_Depend/mozilla/obj-fx-trunk/dist/update/update.snippet.1 [e-mail]:/opt/aus2/build/0/Firefox/trunk/WINNT_x86-msvc/2007081705/en-US/complete.txt
Warning: Identity file /c/Documents does not exist.
scp: /opt/aus2/build/0/Firefox/trunk/WINNT_x86-msvc/2007081705/en-US/complete.txt: No such file or directory

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

-> build & release

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Also seems like tbox should go orange or something if these steps fail...

Comment 4

[Robert Helmer [:rhelmer]]

Looks like the problem is the space in the SSH command in "/c/Documents and Settings"; it's trying to push the full MAR up to AUS so partials can be created.

Comment 5

[J. Paul Reed [:preed]]

Attachment: Quote scp args

This could have a number of unintended consequences (and frankly, I'm not even 100% sure that run_shell_command will honor these quotes, so we'll see), but it's a first stab at the problem.

If I get r+, I'll test it on the win32 tinderbox to make sure it works before checking it in (I'm mostly worried about breaking Linux and Mac).

Comment 6

[Robert Helmer [:rhelmer]]

Comment on attachment 277146 [details] [diff] [review]
Quote scp args

I did a really quick test on Linux, quoting paths in this way seems to work with run_shell_command().

Comment 7

[gabe]

anybody care to check this in and maybe clobber

Comment 8

[J. Paul Reed [:preed]]

So I just tried that patch (http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1187413740.29710.gz&fulltext=1) and it didn't work.

I can't say I'm *hugely* surprised.

I've got another patch floating around on the machine, and will leave it there for tonight's nightly. We'll see if it's any better. I rethought my methodology and did a simplified testcase for MSYS; it seemed to work.

I guess tomorrow's nightly will tell, though. :-)

Comment 9

[M** A****]

Still no partials. Back to the drawing board

Comment 10

[Steffen Wilberg]

AUS should at least offer a complete update. Just filed bug 392726 on that.

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Dumb question: why is this stuff in /Documents and Settings/ anyway?  I believe all of MSYS honors $HOME if it's overridden (e.g. HOME=/foo ssh-add results in "/c/foo/.ssh/identity: No such file or directory").

Also, why not just use "~" instead of grabbing $ENV{'HOME'} ? scp -i ~/.ssh/aus should work fine.

Comment 12

[J. Paul Reed [:preed]]

(In reply to comment #11)
> Dumb question: why is this stuff in /Documents and Settings/ anyway?  I believe
> all of MSYS honors $HOME if it's overridden (e.g. HOME=/foo ssh-add results in
> "/c/foo/.ssh/identity: No such file or directory").

I didn't change any MSYS settings, so I believe MSYS is filling $HOME using Win32's USERPROFILE variable and MSYSizing the PATH. Technically, in the win32 world, C:\Documents and Settings\cltbld is the "home directory," no?

> Also, why not just use "~" instead of grabbing $ENV{'HOME'} ? scp -i ~/.ssh/aus
> should work fine.

I thought about that; I didn't immediately use it because ~ relies on shell expansion, right? And that won't always work, depending on how you actually execute your subprocess.

The weird part of all of this is that in my little test program I wrote this morning, the patch I currently had (which was to escape the spaces) worked fine... but it doesn't work when Tinderbox calls it. :-/

I've got another patch to add an additional level of quoting, which I'm not sure will work, although we have some other perl code that does that (not in Tinderbox); we'll see. After that, I'll try the ~ trick.

After that, I'll look at getting more creative.

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #12)
> (In reply to comment #11)
> > Dumb question: why is this stuff in /Documents and Settings/ anyway?  I believe
> > all of MSYS honors $HOME if it's overridden (e.g. HOME=/foo ssh-add results in
> > "/c/foo/.ssh/identity: No such file or directory").
> 
> I didn't change any MSYS settings, so I believe MSYS is filling $HOME using
> Win32's USERPROFILE variable and MSYSizing the PATH. Technically, in the win32
> world, C:\Documents and Settings\cltbld is the "home directory," no?

Yup, I was suggesting that we just override $HOME to avoid the Documents and Settings problem.

> > Also, why not just use "~" instead of grabbing $ENV{'HOME'} ? scp -i ~/.ssh/aus should work fine.
> 
> I thought about that; I didn't immediately use it because ~ relies on shell
> expansion, right? And that won't always work, depending on how you actually
> execute your subprocess.

Well... technically any other solution where will also depend on shell handling (quoting, etc).  Dunno, it seems simpler to just let the shell take care of it(especially in that one instance), instead of trying to guess the fragile quoting.

anyway, just some ideas.

Comment 14

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

Looks like the Breakpad symbol push will need a similar quoting-fix:
  http://mxr.mozilla.org/seamonkey/source/tools/tinderbox-configs/firefox/win32/tinder-config.pl#228

Eg, http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1187466060.22885.gz&fulltext=1

Transferring symbols... /e/builds/tinderbox/Fx-Trunk-Newref/WINNT_5.2_Depend/mozilla/../2007081813/crashreporter-symbols-2007081813.zip
Executing: program /usr/bin/ssh host stage.mozilla.org, user ffxbld, command scp -v -d -t /mnt/netapp/breakpad/symbols_ffx//
Warning: Identity file /c/Documents does not exist.
Permission denied (publickey).
lost connection
make: *** [uploadsymbols] Error 1

Comment 15

[J. Paul Reed [:preed]]

The latest clobber I did looks like it pushed the snippets correctly:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1187482860.13313.gz&fulltext=1

Partials should be available now; is anyone seeing them yet?

Ironically, the original version of the patch in this bug worked fine:

The short version: I'm le stupid.

The long(er) version: so, tinderbox has this "feature" where you set it into release build mode by symlinking post-mozilla-rel.pl to post-mozilla.pl; MSYS doesn't support symlinks and doesn't do hardlinks on non-NTFS file systems, so I fixed that (bug 372836), but as I was testing out this patch, I uhh... was editing post-mozilla-rel.pl (where it would be checked in), and not copying it to post-mozilla.pl, which the changes I made in bug 372836 would've taken care of, but I turned that mode off because it would clobber what was in CVS with the patch I was testing.

So... yeah... boo on me.

I'm gonna look at the issue cf raised about pushing symbols, and whip up another patch for r?, and then get that checked in, but for now, the new MSYS tinderbox should generate updates as before.

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Offered an update from 2007081813 to 2007081818

Comment 17

[Jim Jeffery not reading bug-mail 1/2/11]

Partial update .mar file created, and successfully updated from 2007081818 to 2007081905. 

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007081905 Minefield/3.0a8pre Firefox/3.0 ID:2007081905

Comment 18

[J. Paul Reed [:preed]]

Attachment: Quote scp args, take II

The original quote scp args patch, now with 90% more paranoia!

I decided to basically only enable this for MSYS.

Also, quote use of $HOME in the tinder-config.pl for this Tinderbox. I looked at the symbol upload stuff in mozilla/toolkit/crashreporter/tools, and it *looks* like quoting the argument in the environment should do it, although it should definitely work with '~', as well (if it doesn't).

Le sigh... quoting.

Comment 19

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

Comment on attachment 277294 [details] [diff] [review]
Quote scp args, take II

This patch review brought to you by 8 hours of jetlag. I feel peachy!

Comment 20

[Robert Helmer [:rhelmer]]

Comment on attachment 277294 [details] [diff] [review]
Quote scp args, take II

Looks good; I think doing this cross-platform (and for cygwin) would work as well.

Comment 22

[(not currently active) Ted Mielczarek]

Comment on attachment 277294 [details] [diff] [review]
Quote scp args, take II

If this works ok, can we preemptively change the tinder-configs for thunderbird/seamonkey like this?  Should be safe either way.

Comment 23

[Robert Kaiser]

Either we can preemptively change that for the community boxen or we need to make sure we don this when resolving bug 392438

Comment 24

[Chris Cooper [:coop] (he/him)]

Comment on attachment 277294 [details] [diff] [review]
Quote scp args, take II

This patch is live on fxnewref now. Haven't checked it in generally.

Comment 25

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

Attachment: Fix up Breakpad symbol push

The tinder-config.pl part of the previous patch isn't sufficient to get symbol push working. At least, not when I went 
  export SYMBOL_SERVER_SSH_KEY="\"/c/Documents and Settings/cltbld/.ssh/ffxbld_dsa\"" 
set the other three vars, and called upload_symbols.sh. The error is
  Warning: Identity file /c/Documents does not exist.
as before.

This patch does work from the command line, without the quotes in the variable definition. I used it to push symbols for clobber builds from 2007081605 until 2007082005 up to the server.

Comment 26

[(not currently active) Ted Mielczarek]

Comment on attachment 277427 [details] [diff] [review]
Fix up Breakpad symbol push

Great, nice and simple.  I think you should check this in ASAP to get symbol upload working again.

Comment 27

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

> (From update of attachment 277427 [details] [diff] [review])
> Great, nice and simple.  I think you should check this in ASAP to get symbol
> upload working again.

Checking in upload_symbols.sh;
/cvsroot/mozilla/toolkit/crashreporter/tools/upload_symbols.sh,v  <--  upload_symbols.sh
new revision: 1.8; previous revision: 1.7
done
 

Comment 28

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

The upload_symbols.sh patch doesn't work in the presence of the tinder-config.pl changes, so we backed the latter out. I've kicked off a clobber build to test.

Yesterday, symbols for the nightlies from 20070816 thru 20070820 were manually pushed to the server, and I pushed todays 2007082105 symbols a few hours ago.

Comment 29

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

Success! Symbol push worked fine for the clobber build of comment #28.

To close this out we just need to make sure everything is into CVS. I think that's  just the first half of attachment 277924 [details] [diff] [review], but I'll verify that tomorrow. It looks like it's a safe change for other platforms so I may just go ahead and land it before the round of nightlies (and be on hand for any fallout).

Comment 30

[J. Paul Reed [:preed]]

(In reply to comment #20)
> (From update of attachment 277294 [details] [diff] [review])
> Looks good; I think doing this cross-platform (and for cygwin) would work as
> well.

Yah, I think that'd probably be better too; I originally was paranoid because I wasn't in a position to watch all three platforms at the time.

I'll go ahead and check the first version in.

cf: can you help me watch the tinderboxen and the builds to make sure there's no fallout?

Comment 31

[J. Paul Reed [:preed]]

(In reply to comment #30)

> cf: can you help me watch the tinderboxen and the builds to make sure there's
> no fallout?

(To be clear: I'm currently watching Tinderbox for badness [obviously], and I'll also check the nightlies tomorrow when I get up, but you're likely to see failure(s) before I am, so I thought I'd ask. :-)

Comment 32

[Chris Cooper [:coop] (he/him)]

Comment on attachment 277294 [details] [diff] [review]
Quote scp args, take II

The post-mozilla-rel.pl portion of this patch is currently running on all three nightly machines on trunk for verification.

Comment 33

[Chris Cooper [:coop] (he/him)]

Taking this bug so preed will stop working on it. ;-)

Comment 34

[Chris Cooper [:coop] (he/him)]

preed landed the post-mozilla-rel.pl patch today. I'll be verifying the nightly builds tomorrow.

Comment 35

[J. Paul Reed [:preed]]

(In reply to comment #33)
> Taking this bug so preed will stop working on it. ;-)

Now people can stop yelling at me that I'm supposed to be on vacation. :-P

Seriously, thanks for signing up to verify tomorrow.

I should've been clearer about the fact that I actually checked in the post-mozilla-rel.pl part in comment 30; bonsai query: 
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=&branchtype=match&dir=&file=&filetype=match&who=preed%25mozilla.com&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-08-19&maxdate=2007-08-22&cvsroot=%2Fcvsroot

Comment 36

[Chris Cooper [:coop] (he/him)]

Everything looks good (builds, snippets, symbols) after the most recent nightly.

Comment 37

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified; been using this daily since then without issue.