From 8044f8fd4e02ea21020cf3a2770d965434a38f9b Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Tue, 9 Jan 2018 11:40:27 -0600 Subject: [PATCH] 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. - [ ] `./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). - [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 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : ebdbefbc7cb6f397a0630f417c9c03e003cfc9ab --- servo/.travis.yml | 12 ++++--- servo/appveyor.yml | 2 +- servo/etc/ci/buildbot_steps.yml | 34 ++++++++++--------- servo/python/requirements.txt | 1 + servo/python/tidy/servo_tidy/tidy.py | 31 +++++++++++------ .../python/tidy/servo_tidy_tests/test_tidy.py | 4 +-- 6 files changed, 50 insertions(+), 34 deletions(-) diff --git a/servo/.travis.yml b/servo/.travis.yml index d67a514c5edb..1354e724483e 100644 --- a/servo/.travis.yml +++ b/servo/.travis.yml @@ -20,11 +20,11 @@ matrix: - 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: @@ -34,7 +34,9 @@ matrix: 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: diff --git a/servo/appveyor.yml b/servo/appveyor.yml index dbb4772330e3..841213809578 100644 --- a/servo/appveyor.yml +++ b/servo/appveyor.yml @@ -3,6 +3,7 @@ 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;\ @@ -48,7 +49,6 @@ cache: # - 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 diff --git a/servo/etc/ci/buildbot_steps.yml b/servo/etc/ci/buildbot_steps.yml index 97ef5d4d78c8..62ba5177841d 100644 --- a/servo/etc/ci/buildbot_steps.yml +++ b/servo/etc/ci/buildbot_steps.yml @@ -1,3 +1,6 @@ +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 @@ -32,11 +35,11 @@ mac-rel-wpt4: 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 @@ -80,13 +83,13 @@ 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 @@ -120,7 +123,7 @@ linux-nightly: 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 @@ -134,19 +137,18 @@ android-nightly: 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 diff --git a/servo/python/requirements.txt b/servo/python/requirements.txt index 11d7910b94d5..85741d93982b 100644 --- a/servo/python/requirements.txt +++ b/servo/python/requirements.txt @@ -15,6 +15,7 @@ pep8 == 1.5.7 pyflakes == 0.8.1 # For buildbot checking +voluptuous == 0.10.5 PyYAML == 3.12 # For test-webidl diff --git a/servo/python/tidy/servo_tidy/tidy.py b/servo/python/tidy/servo_tidy/tidy.py index 2f638abe0993..e12ae4cdeb3e 100644 --- a/servo/python/tidy/servo_tidy/tidy.py +++ b/servo/python/tidy/servo_tidy/tidy.py @@ -17,8 +17,10 @@ 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 @@ -775,15 +777,24 @@ def duplicate_key_yaml_constructor(loader, node, deep=False): 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): @@ -811,8 +822,8 @@ def check_yaml(file_name, contents): 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): diff --git a/servo/python/tidy/servo_tidy_tests/test_tidy.py b/servo/python/tidy/servo_tidy_tests/test_tidy.py index dd4c06db4c0b..432d5c1117e8 100644 --- a/servo/python/tidy/servo_tidy_tests/test_tidy.py +++ b/servo/python/tidy/servo_tidy_tests/test_tidy.py @@ -210,12 +210,12 @@ class CheckTidiness(unittest.TestCase): 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):