servo: Merge #19720 - Update buildbot_steps lint to handle env variables (from aneeshusa:support-env-vars-in-buildbot-steps); r=jdm
authorAneesh Agrawal <aneeshusa@gmail.com>
Tue, 09 Jan 2018 11:40:27 -0600
changeset 452723 12daae128ea3063d31ef6e50ae4ddbb2d3a637e9
parent 452722 865837f77a90a2db5cf24998ac26fdec951c98e7
child 452724 f214ee9699b42437807bc33291276d54996046fb
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #19720 - Update buildbot_steps lint to handle env variables (from aneeshusa:support-env-vars-in-buildbot-steps); r=jdm https://github.com/servo/saltfs/pull/687 added support for specifying environment variables in `buildbot_steps.yml`. Update the servo-tidy buildbot_steps.yml linter to reflect this. Use the voluptuous Python library (BSD 3-clause license) for validation in lieu of a much larger hand-written implementation. Extracted out of #17171. Helps with servo/saltfs#770. <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [x] There are tests for these changes OR - [ ] These changes do not require tests because they change the tests Source-Repo: https://github.com/servo/servo Source-Revision: 6a6dda526961b7bb7b856e8310ffc165d8bd39aa
servo/.travis.yml
servo/appveyor.yml
servo/etc/ci/buildbot_steps.yml
servo/python/requirements.txt
servo/python/tidy/servo_tidy/tidy.py
servo/python/tidy/servo_tidy_tests/test_tidy.py
--- a/servo/.travis.yml
+++ b/servo/.travis.yml
@@ -15,31 +15,33 @@ matrix:
       before_install:
         - sudo add-apt-repository ppa:ubuntu-toolchain-r/test -y
         - sudo add-apt-repository 'deb http://apt.llvm.org/precise/ llvm-toolchain-precise-3.9 main' -y
         - sudo apt-get update -q
         - sudo apt-get install clang-3.9 llvm-3.9 llvm-3.9-runtime -y
         - export LLVM_CONFIG=/usr/lib/llvm-3.9/bin/llvm-config
         - export CC=gcc-5 CXX=g++-5
       script:
-         - RUSTFLAGS='-D warnings' ./mach build -d --verbose
-         - RUSTFLAGS='-D warnings' ./mach test-unit
+         - ./mach build -d --verbose
+         - ./mach test-unit
          - ./mach clean
-         - RUSTFLAGS='-D warnings' ./mach build-geckolib
-         - RUSTFLAGS='-D warnings' ./mach test-stylo
+         - ./mach build-geckolib
+         - ./mach test-stylo
          - bash etc/ci/lockfile_changed.sh
       cache:
         directories:
           - .cargo
           - .servo
           - $HOME/.ccache
       before_cache:
         - ./mach clean-nightlies --keep 2 --force
         - ./mach clean-cargo-cache --keep 2 --force
-      env: CCACHE=/usr/bin/ccache
+      env:
+        CCACHE=/usr/bin/ccache
+        RUSTFLAGS=-Dwarnings
       addons:
         apt:
           sources:
             - ubuntu-toolchain-r-test
             - sourceline: 'ppa:jonathonf/ffmpeg-3'
           packages:
             - cmake
             - dbus-x11
--- a/servo/appveyor.yml
+++ b/servo/appveyor.yml
@@ -1,13 +1,14 @@
 version: 1.0.{build}
 
 environment:
   CCACHE_DIR: "%APPVEYOR_BUILD_FOLDER%\\.ccache"
   RUST_BACKTRACE: 1
+  RUSTFLAGS: -Dwarnings
   # The appveyor image we use has a pretty huge set of things installed... we make the
   # initial PATH something sane so we know what to expect
   PATH: "C:\\windows\\system32;\
     C:\\windows;\
     C:\\windows\\System32\\Wbem;\
     C:\\windows\\System32\\WindowsPowerShell\\v1.0;\
     C:\\ProgramData\\chocolatey\\bin;\
     C:\\Python27;\
@@ -43,13 +44,12 @@ cache:
 # Uncomment these lines to expose RDP access information to the build machine in the build log.
 #init:
 #  - ps: iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1'))
 #
 #on_finish:
 #  - ps: $blockRdp = $true; iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1'))
 
 build_script:
-  - set RUSTFLAGS=-D warnings
   - mach build -d -v
   - mach test-unit
 
 test: off
