Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zlib: clean up zlib.gyp, don't build minizip #276

Merged
merged 7 commits into from
Jan 10, 2015
Merged

Conversation

piscisaureus
Copy link
Contributor

As exposed by #273, the bundled zlib builds an optional extension called 'minizip' that provides filesystem-like APIs for working with zip files.
Node doesn't use it, so let's not build it either.

This patch works on windows but I haven't checked other platforms yet. (\o CI).
What definitely needs to be reviewed is what happens if use_system_zlib is defined; with this patch zlib still gets built but has no sources at all.

R=@bnoordhuis or @shigeki

Revert "src: fix windows build error" and "zlib: support
concatenated gzip files".

This reverts commits be413ac
and 1183ba4.

Treating subsequent bytes as a concatenated zlib stream
breaks npm install.

Conflicts:
	test/parallel/test-zlib-from-multiple-gzip-with-garbage.js
	test/parallel/test-zlib-from-multiple-gzip.js
	test/parallel/test-zlib-from-multiple-huge-gzip.js

Fixes: nodejs/node-v0.x-archive#8962
PR-URL: #240
Reviewed-By: Ben Noordhuis <[email protected]>
@bnoordhuis
Copy link
Member

LGTM, although not building minizip by itself doesn't fix the warnings, that's due to defining Z_HAVE_UNISTD_H. I suggest landing #273 first, then landing this PR on top so that things don't get conflated. --shared-zlib still works with this patch, by the way.

bnoordhuis and others added 6 commits January 9, 2015 20:56
The V8 development branch has unshipped ES6 classes pending resolution
of a number of inheritance edge cases.  Disable classes in io.js for
the sake of feature parity.

See #251 for background and
discussion.

PR-URL: #272
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Domenic Denicola <[email protected]>
Per the discussion in #272, upstream
V8 has disabled Harmony object literals for the time being.  Do the
same for feature parity.

PR-URL: #272
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Domenic Denicola <[email protected]>
use a threading.Event instead of a boolean attribute.

PR-URL: #277
Fixes: #260
Reviewed-By: Ben Noordhuis <[email protected]>
Build the bundled zlib with -DZ_HAVE_UNISTD_H to make the definition of
close(), read() and other unistd.h functions available to gzread.c and
gzwrite.c. It's kind of silly that we have to jump through hoops here
because we never call any of the functions that do I/O directly, but at
least it squelches the -Wimplicit-function-declaration warnings.

PR-URL: #273
Reviewed-by: Bert Belder <[email protected]>
PR-URL: #276
Reviewed-by: Ben Noordhuis <[email protected]>
It's an optional extension that node/iojs doesn't use.

PR-URL: #276
Reviewed-by: Ben Noordhuis <[email protected]>
@piscisaureus piscisaureus merged commit a80b977 into v1.x Jan 10, 2015
@piscisaureus piscisaureus deleted the zlib-gyp-cleanup branch January 10, 2015 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants