User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en) AppleWebKit/418.9.1 (KHTML, like Gecko) Safari/419.3 Build Identifier: XULRunner 1.8.0.9 When revision 1.8 of "mozilla/tools/update-packaging/make_full_update.sh" was used to create a mar, the resultant mar was corrupt. It just hangs the updater process. The cause has been narrowed down to the presence of a space in the name of the file "XUL.framework/Versions/1.8.0.9/plugins/Default Plugin.plugin". When the space was removed, the mar extracted fine. This problem was not present with revision 1.7 of the file. I noticed that a fix was added for https://bugzilla.mozilla.org/show_bug.cgi?id=373121 to the script in revision 1.8 that removed the command to get the list of files from quotes 45,46c45 < list=$(list_files) < eval "files=($list)" --- > files=($(list_files)) This looks like a likely cause for the problem Reproducible: Always Steps to Reproduce: 1. 2. 3.

confirming, based on bug #373908

See also bug 327076 for how this was handled for unpacking a MAR.

as this is affecting trunk nightlies in very bad ways (see bug #373908 which leads to bug #375619), marking critical.

Comment on attachment 259912 [details] [diff] [review] Handle spaces correctly, rev. 1 Looks good to me. Are you intending to leave this line + echo "${1}[$count]=\"$file\"" in when committing it ? Navin, great job on the initial report!

The patch also fixed the problem with libxpistub.dylib (bug 375607) in my testing of complete update generation. Any ideas why that is ?

Fixed on trunk. I bet xpistub was messed up because the eval of the next line failed and caused the file to be overwritten, or something equally evil like that. I've filed bug 375752 to rewrite these scripts in perl so that we can do real error checking and cleanup.

thanks for the quick fix, bsmedberg. note, some work might be required on the nightly build machines (see bug #375852) before I can verify this fixes both bug #375621 and bug #375607.

> I bet xpistub was messed up because the eval of the next line failed You appear to be right, as the xpistub.dylib.bz2 file is gone, see bug #375607 Thanks bsmedberg!

This change broke my ChatZilla build code. In list_files, the working directory is $targetdir, so it tries to create $targetdir/$workdir/temp-filelist and fails.

Yes, something seems to be wrong now. Have those scripts been really tested? One thing I noticed is that make_incremental... only works when given absolute paths (it used to work with relative ones before) and the result is still invalid since it includes not only the patch files but also the original ones.

This is what I've been using for a while (on Cygwin/Windows XP).

I just wasted a BUNCH of time wrestling with this bug. Unfortunately I didn't see the patch from comment 12 until now. I had exactly the problems mentioned here, namely relative paths no longer work for make_full_update (and I suspect inc update) due to placement of the temp file and the matching issue with the extra quotes. I'm running these tools on Ubuntu Dapper. I'd be interested to know what mozilla is running these tools on such that they work. I did notice that the make_full_update code passes in full paths on the command line. Should we re-open this bug or clone it for a new one?

Sorry to hear that John. Reopening this bug to drive the supplemental fix in. Details of why it didn't bite Mozilla are in bug 375852 - basically, we don't use make_incremental_update for releases, and our nightly system doesn't auto-update that code.

Comment on attachment 264891 [details] [diff] [review] fix list_files and filename comparison in make_incremental_update I remember I had to add the "\"$f\"" bit on purpose, but I don't remember why... this makes me nervous. Can't we just take schrep's rewrite of this in python? Unless we need this fix right away.

adding additional (quoted) quotes to just one side of the test (compared with MOZILLA_1_8_BRANCH) looks wrong to me.

(In reply to comment #15) > (From update of attachment 264891 [details] [diff] [review]) > I remember I had to add the "\"$f\"" bit on purpose, but I don't remember > why... this makes me nervous. I'll take a look at this, we hit the same thing when we tried a newer version for Firefox 3.0b2 > Can't we just take schrep's rewrite of this in python? Unless we need this fix > right away. We still need the common.sh fix, if we want to support relative paths. Can we land that separately? It also bit us during the signing step in Fx 3.0b2 (bug 407962). We should rewrite make_complete_update.sh as well, then I think we can ditch the shell tools altogether; schrep's make_incremental_updates.py is only for partials IIRC.

(In reply to comment #15) > (From update of attachment 264891 [details] [diff] [review]) > I remember I had to add the "\"$f\"" bit on purpose, but I don't remember > why... this makes me nervous. It's for MozillaBuild according to the commit log; but we (mozilla.org) only run these tools on Linux anyway right now. I'll see if there's a way to fix for both. > Can't we just take schrep's rewrite of this in python? Unless we need this fix > right away. You're going to think I am insane I'm sure, but we need to call a mix of this stuff (bug 391958) in the meantime, and update-packaging being busted on trunk is blocking us here as well as breaking our signing scripts. I kind of think we should take this patch and purposely bust MozillaBuild, and reopen the MozillaBuild support bug.. I do want to move to calling update-packaging per-platform as part of the build (e.g. bug 410806), but we don't use it right now so we should unbust other platforms and support MozillaBuild by moving away from these old shell tools. We need to leave the shell tools around for a bit in order to keep shipping though until that's in place and well-tested. Let's just not worry about the shell tools running under anything but Linux, IMHO.

Comment on attachment 264891 [details] [diff] [review] fix list_files and filename comparison in make_incremental_update we're just going to suck and break windows until python comes and saves us

Rob Helmer did some testing to confirm that this patch makes relative paths work again. Thanks for the patch tH. Checking in common.sh; /cvsroot/mozilla/tools/update-packaging/common.sh,v <-- common.sh new revision: 1.10; previous revision: 1.9 done Checking in make_incremental_update.sh; /cvsroot/mozilla/tools/update-packaging/make_incremental_update.sh,v <-- make_incremental_update.sh new revision: 1.12; previous revision: 1.11 done