--- a/servo/etc/ci/buildbot_steps.yml
+++ b/servo/etc/ci/buildbot_steps.yml
@@ -1,8 +1,11 @@
+env:
+  RUSTFLAGS: -Dwarnings
+
 mac-rel-wpt1:
   - ./mach clean-nightlies --keep 3 --force
   - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig ./mach build --release
   - ./mach run -r -o output.png
   - ./mach test-wpt-failure
   - ./mach test-wpt --release --processes 4 --total-chunks 6 --this-chunk 1 --log-raw test-wpt.log --log-errorsummary wpt-errorsummary.log --always-succeed
   - ./mach filter-intermittents wpt-errorsummary.log --log-intermittents intermittents.log --log-filteredsummary filtered-wpt-errorsummary.log --tracker-api default --reporter-api default
   - ./mach test-wpt --release --binary-arg=--multiprocess --processes 8 --log-raw test-wpt-mp.log --log-errorsummary wpt-mp-errorsummary.log eventsource
@@ -27,21 +30,21 @@ mac-rel-wpt4:
   - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig ./mach build --release
   - ./mach test-wpt --release --processes 4 --total-chunks 6 --this-chunk 4 --log-raw test-wpt.log --log-errorsummary wpt-errorsummary.log --always-succeed
   - ./mach filter-intermittents wpt-errorsummary.log --log-intermittents intermittents.log --log-filteredsummary filtered-wpt-errorsummary.log --tracker-api default --reporter-api default
   - ./mach test-wpt --release --pref dom.servoparser.async_html_tokenizer.enabled --processes=8 --log-raw test-async-parsing.log --log-errorsummary async-parsing-errorsummary.log --always-succeed domparsing html/syntax html/dom/documents html/dom/dynamic-markup-insertion
   - ./mach filter-intermittents async-parsing-errorsummary.log --log-intermittents async-parsing-intermittents.log --log-filteredsummary filtered-async-parsing-errorsummary.log --tracker-api default --reporter-api default
 
 mac-dev-unit:
   - ./mach clean-nightlies --keep 3 --force
-  - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig RUSTFLAGS=-Dwarnings ./mach build --dev
-  - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig RUSTFLAGS=-Dwarnings ./mach test-unit
-  - env ./mach package --dev
-  - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig RUSTFLAGS=-Dwarnings ./mach build-cef
-  - env RUSTFLAGS=-Dwarnings ./mach build-geckolib
+  - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig ./mach build --dev
+  - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig ./mach test-unit
+  - ./mach package --dev
+  - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig ./mach build-cef
+  - ./mach build-geckolib
   - bash ./etc/ci/lockfile_changed.sh
   - bash ./etc/ci/manifest_changed.sh
 
 mac-rel-css1:
   - ./mach clean-nightlies --keep 3 --force
   - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig ./mach build --release
   - ./mach test-wpt --release --processes 4 --total-chunks 6 --this-chunk 5 --log-raw test-wpt.log --log-errorsummary wpt-errorsummary.log --always-succeed
   - ./mach filter-intermittents wpt-errorsummary.log --log-intermittents intermittents.log --log-filteredsummary filtered-wpt-errorsummary.log --tracker-api default --reporter-api default
@@ -75,23 +78,23 @@ mac-rel-intermittent:
   - ./mach clean-nightlies --keep 3 --force
   - env PKG_CONFIG_PATH=/usr/local/opt/zlib/lib/pkgconfig ./mach build --release
   - ./etc/ci/check_intermittents.sh --log-raw intermittents.log
 
 linux-dev:
   - ./mach clean-nightlies --keep 3 --force
   - ./mach test-tidy --no-progress --all
   - ./mach test-tidy --no-progress --self-test
-  - env CC=gcc-5 CXX=g++-5 RUSTFLAGS=-Dwarnings ./mach build --dev
-  - env CC=gcc-5 CXX=g++-5 RUSTFLAGS=-Dwarnings ./mach test-unit
-  - env ./mach package --dev
-  - env CC=gcc-5 CXX=g++-5 RUSTFLAGS=-Dwarnings ./mach build-cef
-  - env CC=gcc-5 CXX=g++-5 RUSTFLAGS=-Dwarnings ./mach build --dev --no-default-features --features default-except-unstable
-  - env RUSTFLAGS=-Dwarnings ./mach build-geckolib
-  - env RUSTFLAGS=-Dwarnings ./mach test-stylo
+  - env CC=gcc-5 CXX=g++-5 ./mach build --dev
+  - env CC=gcc-5 CXX=g++-5 ./mach test-unit
+  - ./mach package --dev
+  - env CC=gcc-5 CXX=g++-5 ./mach build-cef
+  - env CC=gcc-5 CXX=g++-5 ./mach build --dev --no-default-features --features default-except-unstable
+  - ./mach build-geckolib
+  - ./mach test-stylo
   - bash ./etc/ci/lockfile_changed.sh
   - bash ./etc/ci/manifest_changed.sh
   - bash ./etc/ci/check_no_panic.sh
 
 linux-rel-wpt:
   - ./mach clean-nightlies --keep 3 --force
   - env CC=gcc-5 CXX=g++-5 ./mach build --release --with-debug-assertions
   - ./mach test-wpt-failure
@@ -115,43 +118,42 @@ linux-nightly:
   - ./mach package --release
   - ./mach upload-nightly linux
   - ./mach test-perf
   - python3 ./etc/ci/performance/download_buildbot_timings.py --verbose
   - aws s3 sync --size-only --acl public-read ./etc/ci/performance/output s3://servo-perf
 
 android:
   - ./mach clean-nightlies --keep 3 --force
-  - env ANDROID_SDK=/home/servo/android/sdk/r25.2.3 RUSTFLAGS=-Dwarnings ./mach build --android --dev
+  - env ANDROID_SDK=/home/servo/android/sdk/r25.2.3 ./mach build --android --dev
   - env ANDROID_SDK=/home/servo/android/sdk/r25.2.3 ./mach package --android --dev
   - bash ./etc/ci/lockfile_changed.sh
   - bash ./etc/ci/manifest_changed.sh
   - python ./etc/ci/check_dynamic_symbols.py
 
 android-nightly:
   - ./mach clean-nightlies --keep 3 --force
   - env ANDROID_SDK=/home/servo/android/sdk/r25.2.3 ./mach build --android --release
   - env ANDROID_SDK=/home/servo/android/sdk/r25.2.3 ./mach package --android --release
   - ./mach upload-nightly android
 
 arm32:
   - ./mach clean-nightlies --keep 3 --force
-  - env RUSTFLAGS=-Dwarnings ./mach build --rel --target=arm-unknown-linux-gnueabihf
+  - ./mach build --rel --target=arm-unknown-linux-gnueabihf
   - bash ./etc/ci/lockfile_changed.sh
   - bash ./etc/ci/manifest_changed.sh
 
 arm64:
   - ./mach clean-nightlies --keep 3 --force
-  - env RUSTFLAGS=-Dwarnings ./mach build --rel --target=aarch64-unknown-linux-gnu
+  - ./mach build --rel --target=aarch64-unknown-linux-gnu
   - bash ./etc/ci/lockfile_changed.sh
   - bash ./etc/ci/manifest_changed.sh
 
 windows-msvc-dev:
   - mach.bat clean-nightlies --keep 3 --force
-  - set RUSTFLAGS=-D warnings
   - mach.bat build --dev
   - mach.bat test-unit
   - mach.bat package --dev
   - mach.bat build-geckolib
   - mach.bat test-stylo
 
 windows-msvc-nightly:
   - mach.bat clean-nightlies --keep 3 --force
--- a/servo/python/requirements.txt
+++ b/servo/python/requirements.txt
@@ -10,16 +10,17 @@ setuptools == 18.5
 toml == 0.9.2
 
 # For Python linting
 flake8 == 2.4.1
 pep8 == 1.5.7
 pyflakes == 0.8.1
 
 # For buildbot checking
+voluptuous == 0.10.5
 PyYAML == 3.12
 
 # For test-webidl
 ply == 3.8
 
 # For Cross-platform colored terminal text
 colorama == 0.3.7
 
--- a/servo/python/tidy/servo_tidy/tidy.py
+++ b/servo/python/tidy/servo_tidy/tidy.py
@@ -12,18 +12,20 @@ import fnmatch
 import imp
 import itertools
 import json
 import os
 import re
 import StringIO
 import subprocess
 import sys
+
 import colorama
 import toml
+import voluptuous
 import yaml
 
 from licenseck import MPL, APACHE, COPYRIGHT, licenses_toml, licenses_dep_toml
 
 CONFIG_FILE_PATH = os.path.join(".", "servo-tidy.toml")
 
 # Default configs
 config = {
@@ -770,25 +772,34 @@ def duplicate_key_yaml_constructor(loade
         if key in mapping:
             raise KeyError(key)
         value = loader.construct_object(value_node, deep=deep)
         mapping[key] = value
     return loader.construct_mapping(node, deep)
 
 
 def lint_buildbot_steps_yaml(mapping):
-    # Check for well-formedness of contents
-    # A well-formed buildbot_steps.yml should be a map to list of strings
-    for k in mapping.keys():
-        if not isinstance(mapping[k], list):
-            raise ValueError("Key '{}' maps to type '{}', but list expected".format(k, type(mapping[k]).__name__))
+    from voluptuous import Any, Extra, Required, Schema
 
-        # check if value is a list of strings
-        for item in itertools.ifilter(lambda i: not isinstance(i, str), mapping[k]):
-            raise ValueError("List mapped to '{}' contains non-string element".format(k))
+    # Note: dictionary keys are optional by default in voluptuous
+    env = Schema({Extra: str})
+    commands = Schema([str])
+    schema = Schema({
+        'env': env,
+        Extra: Any(
+            commands,
+            {
+                'env': env,
+                Required('commands'): commands,
+            },
+        ),
+    })
+
+    # Signals errors via exception throwing
+    schema(mapping)
 
 
 class SafeYamlLoader(yaml.SafeLoader):
     """Subclass of yaml.SafeLoader to avoid mutating the global SafeLoader."""
     pass
 
 
 def check_yaml(file_name, contents):
@@ -806,18 +817,18 @@ def check_yaml(file_name, contents):
     try:
         contents = yaml.load(contents, Loader=SafeYamlLoader)
         lint_buildbot_steps_yaml(contents)
     except yaml.YAMLError as e:
         line = e.problem_mark.line + 1 if hasattr(e, 'problem_mark') else None
         yield (line, e)
     except KeyError as e:
         yield (None, "Duplicated Key ({})".format(e.message))
-    except ValueError as e:
-        yield (None, e.message)
+    except voluptuous.MultipleInvalid as e:
+        yield (None, str(e))
 
 
 def check_for_possible_duplicate_json_keys(key_value_pairs):
     keys = [x[0] for x in key_value_pairs]
     seen_keys = set()
     for key in keys:
         if key in seen_keys:
             raise KeyError("Duplicated Key (%s)" % key)
--- a/servo/python/tidy/servo_tidy_tests/test_tidy.py
+++ b/servo/python/tidy/servo_tidy_tests/test_tidy.py
@@ -205,22 +205,22 @@ class CheckTidiness(unittest.TestCase):
 
     def test_yaml_with_duplicate_key(self):
         errors = tidy.collect_errors_for_files(iterFile('duplicate_keys_buildbot_steps.yml'), [tidy.check_yaml], [], print_text=False)
         self.assertEqual('Duplicated Key (duplicate_yaml_key)', errors.next()[2])
         self.assertNoMoreErrors(errors)
 
     def test_non_list_mapped_buildbot_steps(self):
         errors = tidy.collect_errors_for_files(iterFile('non_list_mapping_buildbot_steps.yml'), [tidy.check_yaml], [], print_text=False)
-        self.assertEqual("Key 'non-list-key' maps to type 'str', but list expected", errors.next()[2])
+        self.assertEqual("expected a list for dictionary value @ data['non-list-key']", errors.next()[2])
         self.assertNoMoreErrors(errors)
 
     def test_non_string_list_mapping_buildbot_steps(self):
         errors = tidy.collect_errors_for_files(iterFile('non_string_list_buildbot_steps.yml'), [tidy.check_yaml], [], print_text=False)
-        self.assertEqual("List mapped to 'mapping_key' contains non-string element", errors.next()[2])
+        self.assertEqual("expected str @ data['mapping_key'][0]", errors.next()[2])
         self.assertNoMoreErrors(errors)
 
     def test_lock(self):
         errors = tidy.collect_errors_for_files(iterFile('duplicated_package.lock'), [tidy.check_lock], [], print_text=False)
         msg = """duplicate versions for package `test`
 \t\x1b[93mThe following packages depend on version 0.4.9 from 'crates.io':\x1b[0m
 \t\ttest2
 \t\x1b[93mThe following packages depend on version 0.5.1 from 'crates.io':\x1b[0m"""