Skip to content

Commit

Permalink
[ios] Generalize update_ios_bundle_data.py and add PRESUBMIT checks
Browse files Browse the repository at this point in the history
With this patch, machinery has been added to populate file lists
based on a list of globs, generalizing the approach taken from
update_ios_bundle_data.py. It is also possible to check that these
file lists are kept up to date via PRESUBMIT checks. A support
utility has also been added to simplify the creation of new presubmit
checks. Details on the conventions used in the glob lists are
outlined in update_bundle_filelist.py.

A number of large bundle_data's have been updated in this patch
but there should be no behavioral change.

Regarding rationale: it can be cumbersome to maintain large sources
lists in bundle_data, and while there was a script in //net to
simplify updating some net-specific lists, it wasn't available
elsewhere, nor was there a PRESUBMIT check to remind you that an
update is needed (or how to do it). This patch makes this approach generally available and provides guidance via PRESUBMIT.

credit: thanks to sdefresne@ for suggesting the read_file, PRESUBMIT,
and bundle_data_from_filelist.gni approaches and to dtapuska@ for
suggesting the presubmit_support.py to simplify the various
PRESUBMIT.py files.

Bug: 698042
Change-Id: Ia5e79432ab392ad3530e92cb76de54b012dfe5d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4209248
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100913}
  • Loading branch information
Ian Vollick authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent 68e6f11 commit 00424e5
Show file tree
Hide file tree
Showing 50 changed files with 2,949 additions and 2,334 deletions.
24 changes: 24 additions & 0 deletions build/config/ios/bundle_data_from_filelist.gni
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

assert(current_os == "ios")

template("bundle_data_from_filelist") {
assert(defined(invoker.filelist_name), "Requires setting filelist_name")

_filelist_content = read_file(invoker.filelist_name, "list lines")
bundle_data(target_name) {
forward_variables_from(invoker,
"*",
[
"filelist_name",
"sources",
])
sources = filter_exclude(_filelist_content, [ "#*" ])
if (!defined(outputs)) {
outputs = [ "{{bundle_resources_dir}}/" +
"{{source_root_relative_dir}}/{{source_file_part}}" ]
}
}
}
39 changes: 39 additions & 0 deletions build/ios/presubmit_support.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
"""Presubmit helpers for ios
See https://www.chromium.org/developers/how-tos/depottools/presubmit-scripts
for more details about the presubmit API built into depot_tools.
"""

from . import update_bundle_filelist


def CheckBundleData(input_api, output_api, base, globroot='//'):
root = input_api.change.RepositoryRoot()
filelist = input_api.os_path.join(input_api.PresubmitLocalPath(),
base + '.filelist')
globlist = input_api.os_path.join(input_api.PresubmitLocalPath(),
base + '.globlist')
if globroot.startswith('//'):
globroot = input_api.os_path.join(input_api.change.RepositoryRoot(),
globroot[2:])
else:
globroot = input_api.os_path.join(input_api.PresubmitLocalPath(), globroot)
if update_bundle_filelist.process_filelist(filelist,
globlist,
globroot,
check=True,
verbose=input_api.verbose) == 0:
return []
else:
script = input_api.os_path.join(input_api.change.RepositoryRoot(), 'build',
'ios', 'update_bundle_filelist.py')

return [
output_api.PresubmitError(
'Filelist needs to be re-generated. Please run %s %s %s %s and '
'include the changes in this CL' %
(script, filelist, globlist, globroot))
]
105 changes: 105 additions & 0 deletions build/ios/presubmit_support_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
#!/usr/bin/env python3
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import os.path
import sys
import unittest

sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..'))

from PRESUBMIT_test_mocks import MockInputApi, MockOutputApi
from build.ios import presubmit_support


class BundleDataPresubmit(unittest.TestCase):
def setUp(self):
self.mock_input_api = MockInputApi()
self.mock_input_api.change.RepositoryRoot = lambda: os.path.join(
os.path.dirname(__file__), '..', '..')
self.mock_input_api.PresubmitLocalPath = lambda: os.path.dirname(__file__)
self.mock_output_api = MockOutputApi()
self.mock_input_api.verbose = False

def testBasic(self):
"""
Checks that a glob can be expanded to build a file list and if it
matches the existing file list, we should see no error.
"""
results = presubmit_support.CheckBundleData(self.mock_input_api,
self.mock_output_api,
'test_data/basic', '.')
self.assertEqual(0, len(results))

def testExclusion(self):
"""
Check that globs can be used to exclude files from file lists.
"""
results = presubmit_support.CheckBundleData(self.mock_input_api,
self.mock_output_api,
'test_data/exclusions', '.')
self.assertEqual(0, len(results))

def testDifferentLocalPath(self):
"""
Checks the case where the presubmit directory is not the same as the
globroot, but it is still local (i.e., not relative to the repository
root)
"""
results = presubmit_support.CheckBundleData(
self.mock_input_api, self.mock_output_api,
'test_data/different_local_path', 'test_data')
self.assertEqual(0, len(results))

def testRepositoryRelative(self):
"""
Checks the case where globs are relative to the repository root.
"""
results = presubmit_support.CheckBundleData(
self.mock_input_api, self.mock_output_api,
'test_data/repository_relative')
self.assertEqual(0, len(results))

def testMissingFilesInFilelist(self):
"""
Checks that we do indeed return an error if the filelist is missing a
file. In this case, all of the test .filelist and .globlist files are
excluded.
"""
results = presubmit_support.CheckBundleData(self.mock_input_api,
self.mock_output_api,
'test_data/missing', '.')
self.assertEqual(1, len(results))

def testExtraFilesInFilelist(self):
"""
Checks the case where extra files have been included in the file list.
"""
results = presubmit_support.CheckBundleData(self.mock_input_api,
self.mock_output_api,
'test_data/extra', '.')
self.assertEqual(1, len(results))

def testOrderInsensitive(self):
"""
Checks that we do not trigger an error for cases where the file list is
correct, but in a different order than the globlist expansion.
"""
results = presubmit_support.CheckBundleData(self.mock_input_api,
self.mock_output_api,
'test_data/reorder', '.')
self.assertEqual(0, len(results))

def testUnexpectedHeader(self):
"""
Checks an unexpected header in a file list causes an error.
"""
results = presubmit_support.CheckBundleData(self.mock_input_api,
self.mock_output_api,
'test_data/comment', '.')
self.assertEqual(1, len(results))


if __name__ == '__main__':
unittest.main()
Empty file added build/ios/test_data/bar.html
Empty file.
7 changes: 7 additions & 0 deletions build/ios/test_data/basic.filelist
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# NOTE: this file is generated by build/ios/update_bundle_filelist.py
# If it requires updating, you should get a presubmit error with
# instructions on how to regenerate. Otherwise, do not edit.
test_data/subdirectory/baz.txt
7 changes: 7 additions & 0 deletions build/ios/test_data/basic.globlist
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# NOTE: this file is generated by build/ios/update_bundle_filelist.py
# If it requires updating, you should get a presubmit error with
# instructions on how to regenerate. Otherwise, do not edit.
test_data/subdirectory/*
2 changes: 2 additions & 0 deletions build/ios/test_data/comment.filelist
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# This comment is an unexpected header.
test_data/subdirectory/baz.txt
3 changes: 3 additions & 0 deletions build/ios/test_data/comment.globlist
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Some comment followed by an empty line.

test_data/subdirectory/*
9 changes: 9 additions & 0 deletions build/ios/test_data/different_local_path.filelist
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# NOTE: this file is generated by build/ios/update_bundle_filelist.py
# If it requires updating, you should get a presubmit error with
# instructions on how to regenerate. Otherwise, do not edit.
bar.html
foo.css
subdirectory/baz.txt
2 changes: 2 additions & 0 deletions build/ios/test_data/different_local_path.globlist
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
**
-**list
9 changes: 9 additions & 0 deletions build/ios/test_data/exclusions.filelist
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# NOTE: this file is generated by build/ios/update_bundle_filelist.py
# If it requires updating, you should get a presubmit error with
# instructions on how to regenerate. Otherwise, do not edit.
test_data/bar.html
test_data/foo.css
test_data/subdirectory/baz.txt
2 changes: 2 additions & 0 deletions build/ios/test_data/exclusions.globlist
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
test_data/**
-test_data/**list
8 changes: 8 additions & 0 deletions build/ios/test_data/extra.filelist
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# NOTE: this file is generated by build/ios/update_bundle_filelist.py
# If it requires updating, you should get a presubmit error with
# instructions on how to regenerate. Otherwise, do not edit.
test_data/bar.html
test_data/foo.css
1 change: 1 addition & 0 deletions build/ios/test_data/extra.globlist
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test_data/*.css
Empty file added build/ios/test_data/foo.css
Empty file.
9 changes: 9 additions & 0 deletions build/ios/test_data/missing.filelist
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# NOTE: this file is generated by build/ios/update_bundle_filelist.py
# If it requires updating, you should get a presubmit error with
# instructions on how to regenerate. Otherwise, do not edit.
test_data/bar.html
test_data/foo.css
test_data/subdirectory/baz.txt
4 changes: 4 additions & 0 deletions build/ios/test_data/missing.globlist
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# This should cover every file in test_data/ and its subdirectories (including
# test files).

test_data/**
9 changes: 9 additions & 0 deletions build/ios/test_data/reorder.filelist
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# NOTE: this file is generated by build/ios/update_bundle_filelist.py
# If it requires updating, you should get a presubmit error with
# instructions on how to regenerate. Otherwise, do not edit.
test_data/subdirectory/baz.txt
test_data/foo.css
test_data/bar.html
2 changes: 2 additions & 0 deletions build/ios/test_data/reorder.globlist
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
test_data/**
-test_data/**list
9 changes: 9 additions & 0 deletions build/ios/test_data/repository_relative.filelist
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Copyright 2023 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# NOTE: this file is generated by build/ios/update_bundle_filelist.py
# If it requires updating, you should get a presubmit error with
# instructions on how to regenerate. Otherwise, do not edit.
//build/ios/test_data/bar.html
//build/ios/test_data/foo.css
//build/ios/test_data/subdirectory/baz.txt
2 changes: 2 additions & 0 deletions build/ios/test_data/repository_relative.globlist
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
//build/ios/test_data/**
-//build/ios/test_data/**list
Empty file.
Loading

0 comments on commit 00424e5

Please sign in to comment.