From 255c05926ba22e3d43cd0f51aa2a81939c3c0a11 Mon Sep 17 00:00:00 2001
From: Usta Shrestha <usta@google.com>
Date: Thu, 13 Apr 2023 22:37:06 -0400
Subject: cosmetic: use os.walkdir()

Test: `incremental_build.sh -c "no change" -b prod` and see "VERIFIED Symlink Forest ..." in console log
Bug: NA
Change-Id: I9b81bbf710d7d8056989f8f7debe0927402578eb
---
 scripts/incremental_build/cuj_catalog.py | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/scripts/incremental_build/cuj_catalog.py b/scripts/incremental_build/cuj_catalog.py
index 6db3c8d8..7d2de238 100644
--- a/scripts/incremental_build/cuj_catalog.py
+++ b/scripts/incremental_build/cuj_catalog.py
@@ -64,20 +64,15 @@ def verify_symlink_forest_has_only_symlink_leaves():
   files except for merged BUILD.bazel files"""
 
   top_in_ws = InWorkspace.ws_counterpart(util.get_top_dir())
-  def helper(d: Path):
-    for child in os.scandir(d):
-      child_path: Path = Path(child.path)
-      if child_path.name == 'symlink_forest_version' and d == top_in_ws:
-         continue
-      if child_path.is_symlink():
+
+  for root, dirs, files in os.walk(top_in_ws, topdown=True, followlinks=False):
+    for file in files:
+      if file == 'symlink_forest_version' and top_in_ws.samefile(root):
         continue
-      if child_path.is_file() and child.name != 'BUILD.bazel':
-        # only "merged" BUILD.bazel files expected
-        raise AssertionError(f'{child_path} is an unexpected file')
-      if child_path.is_dir():
-        helper(child_path)
+      f = Path(root).joinpath(file)
+      if file != 'BUILD.bazel' and not f.is_symlink():
+        raise AssertionError(f'{f} unexpected')
 
-  helper(top_in_ws)
   logging.info('VERIFIED Symlink Forest has no real files except BUILD.bazel')
 
 
-- 
cgit v1.2.3


From 4ab91fa0cb077832d9ae57c89097347258526212 Mon Sep 17 00:00:00 2001
From: Usta Shrestha <usta@google.com>
Date: Mon, 17 Apr 2023 13:58:37 -0400
Subject: Fix event ordering in perf script

concurrent events aren't properly handled
e.g. if one run had a->b->c->d and the other had
d->c->b->a there was non-determinism in which cycle
is detected first. And that cycle detection also
modified some global state raising exceptions

Bug: NA
Test: ran `b test` which failed for perf_metrics_test without the
changes in perf_metrics.py

Change-Id: I6773b80c2598a278f4016d50c6dd193c0b500307
---
 scripts/incremental_build/perf_metrics.py      | 11 ++++++-----
 scripts/incremental_build/perf_metrics_test.py |  5 +++++
 scripts/incremental_build/util_test.py         |  4 ++++
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/scripts/incremental_build/perf_metrics.py b/scripts/incremental_build/perf_metrics.py
index 3d074566..64512e73 100644
--- a/scripts/incremental_build/perf_metrics.py
+++ b/scripts/incremental_build/perf_metrics.py
@@ -181,10 +181,12 @@ def _get_column_headers(rows: list[Row], allow_cycles: bool) -> list[str]:
       prev_col = all_cols[col]
 
   acc = []
-  while len(all_cols) > 0:
-    entries = [c for c in all_cols.values()]
-    entries.sort(key=lambda c: f'{c.indegree:03d}{c.header}')
-    entry = entries[0]
+  entries = [c for c in all_cols.values()]
+  while len(entries) > 0:
+    # sorting alphabetically to break ties for concurrent events
+    entries.sort(key=lambda c: c.header, reverse=True)
+    entries.sort(key=lambda c: c.indegree, reverse=True)
+    entry = entries.pop()
     # take only one to maintain alphabetical sort
     if entry.indegree != 0:
       cycle = '->'.join(entry.dfs(entry.header))
@@ -200,7 +202,6 @@ def _get_column_headers(rows: list[Row], allow_cycles: bool) -> list[str]:
       else:
         if not allow_cycles:
           raise ValueError(f'unexpected error for: {n}')
-    all_cols.pop(entry.header)
   return acc
 
 
diff --git a/scripts/incremental_build/perf_metrics_test.py b/scripts/incremental_build/perf_metrics_test.py
index a25c4405..119bdc10 100644
--- a/scripts/incremental_build/perf_metrics_test.py
+++ b/scripts/incremental_build/perf_metrics_test.py
@@ -40,6 +40,7 @@ class PerfMetricsTest(unittest.TestCase):
       Example(['ac', 'bd'], 'abcd'),
       Example(['abe', 'cde'], 'abcde'),
       Example(['ab', 'ba'], 'ab'),
+      Example(['abcde', 'edcba'], 'abcde'),
       Example(['ac', 'abc'], 'abc')
     ]
     for e in examples:
@@ -60,3 +61,7 @@ class PerfMetricsTest(unittest.TestCase):
         with self.assertRaisesRegex(ValueError,
                                     f'event ordering has a cycle {cycle}'):
           _get_column_headers(rows, allow_cycles=False)
+
+
+if __name__ == '__main__':
+  unittest.main()
diff --git a/scripts/incremental_build/util_test.py b/scripts/incremental_build/util_test.py
index 01b5f9c2..b117bd74 100644
--- a/scripts/incremental_build/util_test.py
+++ b/scripts/incremental_build/util_test.py
@@ -101,3 +101,7 @@ class UtilTest(unittest.TestCase):
     for (ts, expected) in examples:
       self.subTest(ts=ts, expected=expected)
       self.assertEqual(period_to_seconds(ts), expected)
+
+
+if __name__ == '__main__':
+  unittest.main()
-- 
cgit v1.2.3