190389 - GRE installer needs to use new version key format

Status: VERIFIED FIXED

(Core Graveyard :: Installer: GRE, defect)

Description

[Sean Su]

The GRE installer currently registers itself as the following in the windows
regsitry:
  HKEY_LOCAL_MACHINE\Software\mozilla.org\GRE\[version]

where [version] is currently in the following format:
  1.3b

This [version] needs to be in the following new format:
  1.3b-[build id]

where [build id] is acquired from dist/include/nsBuildID.h.  '1.3b' should also
be automatically acquired from dist/include/mozilla-config.h, but not necessary
for this particular fix.

Comment 1

[Sean Su]

I'm not sure if the install path should also include the build id as part of the
dir name:

  ie. [program files]\mozilla.org\GRE\1.3b-[build id]

I'll think about it some more...

Comment 2

[Chak Nanga]

We need to make sure that both MfcEmbed and Mozilla are also updated to look for
the new registry key entry as part of fixing this bug.

Comment 3

[Chak Nanga]

Also, what will the build ID be, say, when we're done with a Mozilla milestone?

For ex, when 1.3 final is out the door, will there still be a build ID ?

Comment 4

[Doug Turner (:dougt)]

i will fix both MFC embed and mozilla.

There will always be a build id.

Comment 5

[Juan José Mata]

Sean, should this buildID be included in the same field or create a new field
under the same key?  I'm thinking that buildID is hopefully too much granularity
for GRE clients (assuming we don't have to release GRE revisions every other
day), in case they do need to look at the buildID, then another field would be
fine, but parsing of all the extra info might be overkill in the general case.

Discussed at EDT: topembed+, please target accordingly.

Comment 6

[Chak Nanga]

I agree with Juan on having a separate build ID field under the same key. 

As we start freezing more of our embedding APIs and head towards a binary
distribution model, we can steer embeddors towards "certifying" their apps
against a released version of a GRE rather than against a specific build IDs etc.

Comment 7

[Doug Turner (:dougt)]

the nsBuildID must be at the KEY level so that you have have more than one of
the same milestone GRE installed on your disk.

Comment 8

[Juan José Mata]

Should it then be added one layer deeper, rather than merged together with the
top-level version number?  It can still be a key that way.  Of course that would
mean that when faced with the question "which 1.3b GRE to use?" code should be
smart enough to either specify the actual buildID that it was certified/tested
against or the latest one which hopefully is the most stable available.

Comment 9

[Doug Turner (:dougt)]

i don't think that will work.  

Comment 10

[Sean Su]

Attachment: patch v1.0

This patch will have GRE installer use a usergent value like '1.3b-[build id]'
for all the places in the windows registry needing a useragent value.

This means that it will now create:
  HKEY_LOCAL_MACHINE\Software\mozilla.org\GRE\1.3b-2003013001

where 2003013001 is the build id acquired from mozilla/config/nsBuildID.h.

Comment 11

[Sean Su]

Attachment: patch v1.1

updated the patch to look for nsBuildID.h in the dist/include dir instead of
directly from the mozilla/config dir.

Comment 12

[Sean Su]

this bug needs to be fixed at the same time as a similar bug for mozilla. 
Mozilla needs to also look for the GRE version with the buildID, if not then
this patch will break mozilla.

Comment 13

[Chak Nanga]

I'm concerned that we've still not clearly addressed Juan's points in #5 and #8
- a build ID is too granular for embeddors to write their apps to.

Also, could you please explain the rationale for including the build ID in the
top level reg key or whereever?

Comment 14

[timeless]

i think java and other things do something like this:

majorkey\
@default value=preferedsubkey

so:
[HKEY_LOCAL_MACHINE\Software\mozilla.org\GRE]
@="1.3b"
[HKEY_LOCAL_MACHINE\Software\mozilla.org\GRE\1.3b]
@="2003013001"
[HKEY_LOCAL_MACHINE\Software\mozilla.org\GRE\1.3b\2003013001]

is there something wrong with that approach?

Comment 15

[Sean Su]

I think that the better fix would be bug 180383.  the installer code can already
detect if a GRE is compatible a the file version level (xpcom.dll to be exact).
 The browser code should be doing the same, not just looking at the windows
registry.

Comment 16

[Sean Su]

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

unrequesting the reviews until we figure out how we really want to resolve this
issue.

Comment 17

[Doug Turner (:dougt)]

ssu - we need to land together on this one.  When you land, I must land at the
same time.  

Comment 18

[Sean Su]

nsbeta1+ per samir.

Comment 19

[Sean Su]

oops, forgot adt1

Comment 20

[Sean Su]

Attachment: patch v2.0 (ns tree)

Comment 21

[Sean Su]

Attachment: patch v2.0 (moz tree)

Comment 22

[Sean Su]

The latest patches will automatically have the installer build process parse the
following files for the appripriate build versions:
  [topsrcdir]\config\milestone.txt
  mozilla\config\nsBuildID.h

Once these patches have been checked in, the release nightly and milestone build
scripts will _no_longer_ be required to pass in build versions (the first 2
args) to the following makeall.pl scripts:
  ns\xpinstall\packager\windows\makeall.pl
  mozilla\xpinstall\packager\windows\makeall.pl
  mozilla\xpinstall\packager\win_gre\makeall.pl
  mozilla\xpinstall\packager\win_mfcembed\makeall.pl

These patches will not build generate the new GRE ID (ie: 1.4a_2003030610) right
now as required by this bug.  This is because bug 195840 has not been fixed yet.

So for now, everything will still work as before (with the exception of the
build process changes).  In order to enable the scripts to generate the required
new GRE ID, I'll attach another patch which will simply be a few lines change to
have it turned on.

All the changes in these patches are still required for the new GRE ID to be
properly generated and working with the installers.

Comment 23

[Samir Gehani]

Comment on attachment 116461 [details] [diff] [review]
patch v2.0 (ns tree)


>+$initEmptyValues          = 1;

Please comment about stuffing zeroes for non-existent ``parts'' of the version.

> # set environment vars for use by other .pl scripts called from this script.
>-if($versionParts[2] eq "0")
>+if($versionParts[2] eq "0" || $#versionParts eq 1)

We don't need the second comparison.

>+$ENV{WIZ_greUniqueID}          = StageUtils::GetGreSpecialID("$topsrcdir/../mozilla");

Please document what the special id is, where it comes from providing an
example or two. 

r=sgehani with those changes.

Comment 24

[Samir Gehani]

Comment on attachment 116462 [details] [diff] [review]
patch v2.0 (moz tree)


>+# GetGreFileVersion()
>+#   To build GRE's file version as follows:
>+#     * Use mozilla's milestone version for 1st 2 numbers of version x.x.x.x.
>+#     * Strip out any non numerical chars from mozilla's milestone version.
>+#     * Get the y2k ID from mozilla/config/nsBuildID.h.
>+#     * Split the y2k ID exactly in 2 equal parts and use them for the last
>+#       2 numbers of the version x.x.x.x.
>+#         ie: y2k: 2003030510
>+#             part1: 20030
>+#             part2: 30510

Please comment with a big XXX that this won't work for dates > 65536, e.g.,
20030 71608 (July 16, 2003 8am).  This is a problem and we need to file a bug
to solve it.

>+  $versionMilestone = `perl milestone.pl --topsrcdir $aDirConfigTopSrc`;

Should we execute this using perl routines rather than assuming the
interpreter's name as perl.exe?  I seem to recall some installation having the
``perl5'' interpreter name.  Maybe cls can comment?  Is this a moot issue?

r=sgehani with the above changes and changes to the above patch applied here as
well.

Comment 25

[Sean Su]

Attachment: patch v2.1 (ns tree)

updated patch with sgehani suggested changes.

Comment 26

[Daniel Veditz [:dveditz]]

it's probably "good enough" to distinguish by date, without having to go into
hour granularity (it's good enough for the User Agent, anyway). In that case the
date breaks into year and month/day parts that always fit in a short.

By default two dev builds on the same day will share a GRE, and the force GRE
install mechanism will ensure it's the most recent one. Given that the most
likely reason for two builds on the same day is a blocker re-spin that's
probably desired behavior.

The other thing you could try would be to chop off the "2" in 2003 and spit it 5
and 4 -- 00303 0510 in the example above. We won't roll over until the year 2656
and you get to keep hour granularity if people think it's important. (with the
downside of a harder-to-understand version number.)

Comment 27

[Sean Su]

Attachment: patch v2.1 (moz tree)

updated patch with sgehani suggested changes.

Comment 28

[Sean Su]

Attachment: patch v1.0 (moz tree)

This patch can only be checked in _after_ patches v2.1 have been checked in. 
This patch will simply switch the build scripts from using the simple
milestone.txt version (eg: 1.4a) to using a new define (GRE_BUILD_ID in
nsBuildID.h) that cls is almost done with, which would then return an ID of
whatever GRE_BUILD_ID is set to (should be something like '1.4a_2003030610').

Comment 29

[Daniel Veditz [:dveditz]]

Comment on attachment 116499 [details] [diff] [review]
patch v1.0 (moz tree)

sr=dveditz

Comment 30

[Daniel Veditz [:dveditz]]

Comment on attachment 116496 [details] [diff] [review]
patch v2.1 (ns tree)

>-Parameter=-mmi -ma -app "$ProductName$" -app_path "[SETUP PATH]\$MainExeFile$"
>+Parameter=-mmi -ma -app "$ProductName$ $UserAgent$" -app_path "[SETUP PATH]\$MainExeFile$"

Should it be $ProductNameInternal$? Change if it needs it before checking in

sr=dveditz

Comment 31

[Daniel Veditz [:dveditz]]

Comment on attachment 116497 [details] [diff] [review]
patch v2.1 (moz tree)

>Index: packager/windows/config.it
>===================================================================
>+Parameter=-mmi -ma -app "$ProductName$ $UserAgent$" -app_path "[SETUP PATH]\$MainExeFile$"

$ProductNameInternal$ here too for consistency.

sr=dveditz

Comment 32

[Sean Su]

patches v2.1 (ns and moz) have been checked in.

Leaf, please update the nightly build scripts to not pass the version numbers
(the 1st 2 params) to any of the makeall.pl files.  They are no longer required.

patch v1.0 (moz tree) has not been checked in.  waiting on others before
flipping the switch.

Comment 33

[Sean Su]

patch v1.0 (moz tree) checked in.  Code is now enabled.  marking this bug fixed.

Comment 34

[Patrick]

Not sure if this is the right bug, but comment 2 made check my \COMMON FILES\
folder to see a whole load of mozilla.org\GRE\ folders with different build
numbers. 

Just a side effect for people that don't use uninstall before installing new builds.

Comment 35

[jay garcia]

Patrick: I just posted my scenario in n.p.m.general alluding to the fact that I
ran out of disk space when installing the latest nightly. My /common
files/mozilla.org/GRE/ was 138 megs !!!!

Comment 36

[Daniel Veditz [:dveditz]]

See bug 198081

Comment 37

[Charles Rosendahl]

This is verified - the above bug addresses the installer logic bug to remove
prior GRE installationsref counted to zero.