Gradle plugin: avoid printing duplicate dependency paths by suztomo · Pull Request #2188 · GoogleCloudPlatform/cloud-opensource-java

suztomo · 2021-08-13T19:07:37Z

Avoid printing duplicate dependency paths. For a large dependency graph, the previous approach of not omitting duplicate paths would cause OutOfMemoryError.

Before this change

https://gist.github.com/suztomo/31a67085163a8a99a171cb22345a188b

io.grpc:grpc-core:1.29.0 is at:
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-netty-shaded:1.28.1 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 / io.grpc:grpc-grpclb:1.28.1 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 / io.grpc:grpc-netty-shaded:1.28.1 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / com.google.cloud:google-cloud-core-grpc:1.93.4 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-netty-shaded:1.28.1 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / com.google.cloud:google-cloud-core-grpc:1.93.4 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 / io.grpc:grpc-grpclb:1.28.1 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / com.google.cloud:google-cloud-core-grpc:1.93.4 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / com.google.cloud:google-cloud-core-grpc:1.93.4 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 / io.grpc:grpc-netty-shaded:1.28.1 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / com.google.cloud:google-cloud-core-grpc:1.93.4 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / com.google.cloud:google-cloud-logging:1.101.1 / io.grpc:grpc-core:1.29.0
  :gradle-project:unspecified / io.grpc:grpc-core:1.29.0

Notice com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-netty-shaded:1.28.1 / io.grpc:grpc-core:1.29.0 is appearing twice (1st and 5th lines).

This was bad because the number of paths would become much larger. Suppose there are 5 paths from artifact A to artifact B, 3 paths from artifact B to artifact C, and 6 paths from C to D. Then the previous implementation would print 5 x 3 x 6 = 90 dependency paths.

After this PR

https://gist.github.com/suztomo/202c001c682238ee0fe922383d9a2e4d

io.grpc:grpc-core:1.29.0 is at:
  g:test-123:0.1.0-SNAPSHOT / io.grpc:grpc-core:1.29.0
  g:test-123:0.1.0-SNAPSHOT / com.google.cloud:google-cloud-logging:1.101.1 / io.grpc:grpc-core:1.29.0
  g:test-123:0.1.0-SNAPSHOT / com.google.cloud:google-cloud-logging:1.101.1 / com.google.cloud:google-cloud-core-grpc:1.93.4 / io.grpc:grpc-core:1.29.0
  g:test-123:0.1.0-SNAPSHOT / com.google.cloud:google-cloud-logging:1.101.1 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-netty-shaded:1.28.1 / io.grpc:grpc-core:1.29.0
  g:test-123:0.1.0-SNAPSHOT / com.google.cloud:google-cloud-logging:1.101.1 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 / io.grpc:grpc-core:1.29.0
  g:test-123:0.1.0-SNAPSHOT / com.google.cloud:google-cloud-logging:1.101.1 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 / io.grpc:grpc-netty-shaded:1.28.1 (omitted for duplicate)
  g:test-123:0.1.0-SNAPSHOT / com.google.cloud:google-cloud-logging:1.101.1 / com.google.cloud:google-cloud-core-grpc:1.93.4 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-netty-shaded:1.28.1 (omitted for duplicate)
  g:test-123:0.1.0-SNAPSHOT / com.google.cloud:google-cloud-logging:1.101.1 / com.google.cloud:google-cloud-core-grpc:1.93.4 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 (omitted for duplicate)
  g:test-123:0.1.0-SNAPSHOT / com.google.cloud:google-cloud-logging:1.101.1 / com.google.api:gax-grpc:1.56.0 / io.grpc:grpc-alts:1.28.1 / io.grpc:grpc-grpclb:1.28.1 / io.grpc:grpc-core:1.29.0

io.grpc:grpc-netty-shaded:1.28.1 / io.grpc:grpc-core:1.29.0 appears only once, and it now shows "omitted for duplicate" message.

suztomo · 2021-08-13T19:51:08Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

          }
