-
Notifications
You must be signed in to change notification settings - Fork 48
Fix --quiet (all.sh, check_names.py) #261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3b9bf5e
aa65dc9
960133e
fc16dd3
93ef10f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -281,6 +281,8 @@ setup_quiet_wrappers() | |
| # Note that the cmake wrapper breaks unless we use an absolute path here. | ||
| if [[ -e ${PWD}/framework/scripts/quiet ]]; then | ||
| export PATH=${PWD}/framework/scripts/quiet:$PATH | ||
| # pass on our own QUIET setting to the wrappers | ||
| export QUIET | ||
| fi | ||
| } | ||
|
|
||
|
|
@@ -661,10 +663,18 @@ pre_restore_files () { | |
| # the ones checked into git, take care not to modify them. Whatever | ||
| # this function leaves behind is what the script will restore before | ||
| # each component. | ||
| case "$(head -n1 Makefile)" in | ||
| if ! in_mbedtls_repo; then | ||
| return | ||
| fi | ||
| local makefiles="library/Makefile programs/Makefile programs/fuzz/Makefile tests/Makefile" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Partly preexisting: how about |
||
| if in_3_6_branch; then | ||
| makefiles="Makefile $makefiles" | ||
| fi | ||
| # No root Makefile in development, use the one in library | ||
| case "$(head -n1 library/Makefile)" in | ||
| *[Gg]enerated*) | ||
| git update-index --no-skip-worktree Makefile library/Makefile programs/Makefile tests/Makefile programs/fuzz/Makefile | ||
| git checkout -- Makefile library/Makefile programs/Makefile tests/Makefile programs/fuzz/Makefile | ||
| git update-index --no-skip-worktree $makefiles | ||
| git checkout -- $makefiles | ||
| ;; | ||
| esac | ||
| } | ||
|
|
@@ -887,11 +897,7 @@ pre_generate_files() { | |
| # since make doesn't have proper dependencies, remove any possibly outdate | ||
| # file that might be around before generating fresh ones | ||
| $MAKE_COMMAND neat | ||
| if [ $QUIET -eq 1 ]; then | ||
| $MAKE_COMMAND generated_files >/dev/null | ||
| else | ||
| $MAKE_COMMAND generated_files | ||
| fi | ||
| $MAKE_COMMAND generated_files | ||
| } | ||
|
|
||
| pre_load_helpers () { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,16 @@ | |
| from mbedtls_framework import build_tree | ||
|
|
||
|
|
||
| class MaxLevelFilter(logging.Filter): | ||
| """Allow records with level <= max_level.""" | ||
| def __init__(self, max_level): | ||
| super().__init__() | ||
| self.max_level = max_level | ||
|
|
||
| def filter(self, record): | ||
| return record.levelno <= self.max_level | ||
|
|
||
|
|
||
| # Naming patterns to check against. These are defined outside the NameCheck | ||
| # class for ease of modification. | ||
| PUBLIC_MACRO_PATTERN = r"^(MBEDTLS|PSA|TF_PSA_CRYPTO)_[0-9A-Z_]*[0-9A-Z]$" | ||
|
|
@@ -791,11 +801,13 @@ def parse_symbols(self): | |
| source_dir = os.getcwd() | ||
| build_dir = tempfile.mkdtemp() | ||
| os.chdir(build_dir) | ||
| subprocess.run( | ||
| result = subprocess.run( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Partly preexisting: it would be more robust to use |
||
| ["cmake", "-DGEN_FILES=ON", source_dir], | ||
| universal_newlines=True, | ||
| stdout=subprocess.PIPE, | ||
| check=True | ||
| ) | ||
| self.log.debug(result.stdout) | ||
| subprocess.run( | ||
| ["cmake", "--build", "."], | ||
| env=my_environment, | ||
|
|
@@ -813,7 +825,7 @@ def parse_symbols(self): | |
| os.chdir(source_dir) | ||
| shutil.rmtree(build_dir) | ||
| except subprocess.CalledProcessError as error: | ||
| self.log.debug(error.output) | ||
| self.log.error(error.output) | ||
| raise error | ||
| finally: | ||
| # Put back the original config regardless of there being errors. | ||
|
|
@@ -962,11 +974,13 @@ def parse_symbols(self): | |
| source_dir = os.getcwd() | ||
| build_dir = tempfile.mkdtemp() | ||
| os.chdir(build_dir) | ||
| subprocess.run( | ||
| result = subprocess.run( | ||
| ["cmake", "-DGEN_FILES=ON", source_dir], | ||
| universal_newlines=True, | ||
| stdout=subprocess.PIPE, | ||
| check=True | ||
| ) | ||
| self.log.debug(result.stdout) | ||
| subprocess.run( | ||
| ["cmake", "--build", "."], | ||
| env=my_environment, | ||
|
|
@@ -1178,15 +1192,28 @@ def main(): | |
| parser.add_argument( | ||
| "-q", "--quiet", | ||
| action="store_true", | ||
| help="hide unnecessary text, explanations, and highlights" | ||
| help="only print warnings and errors" | ||
| ) | ||
|
|
||
| args = parser.parse_args() | ||
|
|
||
| # Configure the global logger, which is then passed to the classes below | ||
| log = logging.getLogger() | ||
| log.setLevel(logging.DEBUG if args.verbose else logging.INFO) | ||
| log.addHandler(logging.StreamHandler()) | ||
| log.setLevel(logging.DEBUG if args.verbose else | ||
| logging.INFO if not args.quiet else | ||
| logging.WARN) | ||
|
|
||
| # stdout handler: DEBUG/INFO only | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not send everything to stderr? That's the normal place for informational messages, as opposed to application data which goes to stdout. Separating two logging streams also creates the risk that they'll end up displayed out of order due to buffering (which I think doesn't happen on Jenkins, it did happen on Travis). |
||
| h_out = logging.StreamHandler(sys.stdout) | ||
| h_out.setLevel(logging.DEBUG) | ||
| h_out.addFilter(MaxLevelFilter(logging.INFO)) | ||
|
|
||
| # stderr handler: WARNING and above | ||
| h_err = logging.StreamHandler(sys.stderr) | ||
| h_err.setLevel(logging.WARNING) | ||
|
|
||
| log.addHandler(h_out) | ||
| log.addHandler(h_err) | ||
|
|
||
| try: | ||
| if build_tree.looks_like_tf_psa_crypto_root(os.getcwd()): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to avoid environment variable names that are too generic. Can we use
MBEDTLS_QUIEThere? (Just as the environment variable, not as the name inside the script.) Or would that be too confusing if we have two names and too much work if we rename it everywhere?