292268 - Standardize SeaMonkey package/installer filenames based on toolkit's package-name.mk

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Robert Kaiser]

This is basically the version of 257117 for SeaMonkey.

See https://bugzilla.mozilla.org/show_bug.cgi?id=257117#c11 for an interesting
way to work on that :)

I've been looking into that for the last hours and want to do it.

I think I should also put up an updated version of the document at
https://bugzilla.mozilla.org/attachment.cgi?id=157639 on the mozilla.org site,
either in MLP, perhaps at some other place if we know a better one.

Comment 1

[Robert Kaiser]

Attachment: patch v1: a first step, untested

OK, here's my first try on it.
It's completely untested, but I'd love to get some feedback on it and I hope it
works correctly.

Comment 2

[Robert Kaiser]

Attachment: patch v2: linux tested, also do windows

This is now tested on linux and does work there ("make" and "make installer" in
xpinstall/packager produce packages with the correct names).
I can't test windows and someone still has to do that, but I got the code for
it in the patch now and I hope it works and is correct.

Note that this all is only about the SeaMonkey packaging stuff, the toolkit
packaging stuff already should use that naming (that is, if you don't count
those strange windows installer names).

Comment 3

[Andrew Schultz]

Comment on attachment 182174 [details] [diff] [review]
patch v2: linux tested, also do windows

>Index: mozilla/xpinstall/packager/Makefile.in
>+ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
>+MOZ_PKG_FORMAT	= BZ2
>+else

This makes the non-installer build bzip2ed, but not the installers.  They're
gzipped in deliver.pl.	I guess that's how it's always been, but it seems
silly.

>-PKG_NAME_EXTRAS =
>-ifeq ($(MOZ_ENABLE_XFT)$(MOZ_WIDGET_TOOLKIT),1gtk2)
>-PKG_NAME_EXTRAS := $(PKG_NAME_EXTRAS)-gtk2+xft
>-else
...

This was here (and in unix/Makefile.in) to make the gtk1 and gtk2 builds have
different filenames.  See bug 254477 and bug 283574.

But the patch works as advetised.

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> This makes the non-installer build bzip2ed, but not the installers.  They're
> gzipped in deliver.pl. I guess that's how it's always been, but it seems
> silly.

Hmm, I think we never compressed the installer with anything else than gzip, if
we want to change this, I'd propose to look into that in a different bug.
Actually, I was unsure if defaulting gtk2 to bzip2 even should be done here or
in a different bug.

> This was here (and in unix/Makefile.in) to make the gtk1 and gtk2 builds have
> different filenames.  See bug 254477 and bug 283574.

They still get different filenames, only the other way round, as gtk2 is now the
default. Look into package-name.mk for details.

Comment 5

[Robert Kaiser]

From IRC:

<bsmedberg> KaiRo: Why can't you just use toolkit/mozapps/installer/packager.mk
directly?
<KaiRo> bsmedberg: hmm... I did not look into that, presumably because I didn't
know if and how I can use it. I guess I should look into that
<bsmedberg> KaiRo: I recommend it; it doesn't package the SDK, but I don't think
you want to anyway

I think we can ditch the SDK (who uses it? what is it for anyways?) and should
look into using packager.mk - I'm just not yet sure how exactly to do it, I'll
look into it though.
For the gtk2 BZ2 issue, I've filed bug 297914 against packager.mk, btw :)

Comment 6

[Benjamin Smedberg]

See browser/installer/Makefile.in to see how to use packager.mk (it's simplicity
itself!). Don't be confused by the "installer" there, this is the makefile which
makes Firefox tarball/zip builds.

Comment 7

[Robert Kaiser]

(In reply to comment #6)
> See browser/installer/Makefile.in to see how to use packager.mk (it's simplicity
> itself!). Don't be confused by the "installer" there, this is the makefile which
> makes Firefox tarball/zip builds.

Yes, I already figured this "installer" means "packager" for us, so we do that
stuff in packager/Makefile.in - it looks to me as if we have a few things in
there that aren't specifically in that toolkit or browser file and that I have
to leave in the file, so I hope I got everything right.
I should have a patch with all that, but I still want to test at least the
default stuff on Linux before attaching it here.

Comment 8

[Robert Kaiser]

Attachment: patch v3: use packager.mk for creating binary packages

OK, this removes a lot of stuff from the packager Makefile.in and uses
toolkit's packager.mk instead, the only functional difference should be that we
don't package the SDK any more.

Comment 9

[Benjamin Smedberg]

Comment on attachment 186484 [details] [diff] [review]
patch v3: use packager.mk for creating binary packages

Please keep removing more:

You do not need TAR_CREATE_FLAGS, which are now set in config.mk

You do not need AB_CD = $(MOZ_UI_LOCALE) which is also set by default in
config.mk

Comment 10

[Robert Kaiser]

(In reply to comment #9)
> You do not need [...] which are now set in config.mk

Ah, well, that's one place I didn't look into. Good. I'm happy with all removing
of stuff I can do without breaking packaging :)

Comment 11

[Robert Kaiser]

Attachment: patch v4: remove even more old stuff (checked in)

OK, this addresses bsmedberg's latest comments and removes even more stuff from
packager/Makefile.in in favor of the values config.mk sets.
It still creates a working package with the correct new name for me on
Linux/gtk2, so it seems to work well without those lines :)

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

> I think we can ditch the SDK (who uses it? what is it for anyways?)

people writing C++ extensions as well as (hopefully/one day) embeddors?

I think we should have a gecko sdk _somewhere_.

Comment 13

[Darin Fisher]

The Gecko SDK is going to be an output of the XULRunner nightly builds.  Chase
is working on moving the XULRunner builds to new machines, and after that is
done, we'll move forward with having those builds generate SDKs.  Since the plan
is to move the apps onto XULRunner, having XULRunner builds produce SDKs makes a
lot of sense.

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

ah, cool, sounds good.

Comment 15

[[e-mail]]

Comment on attachment 186507 [details] [diff] [review]
patch v4: remove even more old stuff (checked in)

Note: packager.mk has a hard-coded list of exclusions. This list appears to be
firefox or at least toolkit-specific...

Comment 16

[Robert Kaiser]

Comment on attachment 186507 [details] [diff] [review]
patch v4: remove even more old stuff (checked in)

requesting approval. This is a SeaMonkey-only change to follow the package
naming conventions of aviary, reusing toolkit's files for that as much as
possible.
Note that this might sit around even with a+ for some time, because I want this
to happen coordinated along with rebranding and tinderbox/stanging changes
needed along with that anyways (so that chase only has to look into our stuff
once and testers only have one big change as well).

Comment 17

[Robert Kaiser]

OK, we're ready to go code/patch-wise, but Chase is overloaded currently (no
time before the july 4 holiday) and can't do the infrastructure stuff atm.

We have to coordinate bugs 292268,285696,288199,289846,298901 to make it all
happen at once and make the transition as smooth as possible, so we'll push out
the whole thing a few days (this is all SeaMonkey-only so we don't have to care
about aviary 1.1a2 too much), I hope it'll be ASAP - still need to schedule an
impact date/time with Chase though.

Additionally, we'll have to wait a few days as well to get MoFo reviews on the
announcement which should go public in a tight timeframe around the happening of
those changes.
Please be warned it's coming up but patient at the same time.

Comment 18

[Robert Kaiser]

checked in along with the big rebranding hit of bug 285696.

I just noticed we might need the same changes as for windows makeall.pl for OS/2
as well. I'll leave this bug open until the dust settles and file a bug for the
OS/2 installer then.

Comment 19

[Robert Kaiser]

Attachment: fix win stub installer, v1

The windows stub installer still has a problem:

cat: seamonkey-1.0a.en-US.win32.stub-installer.jst: No such file or directory
missing .js script: seamonkey-1.0a.en-US.win32.stub-installer.js

