threads / discuss / 11432

[STGIT] stg refresh wish (splitting patches/removing files from a patch)

Subject: [STGIT] stg refresh wish (splitting patches/removing files from a patch)

## tl;dr

7 messages between Dec 30, 2007 and Jan 10, 2008.

replies: 6people: 2as markdown or json

Peter Oberndorfer· Dec 30, 2007, 19:03 UTC · lore

Hi, I recently tried to split a stgit patch into 2 parts and it was not as easy as i would like it to be.

How do i exclude a file from a patch(use version of file present in HEAD^) without modifying the working dir?

with plain git i would use something like git reset HEAD^ files_i_do_not_want_in_first_patch git commit --amend git add files_i_do_not_want_in_first_patch git commit

So my idea was to add a --use-index [1] option to stg refresh When it is passed stg refresh will use the current index for the contenst of the refreshed patch instead of looking at the working dir. This would solve my problem[2] and also make it possible to use git-gui for staging hunks.

Do you think this would be a useful/good idea? Or do we want a separate command for removing files from a patch anyway?

Another thing that might be useful (in my scenario) would be a stg commit --top extension which commits at the top end of the stack (unfortunately this will loose the patch history for splitting commits) then i can edit this commits without being afraid of confusing stgit and then stg assimilate /stg repair to make them managed by stg again

Greetings Peter
[1] rename by desire

[2] new way for splitting a patch with extension git reset HEAD^ files_i_do_not_want_in_first_patch stg refresh --use-index stg refresh -e git add files_i_do_not_want_in_first_patch stg new stg refresh --use-index

Peter Oberndorfer· Jan 2, 2008, 19:39 UTC · re: Peter Oberndorfer · lore

[STG PATCH] refresh: add a --index option which takes the contents of the index as the new commit

just like git commit would do without the -a option --- On Sonntag 30 Dezember 2007, Peter Oberndorfer wrote:

Show 19 quoted lines
> Hi,
> I recently tried to split a stgit patch into 2 parts
> and it was not as easy as i would like it to be.
> 
> How do i exclude a file from a patch(use version of file present in HEAD^)
> without modifying the working dir?
> 
> with plain git i would use something like
> git reset HEAD^ files_i_do_not_want_in_first_patch
> git commit --amend
> git add files_i_do_not_want_in_first_patch
> git commit
> 
> So my idea was to add a --use-index [1] option to stg refresh
> When it is passed stg refresh will use the current index for the contenst of the refreshed patch 
> instead of looking at the working dir.
> This would solve my problem[2] and also make it possible to use git-gui for 
> staging hunks.
> 

OK, i got off my ass and hacked together the following patch based on git://repo.or.cz/stgit/kha.git experimental And i used it to create and update this commit a few times :-) Additionally it passes the (minimal) test i added. But since i am not a stgit expert it is highly recommend that somebody else carefully looks at the patch before going wild on real data.

> Do you think this would be a useful/good idea?
> Or do we want a separate command for removing files from a patch anyway?
The question is still open if this is useful for somebody else.
Greetings Peter
PS: i hope i put the right people on CC
 stgit/commands/refresh.py |   25 ++++++++++++++++---
 stgit/stack.py            |    6 ++++
 t/t2700-refresh.sh        |   57 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 83 insertions(+), 5 deletions(-)
diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py
index 6e8ed0c..0420b98 100644
--- a/stgit/commands/refresh.py
+++ b/stgit/commands/refresh.py
@@ -45,6 +45,9 @@ options = [make_option('-f', '--force',
            make_option('--update',
                        help = 'only update the current patch files',
                        action = 'store_true'),
+           make_option('--index',
+                       help = 'use the current contents of the index instead of looking at the working directory',
+                       action = 'store_true'),
            make_option('--undo',
                        help = 'revert the commit generated by the last refresh',
                        action = 'store_true'),
@@ -76,6 +79,14 @@ def func(parser, options, args):
         if not patch:
             raise CmdException, 'No patches applied'
 
+    if options.index:
+        if args or options.update:
+            raise CmdException, \
+                  'Only full refresh is available with the --index option'
+        if options.patch:
+            raise CmdException, \
+                  '--patch is not (yet) compatible with --index option'
+
     if not options.force:
         check_head_top_equal(crt_series)
 
@@ -85,9 +96,10 @@ def func(parser, options, args):
         out.done()
         return
 
-    files = [path for (stat, path) in git.tree_status(files = args, verbose = True)]
+    if not options.index:
+        files = [path for (stat, path) in git.tree_status(files = args, verbose = True)]
 
-    if files or not crt_series.head_top_equal():
+    if options.index or files or not crt_series.head_top_equal():
         if options.patch:
             applied = crt_series.get_applied()
             between = applied[:applied.index(patch):-1]
@@ -105,8 +117,13 @@ def func(parser, options, args):
 
         if autoresolved == 'yes':
             resolved_all()
-        crt_series.refresh_patch(files = files,
-                                 backup = True, notes = options.annotate)
+
+        if options.index:
+            crt_series.refresh_patch(use_index = True,
+                                     backup = True, notes = options.annotate)
+        else:
+            crt_series.refresh_patch(files = files,
+                                     backup = True, notes = options.annotate)
 
         if crt_series.empty_patch(patch):
             out.done('empty patch')
diff --git a/stgit/stack.py b/stgit/stack.py
index 4203931..7d14261 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -668,6 +668,7 @@ class Series(PatchSet):
         config.remove_section('branch.%s.stgit' % self.get_name())
 
     def refresh_patch(self, files = None, message = None, edit = False,
+                      use_index = False,
                       empty = False,
                       show_patch = False,
                       cache_update = True,
@@ -717,6 +718,11 @@ class Series(PatchSet):
         else:
             tree_id = None
 
+        if use_index:
+            tree_id = None
+            files = None
+            cache_update = False
+
         commit_id = git.commit(files = files,
                                message = descr, parents = [bottom],
                                cache_update = cache_update,
diff --git a/t/t2700-refresh.sh b/t/t2700-refresh.sh
index 2e7901c..9eae85d 100755
--- a/t/t2700-refresh.sh
+++ b/t/t2700-refresh.sh
@@ -6,8 +6,10 @@ test_description='Run "stg refresh"'
 
 test_expect_success 'Initialize StGit stack' '
     stg init &&
-    echo expected.txt >> .git/info/exclude &&
+    echo expected*.txt >> .git/info/exclude &&
     echo patches.txt >> .git/info/exclude &&
+    echo show.txt >> .git/info/exclude &&
+    echo diff.txt >> .git/info/exclude &&
     stg new p0 -m "base" &&
     for i in 1 2 3; do
         echo base >> foo$i.txt &&
@@ -62,4 +64,57 @@ test_expect_success 'Refresh bottom patch' '
     diff -u expected.txt patches.txt
 '
 
+cat > expected.txt <<EOF
+p0
+p1
+p4
+EOF
+cat > expected2.txt <<EOF
+diff --git a/foo1.txt b/foo1.txt
+index 728535d..6f34984 100644
+--- a/foo1.txt
++++ b/foo1.txt
+@@ -1,3 +1,4 @@
+ base
+ foo 1
+ bar 1
++baz 1
+EOF
+cat > expected3.txt <<EOF
+diff --git a/foo1.txt b/foo1.txt
+index 6f34984..a80eb63 100644
+--- a/foo1.txt
++++ b/foo1.txt
+@@ -2,3 +2,4 @@ base
+ foo 1
+ bar 1
+ baz 1
++blah 1
+diff --git a/foo2.txt b/foo2.txt
+index 415c9f5..43168f2 100644
+--- a/foo2.txt
++++ b/foo2.txt
+@@ -1,3 +1,4 @@
+ base
+ foo 2
+ bar 2
++baz 2
+EOF
+test_expect_success 'Refresh --index' '
+    stg status &&
+    stg new p4 -m "refresh_index" &&
+    echo baz 1 >> foo1.txt &&
+    git add foo1.txt &&
+    echo blah 1 >> foo1.txt &&
+    echo baz 2 >> foo2.txt &&
+    stg refresh --index &&
+    stg patches foo1.txt > patches.txt &&
+    git diff HEAD^..HEAD > show.txt &&
+    stg diff > diff.txt &&
+    diff -u expected.txt patches.txt &&
+    diff -u expected2.txt show.txt &&
+    diff -u expected3.txt diff.txt &&
+    stg new p5 -m "cleanup again" &&
+    stg refresh
+'
 test_done
-- 
1.5.4.rc2
Karl Hasselström· Jan 7, 2008, 10:56 UTC · re: Peter Oberndorfer · lore

Re: [STG PATCH] refresh: add a --index option which takes the contents of the index as the new commit

On 2008-01-02 20:39:27 +0100, Peter Oberndorfer wrote:
Show 6 quoted lines
> On Sonntag 30 Dezember 2007, Peter Oberndorfer wrote:
>
> > Do you think this would be a useful/good idea? Or do we want a
> > separate command for removing files from a patch anyway?
>
> The question is still open if this is useful for somebody else.
I think it's a useful addition. Thanks!
Show 24 quoted lines
> diff --git a/stgit/stack.py b/stgit/stack.py
> index 4203931..7d14261 100644
> --- a/stgit/stack.py
> +++ b/stgit/stack.py
> @@ -668,6 +668,7 @@ class Series(PatchSet):
>          config.remove_section('branch.%s.stgit' % self.get_name())
>  
>      def refresh_patch(self, files = None, message = None, edit = False,
> +                      use_index = False,
>                        empty = False,
>                        show_patch = False,
>                        cache_update = True,
> @@ -717,6 +718,11 @@ class Series(PatchSet):
>          else:
>              tree_id = None
>  
> +        if use_index:
> +            tree_id = None
> +            files = None
> +            cache_update = False
> +
>          commit_id = git.commit(files = files,
>                                 message = descr, parents = [bottom],
>                                 cache_update = cache_update,

So the use_index parameter to refresh_patch is actually not necessary? In that case I'd rather you didn't add it, since the functions in stgit/stack.py have quite enough parameters already.

> diff --git a/t/t2700-refresh.sh b/t/t2700-refresh.sh
> index 2e7901c..9eae85d 100755
> --- a/t/t2700-refresh.sh
> +++ b/t/t2700-refresh.sh
Bonus points for adding a test case!

I still haven't rebased my patch stack since Catalin accepted most of it just before Christmas. Once I've gotten around to that, I'll take your patch -- hopefully by then updated to not add the exra argument to refresh_patch(). :-)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
Karl Hasselström· Jan 7, 2008, 10:59 UTC · re: Karl Hasselström · lore

Re: [STG PATCH] refresh: add a --index option which takes the contents of the index as the new commit

On 2008-01-07 11:56:12 +0100, Karl Hasselström wrote:
> hopefully by then updated to not add the exra argument to
> refresh_patch(). :-)
And with a sign-off, too, please.
-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
Peter Oberndorfer· Jan 8, 2008, 20:42 UTC · re: Karl Hasselström · lore

[STG PATCH] add a --index option to refresh which takes the contents of the index as the new commit

This allows to stage only certain changes to a file by only adding the desired parts to the index with git-gui, ugit, git add -i or another tool that manipulates the index and then run stg refresh --index it also allows removing a file from a patch by running git reset HEAD^ -- file_to_remove followed by a stg refresh --index

Signed-off-by: Peter Oberndorfer <kumbayo84@arcor.de>
---
On Montag 07 Januar 2008, Karl Hasselström wrote:
Show 10 quoted lines
> On 2008-01-02 20:39:27 +0100, Peter Oberndorfer wrote:
> 
> > On Sonntag 30 Dezember 2007, Peter Oberndorfer wrote:
> >
> > > Do you think this would be a useful/good idea? Or do we want a
> > > separate command for removing files from a patch anyway?
> >
> > The question is still open if this is useful for somebody else.
> 
> I think it's a useful addition. Thanks!
Good since it was useful for me too even while writing this patch :-)
> So the use_index parameter to refresh_patch is actually not necessary?
> In that case I'd rather you didn't add it, since the functions in
> stgit/stack.py have quite enough parameters already.
> 

In the beginning i was afraid it would be to obscure to call it this way with all parameters set to some specific values. But having more parameters does not make it better :-) Done

Show 12 quoted lines
> > diff --git a/t/t2700-refresh.sh b/t/t2700-refresh.sh
> > index 2e7901c..9eae85d 100755
> > --- a/t/t2700-refresh.sh
> > +++ b/t/t2700-refresh.sh
> 
> Bonus points for adding a test case!
> 
> I still haven't rebased my patch stack since Catalin accepted most of
> it just before Christmas. Once I've gotten around to that, I'll take
> your patch -- hopefully by then updated to not add the exra argument
> to refresh_patch(). :-)
> 

Patch now comes with a Signed-off-by and a log message that explains how this feature could be used. It was tested with the testcase, used during development of this patch and on another repo, but still take care when using it :-)

 stgit/commands/refresh.py |   25 ++++++++++++++++---
 t/t2700-refresh.sh        |   57 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 77 insertions(+), 5 deletions(-)
diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py
index 6e8ed0c..952b1b6 100644
--- a/stgit/commands/refresh.py
+++ b/stgit/commands/refresh.py
@@ -45,6 +45,9 @@ options = [make_option('-f', '--force',
            make_option('--update',
                        help = 'only update the current patch files',
                        action = 'store_true'),
+           make_option('--index',
+                       help = 'use the current contents of the index instead of looking at the working directory',
+                       action = 'store_true'),
            make_option('--undo',
                        help = 'revert the commit generated by the last refresh',
                        action = 'store_true'),
@@ -76,6 +79,14 @@ def func(parser, options, args):
         if not patch:
             raise CmdException, 'No patches applied'
 
+    if options.index:
+        if args or options.update:
+            raise CmdException, \
+                  'Only full refresh is available with the --index option'
+        if options.patch:
+            raise CmdException, \
+                  '--patch is not compatible with the --index option'
+
     if not options.force:
         check_head_top_equal(crt_series)
 
@@ -85,9 +96,10 @@ def func(parser, options, args):
         out.done()
         return
 
-    files = [path for (stat, path) in git.tree_status(files = args, verbose = True)]
+    if not options.index:
+        files = [path for (stat, path) in git.tree_status(files = args, verbose = True)]
 
-    if files or not crt_series.head_top_equal():
+    if options.index or files or not crt_series.head_top_equal():
         if options.patch:
             applied = crt_series.get_applied()
             between = applied[:applied.index(patch):-1]
@@ -105,8 +117,13 @@ def func(parser, options, args):
 
         if autoresolved == 'yes':
             resolved_all()
-        crt_series.refresh_patch(files = files,
-                                 backup = True, notes = options.annotate)
+
+        if options.index:
+            crt_series.refresh_patch(cache_update = False,
+                                     backup = True, notes = options.annotate)
+        else:
+            crt_series.refresh_patch(files = files,
+                                     backup = True, notes = options.annotate)
 
         if crt_series.empty_patch(patch):
             out.done('empty patch')
diff --git a/t/t2700-refresh.sh b/t/t2700-refresh.sh
index 2e7901c..9eae85d 100755
--- a/t/t2700-refresh.sh
+++ b/t/t2700-refresh.sh
@@ -6,8 +6,10 @@ test_description='Run "stg refresh"'
 
 test_expect_success 'Initialize StGit stack' '
     stg init &&
-    echo expected.txt >> .git/info/exclude &&
+    echo expected*.txt >> .git/info/exclude &&
     echo patches.txt >> .git/info/exclude &&
+    echo show.txt >> .git/info/exclude &&
+    echo diff.txt >> .git/info/exclude &&
     stg new p0 -m "base" &&
     for i in 1 2 3; do
         echo base >> foo$i.txt &&
@@ -62,4 +64,57 @@ test_expect_success 'Refresh bottom patch' '
     diff -u expected.txt patches.txt
 '
 
+cat > expected.txt <<EOF
+p0
+p1
+p4
+EOF
+cat > expected2.txt <<EOF
+diff --git a/foo1.txt b/foo1.txt
+index 728535d..6f34984 100644
+--- a/foo1.txt
++++ b/foo1.txt
+@@ -1,3 +1,4 @@
+ base
+ foo 1
+ bar 1
++baz 1
+EOF
+cat > expected3.txt <<EOF
+diff --git a/foo1.txt b/foo1.txt
+index 6f34984..a80eb63 100644
+--- a/foo1.txt
++++ b/foo1.txt
+@@ -2,3 +2,4 @@ base
+ foo 1
+ bar 1
+ baz 1
++blah 1
+diff --git a/foo2.txt b/foo2.txt
+index 415c9f5..43168f2 100644
+--- a/foo2.txt
++++ b/foo2.txt
+@@ -1,3 +1,4 @@
+ base
+ foo 2
+ bar 2
++baz 2
+EOF
+test_expect_success 'Refresh --index' '
+    stg status &&
+    stg new p4 -m "refresh_index" &&
+    echo baz 1 >> foo1.txt &&
+    git add foo1.txt &&
+    echo blah 1 >> foo1.txt &&
+    echo baz 2 >> foo2.txt &&
+    stg refresh --index &&
+    stg patches foo1.txt > patches.txt &&
+    git diff HEAD^..HEAD > show.txt &&
+    stg diff > diff.txt &&
+    diff -u expected.txt patches.txt &&
+    diff -u expected2.txt show.txt &&
+    diff -u expected3.txt diff.txt &&
+    stg new p5 -m "cleanup again" &&
+    stg refresh
+'
 test_done
-- 
1.5.4.rc2
Karl Hasselström· Jan 9, 2008, 07:23 UTC · re: Peter Oberndorfer · lore

Re: [STG PATCH] add a --index option to refresh which takes the contents of the index as the new commit

On 2008-01-08 21:42:46 +0100, Peter Oberndorfer wrote:
Show 9 quoted lines
> On Montag 07 Januar 2008, Karl Hasselström wrote:
>
> > So the use_index parameter to refresh_patch is actually not
> > necessary? In that case I'd rather you didn't add it, since the
> > functions in stgit/stack.py have quite enough parameters already.
>
> In the beginning i was afraid it would be to obscure to call it this
> way with all parameters set to some specific values. But having more
> parameters does not make it better :-) Done
Thanks.
> Patch now comes with a Signed-off-by and a log message that explains
> how this feature could be used. It was tested with the testcase,
> used during development of this patch and on another repo, but still
> take care when using it :-)

I may be promising too much now, but hopefully I'll get to this tonight.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
Karl Hasselström· Jan 10, 2008, 00:08 UTC · re: Karl Hasselström · lore

Re: [STG PATCH] add a --index option to refresh which takes the contents of the index as the new commit

On 2008-01-09 08:23:58 +0100, Karl Hasselström wrote:
Show 9 quoted lines
> On 2008-01-08 21:42:46 +0100, Peter Oberndorfer wrote:
>
> > Patch now comes with a Signed-off-by and a log message that
> > explains how this feature could be used. It was tested with the
> > testcase, used during development of this patch and on another
> > repo, but still take care when using it :-)
>
> I may be promising too much now, but hopefully I'll get to this
> tonight.

I've rebased my patch stack on top of Catalin's master now, and put your two patches on top. The --index patch is only in experimental and not stable, since you recommend further testing.

I massaged the commit messages slightly, mainly to get a reasonably short first line and end all sentences with a period (except on the first line, of course!).

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

← back to recent threads