From b48ebf47f4a3a28f13d5d34e51f200bac73a7b66 Mon Sep 17 00:00:00 2001
From: Jerry Zhang <Jerry@skydio.com>
Date: Tue, 12 Mar 2024 13:31:39 -0700
Subject: [PATCH] upload: Allow arbitrary topic order

Now that we're iterating over topics topologically
in the important places, we can remove the check
that requires topics to be in consecutive order.

This means revup is able to pull together relative
chains in any arbitrary order, as long as that order
is valid with no cycles.

Fixes: #156
---
 docs/upload.md       |  4 +---
 revup/topic_stack.py | 31 ++++++++++++-------------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/docs/upload.md b/docs/upload.md
index b5848fd..a2c4505 100644
--- a/docs/upload.md
+++ b/docs/upload.md
@@ -40,9 +40,7 @@ consecutive locally.
 : Optionally specifies a relative topic for this topic. Each topic can
 have at most one relative topic. Commits in this topic will be cherry-picked
 on top of the branch for the relative topic, and the pull request will
-be created targeted to the relative topic's branch. The first commit
-of a relative topic must appear before the first commit of specifying
-topic.
+be created targeted to the relative topic's branch.
 
 **Branches:**
 : Optionally specifies base branches for this topic. Base branches are long
diff --git a/revup/topic_stack.py b/revup/topic_stack.py
index 8a8f8ec..76f614e 100644
--- a/revup/topic_stack.py
+++ b/revup/topic_stack.py
@@ -468,7 +468,8 @@ async def populate_reviews(
         Populate reviews for already-parsed topics. Verify base branch and relative topic info to
         ensure it is valid.
         """
-        seen_topics: Dict[str, Topic] = {}
+        last_topic = None
+        # Copy topics before iterating so it's safe to delete from the original dict
         for name, topic in list(self.topics.items()):
             if limit_topics:
                 if name not in limit_topics:
@@ -497,8 +498,8 @@ async def populate_reviews(
                 raise RevupUsageException(f"Can't specify more than one uploader for topic {name}!")
 
             relative_topic = ""
-            if force_relative_chain and seen_topics:
-                relative_topic = list(seen_topics)[-1]
+            if force_relative_chain and last_topic is not None:
+                relative_topic = last_topic
             elif len(topic.tags[TAG_RELATIVE]) > 1:
                 raise RevupUsageException(
                     "Can't specify more than 1 relative topic per topic! Got"
@@ -510,21 +511,12 @@ async def populate_reviews(
                 # all the base branches for the relative topic. However it can't specify
                 # any base branches the relative topic doesn't have.
                 relative_topic = min(topic.tags[TAG_RELATIVE])
-                if relative_topic not in seen_topics:
-                    if relative_topic in self.topics:
-                        # Relative topics can have interleaved commits, however the first commit of
-                        # the relative topic must come before the first commit of this topic. This
-                        # prevents cycles of relatives.
-                        raise RevupUsageException(
-                            f"Topic '{name}' is relative to '{relative_topic}' but doesn't appear"
-                            " after it"
-                        )
-                    else:
-                        logging.warning(
-                            f"Relative topic '{relative_topic}' not found in stack, assuming it was"
-                            " merged"
-                        )
-                        relative_topic = ""
+                if relative_topic not in self.topics:
+                    logging.warning(
+                        f"Relative topic '{relative_topic}' not found in stack, assuming it was"
+                        " merged"
+                    )
+                    relative_topic = ""
 
             if self.repo_info and self.fork_info and self.fork_info.owner != self.repo_info.owner:
                 if len(topic.tags[TAG_RELATIVE_BRANCH]) > 1:
@@ -566,7 +558,8 @@ async def populate_reviews(
             if auto_add_users in ("a2r", "both"):
                 topic.tags[TAG_REVIEWER].update(topic.tags[TAG_ASSIGNEE])
 
-            seen_topics[name] = topic
+            # Track the last actually used topic for the relative-chain feature
+            last_topic = name
 
         if limit_topics:
             for name in limit_topics: