162079 - installer: lack of objdir support

Status: RESOLVED FIXED

(SeaMonkey :: Installer, defect)

Description

[Bernard Alleysson]

fix for bug 158729 does not work when MOZ_OBJDIR is set in the mozconfig file

Comment 1

[Bernard Alleysson]

Attachment: 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

Comment 2

[hacker formerly known as [e-mail]]

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.

Comment 3

[Bernard Alleysson]

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.

Comment 4

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

installer fails on linux as well if an objdir is used. changing os->all

Comment 5

[Syd Logan]

cc'ing curt and sean. this might intersect with build script cleanup work that
is in progress.

Comment 6

[hacker formerly known as [e-mail]]

Attachment: 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).

Comment 7

[John Keiser (jkeiser)]

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

Comment 8

[Sean Su]

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.

Comment 9

[Sean Su]

samir said it looks fine.

Comment 10

[Benjamin Smedberg]

Attachment: 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.

Comment 11

[Sean Su]

Attachment: 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.

Comment 12

[Sean Su]

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

Comment 13

[Daniel Veditz [:dveditz]]

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

sr=dveditz

Comment 14

[Daniel Veditz [:dveditz]]

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

sr=dveditz

Comment 15

[Daniel Veditz [:dveditz]]

Sean, please check these in

Comment 16

[Sean Su]

patch checked in.  closing bug as fixed.

Comment 17

[hacker formerly known as [e-mail]]

*ahem* You only fixed the problem on windows.

Comment 18

[hacker formerly known as [e-mail]]

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

[Benjamin Smedberg]

Attachment: fix objdir installer for nsBuildID.h

Trivial little patch.

Comment 20

[hacker formerly known as [e-mail]]

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. 

Comment 21

[hacker formerly known as [e-mail]]

Attachment: make deliver.pl objdir aware & fix bogus strip errors in makexpi.pl

Comment 22

[Sean Su]

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

r=ssu

Comment 23

[Sean Su]

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.

Comment 24

[Sean Su]

Attachment: 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.

Comment 25

[Sean Su]

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.

Comment 26

[hacker formerly known as [e-mail]]

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.

Comment 27

[Sean Su]

Attachment: 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.

Comment 28

[hacker formerly known as [e-mail]]

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.

Comment 29

[Sean Su]

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

Comment 30

[Sean Su]

Attachment: patch v1.1

updated patch to trunk cause it was bit-rotted.

Comment 31

[Sean Su]

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

moving r=seawood to here.
seeking sr=

Comment 32

[Daniel (Leaf) Nunes]

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

I will try and get to this review tomorrow.

Comment 33

[Daniel (Leaf) Nunes]

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

woot.

Comment 34

[Sean Su]

Attachment: patch v1.2

tested on a release machine and found minor issues.  this is an update to fix
the minor issues.

Comment 35

[Sean Su]

patch checked in.  pass '-h' to build.pl for a list of options.  '-objdir
[path]' is now supported under win32.

Comment 36

[hacker formerly known as [e-mail]]

The deliver.pl changes never got checked in.  They were waiting for sr from
leaf; do we not need the sr anymore?

Comment 37

[Sean Su]

oops.  sorry.  as far as I'm aware, we still need the sr=.  I'll reassign the
bug to you then.

Comment 38

[Daniel (Leaf) Nunes]

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

sorry! sr=leaf

Comment 39

[hacker formerly known as [e-mail]]

The deliver.pl & makexpi.pl changes have been checked in.

Comment 40

[Ben Bucksch (:BenB)]

Attachment: 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.

Comment 41

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 42

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: 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

[Daniel (Leaf) Nunes]

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

r=leaf