Remove MSVC6 support from the tree

VERIFIED FIXED in mozilla11

Status

()

Core
Build Config
--
trivial
VERIFIED FIXED
8 years ago
5 years ago

People

(Reporter: emk, Assigned: sgautherie)

Tracking

(Blocks: 1 bug)

Trunk
mozilla11
x86
Windows 95
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(status1.9.1 .8-fixed)

Details

(URL)

Attachments

(10 attachments, 2 obsolete attachments)

2.19 KB, patch
ted
: review+
Details | Diff | Splinter Review
1.80 KB, patch
dougt
: review+
Details | Diff | Splinter Review
1.83 KB, patch
dougt
: review+
Details | Diff | Splinter Review
909 bytes, patch
dougt
: review+
Details | Diff | Splinter Review
1.42 KB, patch
graydon
: review+
Details | Diff | Splinter Review
1.08 KB, patch
Robert Longson
: review+
Details | Diff | Splinter Review
1.14 KB, patch
Callek
: review+
Details | Diff | Splinter Review
1.53 KB, patch
bsmedberg
: review+
bsmedberg
: approval2.0+
Details | Diff | Splinter Review
721 bytes, patch
bsmedberg
: review+
Details | Diff | Splinter Review
1.70 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
MSVC6 didn't build the tree far past. It will just confuse readers.
Serge, are you interested in this?
(Assignee)

Comment 1

8 years ago
(In reply to comment #0)
> MSVC6 didn't build the tree far past. It will just confuse readers.

Ftr,
https://developer.mozilla.org/en/Windows_Build_Prerequisites
says VC6 is
*'Yes (Official)' for 1.8.0 and 1.8.1.
*'No' for 1.9.0 and 1.9.1+.

> Serge, are you interested in this?

Maybe. What should be done?
Severity: normal → trivial
(Reporter)

Comment 2

8 years ago
As a first step, we could remove _CC_SUITE=6 and _CC_MAJOR_VERSION=12 code path from configure.in. Then configure will complain "This version of the MSVC compiler, 12.0.x.x , is unsupported."
Then we could remove all MSVC6-specific hacks.
http://mxr.mozilla.org/mozilla-central/search?string=vc6
I think Mook was still building with VC6+patches at some point, but I also think I'm fine with killing off that support. You can't even get your hands on VC6 legally these days.

Comment 4

8 years ago
Nah, I was doing VC71 + patches / various forms of mingw.  Neil(Away) was the one doing VC6.  CCing him... or at leas _a_ Neil; sorry if I picked up the wrong one :)

See also: https://developer.mozilla.org/en/VC6_Build_Instructions
(Assignee)

Comment 5

8 years ago
Created attachment 392938 [details] [diff] [review]
(Av1) configure.in
[Checkin: Comment 7 & 29]
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #392938 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

8 years ago
Blocks: 506493
Flags: in-testsuite-

Comment 6

8 years ago
(In reply to comment #4)
> See also: https://developer.mozilla.org/en/VC6_Build_Instructions
That's only relevant for Firefox 3.0 - all my Mercurial builds use VC7.1 or 8.
(Assignee)

Updated

8 years ago
Blocks: 508808
(Assignee)

Updated

8 years ago
Blocks: 508809
Attachment #392938 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 7

8 years ago
Comment on attachment 392938 [details] [diff] [review]
(Av1) configure.in
[Checkin: Comment 7 & 29]


http://hg.mozilla.org/mozilla-central/rev/59c456691798
Attachment #392938 - Attachment description: (Av1) configure.in → (Av1) configure.in [Checkin: Comment 7]
(Assignee)

Comment 8

8 years ago
Created attachment 393359 [details] [diff] [review]
(Bv1) xpcom/ds/*.cpp
[Checkin: Comment 10]
Attachment #393359 - Flags: review?
(Assignee)

Updated

8 years ago
Attachment #393359 - Flags: review? → review?(doug.turner)

Comment 9

8 years ago
Comment on attachment 393359 [details] [diff] [review]
(Bv1) xpcom/ds/*.cpp
[Checkin: Comment 10]

looks fine.  push it to try and check the wince build.  I was just worried about the REG_QWORD change.
Attachment #393359 - Flags: review?(doug.turner) → review+
(Assignee)

Comment 10

8 years ago
Comment on attachment 393359 [details] [diff] [review]
(Bv1) xpcom/ds/*.cpp
[Checkin: Comment 10]


http://hg.mozilla.org/mozilla-central/rev/b97303f89a50


(In reply to comment #9)
> push it to try and check the wince build.

Passed (all) as try-3485c3d712d6.
Attachment #393359 - Attachment description: (Bv1) xpcom/ds/*.cpp → (Bv1) xpcom/ds/*.cpp [Checkin: Comment 10]
(Assignee)

Comment 11

8 years ago
Created attachment 393433 [details] [diff] [review]
(Cv1) xpcom/io/*.cpp
[Checkin: See comment 15]

Passed as try-844bdb830ca4.

I know nothing about WinCE/WinMo,
so I figured out this patch and comment from a search on internet...

Yet, I wonder if the following would be right and better:
{
804 #ifndef WINCE
...
829 #endif
...
#ifndef WINCE_WINDOWS_MOBILE // or #ifndef WINCE ?
835         case Win_LocalAppdata:
836         {
837             return GetWindowsFolder(CSIDL_LOCAL_APPDATA, aFile);
838         }
#endif
839 #endif  // XP_WIN
}
Attachment #393433 - Flags: review?(doug.turner)
Comment on attachment 393433 [details] [diff] [review]
(Cv1) xpcom/io/*.cpp
[Checkin: See comment 15]

I think you want WINCE (since it is also not defined on Windows CE, not just undefined on windows mobile.)
Attachment #393433 - Flags: review?(doug.turner) → review+
(Assignee)

Comment 13

8 years ago
Comment on attachment 393433 [details] [diff] [review]
(Cv1) xpcom/io/*.cpp
[Checkin: See comment 15]

Noted for WINCE.
What do you think about comment 11?
(Reporter)

Comment 14

8 years ago
GetProductDirectory will be broken if you #ifdef-out LocalAppData.
(Assignee)

Comment 15

8 years ago
Comment on attachment 393433 [details] [diff] [review]
(Cv1) xpcom/io/*.cpp
[Checkin: See comment 15]


http://hg.mozilla.org/mozilla-central/rev/794867a3e942
Cv1, with comment 12 suggestion(s).
Attachment #393433 - Attachment description: (Cv1) xpcom/io/*.cpp → (Cv1) xpcom/io/*.cpp [Checkin: See comment 15]
(Assignee)

Comment 16

8 years ago
Created attachment 394388 [details] [diff] [review]
(Dv1) widget/src/windows/*.cpp
[Checkin: Comment 17]

Passed as sgautherie.bz@free.fr-try-16eb200df084.
Attachment #394388 - Flags: review?(doug.turner)

Updated

8 years ago
Attachment #394388 - Flags: review?(doug.turner) → review+
(Assignee)

Comment 17

8 years ago
Comment on attachment 394388 [details] [diff] [review]
(Dv1) widget/src/windows/*.cpp
[Checkin: Comment 17]


http://hg.mozilla.org/mozilla-central/rev/98b255a26506
Attachment #394388 - Attachment description: (Dv1) widget/src/windows/*.cpp → (Dv1) widget/src/windows/*.cpp [Checkin: Comment 17]
(Assignee)

Comment 18

8 years ago
Created attachment 395049 [details] [diff] [review]
(Ev1) js/src/*
[Checkin: Comment 19]
Attachment #395049 - Flags: review?(graydon)

Updated

8 years ago
Attachment #395049 - Flags: review?(graydon) → review+
(Assignee)

Comment 19

8 years ago
Comment on attachment 395049 [details] [diff] [review]
(Ev1) js/src/*
[Checkin: Comment 19]


http://hg.mozilla.org/mozilla-central/rev/02773bf9abae
Attachment #395049 - Attachment description: (Ev1) js/src/* → (Ev1) js/src/* [Checkin: Comment 19]
(Assignee)

Comment 20

8 years ago
Created attachment 395207 [details] [diff] [review]
(Fv1) layout/svg/base/src/*
[Checkin: Comment 21]

Succeeded on TryServer.
Attachment #395207 - Flags: review?(longsonr)

Updated

8 years ago
Attachment #395207 - Flags: review?(longsonr) → review+
(Assignee)

Comment 21

8 years ago
Comment on attachment 395207 [details] [diff] [review]
(Fv1) layout/svg/base/src/*
[Checkin: Comment 21]


http://hg.mozilla.org/mozilla-central/rev/6d400c5fa1f3
Attachment #395207 - Attachment description: (Fv1) layout/svg/base/src/* → (Fv1) layout/svg/base/src/* [Checkin: Comment 21]
(Assignee)

Updated

8 years ago
Depends on: 511395
(Assignee)

Comment 22

8 years ago
Created attachment 395714 [details] [diff] [review]
(Gv1) /extensions/metrics/src/*.h
[Superseded by bug 616761]

Succeeded as try-8cebe326f61c.
Attachment #395714 - Flags: review?(Jan.Varga)
(Assignee)

Comment 23

8 years ago
Comment on attachment 395714 [details] [diff] [review]
(Gv1) /extensions/metrics/src/*.h
[Superseded by bug 616761]


This reverts part of bug 334044 and bug 335120.
(Assignee)

Comment 24

7 years ago
Created attachment 423359 [details] [diff] [review]
(Hv1-CC) configure.in
[Checkin: Comment 25]

Do it for bug 506493 now that c-1.9.1 has branched.
Attachment #423359 - Flags: review?(bugspam.Callek)

Updated

7 years ago
Attachment #423359 - Flags: review?(bugspam.Callek) → review+
(Assignee)

Comment 25

7 years ago
Comment on attachment 423359 [details] [diff] [review]
(Hv1-CC) configure.in
[Checkin: Comment 25]


http://hg.mozilla.org/comm-central/rev/6ae5e74f759e
Attachment #423359 - Attachment description: (Hv1-CC) configure.in → (Hv1-CC) configure.in [Checkin: Comment 25]
(Assignee)

Comment 26

7 years ago
Comment on attachment 395714 [details] [diff] [review]
(Gv1) /extensions/metrics/src/*.h
[Superseded by bug 616761]


Looking for an active reviewer...
Attachment #395714 - Flags: review?(dbaron)
(Assignee)

Comment 27

7 years ago
Comment on attachment 392938 [details] [diff] [review]
(Av1) configure.in
[Checkin: Comment 7 & 29]


"approval1.9.1.8=?":
Zero risk, build config only.
Attachment #392938 - Flags: approval1.9.1.8?
Attachment #392938 - Flags: approval1.9.1.8? → approval1.9.1.8+
Comment on attachment 392938 [details] [diff] [review]
(Av1) configure.in
[Checkin: Comment 7 & 29]

don't see a lot of point (people haven't figured that out in 8 updates?), but Approved for 1.9.1.8, a=dveditz for release-drivers
(Assignee)

Comment 29

7 years ago
Comment on attachment 392938 [details] [diff] [review]
(Av1) configure.in
[Checkin: Comment 7 & 29]


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/85ba2075840b
Attachment #392938 - Attachment description: (Av1) configure.in [Checkin: Comment 7] → (Av1) configure.in [Checkin: Comment 7 & 29]
(Assignee)

Updated

7 years ago
status1.9.1: --- → .8-fixed
(Assignee)

Comment 30

7 years ago
Comment on attachment 423359 [details] [diff] [review]
(Hv1-CC) configure.in
[Checkin: Comment 25]


Looking for
"approval-thunderbird3.0.2=?":
Zero risk, build config only.
Attachment #423359 - Flags: review?(bugzilla)
Comment on attachment 423359 [details] [diff] [review]
(Hv1-CC) configure.in
[Checkin: Comment 25]

I don't care about this for 1.9.1. There's no advantages here, from what I can tell any issues should be caught by the mozilla-1.9.1 changes, and the mozilla build page says VC 6 isn't supported for 1.9.1 anyway.
Attachment #423359 - Flags: review?(bugzilla) → review-
(Assignee)

Updated

7 years ago
Attachment #423359 - Flags: review-
Comment on attachment 395714 [details] [diff] [review]
(Gv1) /extensions/metrics/src/*.h
[Superseded by bug 616761]

Please find somebody else to review this.
Attachment #395714 - Flags: review?(dbaron) → review?
(Assignee)

Comment 33

7 years ago
Comment on attachment 395714 [details] [diff] [review]
(Gv1) /extensions/metrics/src/*.h
[Superseded by bug 616761]


Brendan, I have no idea who to ask for this review :-/
Attachment #395714 - Flags: review? → review?(brendan)
Attachment #395714 - Flags: review?(brendan) → review?(benjamin)
Attachment #395714 - Flags: review?(benjamin)
(Assignee)

Updated

7 years ago
Blocks: 600218
(Assignee)

Comment 34

7 years ago
Comment on attachment 395714 [details] [diff] [review]
(Gv1) /extensions/metrics/src/*.h
[Superseded by bug 616761]

Brendan: Benjamin cancelled review, I'm still looking for a reviewer...
Attachment #395714 - Flags: review?(brendan)
Attachment #395714 - Flags: review?(brendan)
(Assignee)

Updated

7 years ago
Depends on: 616761
(Assignee)

Updated

7 years ago
Attachment #395714 - Attachment description: (Gv1) /extensions/metrics/src/*.h → (Gv1) /extensions/metrics/src/*.h [Superseded by bug 616761]
Attachment #395714 - Attachment is obsolete: true
Attachment #395714 - Flags: review?(Jan.Varga)
(Assignee)

Comment 35

7 years ago
Created attachment 495834 [details] [diff] [review]
(Iv1) /toolkit/xre/*.cpp
[Checked in: Comment 36]

Unlike (old) comment 15, I'm not adding an |#ifdef WINCE| anymore: see bug 614720.
(But I could if you want one ftb.)
Attachment #495834 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #495834 - Flags: review? → review?(benjamin)
Attachment #495834 - Flags: review?(benjamin)
Attachment #495834 - Flags: review+
Attachment #495834 - Flags: approval2.0+
(Assignee)

Comment 36

7 years ago
Comment on attachment 495834 [details] [diff] [review]
(Iv1) /toolkit/xre/*.cpp
[Checked in: Comment 36]

http://hg.mozilla.org/mozilla-central/rev/37c63f75ff2f
Attachment #495834 - Attachment description: (Iv1) /toolkit/xre/*.cpp → (Iv1) /toolkit/xre/*.cpp [Checked in: Comment 36]
(Assignee)

Updated

7 years ago
Depends on: 617947
(Assignee)

Comment 37

7 years ago
Created attachment 496590 [details] [diff] [review]
(Jv1) nsEventQueue.h
[Checked in: Comment 38]

Passed on
sgautherie.bz@free.fr – Thu Dec 9 08:01:08 2010 PST
http://hg.mozilla.org/try/rev/b1bb787b73cd
Attachment #496590 - Flags: review?(benjamin)
Whiteboard: [approved-patches-landed]
Attachment #496590 - Flags: review?(benjamin) → review+
(Assignee)

Comment 38

6 years ago
Comment on attachment 496590 [details] [diff] [review]
(Jv1) nsEventQueue.h
[Checked in: Comment 38]

http://hg.mozilla.org/mozilla-central/rev/c72ec462f70c
Attachment #496590 - Attachment description: (Jv1) nsEventQueue.h → (Jv1) nsEventQueue.h [Checked in: Comment 38]
(Assignee)

Comment 39

6 years ago
Created attachment 526108 [details] [diff] [review]
(Kv1) nsNavHistoryResult.cpp

Succeeded as
http://tbpl.mozilla.org/?tree=MozillaTry&rev=e6d22b368e6b
Attachment #526108 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #526108 - Flags: review? → review?(mano)
(Assignee)

Comment 40

5 years ago
Mano, ping for review.
Comment on attachment 526108 [details] [diff] [review]
(Kv1) nsNavHistoryResult.cpp

Ugh, sorry! r+apology=mano.
Attachment #526108 - Flags: review?(mano) → review+
(Assignee)

Comment 42

5 years ago
Created attachment 582664 [details] [diff] [review]
(Kv1a) nsNavHistoryResult.cpp: Remove VC6 workaround
[Checked in: Comment 44]

Kv1, unbitrotted.
Attachment #526108 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Whiteboard: [approved-patches-landed] → [c-n: Kv1a] [approved-patches-landed]

Updated

5 years ago
Keywords: checkin-needed
Whiteboard: [c-n: Kv1a] [approved-patches-landed]
Comment on attachment 582664 [details] [diff] [review]
(Kv1a) nsNavHistoryResult.cpp: Remove VC6 workaround
[Checked in: Comment 44]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e48f73ccaf1c
https://hg.mozilla.org/mozilla-central/rev/e48f73ccaf1c

Are there still parts to land here?
(Assignee)

Updated

5 years ago
Attachment #582664 - Attachment description: (Kv1a) nsNavHistoryResult.cpp: Remove VC6 workaround → (Kv1a) nsNavHistoryResult.cpp: Remove VC6 workaround [Checked in: Comment 44]
(Assignee)

Updated

5 years ago
Depends on: 712552
(Assignee)

Updated

5 years ago
Depends on: 712554
(Assignee)

Comment 45

5 years ago
(In reply to Marco Bonardo [:mak] from comment #44)
> Are there still parts to land here?

I filed 2 more blocking bugs on remaining occurrences.

R.Fixed, for this very bug (assigned to me).
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
(Assignee)

Comment 46

5 years ago
V.Fixed, per MXR search.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.