installer: lack of objdir support

RESOLVED FIXED in mozilla1.5alpha

Status

RESOLVED FIXED
17 years ago
14 years ago

People

(Reporter: bernard.alleysson, Assigned: netscape)

Tracking

Trunk
mozilla1.5alpha

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 6 obsolete attachments)

3.23 KB, patch
ssu0262
: review+
dveditz
: superreview+
Details | Diff | Splinter Review
5.67 KB, patch
ssu0262
: review+
dveditz
: superreview+
Details | Diff | Splinter Review
4.99 KB, patch
ssu0262
: review+
Details | Diff | Splinter Review
65.33 KB, patch
Details | Diff | Splinter Review
1.68 KB, patch
Details | Diff | Splinter Review
1.13 KB, patch
leaf
: review+
Details | Diff | Splinter Review
(Reporter)

Description

17 years ago
fix for bug 158729 does not work when MOZ_OBJDIR is set in the mozconfig file
(Reporter)

Comment 1

17 years ago
Created attachment 94773 [details] [diff] [review]
fix build_static.pl when MOZ_OBJDIR set in mozconfig

see http://bugzilla.mozilla.org/show_bug.cgi?id=158729#c22
if this is ok then the same should be done to build.pl
Over to the installer team.  Setting MOZ_OBJDIR in your mozconfig doesn't add it
to the standard build env.  If you run the build script standalone via perl, it
won't know about MOZ_OBJDIR either.
Assignee: seawood → dveditz
Component: Build Config → Installer
QA Contact: granrose → bugzilla
(Reporter)

Comment 3

17 years ago
I have to set the NT env var (set MOZ_OBJDIR=1) for the perl script *and* add
MOZ_OBJDIR to my mozconfig. It's just a quick hack.
installer fails on linux as well if an objdir is used. changing os->all

OS: Windows XP → All
Hardware: PC → All

Comment 5

16 years ago
cc'ing curt and sean. this might intersect with build script cleanup work that
is in progress.
Created attachment 108336 [details] [diff] [review]
objdir support for deliver.pl

This patch allows deliver.pl to be run from anywhere by specifying the
topsrcdir (-s) & the top of the build tree (-o).

Updated

16 years ago
Attachment #94773 - Flags: superreview?(dveditz)

Comment 7

16 years ago
This is important for tinderboxen that want to generate installer builds (for
example to upload them so that people can try them).

Updated

16 years ago
Attachment #94773 - Flags: review?(syd) → review?(ssu)

Comment 8

16 years ago
Comment on attachment 94773 [details] [diff] [review]
fix build_static.pl when MOZ_OBJDIR set in mozconfig

it looks good to me.  However, samir should take a quick look too since it's
his baby.
Attachment #94773 - Flags: review?(ssu) → review+

Comment 9

16 years ago
samir said it looks fine.

Comment 10

16 years ago
Created attachment 116654 [details] [diff] [review]
add command-line option to build.pl

This patch adds a command-line option -objdir to build.pl; there are also
several bugfixes in other files where options were not being passed properly.
It works for me, and it shouldn't break things, but I haven't tested very
thoroughly.

Updated

16 years ago
Attachment #116654 - Flags: review?(ssu)

Comment 11

16 years ago
Created attachment 117625 [details] [diff] [review]
updated from Benjamin Smedberg that is merged with trunk

Benjamin Smedberg's previous patch was bit-rotted.  this new patch has been
updated with recent trunk.
Attachment #116654 - Attachment is obsolete: true

Updated

16 years ago
Attachment #116654 - Flags: review?(ssu)

Comment 12

16 years ago
Comment on attachment 117625 [details] [diff] [review]
updated from Benjamin Smedberg that is merged with trunk

Benjamin, sorry for the delay in reviewing your patch.

I don't have a build where the objdir is elsewhere than the default area.

This patch works fine for the normal case.

r=ssu
Attachment #117625 - Flags: review+
Comment on attachment 117625 [details] [diff] [review]
updated from Benjamin Smedberg that is merged with trunk

sr=dveditz
Attachment #117625 - Flags: superreview+
Comment on attachment 94773 [details] [diff] [review]
fix build_static.pl when MOZ_OBJDIR set in mozconfig

sr=dveditz
Attachment #94773 - Flags: superreview?(dveditz) → superreview+
Sean, please check these in
Assignee: dveditz → ssu

