Git development
 help / color / mirror / Atom feed
* Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)
From: Eric Wong @ 2007-05-13 17:17 UTC (permalink / raw)
  To: git
In-Reply-To: <vpq7irfengj.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Hi,
> 
> I'm using git-svn, which usually works fine, but I occasionally get
> this:
> 
> $ git-svn dcommit           
>         A       file1
>         A       file2
> Network connection closed unexpectedly: Connection closed unexpectedly at /path/to/git-svn line 401
> $
> 
> The failure seems to depend on the commit's I'm pushing, since this is
> reproducible when running several times "dcommit" for the same commit,
> but the same command also usually works fine on the same repositories
> (same git repo, same svn target).
> 
> The svn repository is accessed with svn+ssh://user@machine/path/.

Does the patch in
  http://permalink.gmane.org/gmane.comp.version-control.git/47126
help?

If so, I'm not sure why it should be failing sometimes and not
all the time.  If your local username the same as the username
you're using for svn it shouldn't fail for the reason that patch
above is needed.

Using the native svn or svn+ssh protocol is definitely flakier in my
experience, and there are still known issues with connections closing
during *fetch*.  dcommit is actually a lot more straightforward
as it only connects once, whereas fetch can reconnect to different
parts of the repo for --follow-parent.

> exporting the patch, applying it to an svn checkout, and "svn commit"
> works fine.
> 
> Let me know if I can provide any other usefull information for
> debugging.
> 
> Thanks,
> 
> Debian etch (stable)
> Reproducible with both git 1.5.1.1 and the latest from git:
> 
> $ git-svn --version
> git-svn version 1.5.2.rc3.3.ge347 (svn 1.4.2)

Which version of SVN is running on the server?

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH 2/2] git-svn: fix segfaults due to initial SVN pool being cleared
From: Pierre Habouzit @ 2007-05-13 15:05 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <11790434862131-git-send-email-normalperson@yhbt.net>

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]

On Sun, May 13, 2007 at 01:04:44AM -0700, Eric Wong wrote:
> Some parts of SVN always seem to use it, even if the SVN::Ra
> object we're using is no longer used and we've created a new one
> in its place.  It's also true that only one SVN::Ra connection
> can exist at once...  Using SVN::Pool->new_default when the
> SVN::Ra object is created doesn't seem to help very much,
> either...
> 
> Hopefully this fixes all segfault problems users have been
> experiencing over the past few months.
> 
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
>  git-svn.perl |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index ee69598..5352470 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2904,7 +2904,6 @@ sub new {
>  	my ($class, $url) = @_;
>  	$url =~ s!/+$!!;
>  	return $RA if ($RA && $RA->{url} eq $url);
> -	$RA->{pool}->clear if $RA;
>  
>  	SVN::_Core::svn_config_ensure($config_dir, undef);
>  	my ($baton, $callbacks) = SVN::Core::auth_open_helper([
> -- 
> 1.5.2.rc3.18.gf0c86

  I confirm it fixes every segfault I was able to reproduce with any
prior version :)

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH] cvsserver: Don't send mixed messages to clients
From: Frank Lichtenheld @ 2007-05-13 17:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Martin Langhoff, Frank Lichtenheld

After we send I HATE YOU we should probably exit and not happily
continue with I LOVE YOU and further communication.

Most clients will probably just exit and ignore everything we
send after the I HATE YOU and it is not a security problem
either because we don't really care about the user name anyway.

But it is still the right thing to do.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
---
 git-cvsserver.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index a07c725..fcfb99d 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -105,6 +105,7 @@ if (@ARGV && $ARGV[0] eq 'pserver') {
     unless ($line eq 'anonymous') {
        print "E Only anonymous user allowed via pserver\n";
        print "I HATE YOU\n";
+       exit 1;
     }
     $line = <STDIN>; chomp $line;    # validate the password?
     $line = <STDIN>; chomp $line;
-- 
1.5.1.4

^ permalink raw reply related

* [PATCH] t9400: Add some more cvs update tests
From: Frank Lichtenheld @ 2007-05-13 17:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Frank Lichtenheld

Add some cvs update tests that include various merge
situations. Also add a basic test for update -C
since it fits so well in there.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
---
 t/t9400-git-cvsserver-server.sh |   66 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index d406a88..1d40ee8 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -123,4 +123,70 @@ test_expect_success 'cvs update (re-add deleted file)' \
    test "$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))" = "testfile1/1.4/" &&
    diff -q testfile1 ../testfile1'
 
+cd "$WORKDIR"
+test_expect_success 'cvs update (merge)' \
+  'echo Line 0 >expected &&
+   for i in 1 2 3 4 5 6 7
+   do
+     echo Line $i >>merge
+     echo Line $i >>expected
+   done &&
+   echo Line 8 >>expected &&
+   git add merge &&
+   git commit -q -m "Merge test (pre-merge)" &&
+   git push gitcvs.git >/dev/null &&
+   cd cvswork &&
+   GIT_CONFIG="$git_config" cvs -Q update &&
+   test "$(echo $(grep merge CVS/Entries|cut -d/ -f2,3,5))" = "merge/1.1/" &&
+   diff -q merge ../merge &&
+   ( echo Line 0; cat merge ) >merge.tmp &&
+   mv merge.tmp merge &&  
+   cd "$WORKDIR" &&
+   echo Line 8 >>merge &&
+   git add merge &&
+   git commit -q -m "Merge test (merge)" &&
+   git push gitcvs.git >/dev/null &&
+   cd cvswork &&
+   GIT_CONFIG="$git_config" cvs -Q update &&
+   diff -q merge ../expected'
+
+cd "$WORKDIR"
+test_expect_success 'cvs update (conflict merge)' \
+  '( echo LINE 0; cat merge ) >merge.tmp &&
+   mv merge.tmp merge &&
+   cat >expected.C <<EOF &&
+<<<<<<< merge.mine
+Line 0
+=======
+LINE 0
+>>>>>>> merge.3
+EOF
+   for i in 1 2 3 4 5 6 7 8
+   do
+     echo Line $i >>expected.C
+   done &&
+   git add merge &&
+   git commit -q -m "Merge test (conflict)" &&
+   git push gitcvs.git >/dev/null &&
+   cd cvswork &&
+   GIT_CONFIG="$git_config" cvs -Q update &&
+   diff -q merge ../expected.C'
+
+cd "$WORKDIR"
+test_expect_success 'cvs update (-C)' \
+  'cd cvswork &&
+   GIT_CONFIG="$git_config" cvs -Q update -C &&
+   diff -q merge ../merge'
+
+cd "$WORKDIR"
+test_expect_success 'cvs update (merge no-op)' \
+   'echo Line 9 >>merge &&
+    cp merge cvswork/merge &&
+    git add merge &&
+    git commit -q -m "Merge test (no-op)" &&
+    git push gitcvs.git >/dev/null &&
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs -Q update &&
+    diff -q merge ../merge'
+
 test_done
-- 
1.5.1.4

^ permalink raw reply related

* [PATCH] cvsserver: Add some basic pserver tests
From: Frank Lichtenheld @ 2007-05-13 17:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Frank Lichtenheld
In-Reply-To: <1179078566806-git-send-email-frank@lichtenheld.de>

While we can easily test the cvs <-> git-cvsserver
communication with :fork: and git-cvsserver server
there are some pserver specifics we should test, too.

Currently this are two tests of the pserver authentication.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
---
 t/t9410-git-cvsserver-pserver.sh |   53 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 53 insertions(+), 0 deletions(-)
 create mode 100644 t/t9410-git-cvsserver-pserver.sh

diff --git a/t/t9410-git-cvsserver-pserver.sh b/t/t9410-git-cvsserver-pserver.sh
new file mode 100644
index 0000000..1cf9bc7
--- /dev/null
+++ b/t/t9410-git-cvsserver-pserver.sh
@@ -0,0 +1,53 @@
+#!/bin/sh
+# 
+# Copyright (c) 2007 Frank Lichtenheld
+#
+
+test_description='git-cvsserver pserver access
+
+tests some pserver specific features (like
+authentication).'
+
+. ./test-lib.sh
+
+unset GIT_DIR GIT_CONFIG
+WORKDIR=$(pwd)
+SERVERDIR=$(pwd)/gitcvs.git
+CVSWORK=$(pwd)/cvswork
+export CVSROOT CVSWORK
+
+rm -rf "$CVSWORK" "$SERVERDIR"
+echo >empty &&
+  git add empty &&
+  git commit -q -m "First Commit" &&
+  git clone -q --local --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 &&
+  GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true ||
+  exit 1
+
+test_expect_success 'authentication' \
+  'cat <<EOF | git-cvsserver pserver >log 2>&1 &&
+BEGIN AUTH REQUEST
+$SERVERDIR
+anonymous
+
+END AUTH REQUEST
+EOF
+   tail -n1 log | grep -q "^I LOVE YOU$"'
+
+test_expect_success 'authentication failure (non-anonymous user)' \
+  'if cat <<EOF | git-cvsserver pserver >log 2>&1
+BEGIN AUTH REQUEST
+$SERVERDIR
+git
+
+END AUTH REQUEST
+EOF
+   then
+       false
+   else
+       true
+   fi &&
+   tail -n1 log | grep -q "^I HATE YOU$"'
+
+
+test_done
-- 
1.5.1.4

^ permalink raw reply related

* Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)
From: Martin Eisenhardt @ 2007-05-13 17:49 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, Matthieu.Moy
In-Reply-To: <20070513171707.GA14024@muzzle>

[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]

Hi Eric,
hi Matthieu,
hi list,

On Sunday 13 May 2007 19:17:07 Eric Wong wrote:
> Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> > I'm using git-svn, which usually works fine, but I occasionally get
> > this:
> >
> > $ git-svn dcommit
> >         A       file1
> >         A       file2
> > Network connection closed unexpectedly: Connection closed unexpectedly at
> > /path/to/git-svn line 401 $

I ran into that one, too ... :-(

> > The failure seems to depend on the commit's I'm pushing, since this is
> > reproducible when running several times "dcommit" for the same commit,
> > but the same command also usually works fine on the same repositories
> > (same git repo, same svn target).

I assume that the dcommit runs fine for *other* commits but not for that one 
that produced the error cited above.

In my case, it was simply a problem on my side: I had tried to add a file, 
that already existed in the repository - and subversion rejected the commit. 
Of course, it would be nice if git-svn could simply tell the user about his 
error, i.e., simply copy the output of libsvn or whatever it is using to 
connect to the subversion repo.

Another case where I stumbled upon said error message was when I added an 
empty (sub)directory to a project in subversion and then used git to track 
that project. Since git cannot track an empty directory, it did not appear in 
my git working copy. When I mkdir'd the directory locally (in my git wc) and 
then tried to add files within that repository, I got exactly the same error 
as Matthieu.

>
> Does the patch in
>   http://permalink.gmane.org/gmane.comp.version-control.git/47126
> help?

Not in my case, although this patch resolved the other issue, thanks again 
btw!

Kind regards
Martin Eisenhardt

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] git-svn: fix segfaults due to initial SVN pool being cleared
From: Junio C Hamano @ 2007-05-13 18:17 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20070513150530.GA29082@artemis>

Pierre Habouzit <madcoder@debian.org> writes:

>> Hopefully this fixes all segfault problems users have been
>> experiencing over the past few months.
>> 
>> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ...
>   I confirm it fixes every segfault I was able to reproduce with any
> prior version :)

