git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Some ideas for StGIT
@ 2007-08-03 17:50 Pavel Roskin
  2007-08-03 18:14 ` Andy Parkins
                   ` (3 more replies)
  0 siblings, 4 replies; 32+ messages in thread
From: Pavel Roskin @ 2007-08-03 17:50 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

Hello!

I was recently disappointed to learn that one of the Linux drivers
(bcm43xx_mac80211, to be precise) switched from git to quilt.  I asked
whether StGIT was considered, a discussion followed, and I think the key
points need to be shared with StGIT developers.  I'll add some of my
ideas to the mix.

The main point in favor of quilt is that it allows to edit the patches
with the text editor.  One can pop all patches, edit them and push the
all back.

I don't suggest that StGIT gives up on the git-based storage, but this
mode of operation could be implemented in two ways.

One is to have a command opposite to "export".  It would read the files
that "export" produces, replacing the existing patches.

Another approach would be to reexamine the patch after "stg refresh -es"
and to apply it instead of the original patch.  If the patch doesn't
apply, the options would be to discard the edits or to re-launch the
editor.

Next issue is that it should be possible to create a patch in one
operation.  StGIT follows quilt too closely here in requiring "new" and
"refresh", instead of utilizing the advantage of the workflow that
allows immediate editing of the sources without any commands.

Basically, I want one command that:

1) shows user what was changed
2) allows user to name the patch
3) allows user to describe the patch
4) allows user to exclude files from the patch
5) doesn't require another command to put the changes to the patch

I think the most natural approach would be to enhance "stg new".  I see
"stg new -s" is supposed to show the changes, but it's currently broken.
This is run in a clean StGIT repository with no patches:

$ stg new -s foo
Traceback (most recent call last):
  File "/home/proski/bin/stg", line 43, in <module>
    main()
  File "home/proski/lib/python2.5/site-packages/stgit/main.py", line 284, in main
  File "/usr/lib64/python2.5/new.py", line 82, in func
    
  File "home/proski/lib/python2.5/site-packages/stgit/stack.py", line 842, in new_patch
  File "home/proski/lib/python2.5/site-packages/stgit/stack.py", line 89, in edit_file
  File "home/proski/lib/python2.5/site-packages/stgit/stack.py", line 461, in get_patch
  File "home/proski/lib/python2.5/site-packages/stgit/stack.py", line 148, in __init__
  File "/usr/lib64/python2.5/posixpath.py", line 60, in join
    if b.startswith('/'):
AttributeError: 'NoneType' object has no attribute 'startswith'

Another backtrace in "stg new", also run in a clean StGIT repository with no patches:

$ EDITOR=true stg new todo           
Invoking the editor: "true .stgitmsg.txt" ... done
$ stg export -np
Checking for changes in the working directory ... done
Traceback (most recent call last):
  File "/home/proski/bin/stg", line 43, in <module>
    main()
  File "home/proski/lib/python2.5/site-packages/stgit/main.py", line 284, in main
  File "home/proski/lib/python2.5/site-packages/stgit/commands/export.py", line 137, in func
AttributeError: 'NoneType' object has no attribute 'strip'

Finally, it would be great to have TLS support in the mail command.
Mercurial has it, and looking at their mail.py, it doesn't seem to be
much work.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-03 17:50 Some ideas for StGIT Pavel Roskin
@ 2007-08-03 18:14 ` Andy Parkins
  2007-08-04  5:41   ` Pavel Roskin
  2007-08-03 23:23 ` Yann Dirson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 32+ messages in thread
From: Andy Parkins @ 2007-08-03 18:14 UTC (permalink / raw)
  To: git; +Cc: Pavel Roskin, Catalin Marinas

On Friday 2007, August 03, Pavel Roskin wrote:

> I don't suggest that StGIT gives up on the git-based storage, but this
> mode of operation could be implemented in two ways.

git's shiny new git rebase -i has removed, for me, those times when I needed 
stgit.  Perhaps those who've move from git to quilt would try again when 
1.5.3 is out with the magic that is "rebase -i".


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-03 17:50 Some ideas for StGIT Pavel Roskin
  2007-08-03 18:14 ` Andy Parkins
@ 2007-08-03 23:23 ` Yann Dirson
  2007-08-06  9:49   ` Catalin Marinas
  2007-08-04  6:38 ` Theodore Tso
  2007-08-06  9:36 ` Catalin Marinas
  3 siblings, 1 reply; 32+ messages in thread
From: Yann Dirson @ 2007-08-03 23:23 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Catalin Marinas, git

On Fri, Aug 03, 2007 at 01:50:10PM -0400, Pavel Roskin wrote:
> One is to have a command opposite to "export".  It would read the files
> that "export" produces, replacing the existing patches.

This should be already possible (although I never used it), with "stg
pop -a && stg import --replace ..."

> Another approach would be to reexamine the patch after "stg refresh -es"
> and to apply it instead of the original patch.  If the patch doesn't
> apply, the options would be to discard the edits or to re-launch the
> editor.

Added to wishes: https://gna.org/bugs/index.php?9674


> Next issue is that it should be possible to create a patch in one
> operation.  StGIT follows quilt too closely here in requiring "new" and
> "refresh", instead of utilizing the advantage of the workflow that
> allows immediate editing of the sources without any commands.
> 
> Basically, I want one command that:
> 
> 1) shows user what was changed
> 2) allows user to name the patch
> 3) allows user to describe the patch
> 4) allows user to exclude files from the patch
> 5) doesn't require another command to put the changes to the patch
> 
> I think the most natural approach would be to enhance "stg new".

Sure, something like this could be done.  A syntax like the following
would IMHO fit in how things are done, but does not exactly address 4:

$ stg new <name> -m <msg> -s [--] <files> <to> <add>

Maybe another --exclude flag to reverse the meaning of the listed
files would be a solution, but I'm not thrilled by this idea...


>  I see
> "stg new -s" is supposed to show the changes, but it's currently broken.
> This is run in a clean StGIT repository with no patches:
> 
> $ stg new -s foo

Hm, I'm not sure what -s would be supposed to show here, since we're
asking for the creation of a patch, which currently always starts
empty.

Especially confusing is that if there are already applied patches, the
diff shown is the one of the previous top patch - and if there is no
applied patches, we get the exception you noticed.

I guess -s should be removed for 0.13.1.


> Another backtrace in "stg new", also run in a clean StGIT repository with no patches:

This appears to occur when there is no description file, or when it is
empty.  Thanks for the report.

I also tried with "stg refresh -m ''" to see if it caused the same
problem, but it appears to have another problem instead: it does not
refresh the patch description at all.

My guess is that we should not allow empty patch description (and
maybe fill it with provided patchname).  What did you want to acheieve
precisely with that command ?


> Finally, it would be great to have TLS support in the mail command.
> Mercurial has it, and looking at their mail.py, it doesn't seem to be
> much work.

Added to wishes: https://gna.org/bugs/index.php?9673

