Make package understand OSes other than Windows

RESOLVED FIXED

Status

()

Core
Build Config
RESOLVED FIXED
12 years ago
8 years ago

People

(Reporter: mkaply, Unassigned)

Tracking

({fixed1.8})

Trunk
fixed1.8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

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

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

Comment 1

12 years ago
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)
(Reporter)

Updated

12 years ago
Attachment #174379 - Flags: review?(benjamin)
Attachment #174379 - Flags: review?(benjamin) → review+
(Reporter)

Comment 2

12 years ago
Fixed
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 3

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

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

Comment 4

12 years ago
backed out. need a new patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 5

12 years ago
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.
(Reporter)

Updated

12 years ago
Attachment #174379 - Attachment is obsolete: true
Attachment #176056 - Flags: review?(cmp)

Comment 6

12 years ago
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.
Attachment #176056 - Flags: review?(chase) → review+

Comment 7

12 years ago
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.
(Reporter)

Comment 8

12 years ago
Fix checked in. I'll watch the TB this time :)
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED

Comment 9

12 years ago
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

12 years ago
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
(Reporter)

Comment 11

12 years ago
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

12 years ago
(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.
(Reporter)

Comment 13

12 years ago
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.
Attachment #176056 - Attachment is obsolete: true
Attachment #176800 - Attachment is obsolete: true

Comment 14

12 years ago
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...".
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 15

12 years ago
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.
Attachment #177010 - Flags: review?(chase)

Comment 16

12 years ago
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.
Attachment #177010 - Flags: review?(chase) → review+

Comment 17

12 years ago
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.
(Reporter)

Comment 18

12 years ago
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.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
(Reporter)

Comment 19

12 years ago
Created attachment 198333 [details] [diff] [review]
xpinstall patch for 1.8

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

Comment 20

12 years ago
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.
Attachment #198333 - Flags: approval1.8b5?

Comment 21

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

moving request to 1.8rc1 since 1.8b5 has shipped.
Attachment #198333 - Flags: approval1.8b5? → approval1.8rc1?

Comment 22

12 years ago
Are we sure this didn't break anything? Is this suite only?
(Reporter)

Comment 23

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

The patch does affect all packaging.

Updated

12 years ago
Attachment #198333 - Flags: approval1.8rc1? → approval1.8rc1+

Comment 24

12 years ago
Mike, this needs to land ASAP if it's gonna make the release. Thanks.
(Reporter)

Comment 25

12 years ago
Landed this morning.
Keywords: fixed1.8

Updated

8 years ago
QA Contact: build-config
You need to log in before you can comment on or make changes to this bug.