That's wonderful.  Thanks for testing, everybody.  And thanks
for fixing, Eric.

^ permalink raw reply

* Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)
From: Karl Hasselström @ 2007-05-13 18:24 UTC (permalink / raw)
  To: Martin Eisenhardt; +Cc: git, Eric Wong, Matthieu.Moy
In-Reply-To: <200705131949.38051.list-receive@mneisen.org>

On 2007-05-13 19:49:34 +0200, Martin Eisenhardt wrote:

> Another case where I stumbled upon said error message was when I
> added an empty (sub)directory to a project in subversion and then
> used git to track that project. Since git cannot track an empty
> directory, it did not appear in my git working copy. When I mkdir'd
> the directory locally (in my git wc) and then tried to add files
> within that repository, I got exactly the same error as Matthieu.

It might be a net win to let git-svn handle empty directories by
creating an empty .git-svn-empty-dir file in them, instead of
pretending they don't exist.

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

^ permalink raw reply

* Re: [RFC PATCH] Rename "bury" back to "sink".
From: David Kågedal @ 2007-05-13 18:28 UTC (permalink / raw)
  To: git
In-Reply-To: <20070505201307.GE19253@nan92-1-81-57-214-146.fbx.proxad.net>

Yann Dirson <ydirson@altern.org> writes:

