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

VERIFIED FIXED

Status

Release Engineering
General
P1
blocker
VERIFIED FIXED
10 years ago
4 years ago

People

(Reporter: Jim Jeffery not reading bug-mail 1/2/11, Assigned: coop)

Tracking

({regression})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

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
(Reporter)

Updated

10 years ago
Severity: major → blocker
Version: unspecified → Trunk

Updated

10 years ago
Keywords: regression
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 cltbld@aus2-staging.mozilla.org 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 cltbld@aus2-staging.mozilla.org:/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
Status: UNCONFIRMED → NEW
Component: Software Update → Build & Release
Ever confirmed: true
Product: Firefox → mozilla.org
Version: Trunk → other
-> build & release
Assignee: nobody → build
QA Contact: software.update → preed
Summary: mar files not created since move to WINNT 5.2 fxnewref-win32- Dep Nightly → partial mar files not created since move to WINNT 5.2 fxnewref-win32- Dep Nightly
Also seems like tbox should go orange or something if these steps fail...
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

10 years ago
Created attachment 277146 [details] [diff] [review]
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).
Assignee: build → preed
Status: NEW → ASSIGNED
Attachment #277146 - Flags: review?(rhelmer)

Updated

10 years ago
Priority: -- → P1
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().
Attachment #277146 - Flags: review?(rhelmer) → review+

Comment 7

10 years ago
anybody care to check this in and maybe clobber

Updated

10 years ago
Blocks: 384624

Comment 8

10 years ago
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

10 years ago
Still no partials. Back to the drawing board

Comment 10

10 years ago
AUS should at least offer a complete update. Just filed bug 392726 on that.
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.
(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.
(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.
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
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.
Created attachment 277265 [details]
Offered an update from 2007081813 to 2007081818
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
Created attachment 277294 [details] [diff] [review]
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.
Attachment #277146 - Attachment is obsolete: true
Attachment #277294 - Flags: review?(rhelmer)

Updated

10 years ago
Attachment #277294 - Flags: review?(nrthomas)
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!
Attachment #277294 - Flags: review?(nrthomas) → review+
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.
Attachment #277294 - Flags: review?(rhelmer) → review+
Duplicate of this bug: 392851
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

10 years ago
Either we can preemptively change that for the community boxen or we need to make sure we don this when resolving bug 392438
(Assignee)

Comment 24

10 years ago
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.
Created attachment 277427 [details] [diff] [review]
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.
Attachment #277427 - Flags: review?(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.
Attachment #277427 - Flags: review?(ted.mielczarek) → review+
> (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
 

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.
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).
(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?
(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. :-)
(Assignee)

Comment 32

10 years ago
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.
(Assignee)

Comment 33

10 years ago
Taking this bug so preed will stop working on it. ;-)
Assignee: preed → ccooper
Status: ASSIGNED → NEW
(Assignee)

Comment 34

10 years ago
preed landed the post-mozilla-rel.pl patch today. I'll be verifying the nightly builds tomorrow.
Status: NEW → ASSIGNED
(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
(Assignee)

Comment 36

10 years ago
Everything looks good (builds, snippets, symbols) after the most recent nightly.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Verified; been using this daily since then without issue.
Status: RESOLVED → VERIFIED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.