508760 - Remove MSVC6 support from the tree

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[Masatoshi Kimura [:emk]]

MSVC6 didn't build the tree far past. It will just confuse readers.
Serge, are you interested in this?

Comment 1

[Serge Gautherie (:sgautherie)]

(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?

Comment 2

[Masatoshi Kimura [:emk]]

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

Comment 3

[(not currently active) Ted Mielczarek]

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

[:Mook]

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

Comment 5

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) configure.in [Checkin: Comment 7 & 29]

Comment 6

[[e-mail]]

(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.

Comment 7

[Serge Gautherie (:sgautherie)]

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


http://hg.mozilla.org/mozilla-central/rev/59c456691798

Comment 8

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) xpcom/ds/*.cpp [Checkin: Comment 10]

Comment 9

[Doug Turner (:dougt)]

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.

Comment 10

[Serge Gautherie (:sgautherie)]

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.

Comment 11

[Serge Gautherie (:sgautherie)]

Attachment: (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
}

Comment 12

[Doug Turner (:dougt)]

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.)

Comment 13

[Serge Gautherie (:sgautherie)]

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?

Comment 14

[Masatoshi Kimura [:emk]]

GetProductDirectory will be broken if you #ifdef-out LocalAppData.

Comment 15

[Serge Gautherie (:sgautherie)]

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).

Comment 16

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1) widget/src/windows/*.cpp [Checkin: Comment 17]

Passed as [e-mail]-try-16eb200df084.

Comment 17

[Serge Gautherie (:sgautherie)]

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


http://hg.mozilla.org/mozilla-central/rev/98b255a26506

Comment 18

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1) js/src/* [Checkin: Comment 19]

Comment 19

[Serge Gautherie (:sgautherie)]

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


http://hg.mozilla.org/mozilla-central/rev/02773bf9abae

Comment 20

[Serge Gautherie (:sgautherie)]

Attachment: (Fv1) layout/svg/base/src/* [Checkin: Comment 21]

Succeeded on TryServer.

Comment 21

[Serge Gautherie (:sgautherie)]

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


http://hg.mozilla.org/mozilla-central/rev/6d400c5fa1f3

Comment 22

[Serge Gautherie (:sgautherie)]

Attachment: (Gv1) /extensions/metrics/src/*.h [Superseded by bug 616761]

Succeeded as try-8cebe326f61c.

Comment 23

[Serge Gautherie (:sgautherie)]

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.

Comment 24

[Serge Gautherie (:sgautherie)]

Attachment: (Hv1-CC) configure.in [Checkin: Comment 25]

Do it for bug 506493 now that c-1.9.1 has branched.

Comment 25

[Serge Gautherie (:sgautherie)]

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


http://hg.mozilla.org/comm-central/rev/6ae5e74f759e

Comment 26

[Serge Gautherie (:sgautherie)]

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


Looking for an active reviewer...

Comment 27

[Serge Gautherie (:sgautherie)]

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


"approval1.9.1.8=?":
Zero risk, build config only.

Comment 28

[Daniel Veditz [:dveditz]]

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

Comment 29

[Serge Gautherie (:sgautherie)]

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

Comment 30

[Serge Gautherie (:sgautherie)]

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.

Comment 31

[Mark Banner (:standard8)]

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.

Comment 32

[David Baron :dbaron:]

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

Please find somebody else to review this.

Comment 33

[Serge Gautherie (:sgautherie)]

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 :-/

Comment 34

[Serge Gautherie (:sgautherie)]

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...

Comment 35

[Serge Gautherie (:sgautherie)]

Attachment: (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.)

Comment 36

[Serge Gautherie (:sgautherie)]

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

http://hg.mozilla.org/mozilla-central/rev/37c63f75ff2f

Comment 37

[Serge Gautherie (:sgautherie)]

Attachment: (Jv1) nsEventQueue.h [Checked in: Comment 38]

Passed on
[e-mail] – Thu Dec 9 08:01:08 2010 PST
http://hg.mozilla.org/try/rev/b1bb787b73cd

Comment 38

[Serge Gautherie (:sgautherie)]

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

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

Comment 39

[Serge Gautherie (:sgautherie)]

Attachment: (Kv1) nsNavHistoryResult.cpp

Succeeded as
http://tbpl.mozilla.org/?tree=MozillaTry&rev=e6d22b368e6b

Comment 40

[Serge Gautherie (:sgautherie)]

Mano, ping for review.

Comment 41

[Asaf Romano (gone)]

Comment on attachment 526108 [details] [diff] [review]
(Kv1) nsNavHistoryResult.cpp

Ugh, sorry! r+apology=mano.

Comment 42

[Serge Gautherie (:sgautherie)]

Attachment: (Kv1a) nsNavHistoryResult.cpp: Remove VC6 workaround [Checked in: Comment 44]

Kv1, unbitrotted.

Comment 43

[Dão Gottwald [:dao]]

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

Comment 44

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/e48f73ccaf1c

Are there still parts to land here?

Comment 45

[Serge Gautherie (:sgautherie)]

(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).

Comment 46

[Serge Gautherie (:sgautherie)]

V.Fixed, per MXR search.