> Side note about the "stg move" name: yes it could possible to mistake it
> for "move file" (especially as we don't have "stg mv").  My current
> state of mind would be to drop add/rm/cp from stgit, and move the "stg
> cp" logic to a new git-cp command.  This way, stgit would just be
> about handling series of patches, with git being used for the
> working-copy.  Any opinions on this ?

For this reason, and others, I think "stg reorder" would be better.
Especially if you implement Karl's suggestion of reordering all
patches in an editor.

-- 
David Kågedal

^ permalink raw reply

* [StGIT PATCH 1/3] Make patch deletion test more specific
From: Karl Hasselström @ 2007-05-13 18:27 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

From: Karl Hasselström <kha@treskal.com>

Don't just test the number of applied and unapplied patches; also test
their name and order.

Signed-off-by: Karl Hasselström <kha@treskal.com>
---

 t/t1601-delete-many.sh |   38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/t/t1601-delete-many.sh b/t/t1601-delete-many.sh
index 8648ed1..8eff308 100755
--- a/t/t1601-delete-many.sh
+++ b/t/t1601-delete-many.sh
@@ -10,13 +10,13 @@ test_expect_success \
 test_expect_success \
     'Create five applied and five unapplied patches' \
     '
-    stg new foo0 -m foo0 &&
-    echo foo0 > foo.txt &&
+    stg new p0 -m p0 &&
+    echo p0 > foo.txt &&
     stg add foo.txt &&
     stg refresh &&
     for i in 1 2 3 4 5 6 7 8 9; do
-        stg new foo$i -m foo$i &&
-        echo foo$i >> foo.txt &&
+        stg new p$i -m p$i &&
+        echo p$i >> foo.txt &&
         stg refresh;
     done &&
     stg pop -n 5
@@ -25,31 +25,31 @@ test_expect_success \
 test_expect_success \
     'Delete some patches' \
     '
-    [ $(stg applied | wc -l) -eq 5 ] &&
-    [ $(stg unapplied | wc -l) -eq 5 ] &&
-    stg delete foo7 foo6 foo3 foo4 &&
-    [ $(stg applied | wc -l) -eq 3 ] &&
-    [ $(stg unapplied | wc -l) -eq 3 ]
+    [ "$(echo $(stg applied))" = "p0 p1 p2 p3 p4" ] &&
+    [ "$(echo $(stg unapplied))" = "p5 p6 p7 p8 p9" ] &&
+    stg delete p7 p6 p3 p4 &&
+    [ "$(echo $(stg applied))" = "p0 p1 p2" ] &&
+    [ "$(echo $(stg unapplied))" = "p5 p8 p9" ]
     '
 
 test_expect_success \
     'Delete some more patches, some of which do not exist' \
     '
-    [ $(stg applied | wc -l) -eq 3 ] &&
-    [ $(stg unapplied | wc -l) -eq 3 ] &&
-    ! stg delete foo7 foo8 foo2 foo0 &&
-    [ $(stg applied | wc -l) -eq 3 ] &&
-    [ $(stg unapplied | wc -l) -eq 3 ]
+    [ "$(echo $(stg applied))" = "p0 p1 p2" ] &&
+    [ "$(echo $(stg unapplied))" = "p5 p8 p9" ] &&
+    ! stg delete p7 p8 p2 p0 &&
+    [ "$(echo $(stg applied))" = "p0 p1 p2" ] &&
+    [ "$(echo $(stg unapplied))" = "p5 p8 p9" ]
     '
 
 test_expect_success \
     'Delete a range of patches' \
     '
-    [ $(stg applied | wc -l) -eq 3 ] &&
-    [ $(stg unapplied | wc -l) -eq 3 ] &&
-    stg delete foo1..foo8 &&
-    [ $(stg applied | wc -l) -eq 1 ] &&
-    [ $(stg unapplied | wc -l) -eq 1 ]
+    [ "$(echo $(stg applied))" = "p0 p1 p2" ] &&
+    [ "$(echo $(stg unapplied))" = "p5 p8 p9" ] &&
+    stg delete p1..p8 &&
+    [ "$(echo $(stg applied))" = "p0" ] &&
+    [ "$(echo $(stg unapplied))" = "p9" ]
     '
 
 test_done

^ permalink raw reply related

* [StGIT PATCH 2/3] Test patch order, not just number of patches
From: Karl Hasselström @ 2007-05-13 18:27 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20070513182720.18810.64330.stgit@yoghurt>

From: Karl Hasselström <kha@treskal.com>

In addition to testing the number of applied patches, also test their
order. And test the order of unapplied patches too while we're at it.

Signed-off-by: Karl Hasselström <kha@treskal.com>
---

 t/t2000-sync.sh |   37 +++++++++++++++++++++++++++----------
 1 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/t/t2000-sync.sh b/t/t2000-sync.sh
index 02345bb..69ab1ac 100755
--- a/t/t2000-sync.sh
+++ b/t/t2000-sync.sh
@@ -29,7 +29,9 @@ test_expect_success \
     stg add foo3.txt &&
     stg refresh &&
     stg export &&
-    stg pop
+    stg pop &&
+    [ "$(echo $(stg applied))" = "p1 p2" ] &&
+    [ "$(echo $(stg unapplied))" = "p3" ]
     '
 
 test_expect_success \
@@ -38,15 +40,17 @@ test_expect_success \
     stg branch -c foo base &&
     stg new p1 -m p1 &&
     stg new p2 -m p2 &&
-    stg new p3 -m p3
-    test $(stg applied -c) -eq 3
+    stg new p3 -m p3 &&
+    [ "$(echo $(stg applied))" = "p1 p2 p3" ] &&
+    [ "$(echo $(stg unapplied))" = "" ]
     '
 
 test_expect_success \
     'Synchronise second patch with the master branch' \
     '
     stg sync -b master p2 &&
-    test $(stg applied -c) -eq 3 &&
+    [ "$(echo $(stg applied))" = "p1 p2 p3" ] &&
+    [ "$(echo $(stg unapplied))" = "" ] &&
     test $(cat foo2.txt) = "foo2"
     '
 
@@ -54,7 +58,8 @@ test_expect_success \
     'Synchronise the first two patches with the master branch' \
     '
     stg sync -b master -a &&
-    test $(stg applied -c) -eq 3 &&
+    [ "$(echo $(stg applied))" = "p1 p2 p3" ] &&
+    [ "$(echo $(stg unapplied))" = "" ] &&
     test $(cat foo1.txt) = "foo1" &&
     test $(cat foo2.txt) = "foo2"
     '
@@ -63,7 +68,8 @@ test_expect_success \
     'Synchronise all the patches with the exported series' \
     '
     stg sync -s patches-master/series -a &&
-    test $(stg applied -c) -eq 3 &&
+    [ "$(echo $(stg applied))" = "p1 p2 p3" ] &&
+    [ "$(echo $(stg unapplied))" = "" ] &&
     test $(cat foo1.txt) = "foo1" &&
     test $(cat foo2.txt) = "foo2" &&
     test $(cat foo3.txt) = "foo3"
@@ -73,6 +79,8 @@ test_expect_success \
     'Modify the master patches' \
     '
     stg branch master &&
+    [ "$(echo $(stg applied))" = "p1 p2" ] &&
+    [ "$(echo $(stg unapplied))" = "p3" ] &&
     stg goto p1 &&
     echo bar1 >> foo1.txt &&
     stg refresh &&
@@ -83,6 +91,8 @@ test_expect_success \
     stg goto p3 &&
     echo bar3 >> foo3.txt &&
     stg refresh &&
+    [ "$(echo $(stg applied))" = "p1 p2 p3" ] &&
+    [ "$(echo $(stg unapplied))" = "" ] &&
     stg export &&
     stg branch foo
     '
@@ -91,7 +101,8 @@ test_expect_success \
     'Synchronise second patch with the master branch' \
     '
     stg sync -b master p2 &&
-    test $(stg applied -c) -eq 3 &&
+    [ "$(echo $(stg applied))" = "p1 p2 p3" ] &&
+    [ "$(echo $(stg unapplied))" = "" ] &&
     test $(cat bar2.txt) = "bar2"
     '
 
@@ -104,10 +115,13 @@ test_expect_failure \
 test_expect_success \
     'Restore the stack status after the failed sync' \
     '
-    test $(stg applied -c) -eq 1 &&
+    [ "$(echo $(stg applied))" = "p1" ] &&
+    [ "$(echo $(stg unapplied))" = "p2 p3" ] &&
     stg resolved -a &&
     stg refresh &&
     stg goto p3
+    [ "$(echo $(stg applied))" = "p1 p2 p3" ] &&
+    [ "$(echo $(stg unapplied))" = "" ]
     '
 
 test_expect_failure \
@@ -119,9 +133,12 @@ test_expect_failure \
 test_expect_success \
     'Restore the stack status after the failed sync' \
     '
-    test $(stg applied -c) -eq 3 &&
+    [ "$(echo $(stg applied))" = "p1 p2 p3" ] &&
+    [ "$(echo $(stg unapplied))" = "" ] &&
     stg resolved -a &&
-    stg refresh
+    stg refresh &&
+    [ "$(echo $(stg applied))" = "p1 p2 p3" ] &&
+    [ "$(echo $(stg unapplied))" = "" ]
     '
 
 test_done

^ permalink raw reply related

* [StGIT PATCH 3/3] Use "stg applied" instead of reading the applied file directly
From: Karl Hasselström @ 2007-05-13 18:27 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20070513182720.18810.64330.stgit@yoghurt>

From: Karl Hasselström <kha@treskal.com>



Signed-off-by: Karl Hasselström <kha@treskal.com>
---

 t/t1500-float.sh |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/t/t1500-float.sh b/t/t1500-float.sh
index dbcd8ce..814c9bd 100755
--- a/t/t1500-float.sh
+++ b/t/t1500-float.sh
@@ -20,37 +20,37 @@ test_expect_success \
 	 stg new F -m "f" && echo F >f.txt && stg add f.txt && stg refresh &&
 	 stg new G -m "g" && echo G >g.txt && stg add g.txt && stg refresh &&
 	 stg pop &&
-	 test "`echo \`cat .git/patches/master/applied\``" = "A B C D E F"
+	 test "$(echo $(stg applied))" = "A B C D E F"
 	'
 
 test_expect_success \
 	'Float A to top' \
 	'stg float A &&
-	 test "`echo \`cat .git/patches/master/applied\``" = "B C D E F A"
+	 test "$(echo $(stg applied))" = "B C D E F A"
 	'
 test_expect_success \
 	'Float A to top (noop)' \
 	'stg float A &&
-	 test "`echo \`cat .git/patches/master/applied\``" = "B C D E F A"
+	 test "$(echo $(stg applied))" = "B C D E F A"
 	'
 test_expect_success \
 	'Float B C to top' \
 	'stg float B C &&
-	 test "`echo \`cat .git/patches/master/applied\``" = "D E F A B C"
+	 test "$(echo $(stg applied))" = "D E F A B C"
 	'
 test_expect_success \
 	'Float E A to top' \
 	'stg float E A &&
-	 test "`echo \`cat .git/patches/master/applied\``" = "D F B C E A"
+	 test "$(echo $(stg applied))" = "D F B C E A"
 	'
 test_expect_success \
 	'Float E to top' \
 	'stg float E &&
-	 test "`echo \`cat .git/patches/master/applied\``" = "D F B C A E"
+	 test "$(echo $(stg applied))" = "D F B C A E"
 	'
 test_expect_success \
 	'Float G F to top' \
 	'stg float G F &&
-	 test "`echo \`cat .git/patches/master/applied\``" = "D B C A E G F"
+	 test "$(echo $(stg applied))" = "D B C A E G F"
 	'
 test_done

^ permalink raw reply related

* Re: [PATCH] cvsserver: Add some basic pserver tests
From: Junio C Hamano @ 2007-05-13 18:38 UTC (permalink / raw)
  To: Frank Lichtenheld; +Cc: git
In-Reply-To: <11790785662928-git-send-email-frank@lichtenheld.de>

Frank Lichtenheld <frank@lichtenheld.de> writes:

> While we can easily test the cvs <-> git-cvsserver
> communication with :fork: and git-cvsserver server
> there are some pserver specifics we should test, too.
>
> Currently this are two tests of the pserver authentication.
>
> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
>...
> +test_expect_success 'authentication' \
> +  'cat <<EOF | git-cvsserver pserver >log 2>&1 &&
> +BEGIN AUTH REQUEST
> +$SERVERDIR
> +anonymous
> +
> +END AUTH REQUEST
> +EOF
> +   tail -n1 log | grep -q "^I LOVE YOU$"'

In the past, some people with non-bash shells had trouble with
here text (i.e. <<HERE) inside our test scripts Either our use
of eval to run the test in test_expect_success is not kosher, or
their implementation of eval was slightly broken --- I do not
recall if we ever dug down to the bottom of the problem.  That
is why most tests have here-text to prepare test vectors outside
of tests themselves.

^ permalink raw reply

* ~ and ^
From: Sean @ 2007-05-13 18:44 UTC (permalink / raw)
  To: git


Would it be possible/reasonable to treat a bare ~3 (or ^) on the git command
line as equivalent to "<current branch>~3" ?    At least @{3} already works
this way and it would be nice to do the same for the even more common tilde
and hat operations.  If so, it would be a small convenience to just type:
"git show ~3"

Thanks,
Sean

^ permalink raw reply

* Re: Merging commits together into a super-commit
From: Karl Hasselström @ 2007-05-13 18:43 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Yann Dirson, Carl Worth, Petr Baudis, J. Bruce Fields,
	Linus Torvalds, Johannes Sixt, git
In-Reply-To: <7vy7jtyh8q.fsf@assigned-by-dhcp.cox.net>

On 2007-05-12 12:27:49 -0700, Junio C Hamano wrote:

> Yann Dirson <ydirson@altern.org> writes:
>
> > This supposes there is no valuable branch-level metadata.
> > Currently we have the description - something which could arguably
> > be moved to the git level as well. Otherwise that sounds
> > reasonable to me.
>
> Will it be something like
>
>       [branch "master"]
>               description = "My primary development line"

Yes, exactly. It's just a simple per-branch description string, just
like in your suggestion.

> if so I think that is a reasonable thing to do, from git-core's
> point of view. Obviously, gitk, tig, gitweb and friends can use
> this, too.

Absolutely.

> Are there other per-branch information StGIT wants to keep on an
> active branch that might benefit the core as well?

No, I'm pretty sure there isn't.

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

^ permalink raw reply

* [StGIT PATCH] Compute patch appliedness from commit DAG
From: Karl Hasselström @ 2007-05-13 18:56 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

Don't rely on cached metadata in the "applied" and "unapplied" files
to tell which patches are applied. Instead, consider the patches
reachable from the branch head to be applied, and the rest unapplied.

The order of the applied patches is also taken from the DAG, but we
can't do that for the unapplied patches. So the patch order is saved
to a file whenever it changes, and that file is consulted whenever we
need to compute the order of the unapplied patches.

The point of this excercise is to let users do things such as "git
reset" without confusing stgit. This gives incrased flexibility to
power users, and increased safety to other users. The advantages come
from the removal of redundant metadata: it is no longer possible for
StGIT's appliedness status to get out of sync with the underlying git
commit DAG.

This is how the appliedness and order is computed:

  * First, a single call to git-show-ref gives the hashes of all
    patches and the branch head.

  * Then, "git-rev-list patch1 patch2 patch3 ^branch" lists a small
    set of hashes that contains all the unapplied patches and none of
    the applied patches.

  * Last, "git-rev-list head" lists all commits in the branch. The
    applied patches are listed in the correct order.

This is efficient because none of the two rev-list calls need to look
at more than a small part of the DAG. The first call returns a small
set of commits, and the last call is abandoned before it has time to
look far back in the DAG.

Signed-off-by: Karl Hasselström <kha@treskal.com>
---

I've done some performance testing. First, I ran "stg series" in a
Linus kernel tree with 50 applied and 50 unapplied patches (all
empty). The performance diff was in the noise, indicating that the DAG
walking is indeed cheap per patch, and only has to walk a small part
of the commit DAG. Second, I timed stgit's entire test suite: the time
increased from 1.5 to 2 minutes. So there is a small overhead that can
be seen when you run a gazillion cheap commands, but it remains small
even for big trees and doesn't ruin your day.

Comments and suggestions on the algorithm welcome.

 stgit/commands/commit.py  |    7 -
 stgit/commands/float.py   |    2 
 stgit/commands/imprt.py   |    2 
 stgit/commands/refresh.py |    2 
 stgit/commands/sync.py    |    2 
 stgit/git.py              |    5 +
 stgit/stack.py            |  324 ++++++++++++++++++++++++++++-----------------
 7 files changed, 214 insertions(+), 130 deletions(-)

diff --git a/stgit/commands/commit.py b/stgit/commands/commit.py
index 8af6651..ee17248 100644
--- a/stgit/commands/commit.py
+++ b/stgit/commands/commit.py
@@ -52,15 +52,10 @@ def func(parser, options, args):
     if crt_series.get_protected():
         raise CmdException, 'This branch is protected.  Commit is not permitted'
 
-    crt_head = git.get_head()
-
     print 'Committing %d patches...' % len(applied),
     sys.stdout.flush()
 
-    crt_series.pop_patch(applied[0])
-    git.switch(crt_head)
-
     for patch in applied:
-        crt_series.delete_patch(patch)
+        crt_series.delete_patch_data(patch)
 
     print 'done'
diff --git a/stgit/commands/float.py b/stgit/commands/float.py
index 0e32f6b..8ba76d5 100644
--- a/stgit/commands/float.py
+++ b/stgit/commands/float.py
@@ -48,7 +48,7 @@ def func(parser, options, args):
     check_head_top_equal()
 
     unapplied = crt_series.get_unapplied()
-    applied = crt_series.get_applied()
+    applied = list(crt_series.get_applied()) # a copy, since we'll modify it
     all = unapplied + applied
 
     if options.series:
diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py
index 6fcdc62..884fc15 100644
--- a/stgit/commands/imprt.py
+++ b/stgit/commands/imprt.py
@@ -297,7 +297,7 @@ def __create_patch(filename, message, author_name, author_email,
     else:
         git.apply_patch(diff = diff)
 
-    crt_series.refresh_patch(edit = options.edit,
+    crt_series.refresh_patch(patch, edit = options.edit,
                              show_patch = options.showpatch)
 
     print 'done'    
diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py
index 4cf09ae..8375cdc 100644
--- a/stgit/commands/refresh.py
+++ b/stgit/commands/refresh.py
@@ -133,7 +133,7 @@ def func(parser, options, args):
 
         if autoresolved == 'yes':
             resolved_all()
-        crt_series.refresh_patch(files = args,
+        crt_series.refresh_patch(patch, files = args,
                                  message = options.message,
                                  edit = options.edit,
                                  show_patch = options.showpatch,
diff --git a/stgit/commands/sync.py b/stgit/commands/sync.py
index db52bcd..3196a68 100644
--- a/stgit/commands/sync.py
+++ b/stgit/commands/sync.py
@@ -165,7 +165,7 @@ def func(parser, options, args):
         if git.local_changes(verbose = False):
             # index (cache) already updated by the git merge. The
             # backup information was already reset above
-            crt_series.refresh_patch(cache_update = False, backup = False,
+            crt_series.refresh_patch(p, cache_update = False, backup = False,
                                      log = 'sync')
             print 'done (updated)'
         else:
diff --git a/stgit/git.py b/stgit/git.py
index 837f927..4041c98 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -189,8 +189,11 @@ def _output_one_line(cmd, file_desc = None):
                                                 p.childerr.read().strip())
     return output
 
-def _output_lines(cmd):
+def _output_lines(cmd, input = []):
     p=popen2.Popen3(cmd, True)
+    for line in input:
+        p.tochild.write(line)
+    p.tochild.close()
     lines = p.fromchild.readlines()
     if p.wait():
         raise GitException, '%s failed (%s)' % (str(cmd),
diff --git a/stgit/stack.py b/stgit/stack.py
index d0008bc..c105b21 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -18,11 +18,12 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import sys, os, re
+import sys, os, popen2, re
 
 from stgit.utils import *
 from stgit import git, basedir, templates
 from stgit.config import config
+from sets import Set
 
 
 # stack exception class
@@ -273,6 +274,163 @@ class Patch(StgitObject):
         self.__update_log_ref(value)
 
 
+class PatchorderCache:
+    """An object that keeps track of the patch order for a series, as
+    saved in its patchorder file."""
+    def __init__(self, series):
+        self.__series = series
+        self.__invalidate()
+    def __invalidate(self):
+        self.__patchnames = None
+        self.__position = None
+    def __cache(self):
+        if self.__patchnames != None:
+            return # already cached
+
+        self.__patchnames = []
+        self.__position = {}
+
+        def read_file(fn):
+            for line in file(fn):
+                name = line.strip()
+                assert not name in self.__position
+                self.__position[name] = len(self.__patchnames)
+                self.__patchnames.append(name)
+
+        # If the patchorder file exists, read that. Otherwise, fall
+        # back to the old applied/unapplied files.
+        pof, af, uaf = [os.path.join(self.__series._dir(), fn) for fn in
+                        ['patchorder', 'applied', 'unapplied']]
+        if os.path.isfile(pof):
+            read_file(pof)
+        elif os.path.isfile(af) and os.path.isfile(uaf):
+            read_file(af)
+            read_file(uaf)
+    def set_patchorder(self, new_order):
+        self.__invalidate()
+        f = file(os.path.join(self.__series._dir(), 'patchorder'), 'w')
+        for name in new_order:
+            f.write('%s\n' % name)
+        f.close()
+    def cmp(self, name1, name2):
+        """Compare two patch names to see which patch comes first. If
+        both patches are listed in the patchorder file, sort them by
+        the order they appear there; if one is listed and the other
+        not, the listed patch goes first; and if neither is listed,
+        sort them by their names."""
+        self.__cache()
+        largepos = len(self.__patchnames)
+        pos1 = self.__position.get(name1, largepos)
+        pos2 = self.__position.get(name2, largepos)
+        return cmp((pos1, name1), (pos2, name2))
+
+def read_refs(branch):
+    """Return a mapping from patches and branch head to hashes for a
+    given branch. The patches are listed by name; the branch head is
+    None."""
+    refs = {}
+    patchpat = re.compile(r'^refs/patches/%s/([^\.]+)$' % branch)
+    for line in git._output_lines('git-show-ref'):
+        sha1, ref = line.split()
+        m = re.match(patchpat, ref)
+        if m:
+            refs[m.group(1)] = sha1
+        elif ref == 'refs/heads/%s' % branch:
+            refs[None] = sha1
+    return refs
+
+def unapplied_patches(ref2hash):
+    """Given a map of patch names (and the branch head, keyed by None)
+    to hashes, return the set of unapplied patches."""
+    hash2refs = {}
+    for r, h in ref2hash.iteritems():
+        hash2refs.setdefault(h, Set()).add(r)
+
+    unapplied = Set()
+    for line in git._output_lines(
+        'git-rev-list --stdin',
+        ('%s%s\n' % (['', '^'][ref == None], sha1)
+         for ref, sha1 in ref2hash.iteritems())):
+        for ref in hash2refs.get(line.strip(), []):
+            unapplied.add(ref)
+    return unapplied
+
+def sort_applied_patches(ref2hash):
+    """Given a map of patch names (and the branch head, keyed by None)
+    to hashes, return a list with the applied patches in stack order.
+    All patches in the map must be applied."""
+    hash2refs = {}
+    for r, h in ref2hash.iteritems():
+        if r != None:
+            hash2refs.setdefault(h, Set()).add(r)
+
+    missing = Set(ref for ref in ref2hash.iterkeys() if ref != None)
+    if not missing:
+        return []
+    applied = []
+    grl = popen2.Popen3('git-rev-list %s' % ref2hash[None], True)
+    for line in grl.fromchild:
+        for ref in hash2refs.get(line.strip(), []):
+            applied.append(ref)
+            missing.remove(ref)
+        if not missing:
+            applied.reverse()
+            return applied
+
+    raise StackException, 'Could not find patches: %s' % ', '.join(missing)
+
+class AppliedCache:
+    """An object that keeps track of the appliedness and order of the
+    patches in a patch series."""
+    def __init__(self, series):
+        self.__series = series
+        self.__order = PatchorderCache(series)
+        self.__invalidate()
+    def get_applied(self):
+        self.__cache()
+        return self.__applied
+    def get_unapplied(self):
+        self.__cache()
+        return self.__unapplied
+    def rename(self, oldname, newname):
+        """Rename a patch."""
+        self.__cache()
+        for lst in (self.__applied, self.__unapplied):
+            try:
+                lst[lst.index(oldname)] = newname
+            except ValueError:
+                pass # lst.index() couldn't find the index
+            else:
+                self.__write_patchorder()
+                return
+        raise StackException, 'Unknown patch "%s"' % oldname
+    def __write_patchorder(self):
+        self.__order.set_patchorder(self.get_applied() + self.get_unapplied())
+    def set_patchorder(self, new_order):
+        self.__order.set_patchorder(new_order)
+        self.refresh()
+    def refresh(self):
+        """Re-read patch appliedness info, and write patch order to
+        disk."""
+        self.__invalidate()
+        self.__write_patchorder()
+    def __invalidate(self):
+        self.__applied = None
+        self.__unapplied = None
+    def __cached(self):
+        return (self.__applied != None)
+    def __cache(self):
+        if self.__cached():
+            return
+        patches = read_refs(self.__series.get_branch())
+        unapplied = unapplied_patches(patches)
+        for patch in unapplied:
+            del patches[patch]
+        self.__applied = sort_applied_patches(patches)
+        self.__unapplied = list(unapplied)
+        self.__unapplied.sort(self.__order.cmp)
+
+
 class Series(StgitObject):
     """Class including the operations on series
     """
@@ -292,8 +450,6 @@ class Series(StgitObject):
         self.__refs_dir = os.path.join(self.__base_dir, 'refs', 'patches',
                                        self.__name)
 
-        self.__applied_file = os.path.join(self._dir(), 'applied')
-        self.__unapplied_file = os.path.join(self._dir(), 'unapplied')
         self.__hidden_file = os.path.join(self._dir(), 'hidden')
         self.__descr_file = os.path.join(self._dir(), 'description')
 
@@ -313,6 +469,8 @@ class Series(StgitObject):
         if self.is_initialised() and not os.path.isdir(self.__trash_dir):
             os.makedirs(self.__trash_dir)
 
+        self.__applied_cache = AppliedCache(self)
+
     def __patch_name_valid(self, name):
         """Raise an exception if the patch name is not valid.
         """
@@ -340,11 +498,7 @@ class Series(StgitObject):
     def get_current(self):
         """Return the name of the topmost patch, or None if there is
         no such patch."""
-        try:
-            applied = self.get_applied()
-        except StackException:
-            # No "applied" file: branch is not initialized.
-            return None
+        applied = self.get_applied()
         try:
             return applied[-1]
         except IndexError:
@@ -352,20 +506,10 @@ class Series(StgitObject):
             return None
 
     def get_applied(self):
-        if not os.path.isfile(self.__applied_file):
-            raise StackException, 'Branch "%s" not initialised' % self.__name
-        f = file(self.__applied_file)
-        names = [line.strip() for line in f.readlines()]
-        f.close()
-        return names
+        return self.__applied_cache.get_applied()
 
     def get_unapplied(self):
-        if not os.path.isfile(self.__unapplied_file):
-            raise StackException, 'Branch "%s" not initialised' % self.__name
-        f = file(self.__unapplied_file)
-        names = [line.strip() for line in f.readlines()]
-        f.close()
-        return names
+        return self.__applied_cache.get_unapplied()
 
     def get_hidden(self):
         if not os.path.isfile(self.__hidden_file):
@@ -377,12 +521,12 @@ class Series(StgitObject):
 
     def get_base(self):
         # Return the parent of the bottommost patch, if there is one.
-        if os.path.isfile(self.__applied_file):
-            bottommost = file(self.__applied_file).readline().strip()
-            if bottommost:
-                return self.get_patch(bottommost).get_bottom()
-        # No bottommost patch, so just return HEAD
-        return git.get_head()
+        applied = self.get_applied()
+        if applied:
+            return self.get_patch(applied[0]).get_bottom()
+        else:
+            # No bottommost patch, so just return HEAD
+            return git.get_head()
 
     def get_head(self):
         """Return the head of the branch
@@ -510,8 +654,6 @@ class Series(StgitObject):
 
         self.set_parent(parent_remote, parent_branch)
 
-        self.create_empty_field('applied')
-        self.create_empty_field('unapplied')
         self.create_empty_field('description')
         os.makedirs(os.path.join(self._dir(), 'patches'))
         os.makedirs(self.__refs_dir)
@@ -643,10 +785,6 @@ class Series(StgitObject):
 
             # FIXME: find a way to get rid of those manual removals
             # (move functionality to StgitObject ?)
-            if os.path.exists(self.__applied_file):
-                os.remove(self.__applied_file)
-            if os.path.exists(self.__unapplied_file):
-                os.remove(self.__unapplied_file)
             if os.path.exists(self.__hidden_file):
                 os.remove(self.__hidden_file)
             if os.path.exists(self.__descr_file):
@@ -676,7 +814,7 @@ class Series(StgitObject):
         config.unset('branch.%s.merge' % self.__name)
         config.unset('branch.%s.stgit.parentbranch' % self.__name)
 
-    def refresh_patch(self, files = None, message = None, edit = False,
+    def refresh_patch(self, name, files = None, message = None, edit = False,
                       show_patch = False,
                       cache_update = True,
                       author_name = None, author_email = None,
@@ -685,10 +823,6 @@ class Series(StgitObject):
                       backup = False, sign_str = None, log = 'refresh'):
         """Generates a new commit for the given patch
         """
-        name = self.get_current()
-        if not name:
-            raise StackException, 'No patches applied'
-
         patch = Patch(name, self.__patch_dir, self.__refs_dir)
 
         descr = patch.get_description()
@@ -778,9 +912,10 @@ class Series(StgitObject):
         """Creates a new patch
         """
 
+        appl, unappl = self.get_applied(), self.get_unapplied()
         if name != None:
             self.__patch_name_valid(name)
-            if self.patch_applied(name) or self.patch_unapplied(name):
+            if name in appl or name in unappl:
                 raise StackException, 'Patch "%s" already exists' % name
 
         if not message and can_edit:
@@ -817,28 +952,34 @@ class Series(StgitObject):
 
         if unapplied:
             self.log_patch(patch, 'new')
-
-            patches = [patch.get_name()] + self.get_unapplied()
-
-            f = file(self.__unapplied_file, 'w+')
-            f.writelines([line + '\n' for line in patches])
-            f.close()
+            order = appl + [patch.get_name()] + unappl
         elif before_existing:
             self.log_patch(patch, 'new')
-
-            insert_string(self.__applied_file, patch.get_name())
+            order = [patch.get_name()] + appl + unappl
         else:
-            append_string(self.__applied_file, patch.get_name())
+            order = appl + [patch.get_name()] + unappl
             if refresh:
-                self.refresh_patch(cache_update = False, log = 'new')
+                self.refresh_patch(name, cache_update = False, log = 'new')
+        self.__applied_cache.set_patchorder(order)
+
+
+    def delete_patch_data(self, name):
+        """Deletes the stgit data for a patch."""
+        patch = Patch(name, self.__patch_dir, self.__refs_dir)
+
+        # save the commit id to a trash file
+        write_string(os.path.join(self.__trash_dir, name), patch.get_top())
+
+        patch.delete()
+        if self.patch_hidden(name):
+            self.unhide_patch(name)
 
     def delete_patch(self, name):
         """Deletes a patch
         """
         self.__patch_name_valid(name)
-        patch = Patch(name, self.__patch_dir, self.__refs_dir)
 
-        if self.__patch_is_current(patch):
+        if self.get_current() == name:
             self.pop_patch(name)
         elif self.patch_applied(name):
             raise StackException, 'Cannot remove an applied patch, "%s", ' \
@@ -846,19 +987,8 @@ class Series(StgitObject):
         elif not name in self.get_unapplied():
             raise StackException, 'Unknown patch "%s"' % name
 
-        # save the commit id to a trash file
-        write_string(os.path.join(self.__trash_dir, name), patch.get_top())
-
-        patch.delete()
-
-        unapplied = self.get_unapplied()
-        unapplied.remove(name)
-        f = file(self.__unapplied_file, 'w+')
-        f.writelines([line + '\n' for line in unapplied])
-        f.close()
-
-        if self.patch_hidden(name):
-            self.unhide_patch(name)
+        self.delete_patch_data(name)
+        self.__applied_cache.refresh()
 
     def forward_patches(self, names):
         """Try to fast-forward an array of patches.
@@ -928,13 +1058,7 @@ class Series(StgitObject):
             return 0
 
         git.switch(top)
-
-        append_strings(self.__applied_file, names[0:forwarded])
-
-        f = file(self.__unapplied_file, 'w+')
-        f.writelines([line + '\n' for line in unapplied])
-        f.close()
-
+        self.__applied_cache.refresh()
         return forwarded
 
     def merged_patches(self, names):
@@ -1009,13 +1133,6 @@ class Series(StgitObject):
                           'Use "refresh" after fixing the conflicts or ' \
                           'revert the operation with "push --undo".'
 
-        append_string(self.__applied_file, name)
-
-        unapplied.remove(name)
-        f = file(self.__unapplied_file, 'w+')
-        f.writelines([line + '\n' for line in unapplied])
-        f.close()
-
         # head == bottom case doesn't need to refresh the patch
         if empty or head != bottom:
             if not ex:
@@ -1025,15 +1142,17 @@ class Series(StgitObject):
                     log = 'push(m)'
                 else:
                     log = 'push'
-                self.refresh_patch(cache_update = False, log = log)
+                self.refresh_patch(name, cache_update = False, log = log)
             else:
                 # we store the correctly merged files only for
                 # tracking the conflict history. Note that the
                 # git.merge() operations should always leave the index
                 # in a valid state (i.e. only stage 0 files)
-                self.refresh_patch(cache_update = False, log = 'push(c)')
+                self.refresh_patch(name, cache_update = False, log = 'push(c)')
                 raise StackException, str(ex)
 
+        self.__applied_cache.refresh()
+
         return modified
 
     def undo_push(self):
@@ -1062,10 +1181,7 @@ class Series(StgitObject):
     def pop_patch(self, name, keep = False):
         """Pops the top patch from the stack
         """
-        applied = self.get_applied()
-        applied.reverse()
-        assert(name in applied)
-
+        assert(name in self.get_applied())
         patch = Patch(name, self.__patch_dir, self.__refs_dir)
 
         if git.get_head_file() == self.get_branch():
@@ -1075,24 +1191,7 @@ class Series(StgitObject):
             git.switch(patch.get_bottom(), keep)
         else:
             git.set_branch(self.get_branch(), patch.get_bottom())
-
-        # save the new applied list
-        idx = applied.index(name) + 1
-
-        popped = applied[:idx]
-        popped.reverse()
-        unapplied = popped + self.get_unapplied()
-
-        f = file(self.__unapplied_file, 'w+')
-        f.writelines([line + '\n' for line in unapplied])
-        f.close()
-
-        del applied[:idx]
-        applied.reverse()
-
-        f = file(self.__applied_file, 'w+')
-        f.writelines([line + '\n' for line in applied])
-        f.close()
+        self.__applied_cache.refresh()
 
     def empty_patch(self, name):
         """Returns True if the patch is empty
@@ -1118,7 +1217,8 @@ class Series(StgitObject):
 
         if oldname == newname:
             raise StackException, '"To" name and "from" name are the same'
-
+        if oldname in applied or oldname in unapplied:
+            raise StackException, 'Unknown patch "%s"' % oldname
         if newname in applied or newname in unapplied:
             raise StackException, 'Patch "%s" already exists' % newname
 
@@ -1126,23 +1226,9 @@ class Series(StgitObject):
             self.unhide_patch(oldname)
             self.hide_patch(newname)
 
-        if oldname in unapplied:
-            Patch(oldname, self.__patch_dir, self.__refs_dir).rename(newname)
-            unapplied[unapplied.index(oldname)] = newname
-
-            f = file(self.__unapplied_file, 'w+')
-            f.writelines([line + '\n' for line in unapplied])
-            f.close()
-        elif oldname in applied:
-            Patch(oldname, self.__patch_dir, self.__refs_dir).rename(newname)
-
-            applied[applied.index(oldname)] = newname
-
-            f = file(self.__applied_file, 'w+')
-            f.writelines([line + '\n' for line in applied])
-            f.close()
-        else:
-            raise StackException, 'Unknown patch "%s"' % oldname
+        Patch(oldname, self.__patch_dir, self.__refs_dir).rename(newname)
+        self.__patchorder_cache.rename(oldname, newname)
+        self.__applied_cache.rename(oldname, newname)
 
     def log_patch(self, patch, message):
         """Generate a log commit for a patch

^ permalink raw reply related

* Re: [PATCH 0/3] Fix assorted white space damage
From: Junio C Hamano @ 2007-05-13 18:57 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Marco Costalba, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0705131230230.6410@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> AFAICT the documentation has trailing whitespace also, to indicate that 
> the paragraph does not end here. Correct me if I'm wrong, and these 
> trailing whitespaces are not necessary.

Thanks for the tip --- I did not know about deliberate trailing
whitespaces there.

It should be easy to verify by comparing the build products
before and after trailing whitespace removal, so I will try to
remember doing that.

^ permalink raw reply

* Re: [RFC PATCH] Rename "bury" back to "sink".
From: Karl Hasselström @ 2007-05-13 19:06 UTC (permalink / raw)
  To: David Kågedal; +Cc: git
In-Reply-To: <87zm48wpbs.fsf@morpheus.local>

On 2007-05-13 11:28:23 -0700, David Kågedal wrote:

> For this reason, and others, I think "stg reorder" would be better.
> Especially if you implement Karl's suggestion of reordering all
> patches in an editor.

Sounds very sane to me.

Additionally, in order to appeal to users who like living on the edge,
we should probably consider implementing "stg shuffle".

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

^ permalink raw reply

* Re: ~ and ^
From: Junio C Hamano @ 2007-05-13 19:07 UTC (permalink / raw)
  To: Sean; +Cc: git
In-Reply-To: <20070513144442.b3cba792.seanlkml@sympatico.ca>

Sean <seanlkml@sympatico.ca> writes:

> Would it be possible/reasonable to treat a bare ~3 (or ^) on the git command
> line as equivalent to "<current branch>~3" ?    At least @{3} already works
> this way and it would be nice to do the same for the even more common tilde
> and hat operations.  If so, it would be a small convenience to just type:
> "git show ~3"

Interesting.

At least, "ls ~0" does not seem to get confused by bash to be
the home directory of the root user, so I do not offhand think
of a reason not to have ~$n as a synonym to HEAD~$n.  Would that
be useful?  I dunno.

P.S.

Welcome back to the list ;-)

^ permalink raw reply

* Re: [PATCH (amend)] gitweb: Check if requested object exists
From: Junio C Hamano @ 2007-05-13 19:12 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200705122116.34486.jnareb@gmail.com>

Thanks.

^ permalink raw reply

* Re: ~ and ^
From: Karl Hasselström @ 2007-05-13 19:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sean, git
In-Reply-To: <7v8xbssfti.fsf@assigned-by-dhcp.cox.net>

On 2007-05-13 12:07:21 -0700, Junio C Hamano wrote:

> Sean <seanlkml@sympatico.ca> writes:
>
> > Would it be possible/reasonable to treat a bare ~3 (or ^) on the
> > git command line as equivalent to "<current branch>~3" ? At least
> > @{3} already works this way and it would be nice to do the same
> > for the even more common tilde and hat operations. If so, it would
> > be a small convenience to just type: "git show ~3"
>
> Interesting.

Yes. I think we should have this, even if just for consistency.

> At least, "ls ~0" does not seem to get confused by bash to be the
> home directory of the root user, so I do not offhand think of a
> reason not to have ~$n as a synonym to HEAD~$n.

It seems that ~foo is only expanded to foo's home if there is such a
user. Since all-numeric users probably aren't common, we can probably
get away with it.

> Would that be useful? I dunno.

It would allow you to say

  git log ~5..

instead of

  git log HEAD~5..

so I'd say it is useful.

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

^ permalink raw reply

* Re: [PATCH] cvsserver: Limit config parser to needed options
From: Junio C Hamano @ 2007-05-13 19:16 UTC (permalink / raw)
  To: Frank Lichtenheld; +Cc: git, Martin Langhoff
In-Reply-To: <11790153841188-git-send-email-frank@lichtenheld.de>

Thanks.

^ permalink raw reply

* Re: ~ and ^
From: Sean @ 2007-05-13 19:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8xbssfti.fsf@assigned-by-dhcp.cox.net>

On Sun, 13 May 2007 12:07:21 -0700
Junio C Hamano <junkio@cox.net> wrote:

> Interesting.
> 
> At least, "ls ~0" does not seem to get confused by bash to be
> the home directory of the root user, so I do not offhand think
> of a reason not to have ~$n as a synonym to HEAD~$n.  Would that
> be useful?  I dunno.

Not going to shake the world but it's a small convenience.  Would be
a bit nicer if a lone ~ worked as well without escaping, but I'm not
sure you'd accept a patch to assume "~" whenever git is passed the
users home directory as a lone option ;)

> P.S.
> 
> Welcome back to the list ;-)
> 

Thanks.

Sean

^ permalink raw reply

* Re: Merging commits together into a super-commit
From: Yann Dirson @ 2007-05-13 19:35 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Carl Worth, Petr Baudis, J. Bruce Fields, Linus Torvalds,
	Johannes Sixt, git
In-Reply-To: <7vy7jtyh8q.fsf@assigned-by-dhcp.cox.net>

On Sat, May 12, 2007 at 12:27:49PM -0700, Junio C Hamano wrote:
> Are there other per-branch information StGIT wants to keep on an
> active branch that might benefit the core as well?

Maybe the "protected" attribute, to forbid commands to touch to the
stack ?  Not sure, however, since the semantics would probably be a
bit different in git (eg. just forbid update-ref) and in StGIT (also
protects unapplied patches).

Best regards,
-- 
Yann

^ permalink raw reply

* Re: ~ and ^
From: Sean @ 2007-05-13 19:49 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Junio C Hamano, git
In-Reply-To: <20070513191350.GB14657@diana.vm.bytemark.co.uk>

On Sun, 13 May 2007 21:13:50 +0200
Karl Hasselström <kha@treskal.com> wrote:

> It seems that ~foo is only expanded to foo's home if there is such a
> user. Since all-numeric users probably aren't common, we can probably
> get away with it.

Someone just mentioned to me that ~# is expanded by Bash if you use the
pushd command to keep a directory stack.   So ~1 will stop working
if you use "pushd".   Maybe it'd be more confusing than helpful.  On the
other hand, there's probably no reason for Git to just give an error
message if it receives a properly escaped lone "~", "~#", or "^".

Sean

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox