FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

build: unbreak --with-intl=system-icu build by bnoordhuis · Pull Request #28118 · nodejs/node · GitHub

/ node Public

build: unbreak --with-intl=system-icu build - #28118

Closed
bnoordhuis wants to merge 3 commits into
nodejs:masterfrom
bnoordhuis:fix28052
ConversationCommits (3)ChecksFiles changed
Closed

build: unbreak --with-intl=system-icu build#28118
bnoordhuis wants to merge 3 commits into
nodejs:masterfrom
bnoordhuis:fix28052

Conversation

bnoordhuis commented Jun 7, 2019

Copy link
Copy Markdown
Member

Include directories are configured by the tools/icu/icu-*.gyp files.
The v8.gyp file doesn't need to add them and in fact the way it did
that breaks when building against an external copy of ICU.

Fixes: #28052

CI: https://ci.nodejs.org/job/node-test-commit/29244/

bnoordhuis added 3 commits June 7, 2019 11:38
build: don't swallow pkg-config warnings
aa5e4cc
build: fix icu-i18n pkg-config version check
1a62bc8 
The pkg_config() helper can either return a tuple of None values
(no pkg-config installed) and that was what the check was testing
for, but it can also return a tuple of empty strings when the
package isn't installed.
build: unbreak --with-intl=system-icu build
167f4b0 
Include directories are configured by the tools/icu/icu-*.gyp files.
The v8.gyp file doesn't need to add them and in fact the way it did
that breaks when building against an external copy of ICU.

Fixes: nodejs#28052

This comment has been minimized.

Sign in to view
nodejs-github-bot added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. labels Jun 7, 2019
bnoordhuis added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jun 11, 2019

This comment has been minimized.

Sign in to view

bnoordhuis commented Jun 13, 2019

Copy link
Copy Markdown
Member Author

Looks like the test failure is #27611.

Details 
05:59:40 not ok 2340 sequential/test-cpu-prof
05:59:40   ---
05:59:40   duration_ms: 480.90
05:59:40   severity: fail
05:59:40   exitcode: -15
05:59:40   stack: |-
05:59:40     timeout
05:59:40   ...

This comment has been minimized.

Sign in to view

Trott commented Jun 13, 2019

Copy link
Copy Markdown
Member

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

mhdawson approved these changes Jun 13, 2019
View reviewed changes

mhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Trott commented Jun 14, 2019

Copy link
Copy Markdown
Member

Landed in b614840...3cdd5a2

Trott closed this Jun 14, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request Jun 14, 2019
build: don't swallow pkg-config warnings
16edec8 
PR-URL: nodejs#28118
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott pushed a commit to Trott/io.js that referenced this pull request Jun 14, 2019
build: fix icu-i18n pkg-config version check
30f285d 
The pkg_config() helper can either return a tuple of None values
(no pkg-config installed) and that was what the check was testing
for, but it can also return a tuple of empty strings when the
package isn't installed.

PR-URL: nodejs#28118
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott pushed a commit to Trott/io.js that referenced this pull request Jun 14, 2019
build: unbreak --with-intl=system-icu build
3cdd5a2 
Include directories are configured by the tools/icu/icu-*.gyp files.
The v8.gyp file doesn't need to add them and in fact the way it did
that breaks when building against an external copy of ICU.

Fixes: nodejs#28052
PR-URL: nodejs#28118
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
joyeecheung mentioned this pull request Jun 16, 2019
Closed
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
build: don't swallow pkg-config warnings
e2ad364 
PR-URL: #28118
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
build: fix icu-i18n pkg-config version check
0931f7e 
The pkg_config() helper can either return a tuple of None values
(no pkg-config installed) and that was what the check was testing
for, but it can also return a tuple of empty strings when the
package isn't installed.

PR-URL: #28118
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
build: unbreak --with-intl=system-icu build
7d0ffee 
Include directories are configured by the tools/icu/icu-*.gyp files.
The v8.gyp file doesn't need to add them and in fact the way it did
that breaks when building against an external copy of ICU.

Fixes: #28052
PR-URL: #28118
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR mentioned this pull request Jun 17, 2019
Merged
targos pushed a commit that referenced this pull request Jun 18, 2019
build: don't swallow pkg-config warnings
02fdf5c 
PR-URL: #28118
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jun 18, 2019
build: fix icu-i18n pkg-config version check
eb89c06 
The pkg_config() helper can either return a tuple of None values
(no pkg-config installed) and that was what the check was testing
for, but it can also return a tuple of empty strings when the
package isn't installed.

PR-URL: #28118
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jun 18, 2019
build: unbreak --with-intl=system-icu build
6a2d8e2 
Include directories are configured by the tools/icu/icu-*.gyp files.
The v8.gyp file doesn't need to add them and in fact the way it did
that breaks when building against an external copy of ICU.

Fixes: #28052
PR-URL: #28118
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
kissofire mentioned this pull request Jul 22, 2022
Open
wedataintelligence mentioned this pull request Jul 22, 2022
Open
ron-a20 mentioned this pull request Jul 22, 2022
Open
Dmarch28 mentioned this pull request Jul 22, 2022
Open
aliceUnhinged613 mentioned this pull request Jul 22, 2022
Open
137717unity mentioned this pull request Jul 22, 2022
Open
baby636 mentioned this pull request Jul 22, 2022
Open
molllyn1 mentioned this pull request Jul 23, 2022
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
Trott Trott approved these changes
richardlau richardlau approved these changes
+2 more reviewers
refack refack approved these changes
mhdawson mhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Undefined variable icu_path during configure when system-icu is set

7 participants

Back | FazBrowse Home | New Git URL