Git development
 help / color / mirror / Atom feed
* [PATCH 2/2] Backup the array passed to fetch_pack so we can free items
From: Shawn O. Pearce @ 2007-09-14 22:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

fetch_pack() can call remove_duplicates() on its input array and
this will possibly overwrite an earlier entry with a later one if
there are any duplicates in the input array.  In such a case the
caller here might then attempt to free an item multiple times as
it goes through its cleanup.

I also forgot to free the heads array we pass down into fetch_pack()
when I introduced the allocation of it in this function during my
builtin-fetch cleanup series.  Better free it while we are here
working on related memory management fixes.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 transport.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/transport.c b/transport.c
index 0882edd..0338ed4 100644
--- a/transport.c
+++ b/transport.c
@@ -311,6 +311,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 {
 	struct git_transport_data *data = transport->data;
 	char **heads = xmalloc(nr_heads * sizeof(*heads));
+	char **origh = xmalloc(nr_heads * sizeof(*origh));
 	struct ref *refs;
 	char *dest = xstrdup(transport->url);
 	struct fetch_pack_args args;
@@ -329,11 +330,13 @@ static int fetch_refs_via_pack(struct transport *transport,
 	setup_fetch_pack(&args);
 
 	for (i = 0; i < nr_heads; i++)
-		heads[i] = xstrdup(to_fetch[i]->name);
+		origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
 	refs = fetch_pack(dest, nr_heads, heads, &transport->pack_lockfile);
 
 	for (i = 0; i < nr_heads; i++)
-		free(heads[i]);
+		free(origh[i]);
+	free(origh);
+	free(heads);
 	free_refs(refs);
 	free(dest);
 	return 0;
-- 
1.5.3.1.921.g8c3b

^ permalink raw reply related

* [PATCH 1/2] Fix builtin-fetch memory corruption by not overstepping array
From: Shawn Pearce @ 2007-09-14 22:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

A long time ago Junio added this line to always ensure that the
output array created by remove_duplicates() had a NULL as its
terminating node.  Today none of the downstream consumers of this
array care about a NULL terminator; they only pay attention to the
size of the array (as indicated by nr_heads).  In (nearly?) all
cases passing a NULL element will cause SIGSEGV failures.  So this
NULL terminal is not actually necessary.

Unfortunately we cannot continue to NULL terminate the array at
this point as the array may only have been allocated large enough
to match the input of nr_heads.  If there are no duplicates than
we would be trying to store NULL into heads[nr_heads] and that may
be outside of the array.

My recent series to cleanup builtin-fetch changed the allocation of
the heads array from 256 entries to exactly nr_heads thus ensuring
we were always overstepping the array and causing memory corruption.

Signed-off-by: Shawn Pearce <spearce@blink.home.spearce.org>
---

 This patch is meant for the top of the db/fetch-pack series
 (currently in pu) but it may actually need to be applied back on
 prior versions (e.g. maint).  The code affected seems to predate
 builtin-fetch by quite a bit and we may just be getting lucky that
 the array is always allocated large enough for this extra entry.

 It almost fixes the corruption Junio saw in t3200.  When combined
 with the next patch (2/2) that corruption appears to be fixed on
 both my Mac OS X and my Linux amd64 system.

 builtin-fetch-pack.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index b0936cc..2977a94 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -654,7 +654,6 @@ static int remove_duplicates(int nr_heads, char **heads)
 			heads[dst] = heads[src];
 		dst++;
 	}
-	heads[dst] = 0;
 	return dst;
 }
 
-- 
1.5.3.1.921.g8c3b

^ permalink raw reply related

* Re: [StGit PATCH 03/13] Moved that status function to the status command file
From: David Kågedal @ 2007-09-14 22:36 UTC (permalink / raw)
  To: git
In-Reply-To: <20070914223124.7001.29374.stgit@morpheus.local>

David Kågedal <davidk@lysator.liu.se> writes:

> diff --git a/t/t0002-status.sh b/t/t0002-status.sh
> index 790b9fb..d0c31b2 100755
> --- a/t/t0002-status.sh
> +++ b/t/t0002-status.sh
> @@ -60,6 +60,14 @@ test_expect_success 'Status with an added file' '
>  '
>  
>  cat > expected.txt <<EOF
> +foo/bar
> +EOF
> +test_expect_success 'Status with an added file and -n option' '
> +    stg status -n > output.txt &&
> +    diff -u expected.txt output.txt
> +'
> +
> +cat > expected.txt <<EOF
>  EOF
>  test_expect_success 'Status after refresh' '
>      stg new -m "first patch" &&
>

Oops, that should have been in the first patch.

-- 
David Kågedal

^ permalink raw reply

* [StGit PATCH 13/13] Remove the 'top' field
From: David Kågedal @ 2007-09-14 22:32 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

The top is instead implicitly defined by the patch ref.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/stack.py |   26 +++++++++-----------------
 1 files changed, 9 insertions(+), 17 deletions(-)


diff --git a/stgit/stack.py b/stgit/stack.py
index 00b91c6..94d3eee 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -158,7 +158,6 @@ class Patch(StgitObject):
 
     def create(self):
         os.mkdir(self._dir())
-        self.create_empty_field('top')
 
     def delete(self):
         for f in os.listdir(self._dir()):
@@ -190,11 +189,6 @@ class Patch(StgitObject):
     def __update_log_ref(self, ref):
         git.set_ref(self.__log_ref, ref)
 
-    def update_top_ref(self):
-        top = self.get_top()
-        if top:
-            self.__update_top_ref(top)
-
     def get_old_bottom(self):
         return git.get_commit(self.get_old_top()).get_parent()
 
@@ -205,26 +199,18 @@ class Patch(StgitObject):
         return self._get_field('top.old')
 
     def get_top(self):
-        top = self._get_field('top')
-        try:
-            ref = git.rev_parse(self.__top_ref)
-        except:
-            ref = None
-        assert not ref or top == ref
-        return top
+        return git.rev_parse(self.__top_ref)
 
     def set_top(self, value, backup = False):
         if backup:
-            curr = self._get_field('top')
+            curr = self.get_top()
             self._set_field('top.old', curr)
-        self._set_field('top', value)
         self.__update_top_ref(value)
 
     def restore_old_boundaries(self):
         top = self._get_field('top.old')
 
         if top:
-            self._set_field('top', top)
             self.__update_top_ref(top)
             return True
         else:
@@ -436,7 +422,13 @@ class Series(PatchSet):
                 patch = patch.strip()
                 os.rename(os.path.join(branch_dir, patch),
                           os.path.join(patch_dir, patch))
-                Patch(patch, patch_dir, refs_base).update_top_ref()
+                topfield = os.path.join(patch_dir, patch, 'top')
+                if os.path.isfile(topfield):
+                    top = read_string(topfield, False)
+                else:
+                    top = None
+                if top:
+                    git.set_ref(refs_base + '/' + patch, top)
             set_format_version(1)
 
         # Update 1 -> 2.

^ permalink raw reply related

* [StGit PATCH 12/13] Remove the 'bottom' field
From: David Kågedal @ 2007-09-14 22:32 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

The bottom is instead always calculated from the top by getting its
parent commit.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/commands/sync.py |    1 -
 stgit/stack.py         |   29 +++--------------------------
 2 files changed, 3 insertions(+), 27 deletions(-)


diff --git a/stgit/commands/sync.py b/stgit/commands/sync.py
index 580b5bd..c18d7e0 100644
--- a/stgit/commands/sync.py
+++ b/stgit/commands/sync.py
@@ -148,7 +148,6 @@ def func(parser, options, args):
 
         # reset the patch backup information. That's needed in case we
         # undo the sync but there were no changes made
-        patch.set_bottom(bottom, backup = True)
         patch.set_top(top, backup = True)
 
         # the actual merging (either from a branch or an external file)
diff --git a/stgit/stack.py b/stgit/stack.py
index d9a2e56..00b91c6 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -158,7 +158,6 @@ class Patch(StgitObject):
 
     def create(self):
         os.mkdir(self._dir())
-        self.create_empty_field('bottom')
         self.create_empty_field('top')
 
     def delete(self):
@@ -197,22 +196,10 @@ class Patch(StgitObject):
             self.__update_top_ref(top)
 
     def get_old_bottom(self):
-        old_bottom = self._get_field('bottom.old')
-        old_top = self.get_old_top()
-        assert old_bottom == git.get_commit(old_top).get_parent()
-        return old_bottom
+        return git.get_commit(self.get_old_top()).get_parent()
 
     def get_bottom(self):
-        bottom = self._get_field('bottom')
-        top = self.get_top()
-        assert bottom == git.get_commit(top).get_parent()
-        return self._get_field('bottom')
-
-    def set_bottom(self, value, backup = False):
-        if backup:
-            curr = self._get_field('bottom')
-            self._set_field('bottom.old', curr)
-        self._set_field('bottom', value)
+        return git.get_commit(self.get_top()).get_parent()
 
     def get_old_top(self):
         return self._get_field('top.old')
@@ -232,14 +219,11 @@ class Patch(StgitObject):
             self._set_field('top.old', curr)
         self._set_field('top', value)
         self.__update_top_ref(value)
-        self.get_bottom() # check the assert
 
     def restore_old_boundaries(self):
-        bottom = self._get_field('bottom.old')
         top = self._get_field('top.old')
 
-        if top and bottom:
-            self._set_field('bottom', bottom)
+        if top:
             self._set_field('top', top)
             self.__update_top_ref(top)
             return True
@@ -806,7 +790,6 @@ class Series(PatchSet):
                                committer_name = committer_name,
                                committer_email = committer_email)
 
-        patch.set_bottom(bottom, backup = backup)
         patch.set_top(commit_id, backup = backup)
         patch.set_description(descr)
         patch.set_authname(author_name)
@@ -914,11 +897,8 @@ class Series(PatchSet):
                                    committer_name = committer_name,
                                    committer_email = committer_email)
             # set the patch top to the new commit
-            patch.set_bottom(bottom)
             patch.set_top(commit_id)
         else:
-            assert top != bottom
-            patch.set_bottom(bottom)
             patch.set_top(top)
 
         self.log_patch(patch, 'new')
@@ -972,7 +952,6 @@ class Series(PatchSet):
             if head == bottom:
                 # reset the backup information. No logging since the
                 # patch hasn't changed
-                patch.set_bottom(head, backup = True)
                 patch.set_top(top, backup = True)
 
             else:
@@ -1000,7 +979,6 @@ class Series(PatchSet):
                                      committer_name = committer_name,
                                      committer_email = committer_email)
 
-                    patch.set_bottom(head, backup = True)
                     patch.set_top(top, backup = True)
 
                     self.log_patch(patch, 'push(f)')
@@ -1073,7 +1051,6 @@ class Series(PatchSet):
         if head == bottom:
             # A fast-forward push. Just reset the backup
             # information. No need for logging
-            patch.set_bottom(bottom, backup = True)
             patch.set_top(top, backup = True)
 
             git.switch(top)

^ permalink raw reply related

* [StGit PATCH 11/13] Check bottom and invariants
From: David Kågedal @ 2007-09-14 22:32 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

This code adds some checks that the bottom is actually always the
parent of top.

It also checks that the top is the same as what the patch ref points
to.

This is only do to ensure that the next patches are correct.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/stack.py |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)


diff --git a/stgit/stack.py b/stgit/stack.py
index fdff5a4..d9a2e56 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -197,9 +197,15 @@ class Patch(StgitObject):
             self.__update_top_ref(top)
 
     def get_old_bottom(self):
-        return self._get_field('bottom.old')
+        old_bottom = self._get_field('bottom.old')
+        old_top = self.get_old_top()
+        assert old_bottom == git.get_commit(old_top).get_parent()
+        return old_bottom
 
     def get_bottom(self):
+        bottom = self._get_field('bottom')
+        top = self.get_top()
+        assert bottom == git.get_commit(top).get_parent()
         return self._get_field('bottom')
 
     def set_bottom(self, value, backup = False):
@@ -212,7 +218,13 @@ class Patch(StgitObject):
         return self._get_field('top.old')
 
     def get_top(self):
-        return self._get_field('top')
+        top = self._get_field('top')
+        try:
+            ref = git.rev_parse(self.__top_ref)
+        except:
+            ref = None
+        assert not ref or top == ref
+        return top
 
     def set_top(self, value, backup = False):
         if backup:
@@ -220,6 +232,7 @@ class Patch(StgitObject):
             self._set_field('top.old', curr)
         self._set_field('top', value)
         self.__update_top_ref(value)
+        self.get_bottom() # check the assert
 
     def restore_old_boundaries(self):
         bottom = self._get_field('bottom.old')

^ permalink raw reply related

* [StGit PATCH 09/13] Clear up the semantics of Series.new_patch
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

This patch adds a number of assertions to document and verify the
complex restrictions of the input parameters to the Series.new_patch
function. It also adds the requirement that 'before_existing' and
'commit' cannot be true at the same time when calling it, instead of
updating 'commit' inside the function.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/commands/uncommit.py |    1 +
 stgit/stack.py             |   14 ++++++++++----
 2 files changed, 11 insertions(+), 4 deletions(-)


diff --git a/stgit/commands/uncommit.py b/stgit/commands/uncommit.py
index 3cf2f0a..0cd0fb0 100644
--- a/stgit/commands/uncommit.py
+++ b/stgit/commands/uncommit.py
@@ -129,6 +129,7 @@ def func(parser, options, args):
                      name_email_date(commit.get_author())
         crt_series.new_patch(patchname,
                              can_edit = False, before_existing = True,
+                             commit = False,
                              top = commit_id, bottom = parent,
                              message = commit.get_log(),
                              author_name = author_name,
diff --git a/stgit/stack.py b/stgit/stack.py
index fd19a82..733a241 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -833,9 +833,16 @@ class Series(PatchSet):
                   author_name = None, author_email = None, author_date = None,
                   committer_name = None, committer_email = None,
                   before_existing = False):
