Last Comment Bug 776704 - Default shell build for Windows is broken on zlib.h
: Default shell build for Windows is broken on zlib.h
Status: RESOLVED FIXED
[js:p1:fx17]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: mozilla17
Assigned To: David Mandelin [:dmandelin]
:
:
Mentors:
Depends on: 778560
Blocks: 763651 1039197
  Show dependency treegraph
 
Reported: 2012-07-23 14:20 PDT by David Mandelin [:dmandelin]
Modified: 2014-07-16 01:27 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch, except for build changes (4.31 KB, patch)
2012-07-25 16:54 PDT, David Mandelin [:dmandelin]
no flags Details | Diff | Splinter Review
Patch v2, use define (4.77 KB, patch)
2012-07-26 16:12 PDT, David Mandelin [:dmandelin]
benjamin: review+
Details | Diff | Splinter Review

Description David Mandelin [:dmandelin] 2012-07-23 14:20:05 PDT
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] 2012-07-23 14:58:28 PDT
This is more of a JS problem, since it means something would need being disabled.
Comment 2 David Mandelin [:dmandelin] 2012-07-25 16:54:42 PDT
Created attachment 645949 [details] [diff] [review]
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] 2012-07-25 22:31:16 PDT
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] 2012-07-26 16:12:10 PDT
(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] 2012-07-26 16:12:48 PDT
Created attachment 646400 [details] [diff] [review]
Patch v2, use define
Comment 6 :Benjamin Peterson 2012-07-27 11:11:56 PDT
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] 2012-07-27 11:33:53 PDT
(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] 2012-07-28 18:37:49 PDT
https://hg.mozilla.org/mozilla-central/rev/7a7d4f374bab

Note You need to log in before you can comment on or make changes to this bug.