Closed Bug 316387 Opened 19 years ago Closed 15 years ago

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

Categories

(Firefox Build System :: General, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: kairo, Assigned: kairo)

References

Details

(Keywords: fixed1.9.1)

Attachments

(3 files, 1 obsolete file)

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...
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)
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 ;-)
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.
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+
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)
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)
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+
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.
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?
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
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
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
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
Blocks: 493773
comm-central/SeaMonkey part pushed as http://hg.mozilla.org/comm-central/rev/b3207365e720
Blocks: 493827
Blocks: 493955
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.