Closed Bug 719389 Opened 8 years ago Closed 8 years ago

Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197

Categories

(Core :: Widget: Win32, defect, blocker)

x86
Windows Server 2003
defect
Not set
blocker

Tracking

()

VERIFIED FIXED
mozilla12
Tracking Status
firefox9 --- wontfix
firefox10 - wontfix
firefox11 - verified
firefox-esr10 --- wontfix

People

(Reporter: sgautherie, Assigned: sgautherie)

References

Details

(Keywords: regression)

Attachments

(1 file)

Noticed when trying to build using "--with-windows-version=502".

https://tbpl.mozilla.org/?tree=Try&rev=b6ec764aef44
https://tbpl.mozilla.org/php/getParsedLog.php?id=8663207&tree=Try
WINNT 5.2 try build on 2012-01-19 02:34:26 PST for push b6ec764aef44
{
nsAppShell.cpp

e:/builds/moz2_slave/try-w32/build/widget/src/windows/nsAppShell.cpp(258) : warning C4067: unexpected tokens following preprocessor directive - expected a newline

e:/builds/moz2_slave/try-w32/build/widget/src/windows/nsAppShell.cpp(259) : error C3083: 'widget': the symbol to the left of a '::' must be a type

e:/builds/moz2_slave/try-w32/build/widget/src/windows/nsAppShell.cpp(259) : error C2039: 'StopAudioSession' : is not a member of 'mozilla'

e:/builds/moz2_slave/try-w32/build/widget/src/windows/nsAppShell.cpp(259) : error C3861: 'StopAudioSession': identifier not found

...

make[7]: *** [nsAppShell.obj] Error 2
}
Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

Review of attachment 590205 [details] [diff] [review]:
-----------------------------------------------------------------

Heh, I feel dumb.
Attachment #590205 - Flags: review?(khuey) → review+
Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

https://hg.mozilla.org/mozilla-central/rev/4e12e9a68795


[Approval Request Comment]
Regression caused by (bug #): bug 441197.
User impact if declined: not sure, but blocks building using "--with-windows-version=502".
Testing completed (on m-c, etc.): multiple Try builds and this comment.
Risk to taking this patch (and alternatives if risky): no risk, trivial typo fix.
Attachment #590205 - Attachment description: (Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197 → (Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197 [Checked in: Comment 3]
Attachment #590205 - Flags: approval-mozilla-beta?
Attachment #590205 - Flags: approval-mozilla-aurora?
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

[Triage Comment]
It's not clear what user impact not being able to build with "--with-windows-version=502" has, and thus does not meet the requirement for Aurora/Beta uplift. Please re-nominate once that's known.
Attachment #590205 - Flags: approval-mozilla-beta?
Attachment #590205 - Flags: approval-mozilla-beta-
Attachment #590205 - Flags: approval-mozilla-aurora?
Attachment #590205 - Flags: approval-mozilla-aurora-
Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

(In reply to Alex Keybl [:akeybl] from comment #4)
> It's not clear what user impact not being able to build with
> "--with-windows-version=502" has, and thus does not meet the requirement for
> Aurora/Beta uplift. Please re-nominate once that's known.

"--with-windows-version=502" means:
https://developer.mozilla.org/En/Windows_SDK_versions#section_9
"Windows Server 2003 R2 Platform SDK
 This is your only option if you're on Windows 2000."

This impossibility to build is not part of official builds, but can affect local/customized builds.

Also I didn't check how official builds actually handle this syntax error: I assume they happen to work (around it), but better be correct/sure/safe.
Attachment #590205 - Flags: approval-mozilla-beta?
Attachment #590205 - Flags: approval-mozilla-beta-
Attachment #590205 - Flags: approval-mozilla-aurora?
Attachment #590205 - Flags: approval-mozilla-aurora-
(In reply to Serge Gautherie (:sgautherie) from comment #5)
> This impossibility to build is not part of official builds, but can affect
> local/customized builds.
> 
> Also I didn't check how official builds actually handle this syntax error: I
> assume they happen to work (around it), but better be correct/sure/safe.

Presumably a user can apply this patch to their local build in that case. We'll approve for Aurora, but it's too late in the game for FF10.
Attachment #590205 - Flags: approval-mozilla-beta?
Attachment #590205 - Flags: approval-mozilla-beta-
Attachment #590205 - Flags: approval-mozilla-aurora?
Attachment #590205 - Flags: approval-mozilla-aurora+
https://tbpl.mozilla.org/php/getParsedLog.php?id=8780739&tree=Firefox
WINNT 5.2 mozilla-central build on 2012-01-23 22:08:54 PST for push 3be494e4cc8f
still succeeds.

V.Fixed

*****

(In reply to Alex Keybl [:akeybl] from comment #6)

> Presumably a user can apply this patch to their local build in that case.

Sure.

> We'll approve for Aurora, but it's too late in the game for FF10.

Sad, as iiuc FF10 is meant to be LTS.
Status: RESOLVED → VERIFIED
Keywords: checkin-needed
Whiteboard: [c-n: to m-a]
Depends on: RequireWin7SDK
No longer depends on: PSDK2003R2Removal
Missed aurora; removing checkin-needed to tidy checkin-needed saved search since presume you need to re-request for beta (if still wanted) anyway.
Keywords: checkin-needed
Whiteboard: [c-n: to m-a]
Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

Per comment 8.
Attachment #590205 - Flags: approval-mozilla-beta- → approval-mozilla-beta?
Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

[Triage Comment]
This patch has now baked on m-c (and Aurora 12 since we pushed it out Friday). Approving for Beta 11.
Attachment #590205 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Keywords: checkin-needed
Whiteboard: [c-n: to m-b]
Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

http://hg.mozilla.org/releases/mozilla-beta/rev/004bfb5325af
Attachment #590205 - Attachment description: (Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197 [Checked in: Comment 3] → (Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197 [Checked in: Comments 3 and 11]
Keywords: checkin-needed
Whiteboard: [c-n: to m-b]
You need to log in before you can comment on or make changes to this bug.