Thanks,
-- 
Yann

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-03 18:14 ` Andy Parkins
@ 2007-08-04  5:41   ` Pavel Roskin
  2007-08-04  5:51     ` Shawn O. Pearce
                       ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Pavel Roskin @ 2007-08-04  5:41 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git, Catalin Marinas

Hello, Andy!

On Fri, 2007-08-03 at 19:14 +0100, Andy Parkins wrote:
> On Friday 2007, August 03, Pavel Roskin wrote:
> 
> > I don't suggest that StGIT gives up on the git-based storage, but this
> > mode of operation could be implemented in two ways.
> 
> git's shiny new git rebase -i has removed, for me, those times when I needed 
> stgit.  Perhaps those who've move from git to quilt would try again when 
> 1.5.3 is out with the magic that is "rebase -i".

I don't understand how one option can replace StGIT.  I assume you were
trying to avoid StGIT already, and "git-rebase -i" was just the last
missing piece.

It would be great if you could tell me how your approach would deal with
the issue of editable patches I mentioned already.  In case I was
unclear, here's the quote from one of the developers:

[quote]
Sometimes, I just make patches in quilt, then I do "quilt 
refresh", "quilt pop -a", "cd patches" and modify the patches 
and series file manually, e.g. by moving one patch from one file 
into the other. The "cd ..", "quilt push -a" and off I am. That 
the "database" of quilt is in a known format and I can hack on 
it with an editor is a plus for me :-)
[end of quote]

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04  5:41   ` Pavel Roskin
@ 2007-08-04  5:51     ` Shawn O. Pearce
  2007-08-05  0:08       ` Pavel Roskin
  2007-08-05  0:17       ` Jakub Narebski
  2007-08-04  8:08     ` Yann Dirson
  2007-08-04 14:14     ` Chris Shoemaker
  2 siblings, 2 replies; 32+ messages in thread
From: Shawn O. Pearce @ 2007-08-04  5:51 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Andy Parkins, git, Catalin Marinas

Pavel Roskin <proski@gnu.org> wrote:
> 
> On Fri, 2007-08-03 at 19:14 +0100, Andy Parkins wrote:
> > On Friday 2007, August 03, Pavel Roskin wrote:
> > 
> > > I don't suggest that StGIT gives up on the git-based storage, but this
> > > mode of operation could be implemented in two ways.
> > 
> > git's shiny new git rebase -i has removed, for me, those times when I needed 
> > stgit.  Perhaps those who've move from git to quilt would try again when 
> > 1.5.3 is out with the magic that is "rebase -i".

I agree with Andy.  Aside from the performance issues that I
am currently having with a 55 patch series, "rebase -i" (and its
predecessor script from Dscho) have been a major part of my toolkit,
to the point that I really don't need something like StGIT on
my system.

(Regarding the performance, cherry-picking 55 patches is
slow, especially when many of them would apply trivially with
git-diff|git-apply --index.  Be nice to improve that in 1.5.4.)
 
> I don't understand how one option can replace StGIT.  I assume you were
> trying to avoid StGIT already, and "git-rebase -i" was just the last
> missing piece.

Oh, I'm sure there's features in StGIT that are useful that aren't
available via "rebase -i".  But to be honest, "rebase -i" is good
enough.  It just ain't fast enough.  Editing a patch that is 50
back in the series *sucks*.
 
> It would be great if you could tell me how your approach would deal with
> the issue of editable patches I mentioned already.  In case I was
> unclear, here's the quote from one of the developers:
> 
> [quote]
> Sometimes, I just make patches in quilt, then I do "quilt 
> refresh", "quilt pop -a", "cd patches" and modify the patches 
> and series file manually, e.g. by moving one patch from one file 
> into the other. The "cd ..", "quilt push -a" and off I am. That 
> the "database" of quilt is in a known format and I can hack on 
> it with an editor is a plus for me :-)
> [end of quote]

Uh, the "database" of "rebase -i" is just a chain of commits in a
git repository.  These are a well known format and can be easily
edited with "rebase -i".  This is a real plus for me as the series
can be edited directly in my favorite vi clone, then applied to my
working directory.  ;-)

-- 
Shawn.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-03 17:50 Some ideas for StGIT Pavel Roskin
  2007-08-03 18:14 ` Andy Parkins
  2007-08-03 23:23 ` Yann Dirson
@ 2007-08-04  6:38 ` Theodore Tso
  2007-08-04  8:16   ` Yann Dirson
  2007-08-04 21:35   ` Josef Sipek
  2007-08-06  9:36 ` Catalin Marinas
  3 siblings, 2 replies; 32+ messages in thread
From: Theodore Tso @ 2007-08-04  6:38 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Catalin Marinas, git

On Fri, Aug 03, 2007 at 01:50:10PM -0400, Pavel Roskin wrote:
> Hello!
> 
> I was recently disappointed to learn that one of the Linux drivers
> (bcm43xx_mac80211, to be precise) switched from git to quilt.  I asked
> whether StGIT was considered, a discussion followed, and I think the key
> points need to be shared with StGIT developers.  I'll add some of my
> ideas to the mix.

You might also ask them if they considered "guilt", which uses
text-based patches.  It's a lot easier to use, and if they really want
quilt-like functionality, "guilt" will provide it.

My main reason for avoiding StGIT is the fact that at least in the
past, I've found it very fragile when I forget and use "git checkout"
instead of "stg branch" to switch between branches.  The fact that
sometimes you have to use the StGit variant of basic git commands has
always struck me as confusing, and then recovering when things get
screwed can be exciting.  Usually it's when I start having to cut and
paste SHA1 hashes and running diffs to recreate my patch series after
things get screwed is usually about the time when I wonder why I tried
using StGIT again.  (Don't get me wrong; I'm sure I did something
really stupid, and wrong, that caused things to get screwed up.  My
complaint is that when I do something stupid, StGIT isn't robust and
is painful to recover from.)

That's why I like guilt; it doesn't require that you use alternate stg
commands for manipulating branches, and since it maintains an external
set of text patches, recovering is much easier; worst case I can just
do a "git reset --hard" and then follow it up with a guilt push -a.
And because it's so stupid simple, it's much rarer that something goes
seriously wrong that requires me to use a recovery procedure in the
first place.

Editing patch headers are also a lot easier with guilt; you can just
go ahead and edit them all, and then you can fresh them in git by
doing a "guilt pop -a; guilt push -a".  Since it's not using a
git-based storage, it doesn't have to rewrite the whole patch stack
when you modify a single patch header; you can edit a whole bunch of
text headers and then refresh the git commit series just once.

Of course, that's just my preference; others may find StGIT more
convenient, or folks may find the new rebase -i to be better yet.
YMMV.

     	  		    	 	- Ted

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04  5:41   ` Pavel Roskin
  2007-08-04  5:51     ` Shawn O. Pearce
@ 2007-08-04  8:08     ` Yann Dirson
  2007-08-06 10:01       ` Catalin Marinas
  2007-08-04 14:14     ` Chris Shoemaker
  2 siblings, 1 reply; 32+ messages in thread
From: Yann Dirson @ 2007-08-04  8:08 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Andy Parkins, git, Catalin Marinas

On Sat, Aug 04, 2007 at 01:41:25AM -0400, Pavel Roskin wrote:
> [quote]
> Sometimes, I just make patches in quilt, then I do "quilt 
> refresh", "quilt pop -a", "cd patches" and modify the patches 
> and series file manually, e.g. by moving one patch from one file 
> into the other.
> [end of quote]

FWIW, I have written a couple of scripts to help moving stuff around
between patches.  Those are not yet integrated in stgit proper, and it
happens that the 0.13 tarball does not contain them, they are only
available from the git tree (better use my tree[1], since I updated them
recently).

Most notably relevant to this use are stg-fold-files-from and
stg-dispatch, to move diff hunks between patches.  They only cases
(off the top of my hand) where they do not fit my needs are:
 - when I need to move a part of a diff hunk that is not possible to
 isolate using -U<n> (but I have read interesting things about git-gui
 for such functionnality, so that will surely come one day)
 - when I need to move git-specific diff hunks (moves, permissions,
 etc.), since it uses filterdiff, which is not git-aware (yet ?)

