282335 - Make package understand OSes other than Windows

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Kaply [:mkaply]]

Currently, there is code in stage_mozilla.pl that specifically hardcodes -win.

I'm making changes to pass the OS into this function.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Fix for problem

This patch does a couple things.

1. It allows StageMozilla to take an inOS parameter. Previously it only took
gOSPkg which was NOT the inOS (dos on Windows)

2. It changes make_stage.pl to explicitly use win/mac/unix/os2 (as is
documented)

Comment 2

[Mike Kaply [:mkaply]]

Fixed

Comment 3

[Chase Phillips]

Comment on attachment 174379 [details] [diff] [review]
Fix for problem

bsmedberg gave this review, not me.  CVS log entry should have cited him.

Comment 4

[Mike Kaply [:mkaply]]

backed out. need a new patch.

Comment 5

[Mike Kaply [:mkaply]]

Attachment: Better fix per cmp

This is a much better patch (cmp's idea)

basically move the OS check into stage_mozilla.pl since that is the only place
that needs it.

Comment 6

[Chase Phillips]

Comment on attachment 176056 [details] [diff] [review]
Better fix per cmp

r=cmp

Much better.  Still some questionable things happening throughout the packager
code (and those variable names.. arg!).  This patch is a good start to cleaning
up the flow along the way.

Comment 7

[Peter Weilbacher]

Attachment: Additional fixes for OS/2

There were two instances of "windows" left in os2\makeall.pl and OS check in
make_stage.pl did not want os2. I rediffed against current trunk.

Comment 8

[Mike Kaply [:mkaply]]

Fix checked in. I'll watch the TB this time :)

Comment 9

[Chase Phillips]

Comment on attachment 176056 [details] [diff] [review]
Better fix per cmp

Okay, more fun with this patch is to come.  creature has been orange since the
check-in and bryner points out that stage_gre.pl probably needs to be modified,
as well.

I'm backing out until we understand the issues better and want to have another
go at it.

Comment 10

[Chase Phillips]

Comment on attachment 176056 [details] [diff] [review]
Better fix per cmp

Backed out:

Checking in xpinstall/packager/os2/makeall.pl;
/cvsroot/mozilla/xpinstall/packager/os2/makeall.pl,v  <--  makeall.pl
new revision: 1.18; previous revision: 1.17
done
Checking in xpinstall/packager/stage_mozilla.pl;
/cvsroot/mozilla/xpinstall/packager/stage_mozilla.pl,v	<--  stage_mozilla.pl
new revision: 1.8; previous revision: 1.7
done
Checking in xpinstall/packager/make_stage.pl;
/cvsroot/mozilla/xpinstall/packager/make_stage.pl,v  <--  make_stage.pl
new revision: 1.7; previous revision: 1.6
done
Checking in xpinstall/packager/Makefile.in;
/cvsroot/mozilla/xpinstall/packager/Makefile.in,v  <--	Makefile.in
new revision: 1.65; previous revision: 1.64
done

Comment 11

[Mike Kaply [:mkaply]]

I swear I searched for other cases of StageProduct and didn't find them.

ARGH!

OK, my Windows machine is back and I will do VERY thorough testing of a new
patch tomorrow.

I'm really sorry for the inconvenience.

Comment 12

[Chase Phillips]

(In reply to comment #11)
> I swear I searched for other cases of StageProduct and didn't find them.

You should also search for 'win' and 'dos' in that directory and below.

Comment 13

[Mike Kaply [:mkaply]]

Attachment: The complete fix?

OK, so this patch changes stage_gre.pl and stage_mfcembed.pl as well. Those
were the problems.

I've verified that on my system, I get consistent behavior as without my patch.

Comment 14

[Peter Weilbacher]

The last CVS log I see related to this has checked it out again, so I reopen.
Additionally, a

$line =~ s/\$ProductNameNoVersion\$/$nameProductNoVersion/i;

is missing in os2\makejs.pl, so that an installer says "$ProductNameNoVersion$
must be closed to proceed with installation" instead of "Mozilla must be...".

Comment 15

[Mike Kaply [:mkaply]]

Comment on attachment 177010 [details] [diff] [review]
The complete fix?

I'd really like to try this one more time.

At this point, to do OS/2 packaging builds we have to have a patch in our local
tree.

Comment 16

[Chase Phillips]

Comment on attachment 177010 [details] [diff] [review]
The complete fix?

When mkaply's patch is landed we'll need to watch for any unexpected changes in
packaging behaviour.

Comment 17

[Peter Weilbacher]

Despite comment 16 this didn't go in, did it? Are you planning to do that in the
1.8 timeframe or will it wait until after branching?

Just tried it again in my tree, the patches for the Makefile.in's don't work any
more. But even after fiddling it the way I believe it was meant to be I can't
get it to work under OS/2. Trying to create an installer it always goes the
windows path and then complains. Unless of course I directly run make installer
in xpinstall\packager\os2. It seems that we also need an update to
toolkit\mozapps\installer\packager.mk that sets at least INSTALLER_DIR correctly
for OS/2.

Comment 18

[Mike Kaply [:mkaply]]

This is all checked in now.

Last patch was a change to packager.mk in toolkit.

I don't think this will go in 1.8, but I will put together a patch so this
method can be used on 1.8.

Comment 19

[Mike Kaply [:mkaply]]

Attachment: xpinstall patch for 1.8

This does not include the toolkit/mozapps/installer/packager.mk change.

Comment 20

[Mike Kaply [:mkaply]]

Comment on attachment 198333 [details] [diff] [review]
xpinstall patch for 1.8

Incidentally, this also includes the change from bug 310096 so we can build.

OS/2 needs this so we can build installers for seamonkey.

It's been baking on the trunk for a week.

The rest of the patch from 310096 is not in this diff (it's an xpinstall diff)
but it will go in as well.

Comment 21

[Asa Dotzler [:asa]]

Comment on attachment 198333 [details] [diff] [review]
xpinstall patch for 1.8

moving request to 1.8rc1 since 1.8b5 has shipped.

Comment 22

[Asa Dotzler [:asa]]

Are we sure this didn't break anything? Is this suite only?

Comment 23

[Mike Kaply [:mkaply]]

This change caused no problems - we would have known immediately if there were
packaging issues.

The patch does affect all packaging.

Comment 24

[Asa Dotzler [:asa]]

Mike, this needs to land ASAP if it's gonna make the release. Thanks.

Comment 25

[Mike Kaply [:mkaply]]

Landed this morning.