From e61a4aa8dcfaff48a2b2caf4b277103330bc26f9 Mon Sep 17 00:00:00 2001 From: Pavol Juhas Date: Tue, 21 May 2024 15:45:58 -0700 Subject: [PATCH 1/4] Fix spurious failure of check/all Problem: check/format-incremental fails when invoked with an empty string, because it is not a valid git revision. Solution: Use shell array to store the revision in check/all, that way it is not passed as a sub-command argument if unset. --- check/all | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/check/all b/check/all index 81f0689a736..66e3bfad0ee 100755 --- a/check/all +++ b/check/all @@ -38,20 +38,20 @@ cd "${topdir}" || exit $? errors=() # Parse arguments. -apply_arg="" +apply_arg=( ) only_changed=0 -rev="" +rev=( ) for arg in "$@"; do if [[ "${arg}" == "--only-changed-files" ]]; then only_changed=1 elif [[ "${arg}" == "--apply-format-changes" ]]; then - apply_arg="--apply" - elif [ -z "${rev}" ]; then + apply_arg=( "--apply" ) + elif [[ "${#rev[@]}" == 0 ]]; then if [ "$(git cat-file -t "${arg}" 2> /dev/null)" != "commit" ]; then echo -e "\033[31mNo revision '${arg}'.\033[0m" >&2 exit 1 fi - rev="${arg}" + rev=( "${arg}" ) else echo -e "\033[31mInvalid arguments. Expected [BASE_REVISION] [--only-changed-files] [--apply-format].\033[0m" >&2 exit 1 @@ -73,15 +73,16 @@ echo "Running mypy" check/mypy || errors+=( "check/mypy failed" ) echo "Running incremental format" -check/format-incremental "${rev}" "${apply_arg}" || errors+=( "check/format-incremental failed" ) +check/format-incremental "${rev[@]}" "${apply_arg[@]}" || + errors+=( "check/format-incremental failed" ) if [ ${only_changed} -ne 0 ]; then echo "Running pytest and incremental coverage on changed files" - check/pytest-changed-files-and-incremental-coverage "${rev}" || + check/pytest-changed-files-and-incremental-coverage "${rev[@]}" || errors+=( "check/pytest-changed-files-and-incremental-coverage failed" ) else echo "Running pytest and incremental coverage" - check/pytest-and-incremental-coverage "${rev}" || + check/pytest-and-incremental-coverage "${rev[@]}" || errors+=( "check/pytest-and-incremental-coverage failed" ) fi From f926b3d818ea5ccceb30b1497c68e6e1a64a4e17 Mon Sep 17 00:00:00 2001 From: Pavol Juhas Date: Tue, 21 May 2024 16:24:55 -0700 Subject: [PATCH 2/4] Fix egrep pattern Ensure grep matches `?` as a literal character. --- check/build-changed-protos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check/build-changed-protos b/check/build-changed-protos index c4453373285..43e713a4e7e 100755 --- a/check/build-changed-protos +++ b/check/build-changed-protos @@ -62,7 +62,7 @@ dev_tools/build-protos.sh # Filenames with spaces will be ugly (each part will be listed separately) # but the error logic will still work. -uncommitted=$(git status --porcelain 2>/dev/null | grep -E "^?? cirq-google" | cut -d " " -f 3) +uncommitted=$(git status --porcelain 2>/dev/null | grep -E "^[?][?] cirq-google" | cut -d " " -f 3) if [[ -n "$uncommitted" ]]; then echo -e "\033[31mERROR: Uncommitted generated files found! Please generate and commit these files using dev_tools/build-protos.sh:\033[0m" From e4402ef82aeac29557786a48701c0ff016dd9e0b Mon Sep 17 00:00:00 2001 From: Pavol Juhas Date: Tue, 21 May 2024 16:36:39 -0700 Subject: [PATCH 3/4] Improve validation of git-revision argument in check scripts Allow git tags which point to a commit. --- check/build-changed-protos | 2 +- check/format-incremental | 2 +- check/pylint-changed-files | 2 +- check/pytest-and-incremental-coverage | 2 +- check/pytest-changed-files | 2 +- check/pytest-changed-files-and-incremental-coverage | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/check/build-changed-protos b/check/build-changed-protos index 43e713a4e7e..df58599498b 100755 --- a/check/build-changed-protos +++ b/check/build-changed-protos @@ -32,7 +32,7 @@ cd "${topdir}" || exit $? # Figure out which revision to compare against. if [ -n "$1" ] && [[ $1 != -* ]]; then - if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then + if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then echo -e "\033[31mNo revision '$1'.\033[0m" >&2 exit 1 fi diff --git a/check/format-incremental b/check/format-incremental index 74a253efeea..2f7a8dab078 100755 --- a/check/format-incremental +++ b/check/format-incremental @@ -46,7 +46,7 @@ for arg in "$@"; do elif [[ "${arg}" == "--all" ]]; then only_changed=0 elif [ -z "${rev}" ]; then - if [ "$(git cat-file -t "${arg}" 2> /dev/null)" != "commit" ]; then + if ! git rev-parse --verify --quiet --no-revs "${arg}^{commit}"; then echo -e "\033[31mNo revision '${arg}'.\033[0m" >&2 exit 1 fi diff --git a/check/pylint-changed-files b/check/pylint-changed-files index b335e61d1aa..1e097006992 100755 --- a/check/pylint-changed-files +++ b/check/pylint-changed-files @@ -31,7 +31,7 @@ cd "${topdir}" || exit $? # Figure out which revision to compare against. if [ -n "$1" ] && [[ $1 != -* ]]; then - if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then + if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then echo -e "\033[31mNo revision '$1'.\033[0m" >&2 exit 1 fi diff --git a/check/pytest-and-incremental-coverage b/check/pytest-and-incremental-coverage index e088f4d3864..ba2890ddd88 100755 --- a/check/pytest-and-incremental-coverage +++ b/check/pytest-and-incremental-coverage @@ -45,7 +45,7 @@ done # Figure out which revision to compare against. if [ -n "${BASEREV}" ]; then - if [ "$(git cat-file -t "${BASEREV}" 2> /dev/null)" != "commit" ]; then + if ! git rev-parse --verify --quiet --no-revs "${BASEREV}^{commit}"; then echo -e "\033[31mNo revision '${BASEREV}'.\033[0m" >&2 exit 1 fi diff --git a/check/pytest-changed-files b/check/pytest-changed-files index 8de45dc590d..175dba97321 100755 --- a/check/pytest-changed-files +++ b/check/pytest-changed-files @@ -35,7 +35,7 @@ cd "${topdir}" || exit $? # Figure out which branch to compare against. rest=( "$@" ) if [ -n "$1" ] && [[ $1 != -* ]]; then - if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then + if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then echo -e "\033[31mNo revision '$1'.\033[0m" >&2 exit 1 fi diff --git a/check/pytest-changed-files-and-incremental-coverage b/check/pytest-changed-files-and-incremental-coverage index d5b4a38cc40..a4c120caea4 100755 --- a/check/pytest-changed-files-and-incremental-coverage +++ b/check/pytest-changed-files-and-incremental-coverage @@ -34,7 +34,7 @@ cd "$(git rev-parse --show-toplevel)" || exit 1 # Figure out which revision to compare against. if [ -n "$1" ] && [[ $1 != -* ]]; then - if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then + if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then echo -e "\033[31mNo revision '$1'.\033[0m" >&2 exit 1 fi From 1fc6c95a07027b8dfff90186d7132d372d86188c Mon Sep 17 00:00:00 2001 From: Pavol Juhas Date: Tue, 21 May 2024 16:48:17 -0700 Subject: [PATCH 4/4] Fix check/mypy for dev.env.txt virtual environment Problem: check/mypy passes in the mypy.env.txt environment, but fails on "unnecessary type ignore" in dev.env.txt virtual environment, because it installs IPython with its type information. Solution: Ignore IPython in type checks. --- cirq-core/cirq/contrib/svg/svg_test.py | 2 +- dev_tools/conf/mypy.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cirq-core/cirq/contrib/svg/svg_test.py b/cirq-core/cirq/contrib/svg/svg_test.py index 074651103ee..e00b7f25bdd 100644 --- a/cirq-core/cirq/contrib/svg/svg_test.py +++ b/cirq-core/cirq/contrib/svg/svg_test.py @@ -1,5 +1,5 @@ # pylint: disable=wrong-or-nonexistent-copyright-notice -import IPython.display # type: ignore +import IPython.display import numpy as np import pytest diff --git a/dev_tools/conf/mypy.ini b/dev_tools/conf/mypy.ini index c49032bb0a1..e9fa42a9d15 100644 --- a/dev_tools/conf/mypy.ini +++ b/dev_tools/conf/mypy.ini @@ -17,7 +17,7 @@ follow_imports = silent ignore_missing_imports = true # Non-Google -[mypy-sympy.*,matplotlib.*,proto.*,pandas.*,scipy.*,freezegun.*,mpl_toolkits.*,networkx.*,ply.*,astroid.*,pytest.*,_pytest.*,pylint.*,setuptools.*,qiskit.*,quimb.*,pylatex.*,filelock.*,sortedcontainers.*,tqdm.*,ruamel.*,absl.*,tensorflow_docs.*,ipywidgets.*, cachetools.*] +[mypy-IPython.*,sympy.*,matplotlib.*,proto.*,pandas.*,scipy.*,freezegun.*,mpl_toolkits.*,networkx.*,ply.*,astroid.*,pytest.*,_pytest.*,pylint.*,setuptools.*,qiskit.*,quimb.*,pylatex.*,filelock.*,sortedcontainers.*,tqdm.*,ruamel.*,absl.*,tensorflow_docs.*,ipywidgets.*,cachetools.*] follow_imports = silent ignore_missing_imports = true