397246 - CVS-based ChatZilla language packs

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[Robert Kaiser]

In bug 394315, I introduced "source L10n" (i.e. CVS-based L10n) for ChatZilla. Though the approach taken there works, it's not the ideal way to deal with this, esp. as it results in extensions with the same ID but different content (due to different localizations).

In IRC and newsgroup discussions, we seemed to get to an agreement that the best solution for the future is to go with leaving the core ChatZilla bundling en-US and have language packs for other locales, which are marked dependent on the main ChatZilla package for those applications that can deal with extension dependency.
We also should get AMO to support such extension language packs and even offer compound ChatZilla+locale packages as so-called "Multiple Item Packages" for apps based on toolkit 1.8 and higher.

The target is to get a solution with this for SeaMonkey 2.0 locale repackaging and as a second goal also create XPIs that can be used for Add-On installation on all apps that ChatZilla supports. I'll try to work on those goals in this order.

See the thread in the URL of this bug for the relevant newsgroup discussion.

Comment 1

[Robert Kaiser]

Attachment: WIP patch 1: Make SeaMonkey repackaging build a chatzilla langpack

This patch is not ready for review, this is a work in progress but I'm attaching it here to give you a preview of how I think this can work.
This builds the patch for bug 351715 (a dependent extension only makes sense if there is an extension to depend on). Repackaging of SeaMonkey trunk zip/tarballs works with this, and a "make libs-foo" in extensions/irc/locales (with foo being any locale code present in all-locales) creates the chatzilla langpack. SeaMonkey installers will be a bit harder, and I have not yet touched the normal XPI generation process - as I said, this is WIP. The ChatZilla locales get two new files, which are a defines.inc defining variables to insert into install.rdf and langpack-id.txt containing the langpack ID so that existing langpacks can keep their IDs in the future.

Comment 2

[Axel Hecht [:Pike]]

Comment on attachment 282014 [details] [diff] [review]
WIP patch 1: Make SeaMonkey repackaging build a chatzilla langpack

I'm still not convinced that we're at the stage where we have a build problem, see my post in .l10n and .apps.*.

Anyway, onto this patch.

I don't believe in langpack-id.txt. IMHO, that's making life harder than necessary, I'm not sure how much backwards compat it buys us.

I'd rather see you using the python preprocessor impl than the perl one. That should really just be used for make-jars.pl right now.

I'm much in favour of the "do nothing here" XXX.

I'm not sure that the min and maxVersions in install.rdf will be right, but picking the right ones is a matter of versioning scheme in chatzilla, for one. Having .0 maxVersions sounds wrong, though.

Comment 3

[Robert Kaiser]

(In reply to comment #2)
> I don't believe in langpack-id.txt. IMHO, that's making life harder than
> necessary, I'm not sure how much backwards compat it buys us.

We have at least two existing ChatZilla langpacks that would be broken in upgrading when we "hardcode" this to somthing like langpack-<ab_CD>@chatzilla.mozilla.org, but if ChatZilla maintainers agree that it's OK to break those updates for those two, I'm more than happy to remove that file.

> I'd rather see you using the python preprocessor impl than the perl one. That
> should really just be used for make-jars.pl right now.

I think I just copied that from /browser/locales/Makefile.in - but I guess I should dig out the .py syntax and make a good example :)

> I'm much in favour of the "do nothing here" XXX.

Yes, me as well, but I haven't yet found out how to do it, as a simple ifeq or such doesn't seem to work :(

> I'm not sure that the min and maxVersions in install.rdf will be right, but
> picking the right ones is a matter of versioning scheme in chatzilla, for one.
> Having .0 maxVersions sounds wrong, though.

I copied those from the main ChatZilla install.rdf, but they may also be wrong there.

Comment 4

[:Gijs (out for now; he/him)]

(In reply to comment #2)
> I'm not sure that the min and maxVersions in install.rdf will be right, but
> picking the right ones is a matter of versioning scheme in chatzilla, for one.
> Having .0 maxVersions sounds wrong, though.
> 

> <!-- target: Mozilla, version 1.0 to 1.8 -->

Mozilla 1.8 was never released. Here for AMO satisfaction (we could use 1.7.*, but then it wouldn't show it for older alphas or somesuch)

> <!-- target: Mozilla Firefox, version 1.0 to 3.0 -->

Firefox 3.0 is not here yet. We'll change it when there's an actual release. Right now I don't think we can claim compat with a release that's not even close to finished yet - but we don't want to hassle every month to update it to the new alpha, that'd be useless.

> <!-- target: SeaMonkey, version 1.0 to 2.0 -->
Ditto.

> <!-- target: Flock, version 0.4 to 1.0+ -->
If anyone actually understands Flock's versioning system and thinks this should be something else, they can file a new bug.

> <!-- target: Netscape Navigator, version 9.0b1 to 9.0.* -->
This seems to be fine. :-)

Comment 5

[Robert Kaiser]

Attachment: WIP patch 2: Still only SeaMonkey repackaging, but a bit nicer

Just FYI, here's an update of the WIP work I have - it still doesn't do more than SeaMonkey repackaging, but I addressed several notes with this. Note that though I'm using the python preprocessor now, it has a bug so that the emptyLines filter doesn't work correctly. Other than that, the language pack XPIs generated with this patch work basically fine, as we've tested extensively on trunk and a bit on FF2.
Next thing I'll look at will be manual XPI generation and including install.js in all the packages for backwards compat - we may postpone installer repackaging for a bigger installer change I've been talking about with Rob Strong, which should ease dealing with that.

Comment 6

[Robert Kaiser]

Attachment: patch v1: everything but installer repackaging

Here's a bit that covers everything but SeaMonkey installer repackaging, which can be easily added on top of this work and might be eased by some planned changes to installer code (I'll need to find out if Rob Strong still will get those into the 1.9 timeframe).
The current patch does the following things:
1) When doing zip repackaging or langpacks for SeaMonkey (and the language is in all-locales), a (dependent) chatzilla langpack is created and included in SeaMonkey packages
2) When building through Makefiles with --enable-ui-locale, a full en-US ChatZilla is built, plus a (dependent) chatzilla langpack if the language is supported in all-locales
3) makexpi.sh is reverted to do en-US chatzilla XPIs, makelocalexpi.sh builds the (dependent) langpack.

For testing, pull a l10n/de directory from L10n CVS next to your mozilla/ dir that contains extensions/irc

Comment 7

[Robert Kaiser]

Comment on attachment 286553 [details] [diff] [review]
patch v1: everything but installer repackaging

Gijs, please review the ChatZilla side of things here; Axel, please review the L10n process and approach of the patch. Thanks!

Comment 8

[Robert Kaiser]

Comment on attachment 286553 [details] [diff] [review]
patch v1: everything but installer repackaging

>Index: mozilla/extensions/irc/locales/Makefile.in
>===================================================================
>+DIST_FILES             = generic/install.rdf

Sorry, forgot one detail - make this line
DIST_FILES             = generic/install.rdf generic/install.js
instead, so that the language pack produced by the Makefile process works everywhere.

Comment 9

[James Ross]

Comment on attachment 286553 [details] [diff] [review]
patch v1: everything but installer repackaging

>+XPI_NAME               = chatzilla-$(AB_CD)
>+INSTALL_EXTENSION_ID   = langpack-$(AB_CD)@chatzilla.mozilla.org
>+XPI_PKGNAME            = chatzilla-$(CHATZILLA_VERSION).$(AB_CD)

Is there any particular reason for using a "." between the ChatZilla version and locale? I feel it should be a "-" like the other separators.