-        } else {
-          stack.add(child);
-          recordDependencyPaths(output, stack, targetCoordinates, checkedCircularDependency);


Before this PR, the tree traversal was depth-first manner.

suztomo · 2021-08-13T19:51:32Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+      // or transitive dependencies.
+      Set<ResolvedComponentResult> nodesDependOnTarget = new HashSet<>();
+
+      while (!queue.isEmpty()) {


With this PR, the tree traversal is done by breath-first manner (level-order)

suztomo · 2021-08-13T19:53:29Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+              }
+            } else {
+              ResolvedComponentResultNode childNode = new ResolvedComponentResultNode(child, node);
+              queue.add(childNode);


This is the core of the level-order traversal (breadth-first). The children of the node are added to the queue.

suztomo · 2021-08-13T19:57:18Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+    ListMultimap<String, String> dependencyPaths =
+        groupCoordinatesToDependencyPaths(componentResult, targetCoordinates);


The caller of the method recordDependencyPaths, which is now groupCoordinatesToDependencyPaths is now much cleaner.

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

suztomo · 2021-08-13T20:29:39Z

This still causes my test case.

Edit: it worked with org.gradle.jvmargs=-Xmx4g in gradle.properties

suztomo · 2021-08-13T22:06:50Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+        if (nodesDependOnTarget.contains(item)) {
+          // Do not show duplicate dependency paths. If we know that this node contains dependency
+          // having targetCoordinates in its direct/transitive dependencies, there is no need to
+          // print the dependency paths again.
+          String dependencyPath = node.pathFromRoot() + " (omitted for duplicate)";
+          coordinatesToDependencyPaths.put(targetCoordinates, dependencyPath);


This is the main purpose of this PR.

suztomo · 2021-08-16T13:22:20Z

...ionalTest/groovy/com/google/cloud/tools/dependencies/gradle/BuildStatusFunctionalTest.groovy

+    // Ensure the node closest to the root is printed
+    result.output.contains("g:test-123:0.1.0-SNAPSHOT / io.grpc:grpc-core:1.29.0")
+    // Ensure that the relationship between grpc-netty-shaded to grpc-core is only printed once
+    result.output.count("io.grpc:grpc-netty-shaded:1.28.1 / io.grpc:grpc-core:1.29.0") == 1


This is the effect of this PR.

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

elefeint

Looks great overall, but I'd love it if it were possible to split the BFS + dependency-extraction-and-validation logic into separate pieces. BFS part should be trivial to read, while the individual dependency methods would allow us to focus on only a particular piece at a time (like "enumerate all dependencies of this node" or "does this node cause a circular dependency"?

It's funny that you are actually writing complicated algorithmic code that we all got interviewed on! A rarity.

elefeint · 2021-08-16T13:32:25Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+      if (componentResult.equals(other)) {
+        return true;
+      }
+      if (parent == null) {
+        return false;
+      }
+      return parent.hasParent(other);


You could also skip one extra recursive call by comparing the current node's parent to the passed-in parameter.

Something like this:

if (parent == null) { return false; } else if (parent == other) { return true; } return parent.hasParent(other);

(this was a comment from the start of review, but now that I got to the end, I am no longer sure I had a point there) Would isDescendantOf() be more or less confusing as a method name?

Updated to isDescendantOf.

elefeint · 2021-08-16T13:36:08Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+      List<String> dependencyPathElementsReversed = new ArrayList<>();
+      for (ResolvedComponentResultNode iter = this; iter != null; iter = iter.parent) {
+        dependencyPathElementsReversed.add(formatComponentResult(iter.componentResult));
+      }
+      Collections.reverse(dependencyPathElementsReversed);


You can use ArrayDeque to avoid reversing the list -- it can be used as both stack and queue; you can insert like in stack (reverse) and retrieve like a queue (proper order). Good for both, readability and performance; it's amortized constant time for everything useful.

That's a nice idea. Updating.

Updated to use ArrayDequeue.

elefeint · 2021-08-16T13:40:46Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+    ListMultimap<String, String> coordinatesToDependencyPaths =
+        MultimapBuilder.hashKeys().arrayListValues().build();
+    // No need to print the same circular dependency multiple times
+    Set<ResolvedComponentResult> checkedCircularDependency = new HashSet<>();


How about a name like seen or circularDependencyCandidates? Technically, at the point nodes are added to this set, they have not yet been checked.

Actually an item is added to this collection only when it's a circular dependency.

Added source code comment.

elefeint · 2021-08-16T13:41:11Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+    Set<ResolvedComponentResult> checkedCircularDependency = new HashSet<>();
+
+    for (String targetCoordinates : targetCoordinatesSet) {
+      // Queue of dependnecy nodes. Each node knows its parent.


s/dependnecy/dependency

elefeint · 2021-08-16T13:58:29Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+
+      // Mapping to omit duplicate dependency paths in the output. This is a mapping from Maven
+      // coordinates to dependency nodes that has the dependency of the coordinate in their direct
+      // or transitive dependencies.


This comment sounds like a javadoc for a helper method. Would it make sense to extract BFS for dependencies into some kind of findDependencies() helper?

This comment was outdated. Updated as

// A set to omit duplicate dependency paths in the output. When a node is found to be in this // set while traversing the graph, we do not need to check the children, because we know that // the dependency paths from that node to the targetCoordinates are already added to // coordinatesToDependencyPaths. Set<ResolvedComponentResult> nodesDependOnTarget = new HashSet<>();

I didn't think of a such findDependencies() that would improve code here. But newly added getDependencies should give more clarity on the code.

elefeint · 2021-08-16T14:00:22Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+   * @param targetCoordinatesSet The Maven coordinates to check their dependency paths
+   */
+  private ListMultimap<String, String> groupCoordinatesToDependencyPaths(
+      ResolvedComponentResult componentResult, Set<String> targetCoordinatesSet) {


Is componentResult expected to be the root of dependency tree? Then maybe we could name it this way. Or I could be confused...

Updated to rootProject.

elefeint · 2021-08-16T14:04:36Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+          }
+        }
+
+        if (nodesDependOnTarget.contains(item)) {


Should this if block go above the if (targetCoordinates.equals(coordinates)) {? What if a transitive dependency on targetCoordinates is found before the direct dependency? Or is it a "this should never happen in a gradle resolution" type of situation?

If nodesDependOnTarget.contains(item) is true, then targetCoordinates (the coordinates of item) is not equal to coordinates.

What if a transitive dependency on targetCoordinates is found before the direct dependency?

Would you explain an example?

I was thinking it's possible to have a duplicate if both if (targetCoordinates.equals(coordinates)) from ~10 lines above and this current if statement are both true.

The 2 if-statements don't become true at the same time. I just updated this to the below to deliver a more clear meaning.

if (targetCoordinates.equals(coordinates)) { ... } else if (nodesDependOnTarget.contains(item)) { ... } else { queue.addAll(getDependencies(node)); }

elefeint · 2021-08-16T14:07:29Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+          String dependencyPath = node.pathFromRoot() + " (omitted for duplicate)";
+          coordinatesToDependencyPaths.put(targetCoordinates, dependencyPath);
+
+          for (ResolvedComponentResultNode iter = node; iter != null; iter = iter.parent) {


Since these 3 lines turn up twice (for a direct and all transitive dependencies), could you extract them into a helper method? addParents() or nodesDependOnTarget.addAll(getAllParents()) or some such.

Updated to rootToNode() and using addAll.

elefeint · 2021-08-16T14:13:19Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+          if (dependencyResult instanceof ResolvedDependencyResult) {
+            ResolvedDependencyResult resolvedDependencyResult =
+                (ResolvedDependencyResult) dependencyResult;
+            ResolvedComponentResult child = resolvedDependencyResult.getSelected();
+
+            if (node.hasParent(child)) {
+              // Circular dependency check
+              if (checkedCircularDependency.add(child)) {
+                getLogger()
+                    .error(
+                        "Circular dependency for: "
+                            + resolvedDependencyResult
+                            + "\n The stack is: "
+                            + node.pathFromRoot());
+              }


I'd try to extract this into a causesCircularDependency() or similarly named helper. Just generally, the overall BFS structure will be easier to read if the actually useful pieces of functionality were in helper methods.

Extracted the bigger for-loop into getDependencies.

Renamed getDependencies to findDependencies.

suztomo

Thank you for review. I'll update the code.

suztomo · 2021-08-16T15:58:24Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+      List<String> dependencyPathElementsReversed = new ArrayList<>();
+      for (ResolvedComponentResultNode iter = this; iter != null; iter = iter.parent) {
+        dependencyPathElementsReversed.add(formatComponentResult(iter.componentResult));
+      }
+      Collections.reverse(dependencyPathElementsReversed);


That's a nice idea. Updating.

suztomo · 2021-08-16T16:00:59Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+    ListMultimap<String, String> coordinatesToDependencyPaths =
+        MultimapBuilder.hashKeys().arrayListValues().build();
+    // No need to print the same circular dependency multiple times
+    Set<ResolvedComponentResult> checkedCircularDependency = new HashSet<>();


Actually an item is added to this collection only when it's a circular dependency.

suztomo

@elefeint PTAL.

suztomo · 2021-08-16T20:08:15Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+      if (componentResult.equals(other)) {
+        return true;
+      }
+      if (parent == null) {
+        return false;
+      }
+      return parent.hasParent(other);


Updated to isDescendantOf.

suztomo · 2021-08-16T20:08:25Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+      List<String> dependencyPathElementsReversed = new ArrayList<>();
+      for (ResolvedComponentResultNode iter = this; iter != null; iter = iter.parent) {
+        dependencyPathElementsReversed.add(formatComponentResult(iter.componentResult));
+      }
+      Collections.reverse(dependencyPathElementsReversed);


Updated to use ArrayDequeue.

suztomo · 2021-08-16T20:10:05Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+   * @param targetCoordinatesSet The Maven coordinates to check their dependency paths
+   */
+  private ListMultimap<String, String> groupCoordinatesToDependencyPaths(
+      ResolvedComponentResult componentResult, Set<String> targetCoordinatesSet) {


Updated to rootProject.

suztomo · 2021-08-16T20:11:29Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+    ListMultimap<String, String> coordinatesToDependencyPaths =
+        MultimapBuilder.hashKeys().arrayListValues().build();
+    // No need to print the same circular dependency multiple times
+    Set<ResolvedComponentResult> checkedCircularDependency = new HashSet<>();


Added source code comment.

suztomo · 2021-08-16T20:25:14Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+          }
+        }
+
+        if (nodesDependOnTarget.contains(item)) {


If nodesDependOnTarget.contains(item) is true, then targetCoordinates (the coordinates of item) is not equal to coordinates.

What if a transitive dependency on targetCoordinates is found before the direct dependency?

Would you explain an example?

suztomo · 2021-08-16T20:32:54Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+          String dependencyPath = node.pathFromRoot() + " (omitted for duplicate)";
+          coordinatesToDependencyPaths.put(targetCoordinates, dependencyPath);
+
+          for (ResolvedComponentResultNode iter = node; iter != null; iter = iter.parent) {


Updated to rootToNode() and using addAll.

suztomo · 2021-08-16T20:43:12Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+          if (dependencyResult instanceof ResolvedDependencyResult) {
+            ResolvedDependencyResult resolvedDependencyResult =
+                (ResolvedDependencyResult) dependencyResult;
+            ResolvedComponentResult child = resolvedDependencyResult.getSelected();
+
+            if (node.hasParent(child)) {
+              // Circular dependency check
+              if (checkedCircularDependency.add(child)) {
+                getLogger()
+                    .error(
+                        "Circular dependency for: "
+                            + resolvedDependencyResult
+                            + "\n The stack is: "
+                            + node.pathFromRoot());
+              }


Extracted the bigger for-loop into getDependencies.

suztomo · 2021-08-16T20:45:47Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+    Set<ResolvedComponentResult> checkedCircularDependency = new HashSet<>();
+
+    for (String targetCoordinates : targetCoordinatesSet) {
+      // Queue of dependnecy nodes. Each node knows its parent.


suztomo · 2021-08-16T20:48:24Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+
+      // Mapping to omit duplicate dependency paths in the output. This is a mapping from Maven
+      // coordinates to dependency nodes that has the dependency of the coordinate in their direct
+      // or transitive dependencies.


This comment was outdated. Updated as

// A set to omit duplicate dependency paths in the output. When a node is found to be in this // set while traversing the graph, we do not need to check the children, because we know that // the dependency paths from that node to the targetCoordinates are already added to // coordinatesToDependencyPaths. Set<ResolvedComponentResult> nodesDependOnTarget = new HashSet<>();

suztomo · 2021-08-16T20:51:05Z

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+
+      // Mapping to omit duplicate dependency paths in the output. This is a mapping from Maven
+      // coordinates to dependency nodes that has the dependency of the coordinate in their direct
+      // or transitive dependencies.


I didn't think of a such findDependencies() that would improve code here. But newly added getDependencies should give more clarity on the code.

elefeint

Looks great!

suztomo added 2 commits August 13, 2021 10:16

incorporated changes

24ad399

Merge remote-tracking branch 'origin/master' into path_explosion

d158a82

google-cla bot added the cla: yes label Aug 13, 2021

suztomo force-pushed the path_explosion branch from adce0c0 to dc80a47 Compare August 13, 2021 19:44

Level-order to traverse dependency tree

d38c8db

suztomo force-pushed the path_explosion branch from dc80a47 to d38c8db Compare August 13, 2021 19:46

suztomo changed the title ~~Level-order to traverse Gradle dependency tree~~ Gradle plugin: avoid printing duplicate dependency paths Aug 13, 2021

suztomo commented Aug 13, 2021

View reviewed changes

suztomo closed this Aug 13, 2021

suztomo reopened this Aug 13, 2021

suztomo requested a review from a team August 13, 2021 21:29

suztomo commented Aug 13, 2021

View reviewed changes

Use for-loop to iterate nodes to root

e052b56

suztomo commented Aug 16, 2021

View reviewed changes

elefeint suggested changes Aug 16, 2021

View reviewed changes

Merge remote-tracking branch 'origin/master' into path_explosion

993fb04

suztomo commented Aug 16, 2021

View reviewed changes

Applied review

d6106de

suztomo commented Aug 16, 2021

View reviewed changes

suztomo added 2 commits August 16, 2021 17:17

Clarifying if-statements

701bfff

getDependencies -> findDependencies

fb2b053

elefeint approved these changes Aug 17, 2021

View reviewed changes

suztomo merged commit 062331a into master Aug 17, 2021

		ListMultimap<String, String> dependencyPaths =
		groupCoordinatesToDependencyPaths(componentResult, targetCoordinates);

Conversation

suztomo commented Aug 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before this change

After this PR

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

suztomo commented Aug 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elefeint left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suztomo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

suztomo commented Aug 13, 2021 •

edited

Loading

suztomo commented Aug 13, 2021 •

edited

Loading