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: respect --shared-* flags for inspector deps #7569

Merged
merged 1 commit into from
Jul 9, 2016
Merged

build: respect --shared-* flags for inspector deps #7569

merged 1 commit into from
Jul 9, 2016

Conversation

ChALkeR
Copy link
Member

@ChALkeR ChALkeR commented Jul 6, 2016 •
edited
Loading

Checklist
  • make -j4 test passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

Description of change

Don't build openssl/http_parser/libuv for v8_inspector if corresponding --shared-* flags were passed to the ./configure script.

Fixes: #7478

./configure --shared-openssl --without-intl --shared-http-parser; make -j4 test works and doesn't build openssl and http-parser even with v8_inspector enabled.
#7486 fixed that only for the case when --without-inspector was passed.

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

/cc @bnoordhuis @ofrobots

@ChALkeR ChALkeR added the build Issues and PRs related to build files or the CI. label Jul 6, 2016
@mscdex mscdex added openssl Issues and PRs related to the OpenSSL dependency. libuv Issues and PRs related to the libuv dependency or the uv binding. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. labels Jul 6, 2016
@MylesBorins
Copy link
Contributor

will land tomorrow if CI is green and we have no objects

/cc @nodejs/lts @nodejs/collaborators

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

@ChALkeR ChALkeR added the inspector Issues and PRs related to the V8 inspector protocol label Jul 7, 2016
@bnoordhuis
Copy link
Member

LGTM

@ChALkeR ChALkeR mentioned this pull request Jul 7, 2016
Closed
@ChALkeR
Copy link
Member Author

ChALkeR commented Jul 8, 2016

Note: this also fixes #7583.

@ofrobots
Copy link
Contributor

ofrobots commented Jul 8, 2016

LGTM

@ChALkeR
Copy link
Member Author

ChALkeR commented Jul 9, 2016 •
edited
Loading

The only failing test in CI was test-net-write-slow on FreeBSD, it was known flaky (#7555).
Landing 990c968.

@ChALkeR
build: respect --shared-* flags for inspector deps
990c968 
Don't build openssl/http_parser/libuv for v8_inspector if corresponding
--shared-* flags were passed to the ./configure script.

Fixes: nodejs#7478
Fixes: nodejs#7583
PR-URL: nodejs#7569
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
@ChALkeR ChALkeR merged commit 990c968 into nodejs:master Jul 9, 2016
@MylesBorins MylesBorins mentioned this pull request Jul 11, 2016
Closed
evanlucas pushed a commit that referenced this pull request Jul 13, 2016
@ChALkeR @evanlucas
build: respect --shared-* flags for inspector deps
933ff62 
Don't build openssl/http_parser/libuv for v8_inspector if corresponding
--shared-* flags were passed to the ./configure script.

Fixes: #7478
Fixes: #7583
PR-URL: #7569
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
@evanlucas evanlucas mentioned this pull request Jul 18, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. inspector Issues and PRs related to the V8 inspector protocol libuv Issues and PRs related to the libuv dependency or the uv binding. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

build: --shared-openssl still builds openssl
5 participants
@ChALkeR @MylesBorins @bnoordhuis @ofrobots @mscdex