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 common defines #23426

Merged
merged 1 commit into from
Nov 5, 2018
Merged

build: add common defines #23426

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion common.gypi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
'node_module_version%': '',
'node_with_ltcg%': '',
'node_use_pch%': 'false',
'node_shared_openssl%': 'false',

'node_tag%': '',
'uv_library%': 'static_library',

'clang%': 0,

'openssl_no_asm%': 0,
'openssl_fips%': '',

# Reset this number to 0 on major V8 upgrades.
Expand Down Expand Up @@ -261,6 +263,14 @@
}
}
},

# Defines these mostly for node-gyp to pickup, and warn addon authors of
# imminent V8 deprecations, also to sync how dependencies are configured.
'defines': [
'V8_DEPRECATION_WARNINGS',
'V8_IMMINENT_DEPRECATION_WARNINGS',
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to understand this better, what are the differences between these two?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Example: https://github.com/nodejs/node/pull/23414/files
AFAIU V8_DEPRECATION_WARNINGS makes API points marked with V8_DEPRECATED marked [[deprecated]]
V8_IMMINENT_DEPRECATION_WARNINGS makes thant for API points marked with V8_DEPRECATE_SOON.
Similar to node's policy V8_DEPRECATED are not-recommended for use, and can be removed in the next V8 version. V8_DEPRECATE_SOON can only be demoted to V8_DEPRECATED in a next version.

/CC @hashseed, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ATM we do turn on V8_DEPRECATION_WARNINGS for node build, and native addons builds.

Copy link
Contributor

Choose a reason for hiding this comment

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

So V8_IMMINENT_DEPRECATION_WARNINGS will report DEPRECATE_SOON and V8_DEPRECATION_WARNINGS will report the normal deprecation, right?

],

# Forcibly disable -Werror. We support a wide range of compilers, it's
# simply not feasible to squelch all warnings, never mind that the
# libraries in deps/ are not under our control.
Expand Down Expand Up @@ -503,7 +513,18 @@
'ldflags': [
'-Wl,--export-dynamic',
],
}]
}],
['node_shared_openssl!="true"', {
Copy link
Contributor

Choose a reason for hiding this comment

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

@refack You suggested that to avoid OpenSSL flags being passed to icu (and thus triggering an icu rebuild when switching between shared/non-shared openssl, a change that doesn't effect icu), I could "Add OPENSSL_THREADS to the else clause.", but I'm not sure what you mean by else clause here. Can you elaborate, and I'll make the change and test it?

Copy link
Contributor

Choose a reason for hiding this comment

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

cf. #25135

# `OPENSSL_THREADS` is defined via GYP for openSSL for all architectures.
'defines': [
'OPENSSL_THREADS',
],
}],
['node_shared_openssl!="true" and openssl_no_asm==1', {
'defines': [
'OPENSSL_NO_ASM',
],
}],
],
}
}