Reland "Add export_targets.py to presubmit"

This is a reland of c40a21f Changes: Fixed presubmit failing on Windows due to being unable to find gn, fixed export_targets.py failing on Windows for Googlers due to being unable to find Visual Studio files. Original change's description: > Add export_targets.py to presubmit > > Adds export_targets.py to run as part of presubmit in order to help > prevent breaking Firefox with BUILD.gn changes. > > Bug: chromium:1003151 > Change-Id: I5a7ab00891cd7c094c797e6150f642f803a726b6 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1802038 > Commit-Queue: Jamie Madill <[email protected]> > Reviewed-by: Yuly Novikov <[email protected]> Bug: chromium:1003151 Change-Id: I321ade86f2d969601afb8e1ed61c36bf166887b5 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1842127 Commit-Queue: Brian Sheedy <[email protected]> Commit-Queue: Yuly Novikov <[email protected]> Reviewed-by: Jamie Madill <[email protected]>
kakashidinho · Oct 7, 2019 · cc4d833 · cc4d833
1 parent 6acfca3
commit cc4d833
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 4 deletions.
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
@@ -7,14 +7,25 @@
 for more details on the presubmit API built into depot_tools.
 """
 
-from subprocess import call
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
 
 # Fragment of a regular expression that matches C++ and Objective-C++ implementation files.
 _IMPLEMENTATION_EXTENSIONS = r'\.(cc|cpp|cxx|mm)$'
 
 # Fragment of a regular expression that matches C++ and Objective-C++ header files.
 _HEADER_EXTENSIONS = r'\.(h|hpp|hxx)$'
 
+_PRIMARY_EXPORT_TARGETS = [
+    '//:libEGL',
+    '//:libGLESv1_CM',
+    '//:libGLESv2',
+    '//:translator',
+]
+
 
 def _CheckChangeHasBugField(input_api, output_api):
     """Requires that the changelist have a Bug: field."""
@@ -102,12 +113,46 @@ def gn_files(f):
     return []
 
 
+def _CheckExportValidity(input_api, output_api):
+    outdir = tempfile.mkdtemp()
+    # shell=True is necessary on Windows, as otherwise subprocess fails to find
+    # either 'gn' or 'vpython3' even if they are findable via PATH.
+    use_shell = input_api.is_windows
+    try:
+        try:
+            subprocess.check_output(['gn', 'gen', outdir], shell=use_shell)
+        except subprocess.CalledProcessError as e:
+            return [
+                output_api.PresubmitError(
+                    'Unable to run gn gen for export_targets.py: %s' % e.output)
+            ]
+        export_target_script = os.path.join(input_api.PresubmitLocalPath(), 'scripts',
+                                            'export_targets.py')
+        try:
+            subprocess.check_output(
+                ['vpython3', export_target_script, outdir] + _PRIMARY_EXPORT_TARGETS,
+                stderr=subprocess.STDOUT,
+                shell=use_shell)
+        except subprocess.CalledProcessError as e:
+            if input_api.is_committing:
+                return [output_api.PresubmitError('export_targets.py failed: %s' % e.output)]
+            return [
+                output_api.PresubmitPromptWarning(
+                    'export_targets.py failed, this may just be due to your local checkout: %s' %
+                    e.output)
+            ]
+        return []
+    finally:
+        shutil.rmtree(outdir)
+
+
 def CheckChangeOnUpload(input_api, output_api):
     results = []
     results.extend(_CheckCodeGeneration(input_api, output_api))
     results.extend(_CheckChangeHasBugField(input_api, output_api))
     results.extend(input_api.canned_checks.CheckChangeHasDescription(input_api, output_api))
     results.extend(_CheckNewHeaderWithoutGnChange(input_api, output_api))
+    results.extend(_CheckExportValidity(input_api, output_api))
     results.extend(
         input_api.canned_checks.CheckPatchFormatted(
             input_api, output_api, result_factory=output_api.PresubmitError))
@@ -121,5 +166,6 @@ def CheckChangeOnCommit(input_api, output_api):
         input_api.canned_checks.CheckPatchFormatted(
             input_api, output_api, result_factory=output_api.PresubmitError))
     results.extend(_CheckChangeHasBugField(input_api, output_api))
+    results.extend(_CheckExportValidity(input_api, output_api))
     results.extend(input_api.canned_checks.CheckChangeHasDescription(input_api, output_api))
     return results
diff --git a/scripts/export_targets.py b/scripts/export_targets.py
@@ -56,7 +56,18 @@
 REPO_DIR = pathlib.Path.cwd()
 
 GN_ENV = dict(os.environ)
-GN_ENV['DEPOT_TOOLS_WIN_TOOLCHAIN'] = '0'
+# We need to set DEPOT_TOOLS_WIN_TOOLCHAIN to 0 for non-Googlers, but otherwise
+# leave it unset since vs_toolchain.py assumes that the user is a Googler with
+# the Visual Studio files in depot_tools if DEPOT_TOOLS_WIN_TOOLCHAIN is not
+# explicitly set to 0.
+vs_found = False
+for directory in os.environ['PATH'].split(os.pathsep):
+    vs_dir = os.path.join(directory, 'win_toolchain', 'vs_files')
+    if os.path.exists(vs_dir):
+        vs_found = True
+        break
+if not vs_found:
+    GN_ENV['DEPOT_TOOLS_WIN_TOOLCHAIN'] = '0'
 
 (OUT_DIR, *ROOTS) = sys.argv[1:]
 assert len(ROOTS), 'Usage: export_targets.py OUT_DIR ROOTS...'
@@ -107,7 +118,7 @@ def recurse(key):
 
 try:
     p = run_checked('gn', 'desc', '--format=json', str(OUT_DIR), '*', stdout=subprocess.PIPE,
-                shell=True, env=GN_ENV)
+                env=GN_ENV, shell=(True if sys.platform == 'win32' else False))
 except subprocess.CalledProcessError:
     sys.stderr.buffer.write(b'`gn` failed. Is depot_tools in your PATH?\n')
     exit(1)
@@ -157,8 +168,17 @@ def pre(k):
 assert INCLUDE_REGEX.match(b'#include "foo"')
 assert INCLUDE_REGEX.match(b'\n#include "foo"')
 
+# Most of these are ignored because this script does not currently handle
+# #includes in #ifdefs properly, so they will erroneously be marked as being
+# included, but not part of the source list.
 IGNORED_INCLUDES = {
+    b'compiler/translator/TranslatorESSL.h',
+    b'compiler/translator/TranslatorGLSL.h',
+    b'compiler/translator/TranslatorHLSL.h',
     b'compiler/translator/TranslatorVulkan.h',
+    b'libANGLE/renderer/d3d/DeviceD3D.h',
+    b'libANGLE/renderer/d3d/DisplayD3D.h',
+    b'libANGLE/renderer/d3d/RenderTargetD3D.h',
     b'libANGLE/renderer/d3d/d3d11/winrt/NativeWindow11WinRT.h',
     b'libANGLE/renderer/gl/glx/DisplayGLX.h',
     b'libANGLE/renderer/gl/cgl/DisplayCGL.h',
@@ -188,7 +208,21 @@ def pre(k):
     b'X11',
 }
 
+IGNORED_DIRECTORIES = {
+    '//third_party/glslang',
+    '//third_party/spirv-tools',
+    '//third_party/SwiftShader',
+    '//third_party/vulkan-headers',
+    '//third_party/vulkan-loader',
+    '//third_party/vulkan-tools',
+    '//third_party/vulkan-validation-layers',
+}
+
 def has_all_includes(target_name: str, descs: dict) -> bool:
+    for ignored_directory in IGNORED_DIRECTORIES:
+        if target_name.startswith(ignored_directory):
+            return True
+
     flat = flattened_target(target_name, descs, stop_at_lib=False)
     acceptable_sources = flat.get('sources', []) + flat.get('outputs', [])
     acceptable_sources = {x.rsplit('/', 1)[-1].encode() for x in acceptable_sources}
@@ -262,7 +296,6 @@ def fn(target_name):
     for dep_name in set(desc['deps']):
         dep = descs[dep_name]
         if dep['type'] in LIBRARY_TYPES:
-            assert dep_name.startswith('//:'), dep_name
             dep_libs.add(dep_name[3:])
     desc['deps'] = sortedi(dep_libs)
 

diff --git a/src/libANGLE/renderer/vulkan/BUILD.gn b/src/libANGLE/renderer/vulkan/BUILD.gn
@@ -161,6 +161,7 @@ if (angle_swiftshader) {
   action("angle_swiftshader_icd") {
     deps = [
       ":angle_swiftshader_icd_rename",
+      "$angle_root/third_party/vulkan-headers/src:vulkan_headers",
     ]
     script = "$angle_root/scripts/generate_vulkan_layers_json.py"
     sources = [

diff --git a/util/util.gni b/util/util.gni
@@ -16,6 +16,7 @@ util_sources = [
   "util/util_export.h",
   "util/util_gl.h",
   "util/Event.h",
+  "util/EGLPlatformParameters.h",
   "util/EGLWindow.cpp",
   "util/EGLWindow.h",
   "util/Matrix.cpp",