From 004aedc249bcf5b31fbc34c90c4b0aad665a3490 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 14 Mar 2024 16:35:44 -0700 Subject: [PATCH 1/5] Improve --auto_diff mode for when GMA is the only change. --- Android/firebase_dependencies.gradle | 2 +- scripts/gha/print_matrix_configuration.py | 52 +++++++++++++++++------ 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/Android/firebase_dependencies.gradle b/Android/firebase_dependencies.gradle index 744c568297..f4198cc140 100644 --- a/Android/firebase_dependencies.gradle +++ b/Android/firebase_dependencies.gradle @@ -27,7 +27,7 @@ def firebaseDependenciesMap = [ 'dynamic_links' : ['com.google.firebase:firebase-dynamic-links'], 'firestore' : ['com.google.firebase:firebase-firestore'], 'functions' : ['com.google.firebase:firebase-functions'], - 'gma' : ['com.google.android.gms:play-services-ads:23.0.0', + 'gma' : ['com.google.android.gms:play-services-ads:23.1.0', 'com.google.android.ump:user-messaging-platform:2.2.0'], 'installations' : ['com.google.firebase:firebase-installations'], 'invites' : ['com.google.firebase:firebase-invites'], diff --git a/scripts/gha/print_matrix_configuration.py b/scripts/gha/print_matrix_configuration.py index d47374f95c..92e2289e98 100644 --- a/scripts/gha/print_matrix_configuration.py +++ b/scripts/gha/print_matrix_configuration.py @@ -310,6 +310,22 @@ def print_value(value): print(json.dumps(value)) +def scan_changes_in_file(parm_key, auto_diff, path, requested_api_list): + """Scan for changes in the given file and return APIs that were modified.""" + change_lines = subprocess.check_output(['git', 'diff', '-U0', auto_diff, path]).decode('utf-8').rstrip('\n').split('\n') + change_lines = [l for l in change_lines if l.startswith('-') or l.startswith('+')] + change_lines = [l for l in change_lines if not l.startswith('---') and not l.startswith('+++')] + change_lines = [l for l in change_lines if len(l) > 20] + changed_apis = set() + for line in change_lines: + if ("Google-Mobile-Ads" in line or "GoogleUserMessagingPlatform" in line or + "play-services-ads" in line or "user-messaging-platform" in line): + changed_apis.add("gma") + else: + changed_apis.update(requested_api_list) + changed_apis.intersection_update(requested_api_list) + return ",".join(changed_apis) + def filter_values_on_diff(parm_key, value, auto_diff): """Filter the given key based on a branch diff. @@ -317,37 +333,49 @@ def filter_values_on_diff(parm_key, value, auto_diff): source tree, relative to the given base branch.""" file_list = set(subprocess.check_output(['git', 'diff', '--name-only', auto_diff]).decode('utf-8').rstrip('\n').split('\n')) if parm_key == 'apis': + file_redirects = { + # Custom handling for specific files, to be treated as a different path, + # ignored completely (set to None), or scan the changes more in-depth (set + # to scan_changes function) + "cmake/external/firestore.cmake": "firestore", + "cmake/external/libuv.cmake": "database", + "cmake/external/uWebSockets.cmake": "database", + "ios_pod/Podfile":scan_changes_in_file, + "Android/firebase_dependencies.gradle":scan_changes_in_file, + "release_build_files/Android/firebase_dependencies.gradle":scan_changes_in_file, + } custom_triggers = { # Special handling for several top-level directories. # Any top-level directories set to None are completely ignored. "external": None, "release_build_files": None, + # These two handled by file_redirects above. + "ios_pod": None, + "Android": None, # Uncomment the two below lines when debugging this script, or GitHub # actions related to auto-diff mode. - # ".github": None, - # "scripts": None, + ".github": None, + "scripts": None, # Top-level directories listed below trigger additional APIs being tested. # For example, if auth is touched by a PR, we also need to test functions, # database, firestore, and storage. "auth": "auth,functions,database,firestore,storage", } - file_redirects = { - # Custom handling for specific files, to be treated as a different path or - # ignored completely (set to None). - "cmake/external/firestore.cmake": "firestore", - "cmake/external/libuv.cmake": "database", - "cmake/external/uWebSockets.cmake": "database", - } requested_api_list = set(value.split(',')) filtered_api_list = set() - - for path in file_list: + file_list_array = list(file_list) + for path in file_list_array: if len(path) == 0: continue if path in file_redirects: if file_redirects[path] is None: continue else: - path = os.path.join(file_redirects[path], path) + if callable(file_redirects[path]): + add_paths = file_redirects[path](parm_key, auto_diff, path, requested_api_list) + if add_paths: file_list_array.extend(add_paths.split(",")) + continue + else: + path = os.path.join(file_redirects[path], path) topdir = path.split(os.path.sep)[0] if topdir in custom_triggers: if not custom_triggers[topdir]: continue # Skip ones set to None. From 5765d4e4ed9b38b452ce53881d24241310ef2b99 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 14 Mar 2024 16:36:58 -0700 Subject: [PATCH 2/5] Revert version change. --- Android/firebase_dependencies.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Android/firebase_dependencies.gradle b/Android/firebase_dependencies.gradle index f4198cc140..744c568297 100644 --- a/Android/firebase_dependencies.gradle +++ b/Android/firebase_dependencies.gradle @@ -27,7 +27,7 @@ def firebaseDependenciesMap = [ 'dynamic_links' : ['com.google.firebase:firebase-dynamic-links'], 'firestore' : ['com.google.firebase:firebase-firestore'], 'functions' : ['com.google.firebase:firebase-functions'], - 'gma' : ['com.google.android.gms:play-services-ads:23.1.0', + 'gma' : ['com.google.android.gms:play-services-ads:23.0.0', 'com.google.android.ump:user-messaging-platform:2.2.0'], 'installations' : ['com.google.firebase:firebase-installations'], 'invites' : ['com.google.firebase:firebase-invites'], From 78a23c4c52b0f27ea91ed88b2bdb10e7bc959ee4 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 14 Mar 2024 16:37:28 -0700 Subject: [PATCH 3/5] Remove debug lines. --- scripts/gha/print_matrix_configuration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/gha/print_matrix_configuration.py b/scripts/gha/print_matrix_configuration.py index 92e2289e98..57053e5ab6 100644 --- a/scripts/gha/print_matrix_configuration.py +++ b/scripts/gha/print_matrix_configuration.py @@ -354,8 +354,8 @@ def filter_values_on_diff(parm_key, value, auto_diff): "Android": None, # Uncomment the two below lines when debugging this script, or GitHub # actions related to auto-diff mode. - ".github": None, - "scripts": None, + #".github": None, + #"scripts": None, # Top-level directories listed below trigger additional APIs being tested. # For example, if auth is touched by a PR, we also need to test functions, # database, firestore, and storage. From badb18b90e33c67d7b5d67d0f368f8f3f55da023 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 14 Mar 2024 16:38:21 -0700 Subject: [PATCH 4/5] Early out from loop. --- scripts/gha/print_matrix_configuration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/gha/print_matrix_configuration.py b/scripts/gha/print_matrix_configuration.py index 57053e5ab6..bd5459b827 100644 --- a/scripts/gha/print_matrix_configuration.py +++ b/scripts/gha/print_matrix_configuration.py @@ -323,6 +323,7 @@ def scan_changes_in_file(parm_key, auto_diff, path, requested_api_list): changed_apis.add("gma") else: changed_apis.update(requested_api_list) + break changed_apis.intersection_update(requested_api_list) return ",".join(changed_apis) From 56591a4ea9e3352b738af04852030a584910fa26 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Fri, 15 Mar 2024 11:40:36 -0700 Subject: [PATCH 5/5] Tweak script to handle empty values better. --- scripts/gha/print_matrix_configuration.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/gha/print_matrix_configuration.py b/scripts/gha/print_matrix_configuration.py index bd5459b827..97ede4d808 100644 --- a/scripts/gha/print_matrix_configuration.py +++ b/scripts/gha/print_matrix_configuration.py @@ -325,7 +325,7 @@ def scan_changes_in_file(parm_key, auto_diff, path, requested_api_list): changed_apis.update(requested_api_list) break changed_apis.intersection_update(requested_api_list) - return ",".join(changed_apis) + return ",".join(sorted(changed_apis)) if changed_apis else None def filter_values_on_diff(parm_key, value, auto_diff): """Filter the given key based on a branch diff. @@ -355,8 +355,8 @@ def filter_values_on_diff(parm_key, value, auto_diff): "Android": None, # Uncomment the two below lines when debugging this script, or GitHub # actions related to auto-diff mode. - #".github": None, - #"scripts": None, + # ".github": None, + # "scripts": None, # Top-level directories listed below trigger additional APIs being tested. # For example, if auth is touched by a PR, we also need to test functions, # database, firestore, and storage. @@ -390,7 +390,7 @@ def filter_values_on_diff(parm_key, value, auto_diff): sys.stderr.write("Path '%s' is outside known directories, defaulting to all APIs: %s\n" % (path, value)) return value sys.stderr.write("::warning::Autodetected APIs: %s\n" % ','.join(sorted(filtered_api_list))) - return ','.join(sorted(filtered_api_list)) + return ','.join(sorted(filtered_api_list if filtered_api_list else requested_api_list)) elif parm_key == 'platform': # Quick and dirty check: # For each file: