Skip to content
This repository has been archived by the owner on Jan 4, 2019. It is now read-only.

Commit

Permalink
suppress icu build in gn for muon
Browse files Browse the repository at this point in the history
  • Loading branch information
bridiver committed Jan 11, 2017
1 parent 089fc18 commit fb89355
Show file tree
Hide file tree
Showing 7 changed files with 262 additions and 157 deletions.
6 changes: 3 additions & 3 deletions atom/browser/javascript_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ bool JavascriptEnvironment::Initialize() {
if (!js_flags.empty())
v8::V8::SetFlagsFromString(js_flags.c_str(), js_flags.size());

base::i18n::InitializeICU();
// base::i18n::InitializeICU();

This comment has been minimized.

Copy link
@bsclifton

bsclifton Jan 12, 2017

Member

Should we remove these comments and (optionally) explain how ICU gets initialized?

Which I am curious about. This solution still loads from a snapshot, right? (as seen by having the snapshot_blob and natives_blob). What does that loading?

This comment has been minimized.

Copy link
@bridiver

bridiver Jan 12, 2017

Author Collaborator

I'll just remove them. They were added for a previous fix which didn't work


gin::V8Initializer::LoadV8Snapshot();
gin::V8Initializer::LoadV8Natives();
// gin::V8Initializer::LoadV8Snapshot();
// gin::V8Initializer::LoadV8Natives();
gin::IsolateHolder::Initialize(gin::IsolateHolder::kNonStrictMode,
gin::IsolateHolder::kStableV8Extras,
gin::ArrayBufferAllocator::SharedInstance());
Expand Down
35 changes: 26 additions & 9 deletions build/node/BUILD.gn
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
import("//v8/gni/v8.gni")

config("node_external_config") {
config("build_config") {
defines = [
"NODE_WANT_INTERNALS=1",
"NODE_SHARED_MODE",
]

include_dirs = [
"//v8",
"//v8/include",

]
}

config("node_external_config") {
if (is_linux) {
ldflags =
[ rebase_path("$root_out_dir/lib/libnode.so",
Expand All @@ -15,21 +23,19 @@ config("node_external_config") {
[ rebase_path("$root_out_dir/node.dll.lib",
root_build_dir) ]
} else if (is_mac) {
ldflags =
[ rebase_path("$root_out_dir/libnode.dylib",
root_build_dir) ]
ldflags = [
rebase_path("$root_out_dir/libnode.dylib", root_build_dir),
]
}
}

source_set("node") {
public_configs = [
":node_external_config",
"//electron/build:electron_config",
]

public_deps = [
":build_node",
"//v8:v8",
]
}

Expand Down Expand Up @@ -74,18 +80,29 @@ if (is_mac) {
}

action("build_node") {
public_configs = [
":build_config",
]

# dummy script - these are actually produced separately through gyp
script = "//electron/tools/node_gyp_build.py"

outputs = [
"$root_out_dir/natives_blob.bin",
"$root_out_dir/snapshot_blob.bin",
"$root_out_dir/icudtl.dat",
]
if (is_mac) {
outputs = [ "$root_out_dir/libnode.dylib" ]
outputs += [
"$root_out_dir/libnode.dylib",
]
} else if (is_win) {
outputs = [
outputs += [
"$root_out_dir/node.dll",
"$root_out_dir/node.dll.lib",
]
} else if (is_linux) {
outputs = [ "$root_out_dir/lib/libnode.so" ]
outputs += [ "$root_out_dir/lib/libnode.so" ]
}
}

Expand Down
23 changes: 17 additions & 6 deletions build/node/node.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,6 @@
],
}],
['_target_name=="node"', {
'include_dirs': [
'../v8',
'../v8/include',
],
'cflags!': [
'-fvisibility=hidden',
'-fdata-sections',
Expand Down Expand Up @@ -192,10 +188,25 @@
],
# Node is using networking API but linking with this itself.
'libraries': [ '-lwinmm.lib' ],
'variables': {
'reference_symbols': [
'udata_setCommonData_56',
'u_errorName_56',
'ubidi_setPara_56',
'ucsdet_getName_56',
'uidna_openUTS46_56',
'ulocdata_close_56',
'unorm_normalize_56',
'uregex_matches_56',
'uspoof_open_56',
'usearch_setPattern_56',
'?createInstance@Transliterator@icu_56@@SAPEAV12@AEBVUnicodeString@2@W4UTransDirection@@AEAW4UErrorCode@@@Z',
'??0MeasureFormat@icu_56@@QEAA@AEBVLocale@1@W4UMeasureFormatWidth@@AEAW4UErrorCode@@@Z',
],

This comment has been minimized.

Copy link
@bsclifton

bsclifton Jan 12, 2017

Member

wow- how does this part here work? You're forcing the references... does this make gn drop the warning/error and send it directly to the linker?

This comment has been minimized.

Copy link
@bridiver

bridiver Jan 12, 2017

Author Collaborator

it ensures that the symbols are exported from the node dll

},
'msvs_settings': {
'VCLinkerTool': {
'EnableCOMDATFolding': '1', # disable
'OptimizeReferences': '1', # disable
'ForceSymbolReferences': [ '<@(reference_symbols)' ],
},
},
}],
Expand Down
22 changes: 15 additions & 7 deletions build/node/v8.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
'icu_path': '../../../third_party/icu',
'icu_use_data_file_flag': 1,
# Using libc++ requires building for >= 10.7.
'mac_deployment_target': '10.8',
# 'mac_deployment_target': '10.8',
# Use the standard way of linking with msvc runtime.
'win_use_allocator_shim': 0,
# 'win_use_allocator_shim': 0,
'v8_enable_i18n_support': 1,
# The V8 libraries.
'v8_libraries': '["v8", "v8_snapshot", "v8_nosnapshot", "v8_external_snapshot", "v8_base", "v8_libbase", "v8_libplatform"]',
'v8_use_snapshot': 'true',
'v8_use_external_startup_data': 1,
# 'v8_use_snapshot': 'true',
# 'v8_use_external_startup_data': 1,
'icu_libraries': '["icui18n", "icuuc"]',
},
'target_defaults': {
Expand Down Expand Up @@ -38,13 +38,21 @@
# Use C++11 library.
'CLANG_CXX_LIBRARY': 'libc++', # -stdlib=libc++
},
'defines': [
'U_COMBINED_IMPLEMENTATION',
# Defining "U_COMBINED_IMPLEMENTATION" will add "explicit" for some
# constructors, make sure it doesn' happen.
'UNISTR_FROM_CHAR_EXPLICIT=',
'UNISTR_FROM_STRING_EXPLICIT=',
'U_NO_DEFAULT_INCLUDE_UTF_HEADERS=0',
],
'defines!': [
'U_STATIC_IMPLEMENTATION',
],
'target_conditions': [
['_type=="static_library" and _toolset=="target" and OS=="linux"', {
'standalone_static_library': 1,
}],
['_target_name in <(icu_libraries)', {
'type': 'static_library',
}],
['_target_name in <(v8_libraries) + <(icu_libraries)', {
'xcode_settings': {
'DEAD_CODE_STRIPPING': 'NO', # -Wl,-dead_strip
Expand Down
97 changes: 92 additions & 5 deletions patches/master_patch.patch
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
diff --git a/build/config/chrome_build.gni b/build/config/chrome_build.gni
index c649018a7a0aeb20caeb2bf37c60d57c48630f1b..a3a5b9507dfebf8d010e018ee1a5b11203146a67 100644
index c649018a7a0aeb20caeb2bf37c60d57c48630f1b..e28969349e35c4bdb48c0693ce7604b269a22ca0 100644
--- a/build/config/chrome_build.gni
+++ b/build/config/chrome_build.gni
@@ -8,15 +8,15 @@ declare_args() {
@@ -8,15 +8,17 @@ declare_args() {
# resources).
is_chrome_branded = false

Expand All @@ -17,15 +17,16 @@ index c649018a7a0aeb20caeb2bf37c60d57c48630f1b..a3a5b9507dfebf8d010e018ee1a5b112
# Break chrome.dll into multple pieces based on process type. Only available
# on Windows.
is_multi_dll_chrome = is_win && !is_component_build
}
-
-}
-# Refers to the subdirectory for branding in various places including
-# chrome/app/theme.
-if (is_chrome_branded) {
- branding_path_component = "google_chrome"
-} else {
- branding_path_component = "chromium"
-}
+ muon_build = false
}
diff --git a/build/toolchain/mac/BUILD.gn b/build/toolchain/mac/BUILD.gn
index 0c00e995fffe139ac39badd12790acbad3a78a7a..e5651111ce284f04ce3b5d6157be84c3738e732c 100644
--- a/build/toolchain/mac/BUILD.gn
Expand Down Expand Up @@ -2077,6 +2078,77 @@ index b93ff45d633c9330beaeb77f4772d3d8d5486245..79748cdaa5ff73ccadaecccbb88c4f9f
// Called to read raw (pre-filtered) data from this Job. Reads at most
// |buf_size| bytes into |buf|.
// Possible return values:
diff --git a/remoting/host/BUILD.gn b/remoting/host/BUILD.gn
index eb095bc5777a6b05bc6ad1ef4ef64393dfdf0592..72eefb12d24bef531684a3f486a8e36f3763ab42 100644
--- a/remoting/host/BUILD.gn
+++ b/remoting/host/BUILD.gn
@@ -12,6 +12,7 @@ import("//remoting/remoting_locales.gni")
import("//remoting/remoting_options.gni")
import("//remoting/remoting_version.gni")
import("//remoting/tools/build/remoting_localize.gni")
+import("//third_party/icu/config.gni")

process_version("remoting_version") {
template_file = "//remoting/host/version.h.in"
@@ -1594,9 +1595,14 @@ if (enable_me2me_host) {
"$root_gen_dir/remoting/CREDITS.txt",
"$root_out_dir/remoting/com.google.chrome.remote_assistance.json",
"$root_out_dir/remoting/com.google.chrome.remote_desktop.json",
- "$root_out_dir/icudtl.dat",
]

+ if (icu_use_data_file) {
+ inputs += [
+ "$root_out_dir/icudtl.dat",
+ ]
+ }
+
_generated_files = rebase_path(inputs, root_build_dir)
_generated_files += [ rebase_path("//remoting/resources/chromoting.ico") ]

diff --git a/services/shell/standalone/BUILD.gn b/services/shell/standalone/BUILD.gn
index fb55a656a2d0f9f897225d5651603dfa5bac05bc..47c63aeb45fe222d8dd53fd16ca8768d909d4a5c 100644
--- a/services/shell/standalone/BUILD.gn
+++ b/services/shell/standalone/BUILD.gn
@@ -6,6 +6,8 @@ import("//services/shell/public/cpp/service.gni")
import("//mojo/public/tools/bindings/mojom.gni")
import("//testing/test.gni")

+# workaround for icu linking errors on windows
+if (muon_build) { group("standalone") {} } else {
executable("standalone") {
output_name = "mojo_runner"
sources = [
@@ -18,6 +20,7 @@ executable("standalone") {
"//build/win:default_exe_manifest",
]
}
+}

source_set("lib") {
sources = [
diff --git a/services/tracing/BUILD.gn b/services/tracing/BUILD.gn
index f002ab60b88a8c8f2c9d1676eaba37e12b84ebbe..3c8517cb748e2368058bd363d20c10d7a833aa1c 100644
--- a/services/tracing/BUILD.gn
+++ b/services/tracing/BUILD.gn
@@ -5,6 +5,8 @@
import("//services/shell/public/cpp/service.gni")
import("//services/shell/public/service_manifest.gni")

+# workaround for icu linking errors on windows
+if (muon_build) { group("tracing") {} } else {
service("tracing") {
sources = [
"main.cc",
@@ -27,7 +29,7 @@ service_manifest("manifest") {
name = "tracing"
source = "manifest.json"
}
-
+}
source_set("lib") {
sources = [
"data_sink.cc",
diff --git a/third_party/WebKit/Source/core/dom/DOMArrayBuffer.h b/third_party/WebKit/Source/core/dom/DOMArrayBuffer.h
index e436a079e835ad465a5262249f0a402d58e25f02..872df7f2c47de00abc32a95f7564172090b75527 100644
--- a/third_party/WebKit/Source/core/dom/DOMArrayBuffer.h
Expand Down Expand Up @@ -2187,6 +2259,21 @@ index 82775974231274e6639751139f5c7bc9bbfaad25..8aca09939d20cc48cfa85c507fdea17c
readonly attribute long long lastModified;

// Non-standard APIs
diff --git a/third_party/WebKit/Source/platform/BUILD.gn b/third_party/WebKit/Source/platform/BUILD.gn
index ce25028fe74fa15fe3f9a24dd4188a6407dc7602..67b7f7a9776f16fd9ba2d99185da643982a1ee66 100644
--- a/third_party/WebKit/Source/platform/BUILD.gn
+++ b/third_party/WebKit/Source/platform/BUILD.gn
@@ -186,6 +186,10 @@ executable("character_data_generator") {
"//build/win:default_exe_manifest",
"//third_party/icu",
]
+
+ if (is_mac) {
+ ldflags = [ "-Wl,-rpath,@loader_path/.", ]
+ }
}

# Inspector protocol
diff --git a/third_party/WebKit/Source/web/WebArrayBuffer.cpp b/third_party/WebKit/Source/web/WebArrayBuffer.cpp
index 471ce6a5c66f6a7240732a12596746ef239a48e1..29b0bbdff9dfe9d0e1d1fb943b36bb98a7bea385 100644
--- a/third_party/WebKit/Source/web/WebArrayBuffer.cpp
Expand Down
49 changes: 49 additions & 0 deletions patches/third_party/icu/filter.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
diff --git a/BUILD.gn b/BUILD.gn
index 1dd7c61d5368a643cb9d8cc489f8833cd796cb5f..6a7b287db3357cdaaff4d822002cc4a8b26f4b44 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -3,6 +3,7 @@
# found in the LICENSE file.

import("//third_party/icu/config.gni")
+import("//build/config/chrome_build.gni")

if (is_android) {
import("//build/config/android/rules.gni")
@@ -39,7 +40,9 @@ config("icu_config") {
]

if (!is_component_build) {
+ if (!muon_build) {
defines += [ "U_STATIC_IMPLEMENTATION" ]
+ }
}

include_dirs = [
@@ -122,6 +125,21 @@ config("icu_code") {
}
}

+if (muon_build) {
+ group("icui18n") {
+ public_deps = [ "//electron/build/node" ]
+ public_configs = [ ":icu_config" ]
+ }
+
+ group("icuuc") {
+ public_deps = [ "//electron/build/node" ]
+ public_configs = [ ":icu_config" ]
+ }
+
+ group("icudata") {
+ public_deps = [ "//electron/build/node:build_node" ]
+ }
+} else {
component("icui18n") {
# find source/i18n -maxdepth 1 ! -type d | egrep '\.(c|cpp|h)$' |\
# sort | sed 's/^\(.*\)$/ "\1",/'
@@ -1054,3 +1072,4 @@ if (icu_use_data_file) {
}
}
}
+}
Loading

1 comment on commit fb89355

@bsclifton
Copy link
Member

Choose a reason for hiding this comment

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

Comments left

Great job!! 😄 This was a NASTY issue

Please sign in to comment.