From 68b50f656d20150b5440835782ef820510dfbb5c Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Wed, 28 Jul 2021 13:01:19 -0700 Subject: [PATCH] build: Stop using hermetic clang, libc++, etc This brings our default build config more in line with what is necessary for some platforms anyway: using the system-installed toolchain and sysroot to build everything. We will no longer fetch source or binaries for any specific build tools, such as libc++, clang, gold, binutils, or valgrind. The main part of this change is the changing of default gyp settings in gyp_packager.py. For this, a bug in gyp_packager.py had to be fixed, in which similar GYP_DEFINE key names (such as clang and host_clang) would conflict, causing some defaults not to be installed properly. In order to enable clang=0 by default, some changes had to be made in common.gypi: - compiler macros added to fix a compatibility issue between Chromium's base/mac/ folder and the actual OSX SDK - replaced clang_warning_flags variables with standard cflags settings, plus xcode_settings for OSX - turned off warnings-as-errors for non-shaka code, rather than allow-listing specific warning types, since we can't actually fix those warnings on any platform - disabled two specific warnings in shaka code, both of which are caused by headers from our non-shaka dependencies Also, one warning (missing "override" keyword) has been fixed in vod_media_info_dump_muxer_listener.h. Although these changes were done to make building simpler on a wider array of platforms (arm64, for example), it seems to make the build a bit faster, too. For me, at least, on my main Linux workstation: - "gclient sync" now runs 20-30% faster - "ninja -C out/Release" now runs 5-13% faster The following environment variables are no longer required: - DEPOT_TOOLS_WIN_TOOLCHAIN - MACOSX_DEPLOYMENT_TARGET Documentation, Dockerfiles, and GitHub Actions workflows have been updated to reflect this. The following GYP_DEFINES are no longer required for anyone: - clang=0 - host_clang=0 - clang_xcode=1 - use_allocator=none - use_experimental_allocator_shim=0 Documentation, Dockerfiles, and GitHub Actions workflows have been updated to reflect this. The following repos are no longer dependencies in gclient: - binutils - clang - gold - libc++ - libc++abi - valgrind The following gclient hooks have been removed: - clang - mac_toolchain - sysroot Change-Id: Ie94ccbeec722ab73c291cb7df897d20761a09a70 --- .../custom-actions/build-packager/action.yaml | 2 - DEPS | 49 -------------- Dockerfile | 2 +- docs/linux_profiling.md | 35 +++++++++- docs/source/build_instructions.md | 20 +----- gyp_packager.py | 28 +++++--- kokoro/windows/build.bat | 4 -- packager/common.gypi | 65 ++++++++++--------- .../vod_media_info_dump_muxer_listener.h | 4 +- packager/testing/dockers/Alpine_Dockerfile | 2 +- packager/testing/dockers/ArchLinux_Dockerfile | 3 - packager/third_party/libc++-static/README | 5 ++ 12 files changed, 100 insertions(+), 119 deletions(-) create mode 100644 packager/third_party/libc++-static/README diff --git a/.github/workflows/custom-actions/build-packager/action.yaml b/.github/workflows/custom-actions/build-packager/action.yaml index 82a2a4dc96..0193927139 100644 --- a/.github/workflows/custom-actions/build-packager/action.yaml +++ b/.github/workflows/custom-actions/build-packager/action.yaml @@ -57,9 +57,7 @@ runs: - name: Sync gclient env: - MACOSX_DEPLOYMENT_TARGET: "10.10" GYP_DEFINES: "target_arch=x64 libpackager_type=${{ inputs.lib_type }}_library" - DEPOT_TOOLS_WIN_TOOLCHAIN: "0" GYP_MSVS_VERSION: "2019" GYP_MSVS_OVERRIDE_PATH: "C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise" shell: bash diff --git a/DEPS b/DEPS index 27b00e252c..ce54dab69d 100644 --- a/DEPS +++ b/DEPS @@ -18,28 +18,12 @@ deps = { "src/packager/build": Var("chromium_git") + "/chromium/src/build@f0243d787961584ac95a86e7dae897b9b60ea674", #409966 - "src/packager/buildtools/third_party/libc++/trunk": - Var("github") + "/llvm-mirror/libcxx.git@8c22696675a2c5ea1c79fc64a4d7dfe1c2f4ca8b", - - "src/packager/buildtools/third_party/libc++abi/trunk": - Var("github") + "/llvm-mirror/libcxxabi.git@6092bfa6c153ad712e2fc90c7b9e536420bf3c57", - "src/packager/testing/gmock": Var("chromium_git") + "/external/googlemock@0421b6f358139f02e102c9c332ce19a33faf75be", #566 "src/packager/testing/gtest": Var("chromium_git") + "/external/github.com/google/googletest@6f8a66431cb592dad629028a50b3dd418a408c87", - # Keep bundled binutil scripts but not downloading actual binaries by default. - # Automatic downloading of binutils has been causing problems for some users: - # #164, #412, #440. Using bundled binutils helps reduce linking time, but - # packager codebase is relatively small, so the gain is not significant. - # User can still enable the usage of bundled binutils by running - # 'python src/packager/third_party/binutils/download.py' and set - # 'linux_use_bundled_binutils' and 'linux_use_bundled_gold' to 1 in GYP_DEFINES. - "src/packager/third_party/binutils": - Var("chromium_git") + "/chromium/src/third_party/binutils@8d77853bc9415bcb7bb4206fa2901de7603387db", - # Make sure the version matches the one in # src/packager/third_party/boringssl, which contains perl generated files. "src/packager/third_party/boringssl/src": @@ -70,22 +54,11 @@ deps = { "src/packager/third_party/zlib": Var("chromium_git") + "/chromium/src/third_party/zlib@830b5c25b5fbe37e032ea09dd011d57042dd94df", #408157 - "src/packager/tools/clang": - Var("chromium_git") + "/chromium/src/tools/clang@723b25997f0aab45fe1776a0f74a14782e350f8f", #513983 - "src/packager/tools/gyp": Var("chromium_git") + "/external/gyp@caa60026e223fc501e8b337fd5086ece4028b1c6", - - "src/packager/tools/valgrind": - Var("chromium_git") + "/chromium/deps/valgrind@3a97aa8142b6e63f16789b22daafb42d202f91dc", } deps_os = { - "unix": { # Linux, actually. - # Linux gold build to build faster. - "src/packager/third_party/gold": - Var("chromium_git") + "/chromium/deps/gold@29ae7431b4688df544ea840b0b66784e5dd298fe", - }, "win": { # Required by boringssl. "src/packager/third_party/yasm/source/patched-yasm": @@ -94,28 +67,6 @@ deps_os = { } hooks = [ - { - # Downloads the current stable linux sysroot to build/linux/ if needed. - # This script is a no-op except for linux. - 'name': 'sysroot', - 'pattern': '.', - 'action': ['python', 'src/packager/build/linux/sysroot_scripts/install-sysroot.py', - '--running-as-hook'], - }, - { - # Update the Mac toolchain if necessary. - 'name': 'mac_toolchain', - 'pattern': '.', - 'action': ['python', 'src/packager/build/mac_toolchain.py'], - }, - { - # Pull clang if needed or requested via GYP_DEFINES (GYP_DEFINES="clang=1"). - "name": "clang", - # Skip clang updates on Windows, where we don't use clang. - "condition": "not checkout_win", - "pattern": ".", - "action": ["python", "src/packager/tools/clang/scripts/update.py", "--if-needed"], - }, { # A change to a .gyp, .gypi, or to GYP itself should run the generator. "pattern": ".", diff --git a/Dockerfile b/Dockerfile index ab55f93034..264a0091aa 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,7 +14,7 @@ RUN sed -i \ '/malloc_usable_size/a \\nstruct mallinfo {\n int arena;\n int hblkhd;\n int uordblks;\n};' \ /usr/include/malloc.h -ENV GYP_DEFINES='clang=0 use_experimental_allocator_shim=0 use_allocator=none musl=1' +ENV GYP_DEFINES='musl=1' # Alpine does not support python3 yet, but depot_tools enabled python3 # by default. Disable python3 explicitly for now. # See https://github.com/google/shaka-packager/issues/763 for details. diff --git a/docs/linux_profiling.md b/docs/linux_profiling.md index ccf39adba8..9999823286 100644 --- a/docs/linux_profiling.md +++ b/docs/linux_profiling.md @@ -1,9 +1,9 @@ # Linux Profiling Profiling code is enabled when the `use_allocator` variable in gyp is set to -`tcmalloc` (currently the default) and `profiling` variable in gyp is set to -`1`. That will build the tcmalloc library, including the cpu profiling and heap -profiling code into shaka-packager, e.g. +`tcmalloc` and `profiling` variable in gyp is set to `1`. That will build the +tcmalloc library, including the cpu profiling and heap profiling code into +shaka-packager, e.g. GYP_DEFINES='profiling=1 use_allocator="tcmalloc"' gclient runhooks @@ -75,6 +75,35 @@ Or you can use gdb to attach at any point: 3. The filename will be printed on the console, e.g. "`Dumping heap profile to heap.0001.heap (foobar)`" + +## Thread sanitizer (tsan) + +To compile with the thread sanitizer library (tsan), you must set clang as your +compiler and set the `tsan=1` and `tsan_blacklist` configs: + + CC=clang CXX=clang++ GYP_DEFINES="tsan=1 tsan_blacklist=/path/to/src/packager/tools/memory/tsan_v2/ignores.txt" gclient runhooks + +NOTE: tsan and asan cannot be used at the same time. + + +## Adddress sanitizer (asan) + +To compile with the address sanitizer library (asan), you must set clang as your +compiler and set the `asan=1` config: + + CC=clang CXX=clang++ GYP_DEFINES="asan=1" gclient runhooks + +NOTE: tsan and asan cannot be used at the same time. + + +## Leak sanitizer (lsan) + +To compile with the leak sanitizer library (lsan), you must set clang as your +compiler and set the `lsan=1` config: + + CC=clang CXX=clang++ GYP_DEFINES="lsan=1" gclient runhooks + + ## Reference [Linux Profiling in Chromium](https://chromium.googlesource.com/chromium/src/+/master/docs/linux_profiling.md) diff --git a/docs/source/build_instructions.md b/docs/source/build_instructions.md index 92f3a13be8..210de16df5 100644 --- a/docs/source/build_instructions.md +++ b/docs/source/build_instructions.md @@ -93,10 +93,6 @@ If you don't have Administrator access, you can add a user-level PATH environment variable and put `C:\src\depot_tools` at the front, but if your system PATH has a Python in it, you will be out of luck. -Also, add a DEPOT_TOOLS_WIN_TOOLCHAIN system variable in the same way, and set -it to 0. This tells depot_tools to use your locally installed version of Visual -Studio (by default, depot_tools will try to use a google-internal version). - From a cmd.exe shell, run the command gclient (without arguments). On first run, gclient will install all the Windows-specific bits needed to work with the code, including msysgit and python. @@ -170,12 +166,6 @@ you can change build system to `make` by overriding `GYP_GENERATORS`: $ GYP_GENERATORS='make' gclient runhooks ``` -Another example, you can also disable clang by overriding `GYP_DEFINES`: - -```shell -$ GYP_DEFINES='clang=0' gclient runhooks -``` - #### Windows The instructions are similar, except that Windows allows using either `/` or `\` @@ -263,10 +253,10 @@ $ sed -i \ /usr/include/malloc.h ``` -We also need to disable clang and some other features to make it work with musl: +We also need to enable musl in the build config: ```shell -export GYP_DEFINES='clang=0 use_experimental_allocator_shim=0 use_allocator=none musl=1' +export GYP_DEFINES='musl=1' ``` ### Arch Linux @@ -288,12 +278,6 @@ $ gpg --keyserver pgp.mit.edu --recv-keys F7E48EDB $ makepkg -si ``` -Optionally, disable clang to build with gcc: - -```shell -$ export GYP_DEFINES='clang=0' -``` - ### Debian Same as Ubuntu. diff --git a/gyp_packager.py b/gyp_packager.py index 801be26824..1152bbd905 100755 --- a/gyp_packager.py +++ b/gyp_packager.py @@ -11,10 +11,6 @@ Build instructions: 1. Setup gyp: ./gyp_packager.py or use gclient runhooks -clang is enabled by default, which can be disabled by overriding -GYP_DEFINE environment variable, i.e. -"GYP_DEFINES='clang=0' gclient runhooks". - Ninja is the default build system. User can also change to make by overriding GYP_GENERATORS to make, i.e. "GYP_GENERATORS='make' gclient runhooks". @@ -74,18 +70,34 @@ if __name__ == '__main__': 'linux_use_bundled_binutils': 0, 'linux_use_bundled_gold': 0, 'linux_use_gold_flags': 0, + 'clang': 0, + 'host_clang': 0, + 'clang_xcode': 1, + 'use_allocator': 'none', + 'mac_deployment_target': '10.10', + 'use_experimental_allocator_shim': 0, 'clang_use_chrome_plugins': 0} - gyp_defines = os.environ.get('GYP_DEFINES', '') + gyp_defines_str = os.environ.get('GYP_DEFINES', '') + user_gyp_defines_map = {} + for term in gyp_defines_str.split(' '): + if term: + key, value = term.strip().split('=') + user_gyp_defines_map[key] = value + for key, value in _DEFAULT_DEFINES.items(): - if key not in gyp_defines: - gyp_defines += ' {0}={1}'.format(key, value) - os.environ['GYP_DEFINES'] = gyp_defines.strip() + if key not in user_gyp_defines_map: + gyp_defines_str += ' {0}={1}'.format(key, value) + os.environ['GYP_DEFINES'] = gyp_defines_str.strip() # Default to ninja, but only if no generator has explicitly been set. if 'GYP_GENERATORS' not in os.environ: os.environ['GYP_GENERATORS'] = 'ninja' + # By default, don't download our own toolchain for Windows. + if 'DEPOT_TOOLS_WIN_TOOLCHAIN' not in os.environ: + os.environ['DEPOT_TOOLS_WIN_TOOLCHAIN'] = '0' + # There shouldn't be a circular dependency relationship between .gyp files, # but in Chromium's .gyp files, on non-Mac platforms, circular relationships # currently exist. The check for circular dependencies is currently diff --git a/kokoro/windows/build.bat b/kokoro/windows/build.bat index 65314ba548..39c5423c70 100644 --- a/kokoro/windows/build.bat +++ b/kokoro/windows/build.bat @@ -19,10 +19,6 @@ python %PACKAGERDIR%\kokoro\deps_replacer.py "https://github.com" "https://githu cd %PACKAGERDIR%\.. -:: Disable downloading google-internal version of WIN TOOLCHAIN. Use locally -:: installed version instead. -set DEPOT_TOOLS_WIN_TOOLCHAIN=0 - :: Note that gclient file is a batch script, so 'call' must be used to wait for :: the result. :: Also gclient turns off echo, so echo is re-enabled after the command. diff --git a/packager/common.gypi b/packager/common.gypi index 90be5b4e31..f5689a04d0 100644 --- a/packager/common.gypi +++ b/packager/common.gypi @@ -41,25 +41,38 @@ ], }, 'target_defaults': { + # These defines make the contents of base/mac/foundation_util.h compile + # against the standard OSX SDK, by renaming Chrome's opaque type and using + # the real OSX type. This was not necessary before we switched away from + # using hermetic copies of clang and the sysroot to build. + 'defines': [ + 'OpaqueSecTrustRef=__SecACL', + 'OpaqueSecTrustedApplicationRef=__SecTrustedApplication', + ], 'conditions': [ ['shaka_code==1', { 'include_dirs': [ '.', '..', ], - 'variables': { - 'clang_warning_flags': [ - '-Wimplicit-fallthrough', - ], - # Revert the relevant settings in Chromium's common.gypi. - 'clang_warning_flags_unset': [ - '-Wno-char-subscripts', - '-Wno-unneeded-internal-declaration', - '-Wno-covered-switch-default', - - # C++11-related flags: - '-Wno-c++11-narrowing', - '-Wno-reserved-user-defined-literal', + 'cflags': [ + # This is triggered by logging macros. + '-Wno-implicit-fallthrough', + # Triggered by unit tests, which override things in mocks. gmock + # doesn't mark them as override. An upgrade may help. TODO: try + # upgrading gmock. + '-Wno-inconsistent-missing-override', + # Triggered by base/time/time.h when using clang, but NOT on Mac. + '-Wno-implicit-const-int-float-conversion', + ], + 'xcode_settings': { + 'WARNING_CFLAGS': [ + # This is triggered by logging macros. + '-Wno-implicit-fallthrough', + # Triggered by unit tests, which override things in mocks. gmock + # doesn't mark them as override. An upgrade may help. TODO: try + # upgrading gmock. + '-Wno-inconsistent-missing-override', ], }, # TODO(kqyang): Fix these msvs warnings. @@ -75,10 +88,17 @@ }, { # We do not have control over non-shaka code. Disable some warnings to # make build pass. + 'cflags': [ + '-Wno-error', + ], 'variables': { 'clang_warning_flags': [ - '-Wno-tautological-constant-compare', - '-Wno-unguarded-availability', + '-Wno-error', + ], + }, + 'xcode_settings': { + 'WARNING_CFLAGS': [ + '-Wno-error', ], }, 'msvs_disabled_warnings': [ @@ -87,19 +107,6 @@ # the code in CJK environment if there is non-ASCII characters # in the file. ], - 'cflags': [ - # TODO(modmaker): Remove once Chromium base is removed. - '-Wno-deprecated-declarations', - ], - 'conditions': [ - ['clang==0', { - 'cflags': [ - '-Wno-dangling-else', - '-Wno-deprecated-declarations', - '-Wno-unused-function', - ], - }], - ], }], ['musl==1', { 'defines': [ @@ -114,7 +121,7 @@ # warning in musl's sys/errno.h. '-Werror', ], - }] + }], ], }, } diff --git a/packager/media/event/vod_media_info_dump_muxer_listener.h b/packager/media/event/vod_media_info_dump_muxer_listener.h index eea91e483c..6b43b1964a 100644 --- a/packager/media/event/vod_media_info_dump_muxer_listener.h +++ b/packager/media/event/vod_media_info_dump_muxer_listener.h @@ -50,7 +50,9 @@ class VodMediaInfoDumpMuxerListener : public MuxerListener { int64_t start_time, int64_t duration, uint64_t segment_file_size) override; - void OnKeyFrame(int64_t timestamp, uint64_t start_byte_offset, uint64_t size); + void OnKeyFrame(int64_t timestamp, + uint64_t start_byte_offset, + uint64_t size) override; void OnCueEvent(int64_t timestamp, const std::string& cue_data) override; /// @} diff --git a/packager/testing/dockers/Alpine_Dockerfile b/packager/testing/dockers/Alpine_Dockerfile index 5e245661b6..460ba550b7 100644 --- a/packager/testing/dockers/Alpine_Dockerfile +++ b/packager/testing/dockers/Alpine_Dockerfile @@ -15,7 +15,7 @@ RUN sed -i \ '/malloc_usable_size/a \\nstruct mallinfo {\n int arena;\n int hblkhd;\n int uordblks;\n};' \ /usr/include/malloc.h -ENV GYP_DEFINES='clang=0 use_experimental_allocator_shim=0 use_allocator=none musl=1' +ENV GYP_DEFINES='musl=1' # Alpine does not support python3 yet, but depot_tools enabled python3 # by default. Disable python3 explicitly for now. # See https://github.com/google/shaka-packager/issues/763 for details. diff --git a/packager/testing/dockers/ArchLinux_Dockerfile b/packager/testing/dockers/ArchLinux_Dockerfile index c4a160f206..5227c9a31d 100644 --- a/packager/testing/dockers/ArchLinux_Dockerfile +++ b/packager/testing/dockers/ArchLinux_Dockerfile @@ -11,8 +11,5 @@ WORKDIR / RUN git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git ENV PATH /depot_tools:$PATH -# Clang needs libtinfo.so.5 which is not available on ArchLinux. -ENV GYP_DEFINES='clang=0' - # Build and run this docker by mapping shaka-packager with # -v "shaka-packager:/shaka-packager". diff --git a/packager/third_party/libc++-static/README b/packager/third_party/libc++-static/README new file mode 100644 index 0000000000..7e509d1c35 --- /dev/null +++ b/packager/third_party/libc++-static/README @@ -0,0 +1,5 @@ +This is a placeholder. Shaka Packager depends on an old version of the +Chromium build system, which unconditionally adds this folder to the library +search path. We no longer fetch a copy of libc++-static, but chromium's +common.gypi file can't be configured to exclude this from the linker path. +Therefore we just add a placeholder here to silence linker warnings.