Revert "[ssci] Added CheckChromiumMetadataFiles in presubmit_canned_checks"

This reverts commit a1cfc693af.

Reason for revert: causing presubmit errors downstream

Original change's description:
> [ssci] Added CheckChromiumMetadataFiles in presubmit_canned_checks
>
> Bug: b:277147404
> Change-Id: I14a2f11b256bc85fdfe225443ef533c38463ca3e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4796694
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Reviewed-by: Rachael Newitt <renewitt@google.com>
> Commit-Queue: Anne Redulla <aredulla@google.com>

Bug: b:277147404
Change-Id: I83f52494bc1a3a786505b8b74b2053269baa6e8e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4803286
Commit-Queue: Anne Redulla <aredulla@google.com>
Auto-Submit: Anne Redulla <aredulla@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Anne Redulla <aredulla@google.com>
changes/86/4803286/2
Anne Redulla 2 years ago committed by LUCI CQ
parent a1cfc693af
commit 760f8bcfb9

@ -1,4 +0,0 @@
# Validation for Chromium's Third Party Metadata Files
This directory contains the code to validate Chromium's third party metadata
files, i.e. `README.chromium` files.

@ -119,7 +119,7 @@ class DependencyMetadata:
if repeated_field_info: if repeated_field_info:
repeated = ", ".join(repeated_field_info) repeated = ", ".join(repeated_field_info)
error = vr.ValidationError( error = vr.ValidationError(
f"Multiple entries for the same field: {repeated}.") f"Multiple entries for the same field: {repeated}")
error.set_tag(tag="reason", value="repeated field") error.set_tag(tag="reason", value="repeated field")
results.append(error) results.append(error)
@ -128,7 +128,7 @@ class DependencyMetadata:
for field in required_fields: for field in required_fields:
if field not in self._metadata: if field not in self._metadata:
field_name = field.get_name() field_name = field.get_name()
error = vr.ValidationError(f"Required field '{field_name}' is missing.") error = vr.ValidationError(f"Required field '{field_name}' is missing")
error.set_tag(tag="reason", value="missing required field") error.set_tag(tag="reason", value="missing required field")
results.append(error) results.append(error)

@ -1,4 +1,4 @@
Name: Test-A README for Chromium metadata (0 errors, 0 warnings) Name: Test-A README for Chromium metadata
Short Name: metadata-test-valid Short Name: metadata-test-valid
URL: https://www.example.com/metadata, URL: https://www.example.com/metadata,
https://www.example.com/parser https://www.example.com/parser
@ -21,7 +21,7 @@ EXCEPT:
-------------------- DEPENDENCY DIVIDER -------------------- -------------------- DEPENDENCY DIVIDER --------------------
Name: Test-B README for Chromium metadata (4 errors, 1 warning) Name: Test-B README for Chromium metadata
SHORT NAME: metadata-test-invalid SHORT NAME: metadata-test-invalid
URL: file://home/drive/chromium/src/metadata URL: file://home/drive/chromium/src/metadata
Version:0 Version:0
@ -37,7 +37,7 @@ Local Modifications: None.
-------------------- DEPENDENCY DIVIDER -------------------- -------------------- DEPENDENCY DIVIDER --------------------
-------------------- DEPENDENCY DIVIDER -------------------- -------------------- DEPENDENCY DIVIDER --------------------
Name: Test-C README for Chromium metadata (5 errors, 1 warning) Name: Test-C README for Chromium metadata
URL: https://www.example.com/first URL: https://www.example.com/first
URL: https://www.example.com/second URL: https://www.example.com/second
Version: N/A Version: N/A

@ -1,32 +0,0 @@
Name: Test-A README for Chromium metadata
Short Name: metadata-test-valid
URL: https://www.example.com/metadata,
https://www.example.com/parser
Version: 1.0.12
Date: 2020-12-03
License: Apache, 2.0 and MIT
License File: LICENSE
Security Critical: yes
Shipped: yes
CPEPrefix: unknown
This line should be ignored because CPEPrefix is a one-liner field.
Description:
A test metadata file, with a
multi-line description.
Local Modifications:
None,
EXCEPT:
* nothing.
-------------------- DEPENDENCY DIVIDER --------------------
Name: Test-B README for Chromium metadata
Short Name: metadata-test-valid-again
URL: https://www.example.com/metadata
Version: 1.0.12
Date: 2020-12-03
License: Apache, 2.0 and MIT
License File: LICENSE
Security Critical: yes
Shipped: yes

@ -1,20 +0,0 @@
Name: Test-A README for Chromium metadata
Short Name: metadata-test-valid
URL: https://www.example.com/metadata,
https://www.example.com/parser
Version: 1.0.12
Date: 2020-12-03
License: Apache, 2.0 and MIT
License File: LICENSE
Security Critical: yes
Shipped: yes
CPEPrefix: unknown
This line should be ignored because CPEPrefix is a one-liner field.
Description:
A test metadata file, with a
multi-line description.
Local Modifications:
None,
EXCEPT:
* nothing.

@ -18,37 +18,8 @@ import metadata.parse
class ParseTest(unittest.TestCase): class ParseTest(unittest.TestCase):
def test_parse_single(self): def test_parse(self):
"""Check parsing works for a single dependency's metadata.""" filepath = os.path.join(_THIS_DIR, "data", "README.chromium.test")
filepath = os.path.join(_THIS_DIR, "data",
"README.chromium.test.single-valid")
all_metadata = metadata.parse.parse_file(filepath)
self.assertEqual(len(all_metadata), 1)
self.assertListEqual(
all_metadata[0].get_entries(),
[
("Name", "Test-A README for Chromium metadata"),
("Short Name", "metadata-test-valid"),
("URL", "https://www.example.com/metadata,\n"
" https://www.example.com/parser"),
("Version", "1.0.12"),
("Date", "2020-12-03"),
("License", "Apache, 2.0 and MIT"),
("License File", "LICENSE"),
("Security Critical", "yes"),
("Shipped", "yes"),
("CPEPrefix", "unknown"),
("Description", "A test metadata file, with a\n"
" multi-line description."),
("Local Modifications", "None,\nEXCEPT:\n* nothing."),
],
)
def test_parse_multiple(self):
"""Check parsing works for multiple dependencies' metadata."""
filepath = os.path.join(_THIS_DIR, "data",
"README.chromium.test.multi-invalid")
all_metadata = metadata.parse.parse_file(filepath) all_metadata = metadata.parse.parse_file(filepath)
# Dependency metadata with no entries at all are ignored. # Dependency metadata with no entries at all are ignored.
@ -58,8 +29,7 @@ class ParseTest(unittest.TestCase):
self.assertListEqual( self.assertListEqual(
all_metadata[0].get_entries(), all_metadata[0].get_entries(),
[ [
("Name", ("Name", "Test-A README for Chromium metadata"),
"Test-A README for Chromium metadata (0 errors, 0 warnings)"),
("Short Name", "metadata-test-valid"), ("Short Name", "metadata-test-valid"),
("URL", "https://www.example.com/metadata,\n" ("URL", "https://www.example.com/metadata,\n"
" https://www.example.com/parser"), " https://www.example.com/parser"),
@ -81,8 +51,7 @@ class ParseTest(unittest.TestCase):
self.assertListEqual( self.assertListEqual(
all_metadata[1].get_entries(), all_metadata[1].get_entries(),
[ [
("Name", ("Name", "Test-B README for Chromium metadata"),
"Test-B README for Chromium metadata (4 errors, 1 warning)"),
("SHORT NAME", "metadata-test-invalid"), ("SHORT NAME", "metadata-test-invalid"),
("URL", "file://home/drive/chromium/src/metadata"), ("URL", "file://home/drive/chromium/src/metadata"),
("Version", "0"), ("Version", "0"),
@ -99,8 +68,7 @@ class ParseTest(unittest.TestCase):
self.assertListEqual( self.assertListEqual(
all_metadata[2].get_entries(), all_metadata[2].get_entries(),
[ [
("Name", ("Name", "Test-C README for Chromium metadata"),
"Test-C README for Chromium metadata (5 errors, 1 warning)"),
("URL", "https://www.example.com/first"), ("URL", "https://www.example.com/first"),
("URL", "https://www.example.com/second"), ("URL", "https://www.example.com/second"),
("Version", "N/A"), ("Version", "N/A"),

@ -1,76 +0,0 @@
#!/usr/bin/env python3
# Copyright 2023 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import os
import sys
import unittest
_THIS_DIR = os.path.abspath(os.path.dirname(__file__))
# The repo's root directory.
_ROOT_DIR = os.path.abspath(os.path.join(_THIS_DIR, "..", ".."))
# Add the repo's root directory for clearer imports.
sys.path.insert(0, _ROOT_DIR)
import metadata.validate
class ValidateTest(unittest.TestCase):
def test_validate_file(self):
# Validate a valid file (no errors or warnings).
test_filepath = os.path.join(_THIS_DIR, "data",
"README.chromium.test.multi-valid")
results = metadata.validate.validate_file(
filepath=test_filepath,
repo_root_dir=_THIS_DIR,
)
self.assertEqual(len(results), 0)
# Validate an invalid file (both errors and warnings).
test_filepath = os.path.join(_THIS_DIR, "data",
"README.chromium.test.multi-invalid")
results = metadata.validate.validate_file(
filepath=test_filepath,
repo_root_dir=_THIS_DIR,
)
self.assertEqual(len(results), 11)
error_count = 0
warning_count = 0
for result in results:
if result.is_fatal():
error_count += 1
else:
warning_count += 1
self.assertEqual(error_count, 9)
self.assertEqual(warning_count, 2)
def test_check_file(self):
# Check a valid file (no errors or warnings).
test_filepath = os.path.join(_THIS_DIR, "data",
"README.chromium.test.multi-valid")
errors, warnings = metadata.validate.check_file(
filepath=test_filepath,
repo_root_dir=_THIS_DIR,
)
self.assertEqual(len(errors), 0)
self.assertEqual(len(warnings), 0)
# Check an invalid file (both errors and warnings).
test_filepath = os.path.join(_THIS_DIR, "data",
"README.chromium.test.multi-invalid")
errors, warnings = metadata.validate.check_file(
filepath=test_filepath,
repo_root_dir=_THIS_DIR,
)
# TODO(aredulla): update this test once validation errors can be returned
# as errors.
# self.assertEqual(len(errors), 9)
# self.assertEqual(len(warnings), 2)
self.assertEqual(len(errors), 0)
self.assertEqual(len(warnings), 11)
if __name__ == "__main__":
unittest.main()

@ -1,80 +0,0 @@
#!/usr/bin/env python3
# Copyright 2023 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import os
import sys
from typing import List, Tuple
_THIS_DIR = os.path.abspath(os.path.dirname(__file__))
# The repo's root directory.
_ROOT_DIR = os.path.abspath(os.path.join(_THIS_DIR, ".."))
# Add the repo's root directory for clearer imports.
sys.path.insert(0, _ROOT_DIR)
import metadata.parse
import metadata.validation_result as vr
def validate_file(filepath: str,
repo_root_dir: str) -> List[vr.ValidationResult]:
"""Validate the metadata file."""
if not os.path.exists(filepath):
result = vr.ValidationError(f"'{filepath}' not found.")
result.set_tag(tag="reason", value="file not found")
return [result]
# Get the directory the metadata file is in.
parent_dir = os.path.dirname(filepath)
results = []
dependencies = metadata.parse.parse_file(filepath)
for dependency in dependencies:
results.extend(
dependency.validate(
source_file_dir=parent_dir,
repo_root_dir=repo_root_dir,
))
return results
def check_file(filepath: str,
repo_root_dir: str) -> Tuple[List[str], List[str]]:
"""Run metadata validation on the given file, and return all validation
errors and validation warnings.
Args:
filepath: the path to a metadata file,
e.g. "/chromium/src/third_party/libname/README.chromium"
repo_root_dir: the repository's root directory; this is needed to construct
file paths to license files.
Returns:
error_messages: the fatal validation issues present in the file;
i.e. presubmit should fail.
warning_messages: the non-fatal validation issues present in the file;
i.e. presubmit should still pass.
"""
error_messages = []
warning_messages = []
for result in validate_file(filepath, repo_root_dir):
# Construct the message.
if result.get_tag("reason") == "file not found":
message = result.get_message(postscript="", width=60)
else:
message = result.get_message(width=60)
# TODO(aredulla): Actually distinguish between validation errors and
# warnings. The quality of metadata is currently being uplifted, but is not
# yet guaranteed to pass validation. So for now, all validation results will
# be returned as warnings so CLs are not blocked by invalid metadata in
# presubmits yet.
# if result.is_fatal():
# error_messages.append(message)
# else:
warning_messages.append(message)
return error_messages, warning_messages

@ -7,11 +7,6 @@ import textwrap
from typing import Dict, Union from typing import Dict, Union
_CHROMIUM_METADATA_PRESCRIPT = "Third party metadata issue:"
_CHROMIUM_METADATA_POSTSCRIPT = ("Check //third_party/README.chromium.template "
"for details.")
class ValidationResult: class ValidationResult:
"""Base class for validation issues.""" """Base class for validation issues."""
def __init__(self, message: str, fatal: bool): def __init__(self, message: str, fatal: bool):
@ -38,27 +33,20 @@ class ValidationResult:
def get_all_tags(self) -> Dict[str, str]: def get_all_tags(self) -> Dict[str, str]:
return dict(self._tags) return dict(self._tags)
def get_message(self, def get_message(self, width: int = 0) -> str:
prescript: str = _CHROMIUM_METADATA_PRESCRIPT,
postscript: str = _CHROMIUM_METADATA_POSTSCRIPT,
width: int = 0) -> str:
components = [prescript, self._message, postscript]
message = " ".join(
[component for component in components if len(component) > 0])
if width > 0: if width > 0:
return textwrap.fill(text=message, width=width) return textwrap.fill(text=self._message, width=width)
return message return self._message
class ValidationError(ValidationResult): class ValidationError(ValidationResult):
"""Fatal validation issue. Presubmit should fail.""" """Fatal validation issue. Presubmit should fail."""
def __init__(self, message: str): def __init__(self, message: str, **tags: Dict[str, str]):
super().__init__(message=message, fatal=True) super().__init__(message=message, fatal=True, **tags)
class ValidationWarning(ValidationResult): class ValidationWarning(ValidationResult):
"""Non-fatal validation issue. Presubmit should pass.""" """Non-fatal validation issue. Presubmit should pass."""
def __init__(self, message: str): def __init__(self, message: str, **tags: Dict[str, str]):
super().__init__(message=message, fatal=False) super().__init__(message=message, fatal=False, **tags)

@ -10,8 +10,6 @@ import io as _io
import os as _os import os as _os
import zlib import zlib
import metadata.validate
_HERE = _os.path.dirname(_os.path.abspath(__file__)) _HERE = _os.path.dirname(_os.path.abspath(__file__))
# These filters will be disabled if callers do not explicitly supply a # These filters will be disabled if callers do not explicitly supply a
@ -377,33 +375,6 @@ def CheckChangeHasNoCrAndHasOnlyOneEol(input_api, output_api,
items=eof_files)) items=eof_files))
return outputs return outputs
def CheckChromiumMetadataFiles(input_api, output_api, file_filter=None):
"""Check affected Chromium metadata files are valid."""
# Restrict this check to README.chromium files if the file filter is not
# specified.
if file_filter is None:
file_filter = lambda f: f.LocalPath().endswith('README.chromium')
# The repo's root directory is required to check license files.
repo_root_dir = input_api.change.RepositoryRoot()
outputs = []
for f in input_api.AffectedFiles(file_filter=file_filter):
errors, warnings = metadata.validate.check_file(
filepath=f.AbsoluteLocalPath(),
repo_root_dir=repo_root_dir,
)
for warning in warnings:
outputs.append(output_api.PresubmitPromptWarning(warning, [f]))
for error in errors:
outputs.append(output_api.PresubmitError(error, [f]))
return outputs
def CheckGenderNeutral(input_api, output_api, source_file_filter=None): def CheckGenderNeutral(input_api, output_api, source_file_filter=None):
"""Checks that there are no gendered pronouns in any of the text files to be """Checks that there are no gendered pronouns in any of the text files to be
submitted. submitted.
@ -427,6 +398,7 @@ def CheckGenderNeutral(input_api, output_api, source_file_filter=None):
return [] return []
def _ReportErrorFileAndLine(filename, line_num, dummy_line): def _ReportErrorFileAndLine(filename, line_num, dummy_line):
"""Default error formatter for _FindNewViolationsOfRule.""" """Default error formatter for _FindNewViolationsOfRule."""
return '%s:%s' % (filename, line_num) return '%s:%s' % (filename, line_num)

Loading…
Cancel
Save