Closed Bug 298066 (vc71) Opened 19 years ago Closed 19 years ago

Switch Windows builds to using VC7.1 instead of VC6

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 323931

People

(Reporter: vlad, Unassigned)

References

Details

Compiling pixman (a component of cairo) results in some buggy code with VC6. 
Specifically, some of the low-level pixel manipulation functions such as IcOver
generate bogus results.  They can in some cases be fixed by expanding out macros
and introducing extra temporary variables; however, doing that would require
finding all the potential cases where the problems occur.

Since we're going to be moving gung-ho to cairo on trunk once we branch for 1.8,
this would be a perfect time to move over to VC7.1.

I've done side-by-side builds with VC6 and VC7.1, and the resulting binary sizes
are identical.  However, the VC7.1 builds dynamically link with
MSVC71.dll/MSVCR71.dll -- most likely just for operator new/delete, but I
haven't confirmed that.

So, opening this bug for tracking switching to VC7.1 once we start on 1.9 work.
MSVC71.dll is definitely not just operator new/delete: it's the entire C stdlib.
If we can resolve the tech issues with doing VC7.1 builds, I can guarantee
resources to transition at least the Windows Firefox trunk build system from VC6
to VC7.1, possibly more.

Would it help to have a Tinderbox cycle using VC7.1 so issues could be worked
through and tested with the help of a live build system?
(In reply to comment #1)
> MSVC71.dll is definitely not just operator new/delete: it's the entire C stdlib.

I was under the (mistaken) impression that we used very little, and instead
implemented most everything in nspr -- but looking at objdumps reveals that
that's not the case.  We use a few dozen symbols from MSVCR71.dll (essentially
-- various str functions, some stdio functions, math functions, qsort).

From MSVCP71.dll, we seem to only use std::memory.

So, maybe we can remove the dependency on MSVCP71, and just ship MSVCR71?  It's
around 350k uncompressed.  I'm not sure what that would do for our ABI
compatability; darin might have some input there.

Setting up a VC71 tinderbox once we branch would probably be a good idea in any
case; we know that the code already builds under VC71, and it would be good to
have nightly builds to compare performance with.
can we do that and still have our binaries use GPL as one of the licenses?
Do our binaries have GPL as one of the licenses?  Our binaries have a EULA which
applies to the binaries, which is distinct from the source license as permitted
(you might even say required) by the MPL.
in the past gerv has explicitly red flagged us switching to shipping those
libraries.

speaking of new, vc6 returns null on failure which makes our null checks happy,
but someone helpfully improved our vc7 codepaths so that they throw instead,
which means that we almost always get an unhandled exception when we do manage
to run out of memory.
(In reply to comment #6)
> speaking of new, vc6 returns null on failure which makes our null checks happy,
> but someone helpfully improved our vc7 codepaths so that they throw instead,
> which means that we almost always get an unhandled exception when we do manage
> to run out of memory.

That's an option in the VC7 compile/includes/whichever.  We can get the VC6 new
behaviour (assuming we don't do our own operator new/delete to avoid MSVCP).

I don't remember red-flagging the shipping of VC71 DLLs. That doesn't mean I
didn't, of course. Could you point me at the relevant bug?

Legally, we use the "alternative binary licence" provisions in the MPL to ship
the Foundation's Firefox binaries under the Firefox EULA. This would not be
prevented by moving to VC 7, even if that required us to ship MSVC71.dll etc. 

However, requiring those libraries would make it hard (post-relicensing) for
anyone to use and ship the code under the GPL, because they would be required to
include GPL-incompatible libraries. (At least, I assume the licence for the VC 7
runtimes is GPL-incompatible.) Which would be a shame.

If we switch, does that mean over time that building with VC 6 would break?

Gerv

It was my understanding that the installer currently ships the VS 6 dlls
required as-required-by-microsoft for those libraries.  Assuming this is still
the case, I don't see how things would have changed for 7.1...
The EULA for the vc6 redistributables is significantly different than that for
vc7.1 (the 7.1 redistributable EULA requires us to prohibit various kinds of
subsequent distribution).

I personall think that static linking of the runtime libs may be a better
solution than shipping the runtime libs.
isn't it the GPL which forbids linking to non-system libraries, which vs 7.1's
are (to my knowledge)?
We need to make sure we don't rush into changes before 1.1 that will cause our
builds to break the 5.0 MB ceiling (4.9MB displayed in IE's download dialog).
This means some research into compile flags that produce the smallest, fastest
code. It would be good to see the results of such research. 
The concensus seems to be that we should do something about this; we should
figure out what that is sooner rather than later, so nominating this for
blocking-1.8b3+.    I agree with bsmedberg that static linking is probably the
best way to go, to both minimize shipping pain and code size.
Flags: blocking1.8b3?
biesi: the relevant exception in the GPL would not apply here:

"However, as a special exception, the source code distributed need not include
anything that is normally distributed (in either source or binary form) with the
major components (compiler, kernel, and so on) of the operating system on which
the executable runs, unless that component itself accompanies the executable."

(Note the last part.) Now it could be that, for the purposes of shipping a GPLed
browser based on Firefox post-relicensing, there's no change between using VC6
and VC7 - neither could be shipped. However, requiring VC7 means such a browser
would run on a lot fewer systems. Which would be unfortunate.

The problems don't end there. I'd need to re-review the relevant EULAs, but I
believe that the VC7 redistributables present a problem even for our current
usage. I'll try and look at this ASAP.
Are we ever intending to ship our builds under the GPL?  I don't think we are,
and those MoFo-distributed builds are what this bug is about.  If others want to
distribute GPL-covered binaries, they'll have to build them, and can therefore
use mingw or cygwin or VC6 to produce them.
Shaver: sure, as long as it's still possible to use VC6. I was under the
impression that the proposed changes would make our stuff build only with VC7 -
at least, if you wanted a non-buggy build.

And apart from that issue, we still need to check we can redistribute the VC7
libs at all. I will try and do that soon; anyone else's views (and links to the
relevant EULAs) would also be appreciated.

Gerv
I doubt they'll ever be able to get a fully non-buggy build with VC6, though
with some brute force it certainly may be possible, especially combined with
VC7-produced object files.

However, we'll always build with mingw, so that should be enough.  I'll dig up
the EULAs tonight if someone doesn't beat me to it.
it is not possible to build a full-featured build with mingw, is it?
It is the case _today_ that you can't get a non-buggy VC6 build; we're proposing
that we use VC7.1 for our builds so that we don't have to continue to do
whatever horrible pre-assemble/manual-macro-expand/etc. things we settle on in
order to ship 1.1 against VC6.

I'm with vlad, though: people know where to find mingw if they want it.  (And
shouldn't a GPL purist not want to use MSVC anyway? *wink*)
Depends on: 298916
Filed bug 298916 based on comment 16 by Gerv to track work on whether we
can/can't redistribute VC7/VC7.1 libs.
per shaver, not for 1.1, we'll go for a different solution as necessary
Flags: blocking1.8b3? → blocking1.8b3-
We also need to be sure meeting the installation requirements with VC7.1 does
not make the initial download experience larger than 4.9 reported MB in IE's
download window.

    /|    +---+
   / |    |   |
  /  |    +---+
 +---+- o     |
     |    ----+
Alias: vc71
(In reply to comment #22)
> We also need to be sure meeting the installation requirements with VC7.1 does
> not make the initial download experience larger than 4.9 reported MB in IE's
> download window.
> 
>     /|    +---+
>    / |    |   |
>   /  |    +---+
>  +---+- o     |
>      |    ----+


4.9 is such a crappy rating.  I think we need to go for at least a 9/10!


(In reply to comment #22)
>     /|    +---+
>    / |    |   |
>   /  |    +---+
>  +---+- o     |
>      |    ----+

Vlad reports we're already experiencing issues with VC6 and the hoops we jump
through to set up build systems with VC6 are really painful.  Care to put your
fingers where your largish-4.9-ASCII-Art is and help us get the product ready to
be ported over?
What I'm saying is that a product requirement of Firefox historically has been a
< 5.0 MB download. There are a number of areas that Firefox can potentially be
trimmed to avoid breaching this as new features arrive:

- internet search service cruft
- unused xpinstall features
- more unused chrome and old xpfe cruft (browserinstance, etc)

Also, better compression flag tuning. etc. etc. But doing this all takes time.
Yes, best to start thinking about it now. I would love to replace the Internet
Search Service for 2.0 with something far leaner, more flexible and bug free. 

My concern is that once the threshold is crossed, size will balloon and pretty
soon we'll be as big as Seamonkey again, except without the benefit of a mail
client. 

My goal is to keep this as software people have few questions about downloading.
That's a goal that isn't really concerned about the compiler suite used to
produce it. I shout this loudly and frequently because Firefox's goal is to make
software that people want to use, software that's easy to produce is important,
but not an end unto itself.
I'll add as a fyi: At Apple, Gramps instituted a policy for the Safari team that
no net performance regressions be introduced. If you slow something down, you
have to check something in that speeds it up again. I know we don't want to be
giving people an excuse to slow things down, but sometimes shit happens. The
download size thing is often a bit different - important new features come
along, e.g. a XULRunner architecture, SVG etc. I still think we should be
focused on "net status quo or decline".


One final clarification: I am not arguing against 7.1 - it is an inevitability
and 6.0 will die. I want to know though that maintaining a trim download size is
on everybody's mind because IMO it is closely related to uptake. 

No further spam from me, strategies discussing the download size issue and how
to deal with it can be found and appended to: http://wiki.mozilla.org/Download_Size
(In reply to comment #27)
> One final clarification: I am not arguing against 7.1 - it is an inevitability
> and 6.0 will die. I want to know though that maintaining a trim download size is
> on everybody's mind because IMO it is closely related to uptake. 

I think download size is on people's minds; but for this, I think the key will
be actually getting a VC71 build with no external dependencies, and then looking
at the size.  I don't think we have a good idea of what the size we're starting
out at is yet, and how much we have to trim.

I'm going to dup this on the switch to using VC8 bug to copy all the emails over.  Future discussion should go in to that bug.

*** This bug has been marked as a duplicate of 323931 ***
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → DUPLICATE
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.