Last Comment Bug 282335 - Make package understand OSes other than Windows
: Make package understand OSes other than Windows
Status: RESOLVED FIXED
: fixed1.8
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-02-15 07:14 PST by Mike Kaply [:mkaply]
Modified: 2009-09-17 13:47 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix for problem (3.38 KB, patch)
2005-02-15 07:16 PST, Mike Kaply [:mkaply]
benjamin: review+
Details | Diff | Splinter Review
Better fix per cmp (5.07 KB, patch)
2005-03-02 10:07 PST, Mike Kaply [:mkaply]
chase: review+
Details | Diff | Splinter Review
Additional fixes for OS/2 (5.61 KB, patch)
2005-03-08 16:08 PST, Peter Weilbacher
no flags Details | Diff | Splinter Review
The complete fix? (13.49 KB, patch)
2005-03-10 06:44 PST, Mike Kaply [:mkaply]
chase: review+
Details | Diff | Splinter Review
xpinstall patch for 1.8 (15.59 KB, patch)
2005-10-03 11:45 PDT, Mike Kaply [:mkaply]
asa: approval1.8rc1+
Details | Diff | Splinter Review

Description Mike Kaply [:mkaply] 2005-02-15 07:14:23 PST
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] 2005-02-15 07:16:44 PST
Created attachment 174379 [details] [diff] [review]
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] 2005-02-18 09:48:50 PST
Fixed
Comment 3 Chase Phillips 2005-02-18 15:38:42 PST
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] 2005-02-21 14:09:45 PST
backed out. need a new patch.
Comment 5 Mike Kaply [:mkaply] 2005-03-02 10:07:49 PST
Created attachment 176056 [details] [diff] [review]
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 2005-03-07 17:43:43 PST
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 2005-03-08 16:08:23 PST
Created attachment 176800 [details] [diff] [review]
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] 2005-03-09 10:50:14 PST
Fix checked in. I'll watch the TB this time :)
Comment 9 Chase Phillips 2005-03-09 16:34:24 PST
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 2005-03-09 16:37:18 PST
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] 2005-03-09 16:42:49 PST
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 2005-03-09 16:54:15 PST
(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] 2005-03-10 06:44:49 PST
Created attachment 177010 [details] [diff] [review]
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 2005-03-19 01:20:13 PST
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] 2005-06-20 07:20:25 PDT
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 2005-07-01 12:37:28 PDT
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 2005-07-27 09:01:46 PDT
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] 2005-10-03 08:24:53 PDT
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] 2005-10-03 11:45:33 PDT
Created attachment 198333 [details] [diff] [review]
xpinstall patch for 1.8

This does not include the toolkit/mozapps/installer/packager.mk change.
Comment 20 Mike Kaply [:mkaply] 2005-10-03 11:48:08 PDT
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] 2005-10-06 21:59:59 PDT
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] 2005-10-10 14:33:33 PDT
Are we sure this didn't break anything? Is this suite only?
Comment 23 Mike Kaply [:mkaply] 2005-10-11 08:55:52 PDT
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] 2005-10-12 12:04:55 PDT
Mike, this needs to land ASAP if it's gonna make the release. Thanks.
Comment 25 Mike Kaply [:mkaply] 2005-10-12 12:30:45 PDT
Landed this morning.

Note You need to log in before you can comment on or make changes to this bug.