(in short, there are lots of dev to do in/around stgit, but there are
not as many contributors as there is for git - hint, hint ;)

If there are other typical situations where they need to edit patches,
I'd be interested to hear about them.  Not to avoid implementing patch
edition in stgit, since it is occasionally useful to fix a typo when
reviewing at refresh time, but to see what higher-level tools we could
provide.

Best regards,
-- 
Yann

[1] gitweb at http://repo.or.cz/w/stgit/ydirson.git

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04  6:38 ` Theodore Tso
@ 2007-08-04  8:16   ` Yann Dirson
  2007-08-04 21:35   ` Josef Sipek
  1 sibling, 0 replies; 32+ messages in thread
From: Yann Dirson @ 2007-08-04  8:16 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Pavel Roskin, Catalin Marinas, git

On Sat, Aug 04, 2007 at 02:38:58AM -0400, Theodore Tso wrote:
> My main reason for avoiding StGIT is the fact that at least in the
> past, I've found it very fragile when I forget and use "git checkout"
> instead of "stg branch" to switch between branches.

FWIW, I exclusively use git-checkout to switch between git branches
and stgit stacks, and it works like a charm.  I don't remember ever
seeing a problem with this, so I guess it has been fixed for a long
time.

But yes, there are still robustness issues with stgit.  It looks like
the user base is small, since there are not so many bugreports.  We
tend to take care about the workfows we use, and people with other
workflows obviously should tell us what gets wrong :)

Best regards,
-- 
Yann

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04  5:41   ` Pavel Roskin
  2007-08-04  5:51     ` Shawn O. Pearce
  2007-08-04  8:08     ` Yann Dirson
@ 2007-08-04 14:14     ` Chris Shoemaker
  2007-08-04 15:22       ` Johannes Schindelin
  2 siblings, 1 reply; 32+ messages in thread
From: Chris Shoemaker @ 2007-08-04 14:14 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Andy Parkins, git, Catalin Marinas

On Sat, Aug 04, 2007 at 01:41:25AM -0400, Pavel Roskin wrote:
> Hello, Andy!
> 
> On Fri, 2007-08-03 at 19:14 +0100, Andy Parkins wrote:
> > On Friday 2007, August 03, Pavel Roskin wrote:
> > 
> > > I don't suggest that StGIT gives up on the git-based storage, but this
> > > mode of operation could be implemented in two ways.
> > 
> > git's shiny new git rebase -i has removed, for me, those times when I needed 
> > stgit.  Perhaps those who've move from git to quilt would try again when 
> > 1.5.3 is out with the magic that is "rebase -i".
> 
> I don't understand how one option can replace StGIT.  I assume you were
> trying to avoid StGIT already, and "git-rebase -i" was just the last
> missing piece.

FWIW, I'm in the same camp.  I'm a huge fan of quilt, and used it
extensively and with large stacks.  (Actually, I still use it whenever
I don't want to bother with importing-to-git a large CVS or SVN
project that I'm tracking.)  When I started using git (and up until
the first time I used git-rebase -i), I assumed I'd eventually have to
use one of the quilt-like add-ons, but I wanted to hold off a little
while until I was comfortable with core-git.

But, after using git-rebase -i, I can't see why I'd need any
quilt-like add-on.  Every time I use git-rebase -i, it's like I'm
editing the patch stack.

> It would be great if you could tell me how your approach would deal with
> the issue of editable patches I mentioned already.  In case I was
> unclear, here's the quote from one of the developers:
> 
> [quote]
> Sometimes, I just make patches in quilt, then I do "quilt 
> refresh", "quilt pop -a", "cd patches" and modify the patches 
> and series file manually, e.g. by moving one patch from one file 
> into the other. 

Well, there are many different ways one might want to modify the
stack, but I find that most of them are quite easy with git-rebase -i.
IMO, here are things that are easier with git-rebase -i than with an
external patch stack:

   - editing the headers (git-rebase makes it easy to find/select the
       patch and even opens the editor for me)
   - reordering patches
   - combining patches (squashing)
   - moving one file's diff from one patch to another

IMO, here are some things that would probably be easier with an external
patch stack:

   - directly editing the diff hunks
   - moving single diff hunks between patches

Maybe there are others, too, but these are things I just don't do
nearly as frequently as the things that git-rebase -i is good at.  (I
use git-rebase -i *constantly*).

> The "cd ..", "quilt push -a" and off I am. That 
> the "database" of quilt is in a known format and I can hack on 
> it with an editor is a plus for me :-)
> [end of quote]

That sounds more like an argument from familiarity than anything else.
Nobody (reasonable) directly hacks git's internal binary format.  The
"known format" I can hack with my editor is just the content itself.
Honestly, when you have commit-handling that is as good as git's,
there's really very little appeal left to editing the diffs directly.

-chris

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04 14:14     ` Chris Shoemaker
@ 2007-08-04 15:22       ` Johannes Schindelin
  0 siblings, 0 replies; 32+ messages in thread
From: Johannes Schindelin @ 2007-08-04 15:22 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: git

Hi,

On Sat, 4 Aug 2007, Chris Shoemaker wrote:

> IMO, here are some things that would probably be easier with an external
> patch stack:
> 
>    - directly editing the diff hunks
>    - moving single diff hunks between patches
> 
> Maybe there are others, too, but these are things I just don't do
> nearly as frequently as the things that git-rebase -i is good at.  (I
> use git-rebase -i *constantly*).

Good to hear!  (I almost missed this mail, since I usually skip the StGit 
mails.)

> > The "cd ..", "quilt push -a" and off I am. That 
> > the "database" of quilt is in a known format and I can hack on 
> > it with an editor is a plus for me :-)
> > [end of quote]
> 
> That sounds more like an argument from familiarity than anything else.
> Nobody (reasonable) directly hacks git's internal binary format.  The
> "known format" I can hack with my editor is just the content itself.
> Honestly, when you have commit-handling that is as good as git's,
> there's really very little appeal left to editing the diffs directly.

Of course, you _could_ just export the patches as one mbox, edit them, and 
reapply them:

	git format-patch --stdout HEAD~4 > mbox.txt
	$EDITOR mbox.txt # even moving hunks
	git reset --hard HEAD~4
	git am mbox.txt

If the need is great enough, it should be easy to hack something like this 
into git rebase -i.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04  6:38 ` Theodore Tso
  2007-08-04  8:16   ` Yann Dirson
@ 2007-08-04 21:35   ` Josef Sipek
  2007-08-05  0:12     ` Pavel Roskin
  1 sibling, 1 reply; 32+ messages in thread
From: Josef Sipek @ 2007-08-04 21:35 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Pavel Roskin, Catalin Marinas, git

On Sat, Aug 04, 2007 at 02:38:58AM -0400, Theodore Tso wrote:
> On Fri, Aug 03, 2007 at 01:50:10PM -0400, Pavel Roskin wrote:
> > Hello!
> > 
> > I was recently disappointed to learn that one of the Linux drivers
> > (bcm43xx_mac80211, to be precise) switched from git to quilt.  I asked
> > whether StGIT was considered, a discussion followed, and I think the key
> > points need to be shared with StGIT developers.  I'll add some of my
> > ideas to the mix.
> 
> You might also ask them if they considered "guilt", which uses
> text-based patches.  It's a lot easier to use, and if they really want
> quilt-like functionality, "guilt" will provide it.
 
I guess I should chime in.

The goal behind guilt is to be very simple yet powerful enough to do what is
necessary to maintain a piece of kernel code (read: to scratch my own itch).

> And because it's so stupid simple, it's much rarer that something goes
> seriously wrong that requires me to use a recovery procedure in the
> first place.

Thanks! :)

> Editing patch headers are also a lot easier with guilt; you can just
> go ahead and edit them all, and then you can fresh them in git by
> doing a "guilt pop -a; guilt push -a".  Since it's not using a
> git-based storage, it doesn't have to rewrite the whole patch stack
> when you modify a single patch header; you can edit a whole bunch of
> text headers and then refresh the git commit series just once.

FYI: v0.27 allows you to edit the patch headers with: guilt header -e

Pavel, if you have any questions, you know where to ask :)

Josef 'Jeff' Sipek.

-- 
Don't drink and derive. Alcohol and algebra don't mix.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04  5:51     ` Shawn O. Pearce
@ 2007-08-05  0:08       ` Pavel Roskin
  2007-08-05  0:17       ` Jakub Narebski
  1 sibling, 0 replies; 32+ messages in thread
From: Pavel Roskin @ 2007-08-05  0:08 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Andy Parkins, git, Catalin Marinas

On Sat, 2007-08-04 at 01:51 -0400, Shawn O. Pearce wrote:

> I agree with Andy.  Aside from the performance issues that I
> am currently having with a 55 patch series, "rebase -i" (and its
> predecessor script from Dscho) have been a major part of my toolkit,
> to the point that I really don't need something like StGIT on
> my system.
> 
> (Regarding the performance, cherry-picking 55 patches is
> slow, especially when many of them would apply trivially with
> git-diff|git-apply --index.  Be nice to improve that in 1.5.4.)

I understand that "git-rebase -i" is good for keeping local changes
up-to-date, but the real issue is managing the local patches so that
they can be enhanced to the point that they are ready for submission.

That includes such things as moving chunks between patches, editing
descriptions, joining patches together, sorting patches into groups by
their urgency and so on.  Keeping the patches up-to-date is just one
aspect.

Anyway, I'm going to give "git-rebase -i" a try.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04 21:35   ` Josef Sipek
@ 2007-08-05  0:12     ` Pavel Roskin
  0 siblings, 0 replies; 32+ messages in thread
From: Pavel Roskin @ 2007-08-05  0:12 UTC (permalink / raw)
  To: Josef Sipek; +Cc: Theodore Tso, Catalin Marinas, git

On Sat, 2007-08-04 at 17:35 -0400, Josef Sipek wrote:

> FYI: v0.27 allows you to edit the patch headers with: guilt header -e

> Pavel, if you have any questions, you know where to ask :)

Thanks everybody.  guilt and "git-rebase -i" are on my list of things to
try.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04  5:51     ` Shawn O. Pearce
  2007-08-05  0:08       ` Pavel Roskin
@ 2007-08-05  0:17       ` Jakub Narebski
  2007-08-05  2:31         ` Shawn O. Pearce
  1 sibling, 1 reply; 32+ messages in thread
From: Jakub Narebski @ 2007-08-05  0:17 UTC (permalink / raw)
  To: git

Shawn O. Pearce wrote:

> (Regarding the performance, cherry-picking 55 patches is
> slow, especially when many of them would apply trivially with
> git-diff|git-apply --index.  Be nice to improve that in 1.5.4.)

Perhaps in the future you would be able to use -i/--interactive mode
in merge driven git-rebase (git rebase --merge -i <base>), which I think
should be faster.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-05  0:17       ` Jakub Narebski
@ 2007-08-05  2:31         ` Shawn O. Pearce
  2007-08-05  3:32           ` Junio C Hamano
  2007-08-05 13:39           ` Josef Sipek
  0 siblings, 2 replies; 32+ messages in thread
From: Shawn O. Pearce @ 2007-08-05  2:31 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Jakub Narebski <jnareb@gmail.com> wrote:
> Shawn O. Pearce wrote:
> 
> > (Regarding the performance, cherry-picking 55 patches is
> > slow, especially when many of them would apply trivially with
> > git-diff|git-apply --index.  Be nice to improve that in 1.5.4.)
> 
> Perhaps in the future you would be able to use -i/--interactive mode
> in merge driven git-rebase (git rebase --merge -i <base>), which I think
> should be faster.

Heh.  You're talking to someone who actually knows what he is talking
about here, and was actually involved in some of these tools...
Let me fill the reader in on what's really happening...

`git-rebase` (non -i, non -m) uses `format-patch|am` to apply the
changes of each commit it is rebasing.  This is insanely fast as
builtin diff and apply routines are quite efficient.  It sometimes
fails due to patches not applying cleanly.  In those cases you
have to either hand edit the patch, apply and continue the rebase,
or abort the rebase and restart with the -m flag so it uses a full
three-way file merge.

`git-rebase -m` (aka --merge) uses git-merge-recursive to apply the
changes of each commit it is rebasing.  (That's the merge part!)
However merge-recursive usually takes longer to run then the above
`format-patch|am` pipeline, and that is why -m is not the default.
But it does handle cases `format-patch|am` cannot do automatically.

For quite a long time now both `git-revert` and `git-cherry-pick`
(which are actually the same program!) have also been using
git-merge-recursive as their implementation to revert or apply the
commit's change.  This allows them to perform changes that also
involve renames, as well as to apply some changes that might fail
as a patch but succeed when done as a three-way file merge.

So really `revert`, `cherry-pick`, `rebase -m` (and also `am -3`
as it also uses merge-recursive) are all the same underlying
implementation.  The major differences between them is what they
do *after* the changes have been applied, and which direction the
change goes (e.g. revert undoes the change).

Now the new `git-rebase -i` is really just a complicated loop around
`cherry-pick`.  Really.  Go look at the code, it never calls anything
except cherry-pick.  So `rebase -i` is actually `rebase --merge -i`.
That's why its sluggish.

Why is merge-recursive sluggish?  It does rename detection.
It does full three-way file merges, rather than just applying
a patch.  It also tries to do a three-way read-tree before doing
file level merges.  git-apply does none of these things, and is
faster because of it.

So that future you speak of above is today.  Its also not faster,
its slower.  Faster would be to do something like `format-patch|am -3`
so that merge-recursive is only invoked if git-apply was unable to
apply the patch automatically.  Except we'd want to save the original
tree data so we can do proper rename detection when merge-recursive
is fired up.

-- 
Shawn.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-05  2:31         ` Shawn O. Pearce
@ 2007-08-05  3:32           ` Junio C Hamano
  2007-08-05 13:39           ` Josef Sipek
  1 sibling, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2007-08-05  3:32 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jakub Narebski, git

"Shawn O. Pearce" <spearce@spearce.org> writes:

> So really `revert`, `cherry-pick`, `rebase -m` (and also `am -3`
> as it also uses merge-recursive) are all the same underlying
> implementation.

Minor factual correction.  "am -3" uses merge-recursive *ONLY*
when patch does not apply, so it is often the best of both
worlds, as long as your changes do not involve renames.  And
that is what the default rebase uses.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-05  2:31         ` Shawn O. Pearce
  2007-08-05  3:32           ` Junio C Hamano
@ 2007-08-05 13:39           ` Josef Sipek
  2007-08-05 13:56             ` Johannes Schindelin
  1 sibling, 1 reply; 32+ messages in thread
From: Josef Sipek @ 2007-08-05 13:39 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jakub Narebski, git

On Sat, Aug 04, 2007 at 10:31:30PM -0400, Shawn O. Pearce wrote:
...
[rebase is complex but fun]

Great, but does git have something that could replace
$QUILT_LIKE_APP refresh?

Sure, if you can take 2 commits and collapse them into one you could fake it
by creating a dummy commit with the new changes, and then collapsing, but
that's nasty - and reflog might not like that much :)

Josef 'Jeff' Sipek.

-- 
I'm somewhere between geek and normal.
		- Linus Torvalds

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-05 13:39           ` Josef Sipek
@ 2007-08-05 13:56             ` Johannes Schindelin
  2007-08-05 14:06               ` Josef Sipek
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Schindelin @ 2007-08-05 13:56 UTC (permalink / raw)
  To: Josef Sipek; +Cc: Shawn O. Pearce, Jakub Narebski, git

Hi,

On Sun, 5 Aug 2007, Josef Sipek wrote:

> On Sat, Aug 04, 2007 at 10:31:30PM -0400, Shawn O. Pearce wrote:
> ...
> [rebase is complex but fun]
> 
> Great, but does git have something that could replace
> $QUILT_LIKE_APP refresh?

What does "refresh"?  (I never used quilt, and probably never will, since 
rebase -i does what I need.)

> Sure, if you can take 2 commits and collapse them into one you could 
> fake it by creating a dummy commit with the new changes, and then 
> collapsing, but that's nasty - and reflog might not like that much :)

IIUC you want to edit/amend a patch in the middle of a series?  Two ways 
to go about it:

	1) (preferred)

		* start rebase -i
		* mark the commit as "edit"
		* wait until rebase stops to let you edit it
		* edit, test, commit --amend
		* rebase --continue

	2) (not so preferred, but often convenient)

		* fix bug
		* commit with a dummy message
		* rebase -i
		* move commit just after the commit-to-edit
		* mark second as "squash"
		* when the editor comes up, just delete the second 
		  commit's message, and possibly adjust the first's

Hth,
Dscho

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-05 13:56             ` Johannes Schindelin
@ 2007-08-05 14:06               ` Josef Sipek
  2007-08-05 14:15                 ` Johannes Schindelin
  0 siblings, 1 reply; 32+ messages in thread
From: Josef Sipek @ 2007-08-05 14:06 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Shawn O. Pearce, Jakub Narebski, git

On Sun, Aug 05, 2007 at 02:56:23PM +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Sun, 5 Aug 2007, Josef Sipek wrote:
> 
> > On Sat, Aug 04, 2007 at 10:31:30PM -0400, Shawn O. Pearce wrote:
> > ...
> > [rebase is complex but fun]
> > 
> > Great, but does git have something that could replace
> > $QUILT_LIKE_APP refresh?
> 
> What does "refresh"?  (I never used quilt, and probably never will, since 
> rebase -i does what I need.)

You understood correctly...see below.

> > Sure, if you can take 2 commits and collapse them into one you could 
> > fake it by creating a dummy commit with the new changes, and then 
> > collapsing, but that's nasty - and reflog might not like that much :)
> 
> IIUC you want to edit/amend a patch in the middle of a series?  Two ways 
> to go about it:
> 
> 	1) (preferred)
> 
> 		* start rebase -i
> 		* mark the commit as "edit"
> 		* wait until rebase stops to let you edit it
> 		* edit, test, commit --amend
> 		* rebase --continue

Ewww...that doesn't seem to scale (read: far too much to type) :) Here's a
quilt/guilt/stgit equivalent:

	$APP push <patchname>

or (depending on where you are in the patch stack)

	$APP pop <patchname>

	<edit>

	$APP refresh # this is the commit --amend part

Josef 'Jeff' Sipek.

-- 
The reasonable man adapts himself to the world; the unreasonable one
persists in trying to adapt the world to himself. Therefore all progress
depends on the unreasonable man.
		- George Bernard Shaw

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-05 14:06               ` Josef Sipek
@ 2007-08-05 14:15                 ` Johannes Schindelin
  2007-08-05 14:57                   ` Josef Sipek
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Schindelin @ 2007-08-05 14:15 UTC (permalink / raw)
  To: Josef Sipek; +Cc: Shawn O. Pearce, Jakub Narebski, git

Hi,

On Sun, 5 Aug 2007, Josef Sipek wrote:

> On Sun, Aug 05, 2007 at 02:56:23PM +0100, Johannes Schindelin wrote:
> > 
> > On Sun, 5 Aug 2007, Josef Sipek wrote:
> > 
> > > Sure, if you can take 2 commits and collapse them into one you could 
> > > fake it by creating a dummy commit with the new changes, and then 
> > > collapsing, but that's nasty - and reflog might not like that much 
> > > :)
> > 
> > IIUC you want to edit/amend a patch in the middle of a series?  Two ways 
> > to go about it:
> > 
> > 	1) (preferred)
> > 
> > 		* start rebase -i
> > 		* mark the commit as "edit"
> > 		* wait until rebase stops to let you edit it
> > 		* edit, test, commit --amend
> > 		* rebase --continue
> 
> Ewww...that doesn't seem to scale (read: far too much to type) :) Here's a
> quilt/guilt/stgit equivalent:
> 
> 	$APP push <patchname>
> 
> or (depending on where you are in the patch stack)
> 
> 	$APP pop <patchname>
> 
> 	<edit>
> 
> 	$APP refresh # this is the commit --amend part

Yeah.  Sounds like you'd just need a "--edit-this $commit" flag to rebase 
-i.

Out of curiousity, what happens if you say "push" several times, 
_without_ popping the patch?  And what happens if you "push" several times 
with the _same_ patchname?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-05 14:15                 ` Johannes Schindelin
@ 2007-08-05 14:57                   ` Josef Sipek
  0 siblings, 0 replies; 32+ messages in thread
From: Josef Sipek @ 2007-08-05 14:57 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Shawn O. Pearce, Jakub Narebski, git

On Sun, Aug 05, 2007 at 03:15:29PM +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Sun, 5 Aug 2007, Josef Sipek wrote:
> 
> > On Sun, Aug 05, 2007 at 02:56:23PM +0100, Johannes Schindelin wrote:
> > > 
> > > On Sun, 5 Aug 2007, Josef Sipek wrote:
> > > 
> > > > Sure, if you can take 2 commits and collapse them into one you could 
> > > > fake it by creating a dummy commit with the new changes, and then 
> > > > collapsing, but that's nasty - and reflog might not like that much 
> > > > :)
> > > 
> > > IIUC you want to edit/amend a patch in the middle of a series?  Two ways 
> > > to go about it:
> > > 
> > > 	1) (preferred)
> > > 
> > > 		* start rebase -i
> > > 		* mark the commit as "edit"
> > > 		* wait until rebase stops to let you edit it
> > > 		* edit, test, commit --amend
> > > 		* rebase --continue
> > 
> > Ewww...that doesn't seem to scale (read: far too much to type) :) Here's a
> > quilt/guilt/stgit equivalent:
> > 
> > 	$APP push <patchname>
> > 
> > or (depending on where you are in the patch stack)
> > 
> > 	$APP pop <patchname>
> > 
> > 	<edit>
> > 
> > 	$APP refresh # this is the commit --amend part
> 
> Yeah.  Sounds like you'd just need a "--edit-this $commit" flag to rebase 
> -i.

Still, very inconvenient (IMO) when you are working _on_ lots of patches.

> Out of curiousity, what happens if you say "push" several times, 
> _without_ popping the patch?  And what happens if you "push" several times 
> with the _same_ patchname?

In addition to the patches, there's also ordering information for those
patches (the series file) - in the simplest case it is a stack in the
traditinal Comp Sci meaning. IOW, if you have patches {foo,bar,baz} and you
want to push "bar", the patch "foo" will also get pushed. When you push a
patch, the software makes note of the fact (status file in Guilt).

If you try to push the same patch twice, it'll tell you that the patch is
already applied. The Mercurial Book has a section about Mercurial Queues
(Mercurial's implementation of something similar to Guilt/stgit) which
describes the concepts rather well [1]. I think that figure 12.10 [2] really
explains the whole thing pretty well. Of course, there is always the quilt
doc (big PDF which has the whole history, etc., etc.).

Hrm, it just occured to me that the quilt-way of managing patches is the
basic concept of a turing machine - infinite tape which is seekable and you
can read/write to the "current" position (topmost applied patch). :)

Josef 'Jeff' Sipek.

[1] http://hgbook.red-bean.com/hgbookch12.html#x16-27200012.5
[2] http://hgbook.red-bean.com/hgbookch12.html#x16-27600310

-- 
Computer Science is no more about computers than astronomy is about
telescopes.
		- Edsger Dijkstra

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-03 17:50 Some ideas for StGIT Pavel Roskin
                   ` (2 preceding siblings ...)
  2007-08-04  6:38 ` Theodore Tso
@ 2007-08-06  9:36 ` Catalin Marinas
  2007-08-06  9:56   ` Karl Hasselström
  2007-08-06 17:17   ` Pavel Roskin
  3 siblings, 2 replies; 32+ messages in thread
From: Catalin Marinas @ 2007-08-06  9:36 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git

Hi Pavel,

All the interesting discussion usually happen during my holidays :-).

On 03/08/2007, Pavel Roskin <proski@gnu.org> wrote:
> I was recently disappointed to learn that one of the Linux drivers
> (bcm43xx_mac80211, to be precise) switched from git to quilt.  I asked
> whether StGIT was considered, a discussion followed, and I think the key
> points need to be shared with StGIT developers.  I'll add some of my
> ideas to the mix.

Thanks for the feedback.

> The main point in favor of quilt is that it allows to edit the patches
> with the text editor.  One can pop all patches, edit them and push the
> all back.

If this is the main feature they need, they probably don't need git at
all and quilt would be enough. I was using quilt before starting StGIT
but the main problem I had with plain patches approach was the
conflict solving.

StGIT does a 'git-diff | git-apply' as a patch push optimization and
we could even cache the diff but the current algorithm is that if
git-apply fails, StGIT falls back to a three-way merge and even an
interactive user merge (via xxdiff for example). I find the three-way
merging (automatic or interactive) much more powerful than fuzzy patch
application.

If we would allow patch editing, the 'stg push' algorithms wouldn't
know when git-apply failed because the patch was edited or the base
was changed. Falling back to the three-way merge would lose the edited
patch. If one doesn't need three-way merging, quilt is good enough.

Other advantages of the three-way merging is the detection of full
patches or hunks merged upstream (the former can also be achieved by
testing the reverse-application of the patches).

I don't usually edit patches during development, I prefer to edit the
source files and review the diff. It happens many times to move hunks
between patches but I usually towards the bottom patches in the stack
(using stg export and emacs) and the three-way merging automatically
removes the merged hunks from top patches.

> I don't suggest that StGIT gives up on the git-based storage, but this
> mode of operation could be implemented in two ways.
>
> One is to have a command opposite to "export".  It would read the files
> that "export" produces, replacing the existing patches.

As Yann said, we already have 'stg import --replace'. I mainly use
this feature with series sent to me and when they need some editing to
apply cleanly. There is also 'stg import --ignore' to ignore the
patches already applied (mainly when the importing fails in the middle
of a series, there is no need to re-import the first patches).

> Another approach would be to reexamine the patch after "stg refresh -es"
> and to apply it instead of the original patch.  If the patch doesn't
> apply, the options would be to discard the edits or to re-launch the
> editor.

That's an interesting idea but maybe we should have a separate command
like --edit-full to edit the full patch + log (part of the
functionality already available in import).

> Next issue is that it should be possible to create a patch in one
> operation.  StGIT follows quilt too closely here in requiring "new" and
> "refresh", instead of utilizing the advantage of the workflow that
> allows immediate editing of the sources without any commands.
>
> Basically, I want one command that:
>
> 1) shows user what was changed
> 2) allows user to name the patch
> 3) allows user to describe the patch
> 4) allows user to exclude files from the patch
> 5) doesn't require another command to put the changes to the patch
>
> I think the most natural approach would be to enhance "stg new".  I see
> "stg new -s" is supposed to show the changes, but it's currently broken.

Thanks for reporting this. I don't use the --showpatch options much
and we don't have any tests (yet) for the interactive options.

> Finally, it would be great to have TLS support in the mail command.
> Mercurial has it, and looking at their mail.py, it doesn't seem to be
> much work.

Indeed, the SMTP Python objects already provide support for TLS via starttls().

-- 
Catalin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-03 23:23 ` Yann Dirson
@ 2007-08-06  9:49   ` Catalin Marinas
  2007-08-06 13:26     ` Pavel Roskin
  0 siblings, 1 reply; 32+ messages in thread
From: Catalin Marinas @ 2007-08-06  9:49 UTC (permalink / raw)
  To: Yann Dirson; +Cc: Pavel Roskin, git

On 04/08/2007, Yann Dirson <ydirson@altern.org> wrote:
> On Fri, Aug 03, 2007 at 01:50:10PM -0400, Pavel Roskin wrote:
> >  I see
> > "stg new -s" is supposed to show the changes, but it's currently broken.
> > This is run in a clean StGIT repository with no patches:
> >
> > $ stg new -s foo
>
> Hm, I'm not sure what -s would be supposed to show here, since we're
> asking for the creation of a patch, which currently always starts
> empty.

The story for the 'new -s' option was that with StGIT (not possible
with Quilt), one can start modifying the local tree and only create a
patch afterwards. The newly created patch is always empty, even if
there were local changes and showing them was useful for writing the
patch description. One can use refresh for checking the changes in.
Indeed, the 'new' command can be improved to have part of the
'refresh' functionality, though I don't really like this duplication.

> Especially confusing is that if there are already applied patches, the
> diff shown is the one of the previous top patch

Are you sure it doesn't only show the local changes (which you might
want to add in a new patch)?

> - and if there is no
> applied patches, we get the exception you noticed.

That's a bug, indeed.

> I guess -s should be removed for 0.13.1.

I'll still like to keep it for the rare cases when I use the diff to
write the patch description.

> I also tried with "stg refresh -m ''" to see if it caused the same
> problem, but it appears to have another problem instead: it does not
> refresh the patch description at all.
>
> My guess is that we should not allow empty patch description (and
> maybe fill it with provided patchname).

I think we should put some default patch description.

-- 
Catalin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-06  9:36 ` Catalin Marinas
@ 2007-08-06  9:56   ` Karl Hasselström
  2007-08-06 12:42     ` Pavel Roskin
  2007-08-06 17:17   ` Pavel Roskin
  1 sibling, 1 reply; 32+ messages in thread
From: Karl Hasselström @ 2007-08-06  9:56 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Pavel Roskin, git

On 2007-08-06 10:36:25 +0100, Catalin Marinas wrote:

> On 03/08/2007, Pavel Roskin <proski@gnu.org> wrote:
>
> > Another approach would be to reexamine the patch after "stg
> > refresh -es" and to apply it instead of the original patch. If the
> > patch doesn't apply, the options would be to discard the edits or
> > to re-launch the editor.
>
> That's an interesting idea but maybe we should have a separate
> command like --edit-full to edit the full patch + log (part of the
> functionality already available in import).

I never really understood why commit message editing had to be part of
the "refresh" command. If it were a separate command and not tied to
refresh, we could allow editing the message (and author, committer,
date, ...) of any commit in the stack -- since the tree objects would
be unchanged, we could just reuse the same tree objects when rewriting
the commit objects on top of it.

That's obviously not going to work if we allow editing of the patch.
But patch editing isn't a good fit as a refresh switch either, since
it's not at all related to replacing the tree of the current patch
with the working tree.

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-04  8:08     ` Yann Dirson
@ 2007-08-06 10:01       ` Catalin Marinas
  0 siblings, 0 replies; 32+ messages in thread
From: Catalin Marinas @ 2007-08-06 10:01 UTC (permalink / raw)
  To: Yann Dirson; +Cc: Pavel Roskin, Andy Parkins, git

On 04/08/2007, Yann Dirson <ydirson@altern.org> wrote:
> FWIW, I have written a couple of scripts to help moving stuff around
> between patches.  Those are not yet integrated in stgit proper, and it
> happens that the 0.13 tarball does not contain them, they are only
> available from the git tree (better use my tree[1], since I updated them
> recently).

That's probably because I haven't updated the MANIFEST.in file (I
don't look in the contrib directory much :-)).

-- 
Catalin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-06  9:56   ` Karl Hasselström
@ 2007-08-06 12:42     ` Pavel Roskin
  2007-08-06 13:52       ` Karl Hasselström
  0 siblings, 1 reply; 32+ messages in thread
