Patchwork D3756: rebase: no need to backup rebased csets while aborting

login
register
mail settings
Submitter phabricator
Date June 16, 2018, 1:47 p.m.
Message ID <differential-rev-PHID-DREV-ejyzqehhr4aqs2reqaih-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/32196/
State Superseded
Headers show

Comments

phabricator - June 16, 2018, 1:47 p.m.
khanchi97 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3756

AFFECTED FILES
  hgext/rebase.py
  tests/test-rebase-abort.t

CHANGE DETAILS




To: khanchi97, #hg-reviewers
Cc: mercurial-devel
phabricator - June 16, 2018, 8:15 p.m.
indygreg added subscribers: durin42, indygreg.
indygreg added a comment.


  I'm unsure about this change. On one hand, the comment (which appears to have been added to mpm several years ago) implies that we never should have generated backups in this case. On the other, one of the key rules of a VCS is "don't eat my data." Even though we are aborting the operation, I could see some scenarios where someone would want a backup of the aborted/partially-completed rebase.
  
  @durin42: what do you think?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3756

To: khanchi97, #hg-reviewers
Cc: indygreg, durin42, mercurial-devel
phabricator - June 18, 2018, 3:20 a.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D3756#58937, @indygreg wrote:
  
  > I'm unsure about this change. On one hand, the comment (which appears to have been added to mpm several years ago) implies that we never should have generated backups in this case. On the other, one of the key rules of a VCS is "don't eat my data." Even though we are aborting the operation, I could see some scenarios where someone would want a backup of the aborted/partially-completed rebase.
  >
  > @durin42: what do you think?
  
  
  I think the paranoia is probably better overall? Maybe update the comment?
  
  (I don't feel strongly.)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3756

To: khanchi97, #hg-reviewers
Cc: indygreg, durin42, mercurial-devel
sushil khanchi - June 18, 2018, 3:46 a.m.
okay, I think following the key rule of VCS would be better. Shall I update
the comment instead?

On Mon, Jun 18, 2018 at 8:50 AM, durin42 (Augie Fackler) <
phabricator@mercurial-scm.org> wrote:

> durin42 added a comment.
>
>
>   In https://phab.mercurial-scm.org/D3756#58937, @indygreg wrote:
>
>   > I'm unsure about this change. On one hand, the comment (which appears
> to have been added to mpm several years ago) implies that we never should
> have generated backups in this case. On the other, one of the key rules of
> a VCS is "don't eat my data." Even though we are aborting the operation, I
> could see some scenarios where someone would want a backup of the
> aborted/partially-completed rebase.
>   >
>   > @durin42: what do you think?
>
>
>   I think the paranoia is probably better overall? Maybe update the
> comment?
>
>   (I don't feel strongly.)
>
> REPOSITORY
>   rHG Mercurial
>
> REVISION DETAIL
>   https://phab.mercurial-scm.org/D3756
>
> To: khanchi97, #hg-reviewers
> Cc: indygreg, durin42, mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/tests/test-rebase-abort.t b/tests/test-rebase-abort.t
--- a/tests/test-rebase-abort.t
+++ b/tests/test-rebase-abort.t
@@ -115,7 +115,6 @@ 
 Abort (should clear out unsupported merge state):
 
   $ hg rebase --abort
-  saved backup bundle to $TESTTMP/a/.hg/strip-backup/3e046f2ecedb-6beef7d5-backup.hg
   rebase aborted
   $ hg debugmergestate
   no merge state found
@@ -406,7 +405,6 @@ 
   [255]
 
   $ hg rebase --abort
-  saved backup bundle to $TESTTMP/interrupted/.hg/strip-backup/3d8812cf300d-93041a90-backup.hg
   rebase aborted
   $ hg log -G --template "{rev} {desc} {bookmarks}"
   o  6 no-a
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1573,7 +1573,7 @@ 
             # Strip from the first rebased revision
             if rebased:
                 # no backup of rebased cset versions needed
-                repair.strip(repo.ui, repo, strippoints)
+                repair.strip(repo.ui, repo, strippoints, backup=False)
 
         if activebookmark and activebookmark in repo._bookmarks:
             bookmarks.activate(repo, activebookmark)