-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
Releases/m71 #3
base: master
Are you sure you want to change the base?
Releases/m71 #3
Conversation
Rejected patch: `ortclib-sdk/webrtc/xplatform/chromium/build/config/compiler/BUILD.gn` ````diff diff a/config/compiler/BUILD.gn b/config/compiler/BUILD.gn (rejected hunks) @@ -104,6 +104,9 @@ declare_args() { # on by default when goma is enabled because setting this to true may make # it harder to debug binaries. strip_absolute_paths_from_debug_symbols = false + + # WebRTC does not want to switch to C++14 yet. + use_cxx11 = true } if (is_clang && !is_nacl) { ````
…arget_cpu. Removed link flag /DYNAMICBASE:NO in debug configuration for arm and arm64. Removed simd/jsimd_arm.C and simd/jsimd_arm_neon.S files from sources for arm and arm64 for winuwp target. Disabled warning 4267 for arm and arm64. By default it was disabled for x64.
…development environment, for x86 (task #246)
…m-merge' into releases/m62
…/m66 # Conflicts: # config/win/BUILD.gn # toolchain/win/BUILD.gn # toolchain/win/setup_toolchain.py # vs_toolchain.py
… confing/compiler/BUILD.gn when /Zi build option is removed when clang is used!
…23-winuwp-arm # Conflicts: # config/BUILDCONFIG.gn # config/win/BUILD.gn # toolchain/win/BUILD.gn # toolchain/win/setup_toolchain.py
…nuwp that was applied to wrong branch
…-arm # Conflicts: # config/win/BUILD.gn # toolchain/win/BUILD.gn
…/m71 # Conflicts: # .gitignore # config/BUILDCONFIG.gn # config/compiler/BUILD.gn # config/win/BUILD.gn # secondary/testing/gmock/BUILD.gn # secondary/testing/gtest/BUILD.gn # secondary/third_party/libjpeg_turbo/BUILD.gn # toolchain/win/BUILD.gn # toolchain/win/setup_toolchain.py # vs_toolchain.py
/Z7 flag is removed for arm and arm64 when project is built with clang-cl Temporary /EHs-c- is used for arm and arm64 when project is built with clang-cl
Updated cc and ccx tools to support arm64.
….exe and for linking lld-link.exe
@@ -1781,7 +1781,7 @@ config("no_size_t_to_int_warning") { | |||
# Code that currently generates warnings for this can include this | |||
# config to disable them. | |||
config("no_shorten_64_warnings") { | |||
if (current_cpu == "x64") { | |||
if (current_cpu == "x64" || current_cpu == "arm" || current_cpu == "arm64") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about the "arm" -- seems to me this should only be arm64.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was added because of compiler warning 4267 (conversion from 'size_t' to 'type', possible loss of data). Warning is generated for arm and arm64 builds, so it should be added. It could be put in another config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but does it apply to arm? seems to me this is only arm64, no?
[ rebase_path(sysroot) ], | ||
"string") == "True", | ||
"Missing sysroot ($sysroot). To fix, run: build/linux/sysroot_scripts/install-sysroot.py --arch=$_script_arch") | ||
if (exec_script("//build/dir_exists.py", [ rebase_path(sysroot) ], "string") != "True") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something that we'd be able to push back to google? If not, we will need to change this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is added because of broken preparation for Linux x86. Not sure if this fix is still required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you see if "official" has resolved this issue?
] | ||
|
||
#TEMPORARY!!! Clang-cl.exe missing unwind support for Windows arm and arm64. This will generate errors in places where try/catch are used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rephrase in a way that we can push back to google. It's okay to say this can be removed once clang-cl.exe has a feature but better to phrase properly for something we can push as I intend to push it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something that definitely shouldn't be pushed to Google. Added just to see what other issues exist for arm and arm64.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need it in such a way that I can submit to google. We don't want to have differences we can't submit back.
@@ -376,6 +387,20 @@ config("common_linker_setup") { | |||
"/DYNAMICBASE", | |||
] | |||
|
|||
# ASLR makes debugging with windbg difficult because Chrome.exe and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did you find this information about chrome? just wondering where it came from
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is Google comment and it was present in older version of that file. I believe that is from m62.
} else { | ||
rspfile = "{{output}}.rsp" | ||
command = "$env_wrapper$cl /nologo /showIncludes ${clflags} @$rspfile /c {{source}} /Fo{{output}} /Fd\"$pdbname\"" | ||
rspfile_content = "$sys_include_flags{{defines}} {{include_dirs}} {{cflags}} {{cflags_cc}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rsp consistency ?
toolchain/win/BUILD.gn
Outdated
else if (toolchain_args.current_cpu == "arm"){ | ||
ml = "armasm.exe" | ||
} else { | ||
ml = "armasm64.exe" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it needs to be an explicit check for arm64 in case another CPU is ever added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe assert "Unsupported CPU tartget"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
@@ -276,7 +276,8 @@ def q(s): # Quote s if it contains spaces or other weird characters. | |||
print 'vc_lib_path = ' + gn_helpers.ToGNString(vc_lib_path) | |||
if (target_store != True): | |||
# Path is assumed not to exist for desktop applications | |||
assert vc_lib_atlmfc_path | |||
if (cpu != 'arm' and cpu != 'arm64'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure this is ok for other non uwp builds.
vs_toolchain.py
Outdated
@@ -190,8 +190,12 @@ def _CopyUCRTRuntime(target_dir, source_dir, target_cpu, dll_pattern, suffix): | |||
os.environ.get('WINDOWSSDKDIR', | |||
os.path.expandvars('%ProgramFiles(x86)%' | |||
'\\Windows Kits\\10'))) | |||
ucrt_dll_dirs = os.path.join(win_sdk_dir, 'Redist', 'ucrt', 'DLLs', | |||
target_cpu) | |||
if target_cpu != 'arm64': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd make this an explicit == 'arm64' then default to standard behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
@@ -250,6 +256,8 @@ def _CopyPGORuntime(target_dir, target_cpu): | |||
source = os.path.join(pgo_x86_runtime_dir, runtime) | |||
elif target_cpu == 'x64': | |||
source = os.path.join(pgo_x64_runtime_dir, runtime) | |||
elif target_cpu == 'arm': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No arm64 needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
@@ -1781,7 +1781,7 @@ config("no_size_t_to_int_warning") { | |||
# Code that currently generates warnings for this can include this | |||
# config to disable them. | |||
config("no_shorten_64_warnings") { | |||
if (current_cpu == "x64") { | |||
if (current_cpu == "x64" || current_cpu == "arm" || current_cpu == "arm64") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but does it apply to arm? seems to me this is only arm64, no?
[ rebase_path(sysroot) ], | ||
"string") == "True", | ||
"Missing sysroot ($sysroot). To fix, run: build/linux/sysroot_scripts/install-sysroot.py --arch=$_script_arch") | ||
if (exec_script("//build/dir_exists.py", [ rebase_path(sysroot) ], "string") != "True") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you see if "official" has resolved this issue?
] | ||
|
||
#TEMPORARY!!! Clang-cl.exe missing unwind support for Windows arm and arm64. This will generate errors in places where try/catch are used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need it in such a way that I can submit to google. We don't want to have differences we can't submit back.
config/win/BUILD.gn
Outdated
@@ -421,7 +446,7 @@ config("default_crt") { | |||
configs = [ ":dynamic_crt" ] | |||
} else { | |||
# Desktop Windows: static CRT. | |||
configs = [ ":static_crt" ] | |||
configs = [ ":dynamic_crt" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea why it changed for winuwp; probably a reason but I don't know what it would be; it's been a LONG time like this...
Sergej/20181206 m71 python
Sergej/20190212 m71 arm
Robin/m71 dot net
This change builds maven modules for Cronet. Each maven module includes: 1. POM file defining module and listing dependent modules 2. Jars 3. Proguard files (optional) 4. Native libraries (optional) 5. Resources (optional) 6. Javadocs (optional) We build four maven modules: 1. cronet-api: the Cronet API 2. cronet-embedded: the Cronet native implementation 3. cronet-fallback: the Cronet fallback implementation 4. cronet-common: Cronet implementation in common with #3 and #4 To test the maven modules, this change builds the Cronet sample app using gradle. You can install and run the resulting app by: ninja -C out/Debug cronet_maven_test_build adb install -r out/Debug/cronet_maven/test/build/outputs/apk/debug/test-debug.apk adb install -r out/Debug/cronet_maven/test/build/outputs/apk/androidTest/debug/test-debug-androidTest.apk adb shell am instrument -w org.chromium.cronet_sample_apk.test/android.support.test.runner.AndroidJUnitRunner Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I5ed27e86f3e01dc3a6441ab449bbe942553343af Reviewed-on: https://chromium-review.googlesource.com/998019 Commit-Queue: Paul Jensen <[email protected]> Reviewed-by: Andrei Kapishnikov <[email protected]> Reviewed-by: Eric Stevenson <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#554608} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 10f4dd8996fe4a028ee2aa61cdf84ad4080ef31c
…with gradle" This reverts commit 10f4dd8996fe4a028ee2aa61cdf84ad4080ef31c. Reason for revert: Build failures on 3 bots BUG=838082 Original change's description: > [Cronet] Build maven modules and test by building sample app with gradle > > This change builds maven modules for Cronet. > Each maven module includes: > 1. POM file defining module and listing dependent modules > 2. Jars > 3. Proguard files (optional) > 4. Native libraries (optional) > 5. Resources (optional) > 6. Javadocs (optional) > We build four maven modules: > 1. cronet-api: the Cronet API > 2. cronet-embedded: the Cronet native implementation > 3. cronet-fallback: the Cronet fallback implementation > 4. cronet-common: Cronet implementation in common with #3 and #4 > > To test the maven modules, this change builds the Cronet sample app using gradle. > You can install and run the resulting app by: > ninja -C out/Debug cronet_maven_test_build > adb install -r out/Debug/cronet_maven/test/build/outputs/apk/debug/test-debug.apk > adb install -r out/Debug/cronet_maven/test/build/outputs/apk/androidTest/debug/test-debug-androidTest.apk > adb shell am instrument -w org.chromium.cronet_sample_apk.test/android.support.test.runner.AndroidJUnitRunner > > Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet > Change-Id: I5ed27e86f3e01dc3a6441ab449bbe942553343af > Reviewed-on: https://chromium-review.googlesource.com/998019 > Commit-Queue: Paul Jensen <[email protected]> > Reviewed-by: Andrei Kapishnikov <[email protected]> > Reviewed-by: Eric Stevenson <[email protected]> > Cr-Commit-Position: refs/heads/master@{#554608} [email protected],[email protected],[email protected] # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: I2779ce28f4df2b8dfe8dd2fcf5fc5a3a850c8ee0 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet Reviewed-on: https://chromium-review.googlesource.com/1034534 Reviewed-by: Anita Woodruff <[email protected]> Commit-Queue: Anita Woodruff <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#554723} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: ab3426de8a278364703d1023b946cb2cc6ff23f7
…with gradle" This is a reland of 10f4dd8996fe4a028ee2aa61cdf84ad4080ef31c I disabled the gradle build when !use_platform_icu_alternatives so Android bots continue to build successfully. I also moved cronet_maven/ output directory to cronet/maven/ so it can be uploaded by the bots. Original change's description: > [Cronet] Build maven modules and test by building sample app with gradle > > This change builds maven modules for Cronet. > Each maven module includes: > 1. POM file defining module and listing dependent modules > 2. Jars > 3. Proguard files (optional) > 4. Native libraries (optional) > 5. Resources (optional) > 6. Javadocs (optional) > We build four maven modules: > 1. cronet-api: the Cronet API > 2. cronet-embedded: the Cronet native implementation > 3. cronet-fallback: the Cronet fallback implementation > 4. cronet-common: Cronet implementation in common with #3 and #4 > > To test the maven modules, this change builds the Cronet sample app using gradle. > You can install and run the resulting app by: > ninja -C out/Debug cronet_maven_test_build > adb install -r out/Debug/cronet_maven/test/build/outputs/apk/debug/test-debug.apk > adb install -r out/Debug/cronet_maven/test/build/outputs/apk/androidTest/debug/test-debug-androidTest.apk > adb shell am instrument -w org.chromium.cronet_sample_apk.test/android.support.test.runner.AndroidJUnitRunner > > Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet > Change-Id: I5ed27e86f3e01dc3a6441ab449bbe942553343af > Reviewed-on: https://chromium-review.googlesource.com/998019 > Commit-Queue: Paul Jensen <[email protected]> > Reviewed-by: Andrei Kapishnikov <[email protected]> > Reviewed-by: Eric Stevenson <[email protected]> > Cr-Commit-Position: refs/heads/master@{#554608} Change-Id: Ieba6842b48d0b8e97ca1dc6686080cb31a87ed95 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet [email protected] Eric already approved the build/ change in https://chromium-review.googlesource.com/998019 Change-Id: Ieba6842b48d0b8e97ca1dc6686080cb31a87ed95 Reviewed-on: https://chromium-review.googlesource.com/1037723 Commit-Queue: Paul Jensen <[email protected]> Reviewed-by: Eric Stevenson <[email protected]> Reviewed-by: Andrei Kapishnikov <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#555157} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 5ff415959d5ef4d105cda4cd4fe36f06107f54b9
Use a new `std_cpp17` argument in Windows builds to enable forcing Win32 MSVC builds to use `/std:c++17`. This is already forced for UWP, but is needed for Win32 if a module compiled with C++17 needs to link against the `webrtc.lib` compiled from this repository. Otherwise the layout mismatch between `absl::optional` (non-c++17) and `std::optional` (c++17) produces a link error.
Enable forcing `/std:c++17` via GN argument
No description provided.