-        """Creates a new patch
+        """Creates a new patch, either pointing to an existing commit object,
+        or by creating a new commit object.
         """
 
+        assert commit or (top and bottom)
+        assert not before_existing or (top and bottom)
+        assert not (commit and before_existing)
+        assert (top and bottom) or (not top and not bottom)
+        assert not top or (bottom == git.get_commit(top).get_parent())
+
         if name != None:
             self.__patch_name_valid(name)
             if self.patch_exists(name):
@@ -873,9 +880,6 @@ class Series(PatchSet):
 
         if before_existing:
             insert_string(self.__applied_file, patch.get_name())
-            # no need to commit anything as the object is already
-            # present (mainly used by 'uncommit')
-            commit = False
         elif unapplied:
             patches = [patch.get_name()] + self.get_unapplied()
             write_strings(self.__unapplied_file, patches)
@@ -900,6 +904,8 @@ class Series(PatchSet):
                                    committer_email = committer_email)
             # set the patch top to the new commit
             patch.set_top(commit_id)
+        else:
+            assert top != bottom
 
         self.log_patch(patch, 'new')
 

^ permalink raw reply related

* [StGit PATCH 10/13] Refactor Series.new_patch
From: David Kågedal @ 2007-09-14 22:32 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

This shuffles some code so that the top and bottom never need to be
set to anything other than a valid commit and its parent.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/stack.py |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)


diff --git a/stgit/stack.py b/stgit/stack.py
index 733a241..fdff5a4 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -864,13 +864,6 @@ class Series(PatchSet):
         patch = self.get_patch(name)
         patch.create()
 
-        if not bottom:
-            bottom = head
-        if not top:
-            top = head
-
-        patch.set_bottom(bottom)
-        patch.set_top(top)
         patch.set_description(descr)
         patch.set_authname(author_name)
         patch.set_authemail(author_email)
@@ -889,10 +882,15 @@ class Series(PatchSet):
             set_head = True
 
         if commit:
+            if top:
+                top_commit = git.get_commit(top)
+            else:
+                bottom = head
+                top_commit = git.get_commit(head)
+
             # create a commit for the patch (may be empty if top == bottom);
             # only commit on top of the current branch
             assert(unapplied or bottom == head)
-            top_commit = git.get_commit(top)
             commit_id = git.commit(message = descr, parents = [bottom],
                                    cache_update = False,
                                    tree_id = top_commit.get_tree(),
@@ -903,9 +901,12 @@ class Series(PatchSet):
                                    committer_name = committer_name,
                                    committer_email = committer_email)
             # set the patch top to the new commit
+            patch.set_bottom(bottom)
             patch.set_top(commit_id)
         else:
             assert top != bottom
+            patch.set_bottom(bottom)
+            patch.set_top(top)
 
         self.log_patch(patch, 'new')
 

^ permalink raw reply related

* [StGit PATCH 08/13] Add a 'bottom' parameter to Series.refresh_patch and use it
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

By specifying a bottom for the new patch commit, it is no longer
necessary to update the bottom of the patch before calling
refresh_patch.  This ensures that the patch top always correspond to a
commit object, and the bottom to its parent.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/stack.py |   24 ++++++++----------------
 1 files changed, 8 insertions(+), 16 deletions(-)


diff --git a/stgit/stack.py b/stgit/stack.py
index 03ce218..fd19a82 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -748,7 +748,7 @@ class Series(PatchSet):
                       author_date = None,
                       committer_name = None, committer_email = None,
                       backup = False, sign_str = None, log = 'refresh',
-                      notes = None):
+                      notes = None, bottom = None):
         """Generates a new commit for the topmost patch
         """
         patch = self.get_current_patch()
@@ -780,7 +780,8 @@ class Series(PatchSet):
 
         descr = add_sign_line(descr, sign_str, committer_name, committer_email)
 
-        bottom = patch.get_bottom()
+        if not bottom:
+            bottom = patch.get_bottom()
 
         commit_id = git.commit(files = files,
                                message = descr, parents = [bottom],
@@ -1026,20 +1027,15 @@ class Series(PatchSet):
         unapplied = self.get_unapplied()
         assert(name in unapplied)
 
-        patch = self.get_patch(name)
+        # patch = self.get_patch(name)
         head = git.get_head()
 
-        # The top is updated by refresh_patch since we need an empty
-        # commit
-        patch.set_bottom(head, backup = True)
-        patch.set_top(head, backup = True)
-
         append_string(self.__applied_file, name)
 
         unapplied.remove(name)
         write_strings(self.__unapplied_file, unapplied)
 
-        self.refresh_patch(cache_update = False, log = 'push(m)')
+        self.refresh_patch(bottom = head, cache_update = False, log = 'push(m)')
 
     def push_patch(self, name):
         """Pushes a patch on the stack
@@ -1071,11 +1067,6 @@ class Series(PatchSet):
         ex = None
         modified = False
 
-        # new patch needs to be refreshed.
-        # The current patch is empty after merge.
-        patch.set_bottom(head, backup = True)
-        patch.set_top(head, backup = True)
-
         # Try the fast applying first. If this fails, fall back to the
         # three-way merge
         if not git.apply_diff(bottom, top):
@@ -1103,13 +1094,14 @@ class Series(PatchSet):
                 log = 'push(m)'
             else:
                 log = 'push'
-            self.refresh_patch(cache_update = False, log = log)
+            self.refresh_patch(bottom = head, 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(bottom = head, cache_update = False,
+                               log = 'push(c)')
             raise StackException, str(ex)
 
         return modified

^ permalink raw reply related

* [StGit PATCH 07/13] Clean up Series.refresh_patch
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

This patch does some minor simplifications of the code and updates the
documentation string of Series.refresh_patch.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/stack.py |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)


diff --git a/stgit/stack.py b/stgit/stack.py
index 9b8ed6e..03ce218 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -749,14 +749,12 @@ class Series(PatchSet):
                       committer_name = None, committer_email = None,
                       backup = False, sign_str = None, log = 'refresh',
                       notes = None):
-        """Generates a new commit for the given patch
+        """Generates a new commit for the topmost patch
         """
-        name = self.get_current()
-        if not name:
+        patch = self.get_current_patch()
+        if not patch:
             raise StackException, 'No patches applied'
 
-        patch = self.get_patch(name)
-
         descr = patch.get_description()
         if not (message or descr):
             edit = True
@@ -767,7 +765,7 @@ class Series(PatchSet):
         if not message and edit:
             descr = edit_file(self, descr.rstrip(), \
                               'Please edit the description for patch "%s" ' \
-                              'above.' % name, show_patch)
+                              'above.' % patch.get_name(), show_patch)
 
         if not author_name:
             author_name = patch.get_authname()

^ permalink raw reply related

* [StGit PATCH 06/13] Refactor Series.push_patch
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

Refactor the Series.push_patch function to make the code flow
simpler. It identifies the simple case and handles it early, and
reduces the number of if statements.

Most changes are simply indentation changes.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/stack.py |   87 ++++++++++++++++++++++++++++++--------------------------
 1 files changed, 46 insertions(+), 41 deletions(-)


diff --git a/stgit/stack.py b/stgit/stack.py
index f31f308..9b8ed6e 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -1054,60 +1054,65 @@ class Series(PatchSet):
         head = git.get_head()
         bottom = patch.get_bottom()
         top = patch.get_top()
-
-        ex = None
-        modified = False
-
         # top != bottom always since we have a commit for each patch
+
         if head == bottom:
-            # reset the backup information. No need for logging
+            # A fast-forward push. Just reset the backup
+            # information. No need for logging
             patch.set_bottom(bottom, backup = True)
             patch.set_top(top, backup = True)
 
             git.switch(top)
-        else:
-            # new patch needs to be refreshed.
-            # The current patch is empty after merge.
-            patch.set_bottom(head, backup = True)
-            patch.set_top(head, backup = True)
-
-            # Try the fast applying first. If this fails, fall back to the
-            # three-way merge
-            if not git.apply_diff(bottom, top):
-                # if git.apply_diff() fails, the patch requires a diff3
-                # merge and can be reported as modified
-                modified = True
-
-                # merge can fail but the patch needs to be pushed
-                try:
-                    git.merge(bottom, head, top, recursive = True)
-                except git.GitException, ex:
-                    out.error('The merge failed during "push".',
-                              'Use "refresh" after fixing the conflicts or'
-                              ' revert the operation with "push --undo".')
+            append_string(self.__applied_file, name)
+
+            unapplied.remove(name)
+            write_strings(self.__unapplied_file, unapplied)
+            return False
+
+        # Need to create a new commit an merge in the old patch
+        ex = None
+        modified = False
+
+        # new patch needs to be refreshed.
+        # The current patch is empty after merge.
+        patch.set_bottom(head, backup = True)
+        patch.set_top(head, backup = True)
+
+        # Try the fast applying first. If this fails, fall back to the
+        # three-way merge
+        if not git.apply_diff(bottom, top):
+            # if git.apply_diff() fails, the patch requires a diff3
+            # merge and can be reported as modified
+            modified = True
+
+            # merge can fail but the patch needs to be pushed
+            try:
+                git.merge(bottom, head, top, recursive = True)
+            except git.GitException, ex:
+                out.error('The merge failed during "push".',
+                          'Use "refresh" after fixing the conflicts or'
+                          ' revert the operation with "push --undo".')
 
         append_string(self.__applied_file, name)
 
         unapplied.remove(name)
         write_strings(self.__unapplied_file, unapplied)
 
-        # head == bottom case doesn't need to refresh the patch
-        if head != bottom:
-            if not ex:
-                # if the merge was OK and no conflicts, just refresh the patch
-                # The GIT cache was already updated by the merge operation
-                if modified:
-                    log = 'push(m)'
-                else:
-                    log = 'push'
-                self.refresh_patch(cache_update = False, log = log)
+        if not ex:
+            # if the merge was OK and no conflicts, just refresh the patch
+            # The GIT cache was already updated by the merge operation
+            if modified:
+                log = 'push(m)'
             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)')
-                raise StackException, str(ex)
+                log = 'push'
+            self.refresh_patch(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)')
+            raise StackException, str(ex)
 
         return modified
 

^ permalink raw reply related

* [StGit PATCH 04/13] Split Series.push_patch in two
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

The push_patch() function has a complex control flow and actually does
two different things depending on the 'empty' parameter. This patch
splits in in two functions without other code changes.

Later patches will refactor the code to simplify it.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/commands/common.py |    2 +
 stgit/stack.py           |   62 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 52 insertions(+), 12 deletions(-)


diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index f3fa89d..eaaf5fc 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -184,7 +184,7 @@ def push_patches(patches, check_merged = False):
         out.start('Pushing patch "%s"' % p)
 
         if p in merged:
-            crt_series.push_patch(p, empty = True)
+            crt_series.push_empty_patch(p)
             out.done('merged upstream')
         else:
             modified = crt_series.push_patch(p)
diff --git a/stgit/stack.py b/stgit/stack.py
index 906e6b1..79d6cf3 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -1022,7 +1022,55 @@ class Series(PatchSet):
 
         return merged
 
-    def push_patch(self, name, empty = False):
+    def push_empty_patch(self, name):
+        """Pushes an empty patch on the stack
+        """
+        unapplied = self.get_unapplied()
+        assert(name in unapplied)
+
+        patch = self.get_patch(name)
+
+        head = git.get_head()
+        bottom = patch.get_bottom()
+        top = patch.get_top()
+
+        ex = None
+        modified = False
+
+        # top != bottom always since we have a commit for each patch
+        # just make an empty patch (top = bottom = HEAD). This
+        # option is useful to allow undoing already merged
+        # patches. The top is updated by refresh_patch since we
+        # need an empty commit
+        patch.set_bottom(head, backup = True)
+        patch.set_top(head, backup = True)
+        modified = True
+
+        append_string(self.__applied_file, name)
+
+        unapplied.remove(name)
+        write_strings(self.__unapplied_file, unapplied)
+
+        # head == bottom case doesn't need to refresh the patch
+        if not ex:
+            # if the merge was OK and no conflicts, just refresh the patch
+            # The GIT cache was already updated by the merge operation
+            if modified:
+                log = 'push(m)'
+            else:
+                log = 'push'
+            self.refresh_patch(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)')
+            raise StackException, str(ex)
+
+        return modified
+
+    def push_patch(self, name):
         """Pushes a patch on the stack
         """
         unapplied = self.get_unapplied()
@@ -1038,15 +1086,7 @@ class Series(PatchSet):
         modified = False
 
         # top != bottom always since we have a commit for each patch
-        if empty:
-            # just make an empty patch (top = bottom = HEAD). This
-            # option is useful to allow undoing already merged
-            # patches. The top is updated by refresh_patch since we
-            # need an empty commit
-            patch.set_bottom(head, backup = True)
-            patch.set_top(head, backup = True)
-            modified = True
-        elif head == bottom:
+        if head == bottom:
             # reset the backup information. No need for logging
             patch.set_bottom(bottom, backup = True)
             patch.set_top(top, backup = True)
@@ -1079,7 +1119,7 @@ class Series(PatchSet):
         write_strings(self.__unapplied_file, unapplied)
 
         # head == bottom case doesn't need to refresh the patch
-        if empty or head != bottom:
+        if head != bottom:
             if not ex:
                 # if the merge was OK and no conflicts, just refresh the patch
                 # The GIT cache was already updated by the merge operation

^ permalink raw reply related

* [StGit PATCH 05/13] Remove dead code from push_empty_patch
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

Since the split from push_patch, the push_empty_patch contains some
code that fills no purpose. Remove it and simplify the code.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/stack.py |   33 +++------------------------------
 1 files changed, 3 insertions(+), 30 deletions(-)


diff --git a/stgit/stack.py b/stgit/stack.py
index 79d6cf3..f31f308 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -1029,46 +1029,19 @@ class Series(PatchSet):
         assert(name in unapplied)
 
         patch = self.get_patch(name)
-
         head = git.get_head()
-        bottom = patch.get_bottom()
-        top = patch.get_top()
-
-        ex = None
-        modified = False
 
-        # top != bottom always since we have a commit for each patch
-        # just make an empty patch (top = bottom = HEAD). This
-        # option is useful to allow undoing already merged
-        # patches. The top is updated by refresh_patch since we
-        # need an empty commit
+        # The top is updated by refresh_patch since we need an empty
+        # commit
         patch.set_bottom(head, backup = True)
         patch.set_top(head, backup = True)
-        modified = True
 
         append_string(self.__applied_file, name)
 
         unapplied.remove(name)
         write_strings(self.__unapplied_file, unapplied)
 
-        # head == bottom case doesn't need to refresh the patch
-        if not ex:
-            # if the merge was OK and no conflicts, just refresh the patch
-            # The GIT cache was already updated by the merge operation
-            if modified:
-                log = 'push(m)'
-            else:
-                log = 'push'
-            self.refresh_patch(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)')
-            raise StackException, str(ex)
-
-        return modified
+        self.refresh_patch(cache_update = False, log = 'push(m)')
 
     def push_patch(self, name):
         """Pushes a patch on the stack

^ permalink raw reply related

* [StGit PATCH 02/13] Clear up semantics of tree_status
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

The tree_status() function does a few slightly different things
depending on the arguments.  This patch adds checks that the arguments
are consistent and that the returned value looks good.

It also changes the semantics slightly.  If the 'files' parameter is
None, it will run status on all files.  If 'files' is a list, it will
run status on only those files.  This changes two things:

 1) If 'files' is the empty list, it will run status on no files.
 2) It 'files' is a list, it will never return the status for other files

Clearing this up will make it easier to understand code that is using
this function.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/commands/status.py |    3 +++
 stgit/git.py             |   44 +++++++++++++++++++++++---------------------
 2 files changed, 26 insertions(+), 21 deletions(-)


diff --git a/stgit/commands/status.py b/stgit/commands/status.py
index 156552b..de88ba0 100644
--- a/stgit/commands/status.py
+++ b/stgit/commands/status.py
@@ -82,6 +82,9 @@ def func(parser, options, args):
         else:
             diff_flags = []
 
+        # No args means all files
+        if not args:
+            args = None
         git.status(args, options.modified, options.new, options.deleted,
                    options.conflict, options.unknown, options.noexclude,
                    diff_flags = diff_flags)
diff --git a/stgit/git.py b/stgit/git.py
index 539d699..8dda1ed 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -167,15 +167,20 @@ def exclude_files():
 
 def tree_status(files = None, tree_id = 'HEAD', unknown = False,
                   noexclude = True, verbose = False, diff_flags = []):
-    """Returns a list of pairs - (status, filename)
+    """Get the status of all changed files, or of a selected set of
+    files. Returns a list of pairs - (status, filename).
+
+    If 'files' is None, it will check all files, and optionally all
+    unknown files.  If 'files' is a list, it will only check the files
+    in the list.
     """
+    assert files == None or not unknown
+
     if verbose:
         out.start('Checking for changes in the working directory')
 
     refresh_index()
 
-    if not files:
-        files = []
     cache_files = []
 
     # unknown files
@@ -197,11 +202,14 @@ def tree_status(files = None, tree_id = 'HEAD', unknown = False,
     conflicts = get_conflicts()
     if not conflicts:
         conflicts = []
-    cache_files += [('C', filename) for filename in conflicts]
+    cache_files += [('C', filename) for filename in conflicts
+                    if files == None or filename in files]
 
     # the rest
-    for line in GRun('git-diff-index', *(diff_flags + [tree_id, '--'] + files)
-                     ).output_lines():
+    args = diff_flags + [tree_id]
+    if files != None:
+        args += ['--'] + files
+    for line in GRun('git-diff-index', *args).output_lines():
         fs = tuple(line.rstrip().split(' ',4)[-1].split('\t',1))
         if fs[1] not in conflicts:
             cache_files.append(fs)
@@ -209,6 +217,7 @@ def tree_status(files = None, tree_id = 'HEAD', unknown = False,
     if verbose:
         out.done()
 
+    assert files == None or set(f for s,f in cache_files) <= set(files)
     return cache_files
 
 def local_changes(verbose = True):
@@ -538,9 +547,6 @@ def committer():
 def update_cache(files = None, force = False):
     """Update the cache information for the given files
     """
-    if not files:
-        files = []
-
     cache_files = tree_status(files, verbose = False)
 
     # everything is up-to-date
@@ -569,8 +575,6 @@ def commit(message, files = None, parents = None, allowempty = False,
            committer_name = None, committer_email = None):
     """Commit the current tree to repository
     """
-    if not files:
-        files = []
     if not parents:
         parents = []
 
@@ -712,14 +716,13 @@ def status(files = None, modified = False, new = False, deleted = False,
            diff_flags = []):
     """Show the tree status
     """
-    if not files:
-        files = []
-
-    cache_files = tree_status(files, unknown = True, noexclude = noexclude,
-                                diff_flags = diff_flags)
-    all = not (modified or new or deleted or conflict or unknown)
+    cache_files = tree_status(files,
+                              unknown = (files == None),
+                              noexclude = noexclude,
+                              diff_flags = diff_flags)
+    filtered = (modified or new or deleted or conflict or unknown)
 
-    if not all:
+    if filtered:
         filestat = []
         if modified:
             filestat.append('M')
@@ -735,9 +738,8 @@ def status(files = None, modified = False, new = False, deleted = False,
         cache_files = [x for x in cache_files if x[0] in filestat]
 
     for fs in cache_files:
-        if files and not fs[1] in files:
-            continue
-        if all:
+        assert files == None or fs[1] in files
+        if not filtered:
             out.stdout('%s %s' % (fs[0], fs[1]))
         else:
             out.stdout('%s' % fs[1])

^ permalink raw reply related

* [StGit PATCH 03/13] Moved that status function to the status command file
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

The git.status() was more of a command than a library function, and
was only used in one place.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 stgit/commands/status.py |   39 ++++++++++++++++++++++++++++++++++++---
 stgit/git.py             |   33 ---------------------------------
 t/t0002-status.sh        |    8 ++++++++
 3 files changed, 44 insertions(+), 36 deletions(-)


diff --git a/stgit/commands/status.py b/stgit/commands/status.py
index de88ba0..bbfb5df 100644
--- a/stgit/commands/status.py
+++ b/stgit/commands/status.py
@@ -65,6 +65,39 @@ options = [make_option('-m', '--modified',
                        action = 'store_true')]
 
 
+def status(files = None, modified = False, new = False, deleted = False,
+           conflict = False, unknown = False, noexclude = False,
+           diff_flags = []):
+    """Show the tree status
+    """
+    cache_files = git.tree_status(files,
+                                  unknown = (files == None),
+                                  noexclude = noexclude,
+                                  diff_flags = diff_flags)
+    filtered = (modified or new or deleted or conflict or unknown)
+
+    if filtered:
+        filestat = []
+        if modified:
+            filestat.append('M')
+        if new:
+            filestat.append('A')
+            filestat.append('N')
+        if deleted:
+            filestat.append('D')
+        if conflict:
+            filestat.append('C')
+        if unknown:
+            filestat.append('?')
+        cache_files = [x for x in cache_files if x[0] in filestat]
+
+    for fs in cache_files:
+        assert files == None or fs[1] in files
+        if not filtered:
+            out.stdout('%s %s' % (fs[0], fs[1]))
+        else:
+            out.stdout('%s' % fs[1])
+
 def func(parser, options, args):
     """Show the tree status
     """
@@ -85,6 +118,6 @@ def func(parser, options, args):
         # No args means all files
         if not args:
             args = None
-        git.status(args, options.modified, options.new, options.deleted,
-                   options.conflict, options.unknown, options.noexclude,
-                   diff_flags = diff_flags)
+        status(args, options.modified, options.new, options.deleted,
+               options.conflict, options.unknown, options.noexclude,
+               diff_flags = diff_flags)
diff --git a/stgit/git.py b/stgit/git.py
index 8dda1ed..b112787 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -711,39 +711,6 @@ def merge(base, head1, head2, recursive = False):
     if errors:
         raise GitException, 'GIT index merging failed (possible conflicts)'
 
-def status(files = None, modified = False, new = False, deleted = False,
-           conflict = False, unknown = False, noexclude = False,
-           diff_flags = []):
-    """Show the tree status
-    """
-    cache_files = tree_status(files,
-                              unknown = (files == None),
-                              noexclude = noexclude,
-                              diff_flags = diff_flags)
-    filtered = (modified or new or deleted or conflict or unknown)
-
-    if filtered:
-        filestat = []
-        if modified:
-            filestat.append('M')
-        if new:
-            filestat.append('A')
-            filestat.append('N')
-        if deleted:
-            filestat.append('D')
-        if conflict:
-            filestat.append('C')
-        if unknown:
-            filestat.append('?')
-        cache_files = [x for x in cache_files if x[0] in filestat]
-
-    for fs in cache_files:
-        assert files == None or fs[1] in files
-        if not filtered:
-            out.stdout('%s %s' % (fs[0], fs[1]))
-        else:
-            out.stdout('%s' % fs[1])
-
 def diff(files = None, rev1 = 'HEAD', rev2 = None, diff_flags = []):
     """Show the diff between rev1 and rev2
     """
diff --git a/t/t0002-status.sh b/t/t0002-status.sh
index 790b9fb..d0c31b2 100755
--- a/t/t0002-status.sh
+++ b/t/t0002-status.sh
@@ -60,6 +60,14 @@ test_expect_success 'Status with an added file' '
 '
 
 cat > expected.txt <<EOF
+foo/bar
+EOF
+test_expect_success 'Status with an added file and -n option' '
+    stg status -n > output.txt &&
+    diff -u expected.txt output.txt
+'
+
+cat > expected.txt <<EOF
 EOF
 test_expect_success 'Status after refresh' '
     stg new -m "first patch" &&

^ permalink raw reply related

* [StGit PATCH 01/13] Add some more tests of "stg status" output
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---

 t/t0002-status.sh |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)


diff --git a/t/t0002-status.sh b/t/t0002-status.sh
index ce3b688..790b9fb 100755
--- a/t/t0002-status.sh
+++ b/t/t0002-status.sh
@@ -85,6 +85,12 @@ test_expect_success 'Status after refresh' '
     diff -u expected.txt output.txt
 '
 
+test_expect_success 'Add another file' '
+    echo lajbans > fie &&
+    stg add fie &&
+    stg refresh
+'
+
 test_expect_success 'Make a conflicting patch' '
     stg pop &&
     stg new -m "third patch" &&
@@ -105,6 +111,28 @@ test_expect_success 'Status after conflicting push' '
 '
 
 cat > expected.txt <<EOF
+C foo/bar
+EOF
+test_expect_success 'Status of file' '
+    stg status foo/bar > output.txt &&
+    diff -u expected.txt output.txt
+'
+
+cat > expected.txt <<EOF
+EOF
+test_expect_success 'Status of dir' '
+    stg status foo > output.txt &&
+    diff -u expected.txt output.txt
+'
+
+cat > expected.txt <<EOF
+EOF
+test_expect_success 'Status of other file' '
+    stg status fie > output.txt &&
+    diff -u expected.txt output.txt
+'
+
+cat > expected.txt <<EOF
 M foo/bar
 EOF
 test_expect_success 'Status after resolving the push' '

^ permalink raw reply related

* [StGit PATCH 00/13] Eliminate 'top' and 'bottom' files
From: David Kågedal @ 2007-09-14 22:31 UTC (permalink / raw)
  To: git, catalin.marinas

The following series removes the 'bottom' and 'top' files for each
patch, and instead uses the commit objects to keep track of the
patches.

The patches are based on kha/safe
24a81d7a94cd7c9ad2fc741b0179db5b830cce78 and will conflict with the
'conflict' series in at least one place.

The first ten patches are actually only cleanups and refactoring that
could go in regardless of the last ones.  Some of the changes are not
really necessary, but I did them while digging into the the code, and
believe they are improvements. They should not change the way stg
behaves in any way.

The eleventh one is just a sanity check before applying the last two
that removes the top and bottom files.

The last two patches do the final cleansing.  Obviously, this changes
the format, and the format version should be increased and and update
function be written.  So it's not really ready to go in yet.

Also, there are some documentation changes not included in this.

Maybe I should have sent the first ten patches separately, but this
makes it clearer why I did it.

---

David Kågedal (13):
      Remove the 'top' field
      Remove the 'bottom' field
      Check bottom and invariants
      Refactor Series.new_patch
      Clear up the semantics of Series.new_patch
      Add a 'bottom' parameter to Series.refresh_patch and use it
      Clean up Series.refresh_patch
      Refactor Series.push_patch
      Remove dead code from push_empty_patch
      Split Series.push_patch in two
      Moved that status function to the status command file
      Clear up semantics of tree_status
      Add some more tests of "stg status" output


 stgit/commands/common.py   |    2 
 stgit/commands/status.py   |   42 +++++++++-
 stgit/commands/sync.py     |    1 
 stgit/commands/uncommit.py |    1 
 stgit/git.py               |   61 +++-----------
 stgit/stack.py             |  189 ++++++++++++++++++++++----------------------
 t/t0002-status.sh          |   36 ++++++++
 7 files changed, 185 insertions(+), 147 deletions(-)

-- 
Signature

^ permalink raw reply

* Re: [PATCH] Add tests for documented features of "git reset".
From: Junio C Hamano @ 2007-09-14 22:10 UTC (permalink / raw)
  To: Carlos Rica; +Cc: git, Johannes Schindelin
In-Reply-To: <1b46aba20709141501l6f0f7440hd22b2bd6c4838a0b@mail.gmail.com>

"Carlos Rica" <jasampler@gmail.com> writes:

> 2007/9/11, Junio C Hamano <gitster@pobox.com>:
>> I also suspect this would not pass on CRLF boxes.
>
> I finally removed the hardcoded object IDs (patch attached,
> to be applied on top of the previous patch).
>
> But I don't know what to fix for making the test to pass
> in CRLF boxes.

The blobs created by the test script would be different on CRLF
boxes, and exact object IDs hardcoded in your test would not
match.  That was the only thing I meant.

But now you bring it up, I guess the expected output may not
match as well.  Hmmm..

Anyway, thanks for the update.

^ permalink raw reply

* Re: [PATCH] Remove duplicate note about removing commits with git-filter-branch
From: Johannes Schindelin @ 2007-09-14 22:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: ulrik, git
In-Reply-To: <7vk5qteqto.fsf@gitster.siamese.dyndns.org>

Hi,

On Thu, 13 Sep 2007, Junio C Hamano wrote:

> ulrik <ulrik.sverdrup@gmail.com> writes:
> 
> > A duplicate of an already existing section in the documentation of
> > git-filter-branch was added in commit
> > f95eef15f2f8a336b9a42749f5458c841a5a5d63.
> > This patch removes that redundant section.
> >
> > Signed-off-by: Ulrik Sverdrup <ulrik.sverdrup@gmail.com>
> > ---
> >  Documentation/git-filter-branch.txt |    5 -----
> >  1 files changed, 0 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/git-filter-branch.txt
> > b/Documentation/git-filter-branch.txt
> > index 29bb8ce..c878ed3 100644
> > --- a/Documentation/git-filter-branch.txt
> > +++ b/Documentation/git-filter-branch.txt
> > @@ -220,11 +220,6 @@ git filter-branch --commit-filter '
> >  	fi' HEAD
> >  ------------------------------------------------------------------------------
> >
> > -Note that the changes introduced by the commits, and not reverted by
> > -subsequent commits, will still be in the rewritten branch. If you want
> > -to throw out _changes_ together with the commits, you should use the
> > -interactive mode of gitlink:git-rebase[1].
> > -
> >  The function 'skip_commits' is defined as follows:
> 
> Hmph, sharp eyes.  I suspect that this could have been my
> screwup.

Yep, I confirm this was my screwup.  Thanks for fixing this, Ulrik!

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add tests for documented features of "git reset".
From: Carlos Rica @ 2007-09-14 22:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vr6l5oi4r.fsf@gitster.siamese.dyndns.org>

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

2007/9/11, Junio C Hamano <gitster@pobox.com>:
> I also suspect this would not pass on CRLF boxes.

I finally removed the hardcoded object IDs (patch attached,
to be applied on top of the previous patch).

But I don't know what to fix for making the test to pass
in CRLF boxes. Could it due to the comparison of
expected diffs with files created using <<EOF?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-t7102-reset-Use-tags-to-avoid-hardcoded-revision-ID.patch --]
[-- Type: text/x-patch; name="0001-t7102-reset-Use-tags-to-avoid-hardcoded-revision-ID.patch", Size: 7070 bytes --]

From b4c696b3e5b79d02c228bbc79d0d0e8f6a144453 Mon Sep 17 00:00:00 2001
From: Carlos Rica <jasampler@gmail.com>
Date: Thu, 13 Sep 2007 12:37:45 +0200
Subject: [PATCH] t7102-reset: Use tags to avoid hardcoded revision IDs.

Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
 t/t7102-reset.sh |   83 ++++++++++++++++++++++++++---------------------------
 1 files changed, 41 insertions(+), 42 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 2cad4db..7e3da71 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -21,19 +21,28 @@ test_expect_success 'creating initial files and commits' '
 
 	echo "2nd line 1st file" >>first &&
 	git commit -a -m "modify 1st file" &&
+	git tag modify1 &&
 
 	git rm first &&
 	git mv second secondfile &&
 	git commit -a -m "remove 1st and rename 2nd" &&
+	git tag moves &&
 
 	echo "1st line 2nd file" >secondfile &&
 	echo "2nd line 2nd file" >>secondfile &&
-	git commit -a -m "modify 2nd file"
+	git commit -a -m "modify 2nd file" &&
+	git tag modify2
 '
 # git log --pretty=oneline # to see those SHA1 involved
 
+refs_are_equal() {
+	r1=$(git rev-parse $1) &&
+	r2=$(git rev-parse $2) &&
+	test $r1 = $r2
+}
+
 check_changes () {
-	test "$(git rev-parse HEAD)" = "$1" &&
+	refs_are_equal HEAD $1 &&
 	git diff | git diff .diff_expect - &&
 	git diff --cached | git diff .cached_expect - &&
 	for FILE in *
@@ -56,7 +65,7 @@ test_expect_success 'giving a non existing revision should fail' '
 	! git reset --mixed aaaaaa &&
 	! git reset --soft aaaaaa &&
 	! git reset --hard aaaaaa &&
-	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	check_changes modify2
 '
 
 test_expect_success \
@@ -65,7 +74,7 @@ test_expect_success \
 	! git reset --hard -- first &&
 	! git reset --soft HEAD^ -- first &&
 	! git reset --hard HEAD^ -- first &&
-	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	check_changes modify2
 '
 
 test_expect_success 'giving unrecognized options should fail' '
@@ -77,7 +86,7 @@ test_expect_success 'giving unrecognized options should fail' '
 	! git reset --soft -o &&
 	! git reset --hard --other &&
 	! git reset --hard -o &&
-	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	check_changes modify2
 '
 
 test_expect_success \
@@ -101,7 +110,7 @@ test_expect_success \
 
 	git checkout master &&
 	git branch -D branch1 branch2 &&
-	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	check_changes modify2
 '
 
 test_expect_success \
@@ -124,27 +133,19 @@ test_expect_success \
 
 	git checkout master &&
 	git branch -D branch3 branch4 &&
-	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	check_changes modify2
 '
 
 test_expect_success \
 	'resetting to HEAD with no changes should succeed and do nothing' '
-	git reset --hard &&
-		check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
-	git reset --hard HEAD &&
-		check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
-	git reset --soft &&
-		check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
-	git reset --soft HEAD &&
-		check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
-	git reset --mixed &&
-		check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
-	git reset --mixed HEAD &&
-		check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
-	git reset &&
-		check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
-	git reset HEAD &&
-		check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	git reset --hard && check_changes modify2
+	git reset --hard HEAD && check_changes modify2
+	git reset --soft && check_changes modify2
+	git reset --soft HEAD && check_changes modify2
+	git reset --mixed && check_changes modify2
+	git reset --mixed HEAD && check_changes modify2
+	git reset && check_changes modify2
+	git reset HEAD && check_changes modify2
 '
 
 >.diff_expect
@@ -165,9 +166,8 @@ secondfile:
 EOF
 test_expect_success '--soft reset only should show changes in diff --cached' '
 	git reset --soft HEAD^ &&
-	check_changes d1a4bc3abce4829628ae2dcb0d60ef3d1a78b1c4 &&
-	test "$(git rev-parse ORIG_HEAD)" = \
-			3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	check_changes moves &&
+	refs_are_equal ORIG_HEAD modify2
 '
 
 >.diff_expect
@@ -182,9 +182,9 @@ test_expect_success \
 	'changing files and redo the last commit should succeed' '
 	echo "3rd line 2nd file" >>secondfile &&
 	git commit -a -C ORIG_HEAD &&
-	check_changes 3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d &&
-	test "$(git rev-parse ORIG_HEAD)" = \
-			3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	git tag addline1
+	check_changes addline1 &&
+	refs_are_equal ORIG_HEAD modify2
 '
 
 >.diff_expect
@@ -199,9 +199,8 @@ EOF
 test_expect_success \
 	'--hard reset should change the files and undo commits permanently' '
 	git reset --hard HEAD~2 &&
-	check_changes ddaefe00f1da16864591c61fdc7adb5d7cd6b74e &&
-	test "$(git rev-parse ORIG_HEAD)" = \
-			3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d
+	check_changes modify1 &&
+	refs_are_equal ORIG_HEAD addline1
 '
 
 >.diff_expect
@@ -243,7 +242,7 @@ test_expect_success \
 	echo "1st line 2nd file" >secondfile &&
 	echo "2nd line 2nd file" >>secondfile &&
 	git add secondfile &&
-	check_changes ddaefe00f1da16864591c61fdc7adb5d7cd6b74e
+	check_changes modify1
 '
 
 cat >.diff_expect <<EOF
@@ -271,9 +270,8 @@ secondfile:
 EOF
 test_expect_success '--mixed reset to HEAD should unadd the files' '
 	git reset &&
-	check_changes ddaefe00f1da16864591c61fdc7adb5d7cd6b74e &&
-	test "$(git rev-parse ORIG_HEAD)" = \
-			ddaefe00f1da16864591c61fdc7adb5d7cd6b74e
+	check_changes modify1 &&
+	refs_are_equal ORIG_HEAD modify1
 '
 
 >.diff_expect
@@ -285,7 +283,7 @@ secondfile:
 EOF
 test_expect_success 'redoing the last two commits should succeed' '
 	git add secondfile &&
-	git reset --hard ddaefe00f1da16864591c61fdc7adb5d7cd6b74e &&
+	git reset --hard modify1 &&
 
 	git rm first &&
 	git mv second secondfile &&
@@ -294,7 +292,7 @@ test_expect_success 'redoing the last two commits should succeed' '
 	echo "1st line 2nd file" >secondfile &&
 	echo "2nd line 2nd file" >>secondfile &&
 	git commit -a -m "modify 2nd file" &&
-	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	check_changes modify2
 '
 
 >.diff_expect
@@ -316,10 +314,11 @@ test_expect_success '--hard reset to HEAD should clear a failed merge' '
 	git checkout branch2 &&
 	echo "3rd line in branch2" >>secondfile &&
 	git commit -a -m "change in branch2" &&
+	git tag addline2 &&
 
 	! git pull . branch1 &&
 	git reset --hard &&
-	check_changes 77abb337073fb4369a7ad69ff6f5ec0e4d6b54bb
+	check_changes addline2
 '
 
 >.diff_expect
@@ -332,15 +331,15 @@ EOF
 test_expect_success \
 	'--hard reset to ORIG_HEAD should clear a fast-forward merge' '
 	git reset --hard HEAD^ &&
-	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
+	check_changes modify2 &&
 
 	git pull . branch1 &&
 	git reset --hard ORIG_HEAD &&
-	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
+	check_changes modify2 &&
 
 	git checkout master &&
 	git branch -D branch1 branch2 &&
-	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
+	check_changes modify2
 '
 
 cat > expect << EOF
-- 
1.5.3.1


^ permalink raw reply related

* [PATCH] Remove non-POSIX 'expr index' from instaweb.
From: Bryan Larsen @ 2007-09-14 22:01 UTC (permalink / raw)
  To: git

git-instaweb.sh uses the non-POSIX 'expr index' command, which
is not present on some BSD's.  This patch replaces this usage
with a simple grep expression.

Signed-off-by: Bryan Larsen < bryan@larsen.st>
---
 git-instaweb.sh |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index ce631a0..ef1a4bf 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -30,8 +30,7 @@ test -z "$port" && port=1234

 start_httpd () {
        httpd_only="`echo $httpd | cut -f1 -d' '`"
-       if test "`expr index $httpd_only /`" -eq '1' || \
-                               which $httpd_only >/dev/null
+       if test echo $http_only | grep ^/ || which $httpd_only >/dev/null
        then
                $httpd $fqgitdir/gitweb/httpd.conf
        else
-- 
1.5.3.1

^ permalink raw reply related

* Re: [PATCH] Tiny fix in git-config manual
From: Junio C Hamano @ 2007-09-14 21:48 UTC (permalink / raw)
  To: Bram Schoenmakers; +Cc: git
In-Reply-To: <200709102307.02468.bramschoenmakers@xs4all.nl>

Bram Schoenmakers <bramschoenmakers@xs4all.nl> writes:

> A little problem is shown in the git-config manual, where the possible
> locations for the configuration files is to be found. The dot is a special
> character and hence not visible in the resulting documents. This patch
> fixes this by adding an additional whitespace.

Thanks, but depending on the docbook stylesheet version this
does not seem to do anything or does not seem to fix the issue.

As we usually refer the repository metadata directory $GIT_DIR,
I am inclined to do this instead.

---

 Documentation/git-config.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 5b794f4..a592b61 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -142,7 +142,7 @@ FILES
 If not set explicitly with '--file', there are three files where
 git-config will search for configuration options:
 
-.git/config::
+$GIT_DIR/config::
 	Repository specific configuration file. (The filename is
 	of course relative to the repository root, not the working
 	directory.)

^ permalink raw reply related

* [PATCH] Remove non-POSIX alternation in sed script in instaweb
From: Bryan Larsen @ 2007-09-14 21:41 UTC (permalink / raw)
  To: git

git-instaweb.sh uses alternation in a sed script.  Unfortunately,
this is not POSIX compliant and does not work on some BSD's.  This
patch removes the alternation via the simple expedient of doubling
the number of lines in the sed script.

Signed-off-by: Bryan Larsen <bryan@larsen.st>
---
 git-instaweb.sh |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index b79c6b6..ce631a0 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -207,10 +207,14 @@ EOF
 }

 script='
-s#^\(my\|our\) $projectroot =.*#\1 $projectroot = "'`dirname $fqgitdir`'";#
-s#\(my\|our\) $gitbin =.*#\1 $gitbin = "'$GIT_EXEC_PATH'";#
-s#\(my\|our\) $projects_list =.*#\1 $projects_list = $projectroot;#
-s#\(my\|our\) $git_temp =.*#\1 $git_temp = "'$fqgitdir/gitweb/tmp'";#'
+s#^my $projectroot =.*#my $projectroot = "'`dirname $fqgitdir`'";#
+s#^our $projectroot =.*#our $projectroot = "'`dirname $fqgitdir`'";#
+s#my $gitbin =.*#my $gitbin = "'$GIT_EXEC_PATH'";#
+s#our $gitbin =.*#our $gitbin = "'$GIT_EXEC_PATH'";#
+s#my $projects_list =.*#my $projects_list = $projectroot;#
+s#our $projects_list =.*#our $projects_list = $projectroot;#
+s#my $git_temp =.*#my $git_temp = "'$fqgitdir/gitweb/tmp'";#
+s#our $git_temp =.*#our $git_temp = "'$fqgitdir/gitweb/tmp'";#'

 gitweb_cgi () {
        cat > "$1.tmp" <<\EOFGITWEB
-- 
1.5.3.1

^ permalink raw reply related

* Re: Track /etc directory using Git
From: Nicolas Vilz @ 2007-09-14 21:26 UTC (permalink / raw)
  To: Thomas Harning Jr.; +Cc: martin f krafft, git, Francis Moreau
In-Reply-To: <e47324780709141031t79981b04q3a91984668ea723e@mail.gmail.com>

On Fri, Sep 14, 2007 at 01:31:06PM -0400, Thomas Harning Jr. wrote:
> On 9/14/07, martin f krafft <madduck@madduck.net> wrote:
> > also sprach Francis Moreau <francis.moro@gmail.com> [2007.09.14.1008 +0200]:
> > > Did you find an alternative to git in this case ?
> >
> > No, and I did not look anywhere, but I know of no other VCS that can
> > adequatly track permissions.
> Has anyone checked out metastore?  http://repo.or.cz/w/metastore.git
> ... there's an XML error in there somewhere, so its not loading the
> 'main' page, but http://repo.or.cz/w/metastore.git?a=shortlog should
> work.
> 
> It looks like it could work.... any thoughts on this?

I use that tool. If you just have one branch, it works. With the
commit-hook, which also updates the metadata, you have current
permission tracking. 

There is a lack of a checkout-hook, which sets the permissions, so you
have to remeber todo a metastore -a after you checked out a revision.

But if you have several branches which fork the master branch and try to
rebase the branches on master, you get trouble, because the metadata gets
corrupted somehow. I will think about a solution on this sometime.

Nicolas

^ permalink raw reply

* Re: tags "problem"
From: Junio C Hamano @ 2007-09-14 21:14 UTC (permalink / raw)
  To: Rene Herman; +Cc: git
In-Reply-To: <46EAE189.5000804@gmail.com>

Rene Herman <rene.herman@gmail.com> writes:

> Hi people.
>
> (please keep me in CC)
>
> Was just busy constructing a tree and noticed a "problem" -- no doubt
> caused by me, but help appreciated.
>
> I clone linus' kernel tree into ./local, then do a
>
> 	git checkout -b v22 v2.6.22
>
> to get a 2.6.22 branch. I like to have a simple "git pull" while on
> this branch pull from the upstream stable tree, so as advised earlier,
> I put:
>
> [branch "v22"]
>         remote = linux-2.6.22.y
>         merge = refs/heads/master
> [remote "linux-2.6.22.y"]
>         url =
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.22.y.git
>         fetch = refs/heads/master
>
> in ./git/config. "git pull" now indeed works nicely.

Perhaps more canonical would be to make the latter fetch as:

	fetch = refs/heads/*:refs/remotes/linux-2.6.22.y/*

> I just noticed now though that this doesn't automatiically fetches
> tags as well.

I suspect that fetch without remote tracking does not auto-fetch
the tags.

^ 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