Comment 16

16 years ago
patch checked in.  closing bug as fixed.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
*ahem* You only fixed the problem on windows.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This is still broken for win32 objdir builds as well.  makeall.pl is still
looking for nsBuildID.h at $topsrcdir/config/nsBuildID.h .

Comment 19

16 years ago
Created attachment 118846 [details] [diff] [review]
fix objdir installer for nsBuildID.h

Trivial little patch.

Updated

16 years ago
Attachment #118846 - Flags: superreview?(dveditz)
Attachment #118846 - Flags: review?(ssu)
There's a lot more broken than just the nsBuildID.h issue.  That's just the
first problem you'll hit.  StageUtils.pm needed to be updated to understand
objdir as well as xpinstall/packager/windows/makeall.pl &
xpinstall/packager/win_gre/makeall.pl.  Then make_stage.pl and the assorted
stage_*.pl files.  That's about as far as I got before I got really annoyed with
it. 
Attachment #108336 - Attachment is obsolete: true
Created attachment 118873 [details] [diff] [review]
make deliver.pl objdir aware & fix bogus strip errors in makexpi.pl
Attachment #118873 - Flags: superreview?(dveditz)
Attachment #118873 - Flags: review?(ssu)

Comment 22

16 years ago
Comment on attachment 118846 [details] [diff] [review]
fix objdir installer for nsBuildID.h

r=ssu
Attachment #118846 - Flags: review?(ssu) → review+

Comment 23

16 years ago
Comment on attachment 118846 [details] [diff] [review]
fix objdir installer for nsBuildID.h

I just got a objdir build working on my machine and this patch is not going to
work.  There are other places that need to be fixed as well.
Attachment #118846 - Flags: review+ → review-

Comment 24

16 years ago
Created attachment 119331 [details] [diff] [review]
patch 0.1

Here my crack at fixing the win32 installer build scripts to support objdir. 
This  is working for me, but haven't run a complete test yet.  It might still
need some tweaking.
Attachment #118846 - Attachment is obsolete: true

Comment 25

16 years ago
Comment on attachment 119331 [details] [diff] [review]
patch 0.1

this patch works with mozilla built with and without objdir set *and*/*or* with
ns build with and without objdir set.
Attachment #119331 - Flags: review?(seawood)
Comment on attachment 119331 [details] [diff] [review]
patch 0.1

>-#     * Get the y2k ID from mozilla/config/nsBuildID.h.
>+#     * Get the y2k ID from mozilla $topobjdir/dist/include/nsBuildID.h.

'mozilla' should be removed as well.

> sub GetGreSpecialID
> {
>-  my($fileBuildID)                      = "$aDirMozTopSrc/config/nsBuildID.h";
>+  my($fileBuildID)                      = "$aDirMozTopSrc/dist/include/nsBuildID.h";

Is $aDirMozTopSrc assumed to be $topobjdir ?  It's possible to build with dist
somewhere other than <buildroot>/dist .  The toplevel script appears to support
that configuration so this function should too.

> sub GetGreFileVersion
> {

> sub GetProductY2KVersion
> {

Same here.

The rest of the changes look ok but I can't test them as my laptop hd died. 
Hopefully, I'll get it back next week.
Attachment #119331 - Flags: review?(seawood) → review-

Comment 27

16 years ago
Created attachment 119385 [details] [diff] [review]
patch v1.0

addresses seawood's comments:
>>-#	 * Get the y2k ID from mozilla/config/nsBuildID.h.
>>+#	 * Get the y2k ID from mozilla $topobjdir/dist/include/nsBuildID.h.
>
>'mozilla' should be removed as well.

 Can't because GetGreSpecialID() comes from the mozilla tree.

>> sub GetGreSpecialID
>> {
>>-  my($fileBuildID)	     =	"$aDirMozTopSrc/config/nsBuildID.h";
>>+  my($fileBuildID)	     =	"$aDirMozTopSrc/dist/include/nsBuildID.h";
>
>Is $aDirMozTopSrc assumed to be $topobjdir ?  It's possible to build with
>dist somewhere other than <buildroot>/dist .  The toplevel script appears
>to support that configuration so this function should too.

 $aDirMozTopSrc should be mozilla's topobjdir.	The code already handles that.
 Just the var name doesn't quite reflect this.	Changed the var name to 
 $aDirMozTopObj.

>> sub GetGreFileVersion
>> {
>
>> sub GetProductY2KVersion
>> {
>
>Same here.

 GetGreFileVersion() and GetProductY2KVersion() also already support for
 dist to be elsewhere than the buildroot dir.  Var names also reflect that
 already.  However, the $aDirMozTopSrc is correct.  It needs to know where
 mozilla's topsrcdir is at.
Attachment #119331 - Attachment is obsolete: true

Updated

16 years ago
Attachment #119385 - Flags: review?(seawood)
Comment on attachment 119385 [details] [diff] [review]
patch v1.0

Looks ok to me (minus that $aDirMozTopObj/dist/include/nsBuildID.h issue) but I
still can't test it.
Attachment #119385 - Flags: review?(seawood) → review+

Comment 29

16 years ago
Comment on attachment 118873 [details] [diff] [review]
make deliver.pl objdir aware & fix bogus strip errors in makexpi.pl

r=ssu

works with normal build and objdir build
Attachment #118873 - Flags: review?(ssu) → review+

Updated

16 years ago
Attachment #119385 - Flags: superreview?(leaf)
Attachment #118873 - Flags: superreview?(dveditz) → superreview?(leaf)
Attachment #118846 - Flags: superreview?(dveditz)

Comment 30

16 years ago
Created attachment 125786 [details] [diff] [review]
patch v1.1

updated patch to trunk cause it was bit-rotted.
Attachment #119385 - Attachment is obsolete: true

Updated

16 years ago
Attachment #118873 - Flags: superreview?(leaf)

Updated

16 years ago
Attachment #119385 - Flags: superreview?(leaf)

Comment 31

16 years ago
Comment on attachment 125786 [details] [diff] [review]
patch v1.1

moving r=seawood to here.
seeking sr=
Attachment #125786 - Flags: superreview?(leaf)
Attachment #125786 - Flags: review+

Comment 32

16 years ago
Comment on attachment 125786 [details] [diff] [review]
patch v1.1

I will try and get to this review tomorrow.

Comment 33

16 years ago
Comment on attachment 125786 [details] [diff] [review]
patch v1.1

woot.
Attachment #125786 - Flags: superreview?(leaf) → superreview+

Comment 34

16 years ago
Created attachment 126775 [details] [diff] [review]
patch v1.2

tested on a release machine and found minor issues.  this is an update to fix
the minor issues.
Attachment #125786 - Attachment is obsolete: true

Comment 35

16 years ago
patch checked in.  pass '-h' to build.pl for a list of options.  '-objdir
[path]' is now supported under win32.
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
The deliver.pl changes never got checked in.  They were waiting for sr from
leaf; do we not need the sr anymore?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 37

16 years ago
oops.  sorry.  as far as I'm aware, we still need the sr=.  I'll reassign the
bug to you then.
Assignee: ssu → seawood
Status: REOPENED → NEW
Attachment #118873 - Flags: superreview?(leaf)

Comment 38

16 years ago
Comment on attachment 118873 [details] [diff] [review]
make deliver.pl objdir aware & fix bogus strip errors in makexpi.pl

sorry! sr=leaf
Attachment #118873 - Flags: superreview?(leaf) → superreview+
The deliver.pl & makexpi.pl changes have been checked in.
Status: NEW → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.5alpha

Comment 40

15 years ago
Created attachment 137744 [details] [diff] [review]
Additional fixes for the 1.4 branch

In case anyone wants to have this on the 1.4 branch: I ported the patches in
this bug to the 1.4 branch, but accidently mixed them with other branch
updates, so I'm not sure I can provide a clean patch from my tree. I did run
into 2-3 additional bugs, though (already fixed on the trunk), I am attaching a
fix for them.
I'm trying to use this (on Fedora Core 1), but for some reason the getopts call
in deliver.pl isn't filling in opt_s and opt_o.  If I move the -o and -s
arguments to the beginning of the argument list, it works.
Created attachment 141559 [details] [diff] [review]
fix xpinstall/packager/unix/Makefile.in

This makefile didn't even exist when this patch was checked in.  In any case, I
think this needs to be fixed.

Comment 43

15 years ago
Comment on attachment 141559 [details] [diff] [review]
fix xpinstall/packager/unix/Makefile.in

r=leaf
Attachment #141559 - Flags: review?(leaf) → review+
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.