-	$(PERL) $(topsrcdir)/config/preprocessor.pl $(DEFINES) $(ACDEFINES) -I$(call EXPAND_LOCALE_SRCDIR,toolkit/locales)/defines.inc -I$(LOCALE_SRCDIR)/defines.inc $(srcdir)/generic/install.rdf > $(FINAL_TARGET)/install.rdf
+	$(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $(ACDEFINES) -I$(call EXPAND_LOCALE_SRCDIR,toolkit/locales)/defines.inc -I$(LOCALE_SRCDIR)/defines.inc $(srcdir)/generic/install.rdf > $(FINAL_TARGET)/install.rdf

This makes me sad. :(  (Good job I'll never be running this bit, then.)

Comment 10

[Robert Kaiser]

(In reply to comment #9)
> >+XPI_PKGNAME            = chatzilla-$(CHATZILLA_VERSION).$(AB_CD)
> 
> Is there any particular reason for using a "." between the ChatZilla version
> and locale? I feel it should be a "-" like the other separators.

The dot is correct if we want to follow http://developer.mozilla.org/en/docs/Package_Filename_Convention which I think would be a good idea as it's what we're using all over the place for Mozilla-based packages.

> [perl->python]
> This makes me sad. :(  (Good job I'll never be running this bit, then.)

The perl preprocessor is deprecated and we're slowly but surely fading it out in favor of the python implementation. The long-term goal is to remove all perl usages in our build system in favor to python (not that I'm a fan of python, but it's said to be cleaner - I'd like it much better if we'd have a possibility to use JS from the commandline to drive all that).

Comment 11

[:Gijs (out for now; he/him)]

Comment on attachment 286553 [details] [diff] [review]
patch v1: everything but installer repackaging

> <snip>
>+
>+libs-%: AB_CD=$*
>+libs-%:
>+	# XXX: it would be nice if we could just do nothing if no langpack is being done

Then what do we do now, and why aren't we doing nothing?

>+	$(MAKE) libs AB_CD=$*
>+
>+repackage-zip: $(STAGEDIST)
>+ifneq (en-US,$(AB_CD))
>+	$(RM) -rf $(STAGEDIST)/extensions/$(INSTALL_EXTENSION_ID)
>+	mkdir $(STAGEDIST)/extensions/$(INSTALL_EXTENSION_ID)
>+	$(RM) -r $(DIST)/xpi-stage/chatzilla-$(AB_CD)/chrome/chatzilla
>+	cd $(DIST)/xpi-stage/chatzilla-$(AB_CD) && \
>+	  tar $(TAR_CREATE_FLAGS) - * | ( cd $(STAGEDIST)/extensions/$(INSTALL_EXTENSION_ID) && tar -xf - )

Am I completely misunderstanding this code or are you using tar to create a zip archive? Why not zip, and if you're not creating a zip archive, what the hell else are you doing here? This code is rather convoluted, if you ask me, but perhaps that's just me not being good at Makefiles. :-\

<snip>
>Index: mozilla/extensions/irc/locales/jar.mn
<snip>
> chatzilla.jar:
>+% locale chatzilla @AB_CD@ %locale/@AB_CD@/chatzilla/
Can you add a comment why is this here, what it does and why we need it?

> * locale/@AB_CD@/chatzilla/contents.rdf          (generic/chrome/contents.rdf)
>   locale/@AB_CD@/chatzilla/chatzillaOverlay.dtd  (%chrome/chatzillaOverlay.dtd)
>   locale/@AB_CD@/chatzilla/browserOverlay.dtd    (%chrome/browserOverlay.dtd)
>   locale/@AB_CD@/chatzilla/pref-irc.dtd          (%chrome/pref-irc.dtd)
>   locale/@AB_CD@/chatzilla/chatzilla.dtd         (%chrome/chatzilla.dtd)
>   locale/@AB_CD@/chatzilla/chatzilla.properties  (%chrome/chatzilla.properties)
>   locale/@AB_CD@/chatzilla/config.dtd            (%chrome/config.dtd)

<snip>
>+# Check that requested language is in all-locales file (i.e. exists and it
>+# allowed to be used). Fall back to en-US if not.
>+# FIXME: THIS DOES NOT WORK WITH CYGWIN.

That will need to be fixed, or some people will be unhappy. r- on that - sorry. Would using safeCommand help here? What exactly doesn't work on cygwin?

>+grep -sx "$AB_CD" "$LOCALEDIR/all-locales" > /dev/null
>+if [ $? != 0 ]; then
>+  echo "ERROR: Language $AB_CD is currently not supported."
>   exit 1
> fi
>+if [ $DEBUG -ge 1 ]; then echo "Language   = $AB_CD"; fi
>+
>+
>+# Set up where the actual localisation files are to be found; below $LOCALEDIR
>+# for en-US, in l10n/ repository for other languages.
>+if [ "$AB_CD" = "en-US" ]; then
>+  L10NDIR="$LOCALEDIR/$AB_CD"
>+else
>+  L10NDIR="$FEDIR/../../../l10n/$AB_CD/extensions/irc"
>+fi

Please add a var to override where to look for the non-en-US locales, so people are not *required* to check out the l10n repository as a sibling to their mozilla tree, and also so they don't need to check out everything (not sure if CVS lets you do subtrees only - if it doesn't, I bet mercurial will. Soit.)

>+if [ $DEBUG -ge 1 ]; then echo "L10n dir   = $L10NDIR"; fi
>+
>+
>+# Check directory setup.
> if ! [ -d "$CONFIGDIR" ]; then
>   echo "ERROR: mozilla/config directory (CONFIGDIR) not found."
>   exit 1
> fi
>-
>-if [ -z "$LOCALE" ]; then
>-  echo "ERROR: You need to provide a locale identifier (ab-CD)"
>+if ! [ -d "$FEDIR" ]; then
>+  echo "ERROR: Base ChatZilla directory (FEDIR) not found."
>+  exit 1
>+fi
>+if ! [ -d "$L10NDIR" ]; then
>+  echo "ERROR: Directory with localized files for $AB_CD language (L10NDIR) not found."
>   exit 1
> fi

Please check in the order you automatically set them at the start of the file (so FEDIR before CONFIGDIR, at least). Otherwise people get yelled at for not having a configdir when the problem is actually somewhere else.

<snip>
 
> # Check for required directory layouts.
> echo -n "  Checking XPI structure"
> echo -n .
>-if ! [ -d "xpi-tree-$LOCALE" ]; then mkdir "xpi-tree-$LOCALE"; fi
>+if ! [ -d $XPIROOT ]; then mkdir -p $XPIROOT; fi

Why -p? (msys doesn't know what this means, at least)

> echo -n .
>-if ! [ -d "xpi-tree-$LOCALE/chrome" ]; then mkdir "xpi-tree-$LOCALE/chrome"; fi
>+if ! [ -d $XPIROOT/chrome ]; then mkdir $XPIROOT/chrome; fi
> echo   ".                        done"
> 
> echo -n "  Checking JAR structure"
> echo -n .
>-if ! [ -d jar-tree ]; then mkdir jar-tree; fi
>+if ! [ -d $JARROOT ]; then mkdir -p $JARROOT; fi
> echo   ".                         done"

Ditto?

> # Make Firefox updates.
> echo -n "  Updating Firefox Extension files"
> echo -n .
>-safeCommand sed "--expression=s|@REVISION@|$VERSION|g" "--expression=s|@LOCALE@|$LOCALE|g" '<' "$XPILOCALEFILES/$LOCALE/install.rdf" '>' "$XPIROOT/install.rdf"
>+safeCommand $PERL $CONFIGDIR/preprocessor.pl -DAB_CD=$AB_CD -DCHATZILLA_VERSION=$VERSION -DINSTALL_EXTENSION_ID=langpack-$[e-mail] -I$L10NDIR/../../toolkit/defines.inc -I$L10NDIR/defines.inc "$LOCALEDIR/generic/install.rdf" '>' "$XPIROOT/install.rdf"

Please add a comment here saying what this is doing. :-)
Also, you assume an entire CVS repo is checked out for this work. Is that really necessary? Why? If possible, please try to make sure that this works after only:
  cvs co mozilla/extensions/irc mozilla/config
and the necessary l10n files (l10n/ab-CD/extensions/irc)

> echo -n .
> echo   ".              done"
> 
> 
> # Make Mozilla Suite updates.
> echo -n "  Updating Mozilla Extension files"
> echo -n .
>-safeCommand sed "--expression=s|@REVISION@|$VERSION|g" "--expression=s|@LOCALE@|$LOCALE|g" '<' "$XPILOCALEFILES/$LOCALE/install.js" '>' "$XPIROOT/install.js"
>+safeCommand $PERL $CONFIGDIR/preprocessor.pl -DAB_CD=$AB_CD -DCHATZILLA_VERSION=$VERSION -DINSTALL_EXTENSION_ID=langpack-$[e-mail] -I$L10NDIR/../../toolkit/defines.inc -I$L10NDIR/defines.inc "$LOCALEDIR/generic/install.js" '>' "$XPIROOT/install.js"

Same as above.

<snip>

>Index: mozilla/extensions/irc/xpi/makexpi.sh
<snip>

> # Check for required directory layouts.
> echo -n "  Checking XPI structure"
> echo -n .
>-if ! [ -d xpi-tree ]; then mkdir xpi-tree; fi
>+if ! [ -d $XPIROOT ]; then mkdir -p $XPIROOT; fi

Why -p?

> echo -n .
>-if ! [ -d xpi-tree/chrome ]; then mkdir xpi-tree/chrome; fi
>+if ! [ -d $XPIROOT/chrome ]; then mkdir $XPIROOT/chrome; fi
> echo -n .
>-if ! [ -d xpi-tree/chrome/icons ]; then mkdir xpi-tree/chrome/icons; fi
>+if ! [ -d $XPIROOT/chrome/icons ]; then mkdir $XPIROOT/chrome/icons; fi
> echo -n .
>-if ! [ -d xpi-tree/chrome/icons/default ]; then mkdir xpi-tree/chrome/icons/default; fi
>+if ! [ -d $XPIROOT/chrome/icons/default ]; then mkdir $XPIROOT/chrome/icons/default; fi
> echo -n .
>-if ! [ -d xpi-tree/components ]; then mkdir xpi-tree/components; fi
>+if ! [ -d $XPIROOT/components ]; then mkdir $XPIROOT/components; fi
> echo   ".           done"
> 
> echo -n "  Checking JAR structure"
> echo -n .
>-if ! [ -d jar-tree ]; then mkdir jar-tree; fi
>+if ! [ -d $JARROOT ]; then mkdir -p $JARROOT; fi

Ditto?

<snip>


>Index: mozilla/extensions/irc/locales/generic/install.js

As far as I can tell, this is locale-resources/install.js with the header for localizers removed because it now gets preprocessed. If you like to be perfect, you should really use cvs move for this op (and the whole locale-resources dir should be cvs rm'd at the end of this story whatever the case). I can't say I care particularly much, as the install.js file's history is not very interesting and is unlikely to ever change, seeing as its only used for legacy support. So I guess you can leave it. If you have energy + time to spare, though... ;-)


>Index: mozilla/extensions/irc/locales/generic/install.rdf
>+        <em:maxVersion>3.0.*</em:maxVersion>
>+        <em:maxVersion>2.0.*</em:maxVersion>

If I wasn't scared of getting into a fight with Axel, I'd yell. Seriously though, I don't really think this is the Right thing to do. If anything, hasn't history taught us that Firefox will probably really need to be 3.0.0.* anyway? Meh. I'd prefer this without the *. We'll adjust when there's an actual release we can claim to support. Right now having it be 3.0 is just so dev. instances will always work on trunk without having to keep upping the alpha rev (which would be useless).

Comment 12

[James Ross]

>+# FIXME: THIS DOES NOT WORK WITH CYGWIN.

I should probably explain the FIXME comment, since I added it to makexpi.sh.

First of all, the problem already exists thanks to the earlier patches to makexpi.sh for locale stuff, and would not be new for this patch. I mearly added the FIXME to indicate it was known to not work, and I guess it got copied from there.

Secondly, the problem is that of line endings. Basically, grep wont match the lines because its argument is (for example) "de" and the line in the file (as far as grep is concerned) is "de<CR>".

I'd have to check, but I think it boiled down to the file having OS-correct line ends (CVS doing the right thing) and Cygwin not translating when grep opened the file (Cygwin not doing the right thing). You could probably strace grep and see the bit where Cygwin gets things wrong.

I did have a small fight with grep to make it work, but it has no option to ignore the CR. I suppose it could be optionally allowed in the grep arguments, now I think about it, but I never tried that.

Comment 13

[Robert Kaiser]

(In reply to comment #11)
> >+	# XXX: it would be nice if we could just do nothing if no langpack is being done
> 
> Then what do we do now, and why aren't we doing nothing?

We are currently just creating the en-US locale (again) when calling this target for a non-supported locale. This is suboptimal, as it takes up a bit of time, but it doesn't disturb anything. I'll extend the comment to tell that.

> Am I completely misunderstanding this code or are you using tar to create a zip
> archive?

The repackage-zip target is just called as a part of the repackaging process and a copy is done here. I'll add a comment to point out why the copy is done with tar as opposed to cp.

> >+% locale chatzilla @AB_CD@ %locale/@AB_CD@/chatzilla/
> Can you add a comment why is this here, what it does and why we need it?

Yes, done locally.
> >+# Set up where the actual localisation files are to be found; below $LOCALEDIR
> >+# for en-US, in l10n/ repository for other languages.
> >+if [ "$AB_CD" = "en-US" ]; then
> >+  L10NDIR="$LOCALEDIR/$AB_CD"
> >+else
> >+  L10NDIR="$FEDIR/../../../l10n/$AB_CD/extensions/irc"
> >+fi
> 
> Please add a var to override where to look for the non-en-US locales, so people
> are not *required* to check out the l10n repository as a sibling to their
> mozilla tree, and also so they don't need to check out everything (not sure if
> CVS lets you do subtrees only - if it doesn't, I bet mercurial will. Soit.)

Doing that - and I figured that we actually never work for en_US as it's not in all-locales and integrated by default, so I'll remove the en-US case for that dir.
CVS does let you check out subtrees BTW, how would you go with only a partial tree for the whole make*xpi idea else?

> Please check in the order you automatically set them at the start of the file
> (so FEDIR before CONFIGDIR, at least). Otherwise people get yelled at for not
> having a configdir when the problem is actually somewhere else.

OK, I had copied that from makexpi, changed for both.

> >+if ! [ -d $XPIROOT ]; then mkdir -p $XPIROOT; fi
> 
> Why -p? (msys doesn't know what this means, at least)

If someone provides a custom XPIROOT, it sounds like a good idea to create the parent dir(s) as well as needed.

> >-safeCommand sed "--expression=s|@REVISION@|$VERSION|g" "--expression=s|@LOCALE@|$LOCALE|g" '<' "$XPILOCALEFILES/$LOCALE/install.rdf" '>' "$XPIROOT/install.rdf"
> >+safeCommand $PERL $CONFIGDIR/preprocessor.pl -DAB_CD=$AB_CD -DCHATZILLA_VERSION=$VERSION -DINSTALL_EXTENSION_ID=langpack-$[e-mail] -I$L10NDIR/../../toolkit/defines.inc -I$L10NDIR/defines.inc "$LOCALEDIR/generic/install.rdf" '>' "$XPIROOT/install.rdf"
> 
> Please add a comment here saying what this is doing. :-)

OK, doing so.

> Also, you assume an entire CVS repo is checked out for this work. Is that
> really necessary? Why? If possible, please try to make sure that this works
> after only:
>   cvs co mozilla/extensions/irc mozilla/config
> and the necessary l10n files (l10n/ab-CD/extensions/irc)

We don't need an entire tree, just toolkit/defines.inc which is used across all localizations for the language name used in langpack titles of all kinds.
So, this all works perfectly with mozilla/extensions/irc mozilla/config plus l10n/ab-CD/extensions/irc l10n/ab-CD/toolkit/defines.inc

> >Index: mozilla/extensions/irc/locales/generic/install.js
> 
> As far as I can tell, this is locale-resources/install.js with the header for
> localizers removed because it now gets preprocessed. If you like to be perfect,
> you should really use cvs move for this op (and the whole locale-resources dir
> should be cvs rm'd at the end of this story whatever the case).

Yes, cvs removing the old locale-resources is a good idea once everything's in and working, I just wanted to safe the energy for the cvs copy. Sounds like you're OK with that as well, which is good :)


WRT the .* I'll go with whatever you and Axel can agree on, I don't care too much as long as everything works :)

Comment 14

[Robert Kaiser]

Attachment: patch v1.1: incorporate Gijs' comments

Updated the patch for Gijs' comments, thanks for looking into this.

Comment 15

[:Gijs (out for now; he/him)]

Comment on attachment 286994 [details] [diff] [review]
patch v1.1: incorporate Gijs' comments

OK, r=me without the .* versions (or after debate with Axel, I guess).

Comment 16

[Axel Hecht [:Pike]]

Comment on attachment 286994 [details] [diff] [review]
patch v1.1: incorporate Gijs' comments

I'm cancelling this review request, which is an odd compromise between an r- and an r+ with review comments.

There is stuff like for example the install:: target that should be fixed, the install:: target is dead.

Then there's stuff that I just don't like, but I have no idea if it'd be ever fixed. Not that I'm stepping up saying that it wouldn't ever break, notably the use of perl or python for particular tasks.

Some things are just not cute, like the preprocessing of jar.mn in a temp file, which is a piped command in rules.mk.

Not to mention the endless debate on whether having a non-existent maxversion is bad or not.

Thus I'm leaving this patch with comments and without review. I'm not giving it an r- either, ymmv.

Comment 17

[Robert Kaiser]

Checked in that patch, followups filed for better version dependencies and further SeaMonkey packaging improvements.

Comment 18

[James Ross]

Just a note that this check-in toasted the build, as makexpi.sh was missing a load of quotes. I've commited a fix.

Comment 19

[Robert Kaiser]

Hmm, everything worked fine here, but I could only test on Linux. In that case, makelocalexpi.sh probably has the same problem though.

Comment 20

[James Ross]

I'm always being a deliberate awkward case with any build system, because I deliberately have spaces in my paths - so that I catch this kind of mistake. :)