Patchwork D3887: rebase: add --no-backup option

login
register
mail settings
Submitter phabricator
Date July 5, 2018, 5:38 a.m.
Message ID <differential-rev-PHID-DREV-zddxordyk54zv7yjflmd-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/32622/
State Superseded
Headers show

Comments

phabricator - July 5, 2018, 5:38 a.m.
khanchi97 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Added --no-backup option which gives functionality to
  not save backup copies of files.

REPOSITORY
  rHG Mercurial

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

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

CHANGE DETAILS




To: khanchi97, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - July 5, 2018, 1:07 p.m.
>   Added --no-backup option which gives functionality to
>   not save backup copies of files.

Can you elaborate why we want this option?

I'm -0 on this because it's unclear when the --no-backup option makes a
difference. --abort isn't the only way to strip revisions while rebasing.
phabricator - July 5, 2018, 1:07 p.m.
yuja added a comment.


  >   Added --no-backup option which gives functionality to
  >   not save backup copies of files.
  
  Can you elaborate why we want this option?
  
  I'm -0 on this because it's unclear when the --no-backup option makes a
  difference. --abort isn't the only way to strip revisions while rebasing.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - July 5, 2018, 2:01 p.m.
khanchi97 added a comment.


  In https://phab.mercurial-scm.org/D3887#60749, @yuja wrote:
  
  > >   Added --no-backup option which gives functionality to
  > >   not save backup copies of files.
  >
  > Can you elaborate why we want this option?
  
  
  The idea was to add --no-backup option in all those commands which alter history. For this one, let say if someone wants to abort a rebase in between then a --no-backup option should be available (same as histedit)
  
  > I'm -0 on this because it's unclear when the --no-backup option makes a
  >  difference. --abort isn't the only way to strip revisions while rebasing.
  
  Oh, I thought --abort is the only way to strip revisions. Shall I try to cover all other cases also where we strip csets; Or If it is not required I can leave it here.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: yuja, mercurial-devel
Yuya Nishihara - July 5, 2018, 2:40 p.m.
> The idea was to add --no-backup option in all those commands which alter
> history. For this one, let say if someone wants to abort a rebase in between
> then a --no-backup option should be available (same as histedit)

If he don't want any backup bundle on history editing, it might be better
to add a config knob to turn off the feature than forcing him to always
specify --no-backup.

histedit and rebase are different from strip in that the user doesn't intend
to "remove" changesets.

> Oh, I thought --abort is the only way to strip revisions.

Pre-obsmarker rebase will strip rebased revisions.

> Shall I try to cover all other cases also where we strip csets; Or If it is
> not required I can leave it here.

I don't have strong opinion about that, but it's probably better to add a
test with some comment saying --no-backup doesn't work and should be fixed
later.
phabricator - July 5, 2018, 2:42 p.m.
yuja added a comment.


  > The idea was to add --no-backup option in all those commands which alter
  >  history. For this one, let say if someone wants to abort a rebase in between
  >  then a --no-backup option should be available (same as histedit)
  
  If he don't want any backup bundle on history editing, it might be better
  to add a config knob to turn off the feature than forcing him to always
  specify --no-backup.
  
  histedit and rebase are different from strip in that the user doesn't intend
  to "remove" changesets.
  
  > Oh, I thought --abort is the only way to strip revisions.
  
  Pre-obsmarker rebase will strip rebased revisions.
  
  > Shall I try to cover all other cases also where we strip csets; Or If it is
  >  not required I can leave it here.
  
  I don't have strong opinion about that, but it's probably better to add a
  test with some comment saying --no-backup doesn't work and should be fixed
  later.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - July 5, 2018, 6:41 p.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D3887#60752, @yuja wrote:
  
  > > The idea was to add --no-backup option in all those commands which alter
  > >  history. For this one, let say if someone wants to abort a rebase in between
  > >  then a --no-backup option should be available (same as histedit)
  >
  > If he don't want any backup bundle on history editing, it might be better
  >  to add a config knob to turn off the feature than forcing him to always
  >  specify --no-backup.
  
  
  I agree with Yuya here that a config option is better here.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: pulkit, yuja, mercurial-devel
phabricator - July 6, 2018, 5 a.m.
khanchi97 added a comment.


  @yuja @pulkit Definitely, adding a config option is better than any other alternative. Can I start to work on this? (I mean adding config option which will work for all history editing commands)

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: pulkit, yuja, mercurial-devel
Yuya Nishihara - July 7, 2018, 2:50 a.m.
>   @yuja @pulkit Definitely, adding a config option is better than any other alternative. Can I start to work on this? (I mean adding config option which will work for all history editing commands)

Sounds good to me, though I don't have any good name for the config option.

To be clear, I think `hg strip` should be out of the control of the config
option to be added, because the strip in that case isn't a side effect of
history editing.
phabricator - July 7, 2018, 2:51 a.m.
yuja added a comment.


  >   @yuja @pulkit Definitely, adding a config option is better than any other alternative. Can I start to work on this? (I mean adding config option which will work for all history editing commands)
  
  Sounds good to me, though I don't have any good name for the config option.
  
  To be clear, I think `hg strip` should be out of the control of the config
  option to be added, because the strip in that case isn't a side effect of
  history editing.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: pulkit, yuja, mercurial-devel
phabricator - Aug. 2, 2018, 3:52 p.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D3887#62825, @yuja wrote:
  
  > Queued, thanks.
  >
  > > @@ -829,6 +833,8 @@
  > > 
  > >   userrevs = list(repo.revs(opts.get('auto_orphans')))
  > >   opts['rev'] = [revsetlang.formatspec('%ld and orphan()', userrevs)]
  > >   opts['dest'] = '_destautoorphanrebase(SRC)'
  > > 
  > > +    backup = ui.configbool('ui', 'history-editing-backup')
  > >  +    opts['backup'] = backup
  >
  > This seems getting ugly. Maybe the option can be carried by rbsrt instead?
  >
  >   self.backupf = ui.configbool('ui', 'history-editing-backup')
  >   
  
  
  I like Yuya's suggestion here. @khanchi97 can you please follow-up?

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: pulkit, yuja, mercurial-devel
phabricator - Aug. 2, 2018, 6:04 p.m.
khanchi97 added a comment.


  In https://phab.mercurial-scm.org/D3887#62850, @pulkit wrote:
  
  > In https://phab.mercurial-scm.org/D3887#62825, @yuja wrote:
  >
  > > Queued, thanks.
  > >
  > > > @@ -829,6 +833,8 @@
  > > > 
  > > >   userrevs = list(repo.revs(opts.get('auto_orphans')))
  > > >   opts['rev'] = [revsetlang.formatspec('%ld and orphan()', userrevs)]
  > > >   opts['dest'] = '_destautoorphanrebase(SRC)'
  > > > 
  > > > +    backup = ui.configbool('ui', 'history-editing-backup')
  > > >  +    opts['backup'] = backup
  > >
  > > This seems getting ugly. Maybe the option can be carried by rbsrt instead?
  > >
  > >   self.backupf = ui.configbool('ui', 'history-editing-backup')
  > >   
  >
  >
  > I like Yuya's suggestion here. @khanchi97 can you please follow-up?
  
  
  Okay, I will send a follow-up.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: pulkit, yuja, 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
@@ -501,3 +501,50 @@ 
   rebase aborted
   $ cd ..
 
+=========================
+Test --no-backup option |
+=========================
+  $ hg init nobackup
+  $ cd nobackup
+  $ echo a>a
+  $ hg ci -qAma
+  $ echo b>b
+  $ hg ci -qAmb
+  $ echo c>c
+  $ hg ci -qAmc
+  $ hg up 0 -q
+  $ echo conflict>c
+  $ hg ci -Am "conflict with c"
+  adding c
+  created new head
+  $ hg log -GT "{rev}: {firstline(desc)}\n"
+  @  3: conflict with c
+  |
+  | o  2: c
+  | |
+  | o  1: b
+  |/
+  o  0: a
+  
+  $ hg rebase -s 1 -d .
+  rebasing 1:d2ae7f538514 "b"
+  rebasing 2:177f92b77385 "c"
+  merging c
+  warning: conflicts while merging c! (edit, then use 'hg resolve --mark')
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+When --no-backup flag is not passed:
+  $ hg rebase --abort
+  saved backup bundle to $TESTTMP/nobackup/.hg/strip-backup/518b9bc3afb6-6c356d34-backup.hg
+  rebase aborted
+  $ hg rebase -s 1 -d .
+  rebasing 1:d2ae7f538514 "b"
+  rebasing 2:177f92b77385 "c"
+  merging c
+  warning: conflicts while merging c! (edit, then use 'hg resolve --mark')
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+When --no-backup flag is passed:
+  $ hg rebase --abort --no-backup
+  rebase aborted
+  $ cd ..
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -677,6 +677,7 @@ 
     ('a', 'abort', False, _('abort an interrupted rebase')),
     ('', 'auto-orphans', '', _('automatically rebase orphan revisions '
                                'in the specified revset (EXPERIMENTAL)')),
+    ('', 'no-backup', False, _('do not save backup copies of files')),
      ] + cmdutil.dryrunopts + cmdutil.formatteropts,
     _('[-s REV | -b REV] [-d REV] [OPTION]'))
 def rebase(ui, repo, **opts):
@@ -879,6 +880,7 @@ 
         destspace = opts.get('_destspace')
         contf = opts.get('continue')
         abortf = opts.get('abort')
+        backup = not opts.get('no_backup')
         if opts.get('interactive'):
             try:
                 if extensions.find('histedit'):
@@ -909,7 +911,7 @@ 
                 ms = mergemod.mergestate.read(repo)
                 mergeutil.checkunresolved(ms)
 
-            retcode = rbsrt._prepareabortorcontinue(abortf)
+            retcode = rbsrt._prepareabortorcontinue(abortf, backup=backup)
             if retcode is not None:
                 return retcode
         else: