make version_win.pl able to use the app version from configure vars

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
Build Config
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: Robert Kaiser, Assigned: Robert Kaiser)

Tracking

({fixed1.9.1})

Trunk
mozilla1.9.2a1
x86
Windows XP
fixed1.9.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

12 years ago
This is from bug 288690 comment 7:
"We may want to replace the version number in module.ver as well, I realized in
the last days. I'll probably have to look into how to do it best in another bug
one time..."

We should be able to use MOZ_APP_VERSION in module.ver at least, so that WIN32_MODULE_PRODUCTVERSION_STRING can be set automatically from that var. The bigger problem is how to automate WIN32_MODULE_PRODUCTVERSION which has a different format (4 comma-separated numbers) than our usual version strings...
(Assignee)

Updated

9 years ago
Duplicate of this bug: 480988
(Assignee)

Comment 2

9 years ago
Created attachment 372040 [details] [diff] [review]
Enable application version to be replaced

I haven't tested this patch yet as I have no Windows build box around, but this should do it. Depending on the application version, it calculates the 4-part comma-separated digits-only version identifiers and uses those as well as the app version if specified in module.ver.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #372040 - Flags: review?(bhearsum)
(Assignee)

Comment 3

9 years ago
Created attachment 372041 [details] [diff] [review]
use all those generics in Firefox

I don't know if Firefox wants to take this, but this patch at least enables the full possibility for testing what version_win.pl can do now by using all those replacement strings in module.ver and splash.rc

We intend to do things this way for SeaMonkey, but for Firefox, I'll leave the decision to others ;-)
(Assignee)

Comment 4

9 years ago
Comment on attachment 372040 [details] [diff] [review]
Enable application version to be replaced

> $mfversion = $mpversion = $milestone;
>+if ($appversion eq "") {
>+  $appversion;
>+}

Umm, just spotted that - this should be "$appversion = $milestone;" (for correctness only, we shouldn't actually hit this with our call from version.mk in any case). Fixed locally, but I don't think it's worth to upload another patch just for that.
(Assignee)

Comment 5

9 years ago
Created attachment 372043 [details] [diff] [review]
comm-central/SeaMonkey part

This is the comm-central/SeaMonkey patch I'd like to have going with this, I'll request review when the main patch is accepted (and hopefully can even make 1.9.1).
Comment on attachment 372040 [details] [diff] [review]
Enable application version to be replaced

I don't think I'm the right person to review this.
Attachment #372040 - Flags: review?(bhearsum) → review?(ted.mielczarek)
Comment on attachment 372040 [details] [diff] [review]
Enable application version to be replaced

+if ($appversion eq "") {
+  $appversion;
+}

This doesn't seem to do anything.

+my @appver = split(/\./,$appversion);
+$appver[1] =~s/\D.*$//;
+if (!$appver[2]) {
+    $appver[2] = "0";
+}

Feels like this could be a loop, since you're doing pretty much the same sanitization on each version part.

r=me with those fixed.
Attachment #372040 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 8

9 years ago
Comment on attachment 372041 [details] [diff] [review]
use all those generics in Firefox

I'll do a new iteration of the main patch addressing the comments and probably changing the second similar block to a loop as well, but functionality will stay the same.
Because of that, the patch to actually use it should work fine (only that version numbers changed around so it won't apply cleanly, but it's still trivial to apply).
Attachment #372041 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 9

9 years ago
Comment on attachment 372043 [details] [diff] [review]
comm-central/SeaMonkey part

This depends on the main patch landing on 1.9.1, of course. Same module.ver change can be done for the other comm-central apps as well following this (if wanted) :)
Attachment #372043 - Flags: review?(bugzilla)
(Assignee)

Comment 10

9 years ago
Created attachment 375187 [details] [diff] [review]
Enable application version to be replaced, v1.1

Re-requesting review for sanity checking that I addressed the comments correctly. I didn't actually do more than that, the other 4-part-version sanitation isn't really repeating stuff so doesn't need a loop.
Attachment #372040 - Attachment is obsolete: true
Attachment #375187 - Flags: review?(ted.mielczarek)
Comment on attachment 372043 [details] [diff] [review]
comm-central/SeaMonkey part

Looks good. I've not tested this, but it looks right.

r=Standard8
Attachment #372043 - Flags: review?(bugzilla) → review+
Attachment #375187 - Flags: review?(ted.mielczarek) → review+
Attachment #372041 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 12

9 years ago
Main patch pushed as http://hg.mozilla.org/mozilla-central/rev/6c911e466654 and Firefox generics pushed as http://hg.mozilla.org/mozilla-central/rev/d518bbb7db86 - and copied the fix to js/src/config as http://hg.mozilla.org/mozilla-central/rev/7d50e5e9b8a5 to fix JS test failure.
(Assignee)

Comment 13

9 years ago
Comment on attachment 375187 [details] [diff] [review]
Enable application version to be replaced, v1.1

Requesting branch approval for the main patch. This is build system only and by itself doesn't change anything in the shipped application.
With the other patch to "use generics in Firefox", it can ease the release process by removing the need to touch module.ver in the release process.

Ted, do we want the Firefox part in 1.9.1 as well?
Attachment #375187 - Flags: approval1.9.1?
(Assignee)

Comment 14

9 years ago
By the way, I verified that the fix does work and firefox.exe has the correct values with the patches I checked in as noted in comment #12.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
(Assignee)

Comment 15

9 years ago
Comment on attachment 372041 [details] [diff] [review]
use all those generics in Firefox

Nominating the Firefox part as well.
I personally only want/need the general stuff for not needing special treatment for SeaMonkey release stuff, but getting this Firefox part into 1.9.1 would enable us to make the whole hg-based release automation framework forget about special treatment for module.ver and streamline that part a bit.
It doesn't really influence anything but release stuff, though.
Attachment #372041 - Flags: approval1.9.1?
Comment on attachment 372041 [details] [diff] [review]
use all those generics in Firefox

a191=beltzner
Attachment #372041 - Flags: approval1.9.1? → approval1.9.1+
Attachment #375187 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 375187 [details] [diff] [review]
Enable application version to be replaced, v1.1

a191=beltzner
(Assignee)

Comment 18

9 years ago
Pushed to 1.9.1 as http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bf4904fd83e6 and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c8e5d96d7251 - which means that release automation (which acts on 1.9.1 and higher) will be able to ignore module.ver for version bumps.
Keywords: fixed1.9.1
(Assignee)

Updated

9 years ago
Blocks: 493773
(Assignee)

Comment 19

9 years ago
comm-central/SeaMonkey part pushed as http://hg.mozilla.org/comm-central/rev/b3207365e720
Blocks: 493827
(Assignee)

Updated

9 years ago
Blocks: 493955
You need to log in before you can comment on or make changes to this bug.