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.

Sign up for GitHub

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

Jump to bottom

build: add -DZLIB_CONST when building with --shared-zlib #9077

Closed
wants to merge 1 commit into from
Closed

build: add -DZLIB_CONST when building with --shared-zlib #9077

wants to merge 1 commit into from

Conversation

bradleythughes
Copy link
Member

@bradleythughes bradleythughes commented Oct 13, 2016 •
edited
Loading

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

Description of change

Commit 782620f added the define only when building with the bundled
zlib. Using a shared zlib results in build breakage:

../src/inspector_agent.cc:179:16: error: assigning to 'Bytef *' (aka 'unsigned char *') from incompatible type
      'const uint8_t *' (aka 'const unsigned char *')
  strm.next_in = PROTOCOL_JSON + 3;
               ^ ~~~~~~~~~~~~~~~~~
1 error generated.

@bradleythughes
build: add -DZLIB_CONST when building with --shared-zlib
fc164ac 
Commit 782620f added the define only when building with the bundled
zlib. Using a shared zlib results in build breakage:

../src/inspector_agent.cc:179:16: error: assigning to 'Bytef *' (aka 'unsigned char *') from incompatible type
      'const uint8_t *' (aka 'const unsigned char *')
  strm.next_in = PROTOCOL_JSON + 3;
               ^ ~~~~~~~~~~~~~~~~~
1 error generated.
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Oct 13, 2016
@mscdex mscdex added the zlib Issues and PRs related to the zlib subsystem. label Oct 13, 2016
@Nibbler999 Nibbler999 mentioned this pull request Oct 13, 2016
Closed
@MylesBorins
Copy link
Contributor

LGTM

@bnoordhuis
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/4499/

jasnell
jasnell approved these changes Oct 13, 2016
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jbergstroem
Copy link
Member

LGTM

@MylesBorins
Copy link
Contributor

when this lands it should be imedietely backported to v6.x-staging

@mhdawson
Copy link
Member

LGTM

@MylesBorins
Copy link
Contributor

CI failures are unrelated (npm failure due to sandbox)

As this is non controversial and fixes a break I'll be landing + backporting later this evening if there are no complaints

@MylesBorins MylesBorins self-assigned this Oct 13, 2016
@MylesBorins MylesBorins mentioned this pull request Oct 13, 2016
Closed
@jbergstroem
Copy link
Member

(npm fail has since been fixed)

@mscdex mscdex mentioned this pull request Oct 14, 2016
Merged
@evanlucas
Copy link
Contributor

LGTM. I'm going to go ahead and land so we can get this in to v6.8.1

@evanlucas
Copy link
Contributor

Landed in c457b92. Thanks!

@evanlucas evanlucas closed this Oct 14, 2016
evanlucas pushed a commit that referenced this pull request Oct 14, 2016
@bradleythughes @evanlucas
build: add -DZLIB_CONST when building with --shared-zlib
c457b92 
Commit 782620f added the define only when building with the bundled
zlib. Using a shared zlib results in build breakage:

../src/inspector_agent.cc:179:16: error: assigning to 'Bytef *' (aka 'unsigned char *') from incompatible type
      'const uint8_t *' (aka 'const unsigned char *')
  strm.next_in = PROTOCOL_JSON + 3;
               ^ ~~~~~~~~~~~~~~~~~
1 error generated.

PR-URL: #9077
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
evanlucas pushed a commit that referenced this pull request Oct 14, 2016
@bradleythughes @evanlucas
build: add -DZLIB_CONST when building with --shared-zlib
14f12bf 
Commit 782620f added the define only when building with the bundled
zlib. Using a shared zlib results in build breakage:

../src/inspector_agent.cc:179:16: error: assigning to 'Bytef *' (aka 'unsigned char *') from incompatible type
      'const uint8_t *' (aka 'const unsigned char *')
  strm.next_in = PROTOCOL_JSON + 3;
               ^ ~~~~~~~~~~~~~~~~~
1 error generated.

PR-URL: #9077
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
evanlucas pushed a commit that referenced this pull request Oct 14, 2016
@bradleythughes @evanlucas
build: add -DZLIB_CONST when building with --shared-zlib
8d2206f 
Commit 782620f added the define only when building with the bundled
zlib. Using a shared zlib results in build breakage:

../src/inspector_agent.cc:179:16: error: assigning to 'Bytef *' (aka 'unsigned char *') from incompatible type
      'const uint8_t *' (aka 'const unsigned char *')
  strm.next_in = PROTOCOL_JSON + 3;
               ^ ~~~~~~~~~~~~~~~~~
1 error generated.

PR-URL: #9077
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
evanlucas added a commit that referenced this pull request Oct 14, 2016
@evanlucas
2016-10-14, Version 6.8.1 (Current)
c6a397b 
* build: Fix building with shared zlib. (Bradley T. Hughes) [#9077](#9077)
* stream: fix `Writable` subclass instanceof checks (Anna Henningsen) [#9088](#9088)
* timers: fix regression with clearImmediate() (Brian White) [#9086](#9086)

PR-URL: #9104
evanlucas added a commit that referenced this pull request Oct 15, 2016
@evanlucas
2016-10-14, Version 6.8.1 (Current)
113c697 
* build: Fix building with shared zlib. (Bradley T. Hughes) [#9077](#9077)
* stream: fix `Writable` subclass instanceof checks (Anna Henningsen) [#9088](#9088)
* timers: fix regression with clearImmediate() (Brian White) [#9086](#9086)

PR-URL: #9104
@evantorrie evantorrie mentioned this pull request Oct 15, 2016
Closed
@bradleythughes bradleythughes deleted the shared-zlib-adds-ZLIB_CONST-define branch October 16, 2016 20:48
jasnell pushed a commit that referenced this pull request Oct 17, 2016
@bradleythughes @jasnell
build: add -DZLIB_CONST when building with --shared-zlib
ea011eb 
Commit 782620f added the define only when building with the bundled
zlib. Using a shared zlib results in build breakage:

../src/inspector_agent.cc:179:16: error: assigning to 'Bytef *' (aka 'unsigned char *') from incompatible type
      'const uint8_t *' (aka 'const unsigned char *')
  strm.next_in = PROTOCOL_JSON + 3;
               ^ ~~~~~~~~~~~~~~~~~
1 error generated.

PR-URL: #9077
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Oct 17, 2016
@imyller
2016-10-14, Version 6.8.1 (Current)
c84b420 
    * build: Fix building with shared zlib. (Bradley T. Hughes) [#9077](nodejs/node#9077)
    * stream: fix `Writable` subclass instanceof checks (Anna Henningsen) [#9088](nodejs/node#9088)
    * timers: fix regression with clearImmediate() (Brian White) [#9086](nodejs/node#9086)

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Oct 17, 2016
@imyller
2016-10-14, Version 6.8.1 (Current)
aa4dfb9 
    * build: Fix building with shared zlib. (Bradley T. Hughes) [#9077](nodejs/node#9077)
    * stream: fix `Writable` subclass instanceof checks (Anna Henningsen) [#9088](nodejs/node#9088)
    * timers: fix regression with clearImmediate() (Brian White) [#9086](nodejs/node#9086)

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
@MylesBorins
Copy link
Contributor

since this appears related to inspector changes I'm marking as don't land on v4.x

@addaleax let me know if I'm mistaken here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

Assignees

@MylesBorins MylesBorins

Labels
build Issues and PRs related to build files or the CI. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@bradleythughes @MylesBorins @bnoordhuis @jbergstroem @mhdawson @evanlucas @jasnell @addaleax @mscdex @nodejs-github-bot