From d0abd1cdfa54b4f8c31fa79e583e2c9f628e98e8 Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Mon, 5 Jun 2017 15:59:09 -0400 Subject: [PATCH 1/2] Update buildbot_steps lint to handle env variables 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. Update the tidy self tests to take into account the new error messages. --- python/requirements.txt | 1 + python/tidy/servo_tidy/tidy.py | 31 +++++++++++++++-------- python/tidy/servo_tidy_tests/test_tidy.py | 4 +-- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/python/requirements.txt b/python/requirements.txt index 11d7910b94d..85741d93982 100644 --- a/python/requirements.txt +++ b/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/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index 2f638abe099..e12ae4cdeb3 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/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/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py index dd4c06db4c0..432d5c1117e 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/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): From e9eaadc913c62ee62d90bcacdf61436034543a54 Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Sun, 7 Jan 2018 21:23:53 -0800 Subject: [PATCH 2/2] Deny warnings in rust code globally in CI Use the new environment variable support for `buildbot_steps.yml` to do this globally instead of having to patch every call, which is error prone. Set this variable globally in .travis.yml and appveyor.yml as well. --- .travis.yml | 12 +++++++----- appveyor.yml | 2 +- etc/ci/buildbot_steps.yml | 34 ++++++++++++++++++---------------- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/.travis.yml b/.travis.yml index d67a514c5ed..1354e724483 100644 --- a/.travis.yml +++ b/.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/appveyor.yml b/appveyor.yml index dbb4772330e..84121380957 100644 --- a/appveyor.yml +++ b/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/etc/ci/buildbot_steps.yml b/etc/ci/buildbot_steps.yml index 97ef5d4d78c..62ba5177841 100644 --- a/etc/ci/buildbot_steps.yml +++ b/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