Support save without since

README.rst

@@ -46,12 +46,14 @@ Usage
     ply status
     All patches applied
 
-* Save the last commit as a new patch in the `patch-repo`::
+* Save set of commits to the `patch-repo`::
 
-    ply save        # Without arguments, HEAD^ is assumed
+    # Without --since, any 'new' patches (patches that follow applied patches)
+    # will be saved
+    ply save
 
-    # Explicit since argument, saves into `foo` subdirectory in the patch-repo
-    ply save --prefix=foo HEAD^
+    # Save only the last commit into the 'foo' subdirectory
+    ply save --since=HEAD^ --prefix=foo HEAD^
 
 * Rollback `working-repo` to match upstream::
 
@@ -90,6 +92,7 @@ Usage
 
         ply graph | dot -Tpng > dependencies.png
 
+
 `ply` vs X?
 ===========
 

TODO

@@ -1,7 +1,7 @@
+- BUG: do not save Ply-Patch annotation
+- BUG: changeId is creeping in
+
 - BUG: Make _mutate_series_file support recursive series files
-- ENHANCEMENT: ply save should only require a `since` if no annotations;
-  otherwise it should default to saving all patches after first.
-- CLEANUP: Make _applied_patches return a list, generator is a hassle
 
 WISHLIST
 

plypatch/__init__.py

@@ -49,26 +49,50 @@ def _last_upstream_commit_hash(self):
         The patch before the earliest Ply-Patch annotated commit is the last
         upstream commit.
         """
-        applied = list(self._applied_patches())
+        applied = self._applied_patches()
         if not applied:
             return None
 
         last_applied_hash = applied[-1][0]
         return self.log(cmd_arg='%s^' % last_applied_hash,
                         count=1, pretty='%H').strip()
 
-    def _applied_patches(self):
+    def _applied_patches(self, new_upper_bound=50):
         """Return a list of patches that have already been applied to this
         branch.
 
-        We figure this out by walking backwards from HEAD until we reach a
-        commit without a 'Ply-Patch' commit msg annotation.
+        We can have 3 types of commits, upstream (U), applied (A), and new
+        (N), which makes the history look like:
+
+                        UUU...U    ->   AAA...A    ->    NNN..N
+
+        In theory, any number of 'new' commits could exist, meaning we'd
+        potentially have to search ALL commits in order to determine if any
+        patches have been applied.
+
+        To avoid this costly operation, an upper-bound is given such that if
+        we don't find an applied patch within that number of queries, we
+        consider no patches as having been applied.
         """
+        applied = []
+
         for commit_hash, commit_msg in self._walk_commit_msgs_backwards():
             patch_name = utils.get_patch_annotation(commit_msg)
-            if not patch_name:
+
+            if patch_name:
+                # Patch found, must be within A
+                applied.append((commit_hash, patch_name))
+            elif applied:
+                # Applied patches, but this isn't one: must be at U/A border
                 break
-            yield commit_hash, patch_name
+            elif new_upper_bound > 0:
+                # No applied patches found yet: searching through N
+                new_upper_bound -= 1
+            else:
+                # Exhausted N search, assume no patches applied
+                raise exc.NoPatchesApplied
+
+        return applied
 
     def _store_patch_files(self, patch_names, filenames,
                            parent_patch_name=None):
@@ -346,11 +370,17 @@ def _create_patches(self, since):
         parent_patch_name = utils.get_patch_annotation(commit_msg)
         return filenames, parent_patch_name
 
-    def save(self, since, prefix=None):
+    def save(self, since=None, prefix=None):
         """Save a series of commits as patches into the patch-repo."""
         if self.uncommitted_changes() or self.patch_repo.uncommitted_changes():
             raise exc.UncommittedChanges
 
+        if not since:
+            since = self._last_upstream_commit_hash()
+
+        if not since:
+            raise exc.NoPatchesApplied
+
         if '..' in since:
             raise ValueError(".. not supported at the moment")
 
@@ -403,7 +433,7 @@ def status(self):
         if os.path.exists(self._patch_conflict_path):
             return 'restore-in-progress'
 
-        if len(list(self._applied_patches())) == 0:
+        if len(self._applied_patches()) == 0:
             return 'no-patches-applied'
 
         return 'all-patches-applied'

plypatch/cli.py

@@ -166,13 +166,15 @@ class SaveCommand(CLICommand):
     __command__ = 'save'
 
     def add_arguments(self, subparser):
-        subparser.add_argument('since', action='store')
-        subparser.add_argument('--prefix', action='store')
+        subparser.add_argument('-s', '--since')
+        subparser.add_argument('-p', '--prefix')
 
     def do(self, args):
         """Save set of commits to patch-repo"""
         try:
             self.working_repo.save(args.since, prefix=args.prefix)
+        except plypatch.exc.NoPatchesApplied:
+            die('No patches applied, so cannot detect new patches to save')
         except plypatch.exc.UncommittedChanges:
             die_on_uncommitted_changes()
 

tests/functional/test_everything.py

@@ -275,6 +275,38 @@ def test_ply_based_on_annotation(self):
         # '[s]aving' should be in commit msg, not 'refreshing'
         self.assertIn('aving', commit_msg)
 
+    def test_save_no_since_supplied(self):
+        """A `save` without a `since` argument is a shortcut for save
+        <upstream-hash>.
+
+        If no patches have been applied, then `save` without `since` should
+        raise an exception.
+        """
+        self.write_readme('Now is the time for all good men to come to the'
+                          ' aid of their country.',
+                          commit_msg='There -> Their')
+
+        self.working_repo.save(self.upstream_hash)
+        self.assert_based_on(self.upstream_hash)
+        self.assertEqual(
+            self.upstream_hash, self.working_repo._last_upstream_commit_hash())
+
+        self.write_readme('Now is the time for all good men to come to the'
+                          ' aid of their country!',
+                          commit_msg='Add exclamation point!')
+
+        self.working_repo.save()
+
+        self.working_repo.rollback()
+
+        self.assert_readme('Now is the time for all good men to come to'
+                           ' the aid of there country.')
+
+        self.working_repo.restore()
+
+        self.assert_readme('Now is the time for all good men to come to'
+                           ' the aid of their country!')
+
 
 if __name__ == '__main__':
     unittest.main()