776704 - Default shell build for Windows is broken on zlib.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Mandelin [:dmandelin]]

I get this for a standard Windows build.

d:/sources/mozilla-inbound/js/src/jsutil.cpp(26) : fatal error C1083: Cannot open include file: 'zlib.h': No such file or directory

Ideally we can always build the shell using the normal instructions without having to add any other libraries. That may not end up being exactly the best thing to here--needs some thought--but being able to build on Windows without compression seems like it might be the thing to do.

Comment 1

[Mike Hommey [:glandium]]

This is more of a JS problem, since it means something would need being disabled.

Comment 2

[David Mandelin [:dmandelin]]

Attachment: Patch, except for build changes

Mike, this gets the JS engine to work on Windows. But can you help me with the build system? Right now configure.in just has a MOZ_ZLIB_CHECK, which is opaque to me. I thinkwhat we need is to make it so that on Windows shell builds only, we don't try to use zlib, and somehow know that we're doing this in the source files. I used a #define, assuming that we might set some variable in a configuration .h file, but I don't know if that's really the right thing to do.

Comment 3

[Mike Hommey [:glandium]]

How about
ifdef MOZ_ZLIB_LIBS
DEFINES += -DUSE_ZLIB
endif

in js/src/Makefile.in? The MOZ_ZLIB_LIBS variable should always contain something when zlib is enabled, and be empty when it's not.

Comment 4

[David Mandelin [:dmandelin]]

(In reply to Mike Hommey [:glandium] from comment #3)
> How about
> ifdef MOZ_ZLIB_LIBS
> DEFINES += -DUSE_ZLIB
> endif
> 
> in js/src/Makefile.in? The MOZ_ZLIB_LIBS variable should always contain
> something when zlib is enabled, and be empty when it's not.

Ah, perfect. Thanks! Now I just need to test on non-windows.

Comment 5

[David Mandelin [:dmandelin]]

Attachment: Patch v2, use define

Comment 6

[:Benjamin Peterson]

Comment on attachment 646400 [details] [diff] [review]
Patch v2, use define

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

::: js/src/jsutil.cpp
@@ +22,5 @@
>  
>  #include "js/TemplateLib.h"
>  #include "js/Utility.h"
>  
> +// XXX

What's this for?

Comment 7

[David Mandelin [:dmandelin]]

(In reply to Benjamin Peterson from comment #6)
> Comment on attachment 646400 [details] [diff] [review]
> Patch v2, use define
> 
> Review of attachment 646400 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jsutil.cpp
> @@ +22,5 @@
> >  
> >  #include "js/TemplateLib.h"
> >  #include "js/Utility.h"
> >  
> > +// XXX
> 
> What's this for?

Just random markers left over. Thanks for catching it!

http://hg.mozilla.org/integration/mozilla-inbound/rev/7a7d4f374bab

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7a7d4f374bab