From: Pavel Roskin @ 2007-08-06 12:42 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Catalin Marinas, git

On Mon, 2007-08-06 at 11:56 +0200, Karl Hasselström wrote:

> I never really understood why commit message editing had to be part of
> the "refresh" command. If it were a separate command and not tied to
> refresh, we could allow editing the message (and author, committer,
> date, ...) of any commit in the stack -- since the tree objects would
> be unchanged, we could just reuse the same tree objects when rewriting
> the commit objects on top of it.
> 
> That's obviously not going to work if we allow editing of the patch.
> But patch editing isn't a good fit as a refresh switch either, since
> it's not at all related to replacing the tree of the current patch
> with the working tree.

Purely from the code standpoint, yes, it should be a separate command.
But it may be practical to have both in one command, since I commonly
need to change the description after changing the code.

We need to think what would be convenient for the normal workflow.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-06  9:49   ` Catalin Marinas
@ 2007-08-06 13:26     ` Pavel Roskin
  2007-08-06 15:19       ` Josef Sipek
  0 siblings, 1 reply; 32+ messages in thread
From: Pavel Roskin @ 2007-08-06 13:26 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Yann Dirson, git

On Mon, 2007-08-06 at 10:49 +0100, Catalin Marinas wrote:

> The story for the 'new -s' option was that with StGIT (not possible
> with Quilt), one can start modifying the local tree and only create a
> patch afterwards.

And that's what I really like about StGIT.  I like that I can edit code
without worrying (too much) about the state of the repository.

> The newly created patch is always empty, even if
> there were local changes and showing them was useful for writing the
> patch description. One can use refresh for checking the changes in.
> Indeed, the 'new' command can be improved to have part of the
> 'refresh' functionality, though I don't really like this duplication.

It should be fine as long as the code is reused IMHO.

> > Especially confusing is that if there are already applied patches, the
> > diff shown is the one of the previous top patch
> 
> Are you sure it doesn't only show the local changes (which you might
> want to add in a new patch)?

I confirm this bug.

> I think we should put some default patch description.

I agree.  Sometimes it's too early to write a description.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-06 12:42     ` Pavel Roskin
@ 2007-08-06 13:52       ` Karl Hasselström
  2007-08-23 14:09         ` Catalin Marinas
  0 siblings, 1 reply; 32+ messages in thread
From: Karl Hasselström @ 2007-08-06 13:52 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Catalin Marinas, git

On 2007-08-06 08:42:05 -0400, Pavel Roskin wrote:

> On Mon, 2007-08-06 at 11:56 +0200, Karl Hasselström wrote:
>
> > I never really understood why commit message editing had to be
> > part of the "refresh" command. If it were a separate command and
> > not tied to refresh, we could allow editing the message (and
> > author, committer, date, ...) of any commit in the stack -- since
> > the tree objects would be unchanged, we could just reuse the same
> > tree objects when rewriting the commit objects on top of it.
>
> Purely from the code standpoint, yes, it should be a separate
> command. But it may be practical to have both in one command, since
> I commonly need to change the description after changing the code.

Sure. I don't have any objection to making

  stg refresh -e

be equivalent to

  stg refresh && stg edit-patch-message <topmost-patch>

What I'm objecting to is being forced to refresh when I just want to
edit the message. (And, to a lesser degree, having to manually push
and pop to make the patch topmost before I can edit its message.)

Obviously not annoyed enough to have written a patch for it yet,
though. :-)

> We need to think what would be convenient for the normal workflow.

Of course.

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-06 13:26     ` Pavel Roskin
@ 2007-08-06 15:19       ` Josef Sipek
  0 siblings, 0 replies; 32+ messages in thread
From: Josef Sipek @ 2007-08-06 15:19 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Catalin Marinas, Yann Dirson, git

<shameless plugs>

On Mon, Aug 06, 2007 at 09:26:08AM -0400, Pavel Roskin wrote:
> On Mon, 2007-08-06 at 10:49 +0100, Catalin Marinas wrote:
> 
> > The story for the 'new -s' option was that with StGIT (not possible
> > with Quilt), one can start modifying the local tree and only create a
> > patch afterwards.
> 
> And that's what I really like about StGIT.  I like that I can edit code
> without worrying (too much) about the state of the repository.
 
guilt-new -f <patchname>

> > The newly created patch is always empty, even if
> > there were local changes and showing them was useful for writing the
> > patch description. One can use refresh for checking the changes in.
> > Indeed, the 'new' command can be improved to have part of the
> > 'refresh' functionality, though I don't really like this duplication.
> 
> It should be fine as long as the code is reused IMHO.

Agreed.

> > I think we should put some default patch description.
> 
> I agree.  Sometimes it's too early to write a description.
 
If Guilt doesn't find a description in the patch file during push, it uses
"patch $patchname" as the commit message. This makes it enough of an
eye-sore that you notice before you submit the patches upstream :)

Josef 'Jeff' Sipek.

-- 
Failure is not an option,
It comes bundled with your Microsoft product.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-06  9:36 ` Catalin Marinas
  2007-08-06  9:56   ` Karl Hasselström
@ 2007-08-06 17:17   ` Pavel Roskin
  1 sibling, 0 replies; 32+ messages in thread
From: Pavel Roskin @ 2007-08-06 17:17 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

On Mon, 2007-08-06 at 10:36 +0100, Catalin Marinas wrote:

> > The main point in favor of quilt is that it allows to edit the patches
> > with the text editor.  One can pop all patches, edit them and push the
> > all back.
> 
> If this is the main feature they need, they probably don't need git at
> all and quilt would be enough. I was using quilt before starting StGIT
> but the main problem I had with plain patches approach was the
> conflict solving.

OK, I understand it wasn't a good idea to ask for improvement on behalf
of others.

> StGIT does a 'git-diff | git-apply' as a patch push optimization and
> we could even cache the diff but the current algorithm is that if
> git-apply fails, StGIT falls back to a three-way merge and even an
> interactive user merge (via xxdiff for example). I find the three-way
> merging (automatic or interactive) much more powerful than fuzzy patch
> application.

I agree.  I have no problem with what StGIT does internally.

> If we would allow patch editing, the 'stg push' algorithms wouldn't
> know when git-apply failed because the patch was edited or the base
> was changed. Falling back to the three-way merge would lose the edited
> patch. If one doesn't need three-way merging, quilt is good enough.

I suggest that StGIT saves the original patch and then does interdiff
between the old and the new patch.  The original patch is applied first
just as it's applied now, and then the difference is applied on top of
that.

Temporary files should be kept in case of failure.

> Other advantages of the three-way merging is the detection of full
> patches or hunks merged upstream (the former can also be achieved by
> testing the reverse-application of the patches).

I'm fully with you here.  Having git history can only be a good thing.

> I don't usually edit patches during development, I prefer to edit the
> source files and review the diff. It happens many times to move hunks
> between patches but I usually towards the bottom patches in the stack
> (using stg export and emacs) and the three-way merging automatically
> removes the merged hunks from top patches.

What I normally need to edit is the comments.  Editing the code is
risky, although I may want to rename some badly named variable
introduced by the patch.

> > I don't suggest that StGIT gives up on the git-based storage, but this
> > mode of operation could be implemented in two ways.
> >
> > One is to have a command opposite to "export".  It would read the files
> > that "export" produces, replacing the existing patches.
> 
> As Yann said, we already have 'stg import --replace'.

Thanks!

> > Another approach would be to reexamine the patch after "stg refresh -es"
> > and to apply it instead of the original patch.  If the patch doesn't
> > apply, the options would be to discard the edits or to re-launch the
> > editor.
> 
> That's an interesting idea but maybe we should have a separate command
> like --edit-full to edit the full patch + log (part of the
> functionality already available in import).

I hate to be in a situation when I want to edit something but cannot,
because I didn't run some command before.  What I like about StGIT is
that it allows me to do things my way.

I don't know if I want to change the patch before I see it.

> > Finally, it would be great to have TLS support in the mail command.
> > Mercurial has it, and looking at their mail.py, it doesn't seem to be
> > much work.
> 
> Indeed, the SMTP Python objects already provide support for TLS via starttls().

And hg provides a great example.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-06 13:52       ` Karl Hasselström
@ 2007-08-23 14:09         ` Catalin Marinas
  2007-08-23 14:34           ` Karl Hasselström
  0 siblings, 1 reply; 32+ messages in thread
From: Catalin Marinas @ 2007-08-23 14:09 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Pavel Roskin, git

(cleaning up my inbox after holiday, so my replies might look random)

On 06/08/07, Karl Hasselström <kha@treskal.com> wrote:
> On 2007-08-06 08:42:05 -0400, Pavel Roskin wrote:
> > Purely from the code standpoint, yes, it should be a separate
> > command. But it may be practical to have both in one command, since
> > I commonly need to change the description after changing the code.
>
> Sure. I don't have any objection to making
>
>   stg refresh -e
>
> be equivalent to
>
>   stg refresh && stg edit-patch-message <topmost-patch>

The only objection is the long command name - 'stg edit [<patch>]'
would be just fine. It would also be nice to do (with an additional
option), the equivalent of export - edit - import in case one wants to
also modify the diff.

> What I'm objecting to is being forced to refresh when I just want to
> edit the message. (And, to a lesser degree, having to manually push
> and pop to make the patch topmost before I can edit its message.)

Not necessarily - 'stg refresh -e -p <patch>' does the pop/push for
you and it even uses the fast-forwarding.

-- 
Catalin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Some ideas for StGIT
  2007-08-23 14:09         ` Catalin Marinas
@ 2007-08-23 14:34           ` Karl Hasselström
  0 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2007-08-23 14:34 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Pavel Roskin, git

On 2007-08-23 15:09:48 +0100, Catalin Marinas wrote:

> On 06/08/07, Karl Hasselström <kha@treskal.com> wrote:
>
> > On 2007-08-06 08:42:05 -0400, Pavel Roskin wrote:
> >
> > > Purely from the code standpoint, yes, it should be a separate
> > > command. But it may be practical to have both in one command,
> > > since I commonly need to change the description after changing
> > > the code.
> >
> > Sure. I don't have any objection to making
> >
> >   stg refresh -e
> >
> > be equivalent to
> >
> >   stg refresh && stg edit-patch-message <topmost-patch>
>
> The only objection is the long command name - 'stg edit [<patch>]'
> would be just fine.

Oh, I chose a ridiculously long name on purpose, to make it
unambiguous while at the same time not implying that I had a good name
already thought out. :-)

> It would also be nice to do (with an additional option), the
> equivalent of export - edit - import in case one wants to also
> modify the diff.

Yes. This is probably one of the most asked-for (and least
implemented) features of StGIT.

> > What I'm objecting to is being forced to refresh when I just want
> > to edit the message. (And, to a lesser degree, having to manually
> > push and pop to make the patch topmost before I can edit its
> > message.)
>
> Not necessarily - 'stg refresh -e -p <patch>' does the pop/push for
> you and it even uses the fast-forwarding.

Hmm, that's better. But it shouldn't be a refresh subcommand!

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2007-08-23 14:34 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-03 17:50 Some ideas for StGIT Pavel Roskin
2007-08-03 18:14 ` Andy Parkins
2007-08-04  5:41   ` Pavel Roskin
2007-08-04  5:51     ` Shawn O. Pearce
2007-08-05  0:08       ` Pavel Roskin
2007-08-05  0:17       ` Jakub Narebski
2007-08-05  2:31         ` Shawn O. Pearce
2007-08-05  3:32           ` Junio C Hamano
2007-08-05 13:39           ` Josef Sipek
2007-08-05 13:56             ` Johannes Schindelin
2007-08-05 14:06               ` Josef Sipek
2007-08-05 14:15                 ` Johannes Schindelin
2007-08-05 14:57                   ` Josef Sipek
2007-08-04  8:08     ` Yann Dirson
2007-08-06 10:01       ` Catalin Marinas
2007-08-04 14:14     ` Chris Shoemaker
2007-08-04 15:22       ` Johannes Schindelin
2007-08-03 23:23 ` Yann Dirson
2007-08-06  9:49   ` Catalin Marinas
2007-08-06 13:26     ` Pavel Roskin
2007-08-06 15:19       ` Josef Sipek
2007-08-04  6:38 ` Theodore Tso
2007-08-04  8:16   ` Yann Dirson
2007-08-04 21:35   ` Josef Sipek
2007-08-05  0:12     ` Pavel Roskin
2007-08-06  9:36 ` Catalin Marinas
2007-08-06  9:56   ` Karl Hasselström
2007-08-06 12:42     ` Pavel Roskin
2007-08-06 13:52       ` Karl Hasselström
2007-08-23 14:09         ` Catalin Marinas
2007-08-23 14:34           ` Karl Hasselström
2007-08-06 17:17   ` Pavel Roskin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).