(from creature's build log)

We still build mozilla-win32-stub-installer.js though we expect
seamonkey-1.0a.en-US.win32.stub-installer.js now.

It would be quite complicated to preprocess mozilla-win32-stub-installer.jst to
seamonkey-1.0a.en-US.win32.stub-installer.js (or whatever) in one step from
makeall.pl/makejs.pl, so I figured the best way is to create a correctly named
.jst from the makefile (additionally inserting the correct .exe name into it)
and feed that to makeall.pl

I hope the attached fix works.

Comment 20

[Ian Neal]

Tried the patch and comes back with:
cat: seamonkey-1.0a.en-US.win32.stub-installer.jst: No such file or directory
missing .js script: seamonkey-1.0a.en-US.win32.stub-installer.js

 Error: perl makexpi.pl seamonkey-1.0a.en-US.win32.stub-installer
c:/mozdev/mozilla/suite-opt/stage/mozilla c:/mozdev/mozilla/suite-opt/dist/install
Can't return outside a subroutine at
/cygdrive/c/mozdev/mozilla/xpinstall/packager/windows/makeall.pl line 359.

Comment 21

[Ian Neal]

The following line (93) in makejs.pl will need to be changed from:
@inJstFileSplit   = split(/\./,$inJstFile);
to:
@inJstFileSplit   = split(/\.jst/,$inJstFile);

http://lxr.mozilla.org/seamonkey/source/xpinstall/packager/windows/makejs.pl#93

Comment 22

[Ian Neal]

Attachment: win stub installer fix v1.1

This definitely works but probably not the best/prettiest way to fix it.

Comment 23

[Ian Neal]

Attachment: simpler win stub installer fix v1.2

In the previous patch I was trying to avoid writing stuff to
xpinstall/packager/windows directory but other scripts write files to there so
we might as well too.

Comment 24

[[e-mail]]

Attachment: better? win stub installer fix v1.3

Comment 25

[[e-mail]]

Comment on attachment 188144 [details] [diff] [review]
better? win stub installer fix v1.3

Whoops, this also needs IanN's patch to mozilla-win32-stub-installer.jst from
attachment 188113 [details] [diff] [review] to work properly.

Comment 26

[[e-mail]]

Comment on attachment 188144 [details] [diff] [review]
better? win stub installer fix v1.3

Seems like installer puts its temporary files in the srcdir after all.

Comment 27

[[e-mail]]

Attachment: Use a makefile rule to avoid building the .jst each time

Comment 28

[Ian Neal]

Attachment: tweaked makefile rule patch v1.5 (checked in)

tested and generates correct .js files and installers.

Comment 29

[Benjamin Smedberg]

Comment on attachment 188195 [details] [diff] [review]
tweaked makefile rule patch v1.5 (checked in)

In xpinstall/packager/windows/Makefile.in you have changed the order of
rules.mk and the installer: target. I don't think this is a good idea unless
there is a hidden reason. Please revert this change. Otherwise, r+a=me

Comment 30

[Robert Kaiser]

Comment on attachment 188195 [details] [diff] [review]
tweaked makefile rule patch v1.5 (checked in)

checked in with rules.mk moved after installer target again as per bsmedberg's
comment.

I'll leave the bug open until we now all package names are correct on all major
platforms.

Comment 31

[Robert Kaiser]

(In reply to comment #29)
> I don't think this is a good idea unless
> there is a hidden reason.

OK, there's IS a hidden reason: we need $(AB_CD) to be set correctly.
Checked in the change to put rules.mk before installer target now to test if it
fixes creature bustage.

Note this line in creature output (filename is missing "en-US"):
make[1]: *** No rule to make target
`/cygdrive/c/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpinstall/packager/windows/seamonkey-1.0a..win32.stub-installer.jst',
needed by `installer'.  Stop.

Comment 32

[Robert Kaiser]

Attachment: fix the includes the right way

From #developers:

<bsmedberg> KaiRo: re bug 292268, what you wanted to do was include config.mk
before the installer rule, and rules.mk after it
<KaiRo> bsmedberg: I wonder we don't have that in other makefiles... e.g. in
the main packager makefile, we have rules.mk before any other target
<bsmedberg> KaiRo: because the installer target is the default in this
makefile, instead of "all"
<KaiRo> bsmedberg: ah, ok...

Comment 33

[Robert Kaiser]

OS/2 installer has been fixed by mkaply along with the other needed OS/2 work
and recent checkins.

FIXED.