* [StGit PATCH 00/14] Undo series
@ 2008-06-12 5:34 Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 01/14] Fix typo Karl Hasselström
` (13 more replies)
0 siblings, 14 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:34 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Now with API documentation added all over the place! Especially in
2/14 and 3/14.
The only two things missing before I'll ask for this to be included
are
1. Conversion of stg refresh to the new infrastructure, so that it
can write two separate steps to the stack log (which will allow
undo et.al. to handle it nicely).
2. Log writing optimization: Right now, we always write all the
patches to the log, without trying to reuse the entries for the
ones that haven't changed since last time.
---
Karl Hasselström (14):
Make "stg log" show stack log instead of patch log
Log and undo external modifications
New command: stg redo
New command: stg undo
Move stack reset function to a shared location
Don't write a log entry if there were no changes
Add a --hard flag to stg reset
Log conflicts separately for all commands
Log conflicts separately
New command: stg reset
Add utility function for reordering patches
Write to a stack log when stack is modified
Library functions for tree and blob manipulation
Fix typo
stgit/commands/branch.py | 19 +-
stgit/commands/common.py | 9 +
stgit/commands/diff.py | 2
stgit/commands/files.py | 2
stgit/commands/id.py | 2
stgit/commands/log.py | 169 +++++------------
stgit/commands/mail.py | 2
stgit/commands/patches.py | 2
stgit/commands/redo.py | 52 +++++
stgit/commands/reset.py | 56 +++++
stgit/commands/show.py | 2
stgit/commands/status.py | 3
stgit/commands/undo.py | 49 +++++
stgit/lib/git.py | 197 +++++++++++++++++--
stgit/lib/log.py | 429 ++++++++++++++++++++++++++++++++++++++++++
stgit/lib/stack.py | 16 ++
stgit/lib/transaction.py | 121 +++++++++---
stgit/main.py | 8 +
t/t1400-patch-history.sh | 103 ----------
t/t3100-reset.sh | 151 +++++++++++++++
t/t3101-reset-hard.sh | 56 +++++
t/t3102-undo.sh | 86 ++++++++
t/t3103-undo-hard.sh | 56 +++++
t/t3104-redo.sh | 122 ++++++++++++
t/t3105-undo-external-mod.sh | 68 +++++++
25 files changed, 1491 insertions(+), 291 deletions(-)
create mode 100644 stgit/commands/redo.py
create mode 100644 stgit/commands/reset.py
create mode 100644 stgit/commands/undo.py
create mode 100644 stgit/lib/log.py
delete mode 100755 t/t1400-patch-history.sh
create mode 100755 t/t3100-reset.sh
create mode 100755 t/t3101-reset-hard.sh
create mode 100755 t/t3102-undo.sh
create mode 100755 t/t3103-undo-hard.sh
create mode 100755 t/t3104-redo.sh
create mode 100755 t/t3105-undo-external-mod.sh
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
* [StGit PATCH 01/14] Fix typo
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
@ 2008-06-12 5:34 ` Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 02/14] Library functions for tree and blob manipulation Karl Hasselström
` (12 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:34 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/lib/git.py | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 0e0cccb..6ccdfa7 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -20,7 +20,7 @@ class Immutable(object):
creating a whole new commit object that's exactly like the old one
except for the commit message.)
- The L{Immutable} class doesn't acytually enforce immutability --
+ The L{Immutable} class doesn't actually enforce immutability --
that is up to the individual immutable subclasses. It just serves
as documentation."""
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 02/14] Library functions for tree and blob manipulation
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 01/14] Fix typo Karl Hasselström
@ 2008-06-12 5:34 ` Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 03/14] Write to a stack log when stack is modified Karl Hasselström
` (11 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:34 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Wrap trees and blobs in Python objects (just like commits were already
wrapped), so that StGit code can read and write them.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/lib/git.py | 188 +++++++++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 165 insertions(+), 23 deletions(-)
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 6ccdfa7..a8881f4 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -182,16 +182,142 @@ class Person(Immutable, Repr):
defaults = cls.user())
return cls.__committer
-class Tree(Immutable, Repr):
- """Represents a git tree object."""
- def __init__(self, sha1):
+class GitObject(Immutable, Repr):
+ """Base class for all git objects. One git object is represented by at
+ most one C{GitObject}, which makes it possible to compare them
+ using normal Python object comparison; it also ensures we don't
+ waste more memory than necessary."""
+
+class BlobData(Immutable, Repr):
+ """Represents the data contents of a git blob object."""
+ def __init__(self, string):
+ self.__string = str(string)
+ str = property(lambda self: self.__string)
+ def commit(self, repository):
+ """Commit the blob.
+ @return: The committed blob
+ @rtype: L{Blob}"""
+ sha1 = repository.run(['git', 'hash-object', '-w', '--stdin']
+ ).raw_input(self.str).output_one_line()
+ return repository.get_blob(sha1)
+
+class Blob(GitObject):
+ """Represents a git blob object. All the actual data contents of the
+ blob object is stored in the L{data} member, which is a
+ L{BlobData} object."""
+ typename = 'blob'
+ default_perm = '100644'
+ def __init__(self, repository, sha1):
+ self.__repository = repository
self.__sha1 = sha1
sha1 = property(lambda self: self.__sha1)
def __str__(self):
- return 'Tree<%s>' % self.sha1
+ return 'Blob<%s>' % self.sha1
+ @property
+ def data(self):
+ return BlobData(self.__repository.cat_object(self.sha1))
+
+class ImmutableDict(dict):
+ """A dictionary that cannot be modified once it's been created."""
+ def error(*args, **kwargs):
+ raise TypeError('Cannot modify immutable dict')
+ __delitem__ = error
+ __setitem__ = error
+ clear = error
+ pop = error
+ popitem = error
+ setdefault = error
+ update = error
+
+class TreeData(Immutable, Repr):
+ """Represents the data contents of a git tree object."""
+ @staticmethod
+ def __x(po):
+ if isinstance(po, GitObject):
+ perm, object = po.default_perm, po
+ else:
+ perm, object = po
+ return perm, object
+ def __init__(self, entries):
+ """Create a new L{TreeData} object from the given mapping from names
+ (strings) to either (I{permission}, I{object}) tuples or just
+ objects."""
+ self.__entries = ImmutableDict((name, self.__x(po))
+ for (name, po) in entries.iteritems())
+ entries = property(lambda self: self.__entries)
+ """Map from name to (I{permission}, I{object}) tuple."""
+ def set_entry(self, name, po):
+ """Create a new L{TreeData} object identical to this one, except that
+ it maps C{name} to C{po}.
+
+ @param name: Name of the changed mapping
+ @type name: C{str}
+ @param po: Value of the changed mapping
+ @type po: L{Blob} or L{Tree} or (C{str}, L{Blob} or L{Tree})
+ @return: The new L{TreeData} object
+ @rtype: L{TreeData}"""
+ e = dict(self.entries)
+ e[name] = self.__x(po)
+ return type(self)(e)
+ def del_entry(self, name):
+ """Create a new L{TreeData} object identical to this one, except that
+ it doesn't map C{name} to anything.
+
+ @param name: Name of the deleted mapping
+ @type name: C{str}
+ @return: The new L{TreeData} object
+ @rtype: L{TreeData}"""
+ e = dict(self.entries)
+ del e[name]
+ return type(self)(e)
+ def commit(self, repository):
+ """Commit the tree.
+ @return: The committed tree
+ @rtype: L{Tree}"""
+ listing = ''.join(
+ '%s %s %s\t%s\0' % (mode, obj.typename, obj.sha1, name)
+ for (name, (mode, obj)) in self.entries.iteritems())
+ sha1 = repository.run(['git', 'mktree', '-z']
+ ).raw_input(listing).output_one_line()
+ return repository.get_tree(sha1)
+ @classmethod
+ def parse(cls, repository, s):
+ """Parse a raw git tree description.
+
+ @return: A new L{TreeData} object
+ @rtype: L{TreeData}"""
+ entries = {}
+ for line in s.split('\0')[:-1]:
+ m = re.match(r'^([0-7]{6}) ([a-z]+) ([0-9a-f]{40})\t(.*)$', line)
+ assert m
+ perm, type, sha1, name = m.groups()
+ entries[name] = (perm, repository.get_object(type, sha1))
+ return cls(entries)
+
+class Tree(GitObject):
+ """Represents a git tree object. All the actual data contents of the
+ tree object is stored in the L{data} member, which is a
+ L{TreeData} object."""
+ typename = 'tree'
+ default_perm = '040000'
+ def __init__(self, repository, sha1):
+ self.__sha1 = sha1
+ self.__repository = repository
+ self.__data = None
+ sha1 = property(lambda self: self.__sha1)
+ @property
+ def data(self):
+ if self.__data == None:
+ self.__data = TreeData.parse(
+ self.__repository,
+ self.__repository.run(['git', 'ls-tree', '-z', self.sha1]
+ ).raw_output())
+ return self.__data
+ def __str__(self):
+ return 'Tree<sha1: %s>' % self.sha1
class CommitData(Immutable, Repr):
- """Represents the actual data contents of a git commit object."""
+ """Represents the data contents of a git commit object."""
def __init__(self, tree = NoValue, parents = NoValue, author = NoValue,
committer = NoValue, message = NoValue, defaults = NoValue):
d = make_defaults(defaults)
@@ -238,8 +364,30 @@ class CommitData(Immutable, Repr):
return ('CommitData<tree: %s, parents: %s, author: %s,'
' committer: %s, message: "%s">'
) % (tree, parents, self.author, self.committer, self.message)
+ def commit(self, repository):
+ """Commit the commit.
+ @return: The committed commit
+ @rtype: L{Commit}"""
+ c = ['git', 'commit-tree', self.tree.sha1]
+ for p in self.parents:
+ c.append('-p')
+ c.append(p.sha1)
+ env = {}
+ for p, v1 in ((self.author, 'AUTHOR'),
+ (self.committer, 'COMMITTER')):
+ if p != None:
+ for attr, v2 in (('name', 'NAME'), ('email', 'EMAIL'),
+ ('date', 'DATE')):
+ if getattr(p, attr) != None:
+ env['GIT_%s_%s' % (v1, v2)] = str(getattr(p, attr))
+ sha1 = repository.run(c, env = env).raw_input(self.message
+ ).output_one_line()
+ return repository.get_commit(sha1)
@classmethod
def parse(cls, repository, s):
+ """Parse a raw git commit description.
+ @return: A new L{CommitData} object
+ @rtype: L{CommitData}"""
cd = cls(parents = [])
lines = list(s.splitlines(True))
for i in xrange(len(lines)):
@@ -259,10 +407,11 @@ class CommitData(Immutable, Repr):
assert False
assert False
-class Commit(Immutable, Repr):
+class Commit(GitObject):
"""Represents a git commit object. All the actual data contents of the
commit object is stored in the L{data} member, which is a
L{CommitData} object."""
+ typename = 'commit'
def __init__(self, repository, sha1):
self.__sha1 = sha1
self.__repository = repository
@@ -367,7 +516,8 @@ class Repository(RunWithEnv):
def __init__(self, directory):
self.__git_dir = directory
self.__refs = Refs(self)
- self.__trees = ObjectCache(lambda sha1: Tree(sha1))
+ self.__blobs = ObjectCache(lambda sha1: Blob(self, sha1))
+ self.__trees = ObjectCache(lambda sha1: Tree(self, sha1))
self.__commits = ObjectCache(lambda sha1: Commit(self, sha1))
self.__default_index = None
self.__default_worktree = None
@@ -429,26 +579,18 @@ class Repository(RunWithEnv):
).output_one_line())
except run.RunException:
raise RepositoryException('%s: No such revision' % rev)
+ def get_blob(self, sha1):
+ return self.__blobs[sha1]
def get_tree(self, sha1):
return self.__trees[sha1]
def get_commit(self, sha1):
return self.__commits[sha1]
- def commit(self, commitdata):
- c = ['git', 'commit-tree', commitdata.tree.sha1]
- for p in commitdata.parents:
- c.append('-p')
- c.append(p.sha1)
- env = {}
- for p, v1 in ((commitdata.author, 'AUTHOR'),
- (commitdata.committer, 'COMMITTER')):
- if p != None:
- for attr, v2 in (('name', 'NAME'), ('email', 'EMAIL'),
- ('date', 'DATE')):
- if getattr(p, attr) != None:
- env['GIT_%s_%s' % (v1, v2)] = str(getattr(p, attr))
- sha1 = self.run(c, env = env).raw_input(commitdata.message
- ).output_one_line()
- return self.get_commit(sha1)
+ def get_object(self, type, sha1):
+ return { Blob.typename: self.get_blob,
+ Tree.typename: self.get_tree,
+ Commit.typename: self.get_commit }[type](sha1)
+ def commit(self, objectdata):
+ return objectdata.commit(self)
@property
def head_ref(self):
try:
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 01/14] Fix typo Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 02/14] Library functions for tree and blob manipulation Karl Hasselström
@ 2008-06-12 5:34 ` Karl Hasselström
2008-06-17 10:24 ` Catalin Marinas
2008-06-12 5:34 ` [StGit PATCH 04/14] Add utility function for reordering patches Karl Hasselström
` (10 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:34 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Create a log branch (called <branchname>.stgit) for each StGit branch,
and write to it whenever the stack is modified.
Commands using the new infrastructure write to the log when they
commit a transaction. Commands using the old infrastructure get a log
entry write written for them when they exit, unless they explicitly
ask for this not to happen.
The only thing you can do with this log at the moment is look at it.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/branch.py | 19 ++-
stgit/commands/common.py | 8 +
stgit/commands/diff.py | 2
stgit/commands/files.py | 2
stgit/commands/id.py | 2
stgit/commands/log.py | 2
stgit/commands/mail.py | 2
stgit/commands/patches.py | 2
stgit/commands/show.py | 2
stgit/commands/status.py | 3 -
stgit/lib/git.py | 3 -
stgit/lib/log.py | 254 +++++++++++++++++++++++++++++++++++++++++++++
stgit/lib/stack.py | 9 ++
stgit/lib/transaction.py | 3 -
stgit/main.py | 2
15 files changed, 298 insertions(+), 17 deletions(-)
create mode 100644 stgit/lib/log.py
diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index 50684bb..edbb01c 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -25,7 +25,7 @@ from stgit.commands.common import *
from stgit.utils import *
from stgit.out import *
from stgit import stack, git, basedir
-
+from stgit.lib import log
help = 'manage patch stacks'
usage = """%prog [options] branch-name [commit-id]
@@ -40,7 +40,7 @@ When displaying the branches, the names can be prefixed with
If not given any options, switch to the named branch."""
-directory = DirectoryGotoToplevel()
+directory = DirectoryGotoToplevel(log = False)
options = [make_option('-c', '--create',
help = 'create a new development branch',
action = 'store_true'),
@@ -161,6 +161,7 @@ def func(parser, options, args):
parent_branch = parentbranch)
out.info('Branch "%s" created' % args[0])
+ log.compat_log_entry('branch --create')
return
elif options.clone:
@@ -181,6 +182,8 @@ def func(parser, options, args):
crt_series.clone(clone)
out.done()
+ log.copy_log(log.default_repo(), crt_series.get_name(), clone,
+ 'branch --clone')
return
elif options.delete:
@@ -188,6 +191,7 @@ def func(parser, options, args):
if len(args) != 1:
parser.error('incorrect number of arguments')
__delete_branch(args[0], options.force)
+ log.delete_log(log.default_repo(), args[0])
return
elif options.list:
@@ -195,13 +199,16 @@ def func(parser, options, args):
if len(args) != 0:
parser.error('incorrect number of arguments')
- branches = git.get_heads()
- branches.sort()
+ branches = set(git.get_heads())
+ for br in set(branches):
+ m = re.match(r'^(.*)\.stgit$', br)
+ if m and m.group(1) in branches:
+ branches.remove(br)
if branches:
out.info('Available branches:')
max_len = max([len(i) for i in branches])
- for i in branches:
+ for i in sorted(branches):
__print_branch(i, max_len)
else:
out.info('No branches')
@@ -238,7 +245,7 @@ def func(parser, options, args):
stack.Series(args[0]).rename(args[1])
out.info('Renamed branch "%s" to "%s"' % (args[0], args[1]))
-
+ log.rename_log(log.default_repo(), args[0], args[1], 'branch --rename')
return
elif options.unprotect:
diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index 029ec65..fd02398 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -28,6 +28,7 @@ from stgit.run import *
from stgit import stack, git, basedir
from stgit.config import config, file_extensions
from stgit.lib import stack as libstack
+from stgit.lib import log
# Command exception class
class CmdException(StgException):
@@ -478,8 +479,9 @@ class DirectoryException(StgException):
pass
class _Directory(object):
- def __init__(self, needs_current_series = True):
+ def __init__(self, needs_current_series = True, log = True):
self.needs_current_series = needs_current_series
+ self.log = log
@readonly_constant_property
def git_dir(self):
try:
@@ -512,6 +514,9 @@ class _Directory(object):
).output_one_line()]
def cd_to_topdir(self):
os.chdir(self.__topdir_path)
+ def write_log(self, msg):
+ if self.log:
+ log.compat_log_entry(msg)
class DirectoryAnywhere(_Directory):
def setup(self):
@@ -536,6 +541,7 @@ class DirectoryHasRepositoryLib(_Directory):
"""For commands that use the new infrastructure in stgit.lib.*."""
def __init__(self):
self.needs_current_series = False
+ self.log = False # stgit.lib.transaction handles logging
def setup(self):
# This will throw an exception if we don't have a repository.
self.repository = libstack.Repository.default()
diff --git a/stgit/commands/diff.py b/stgit/commands/diff.py
index fd6be34..8966642 100644
--- a/stgit/commands/diff.py
+++ b/stgit/commands/diff.py
@@ -42,7 +42,7 @@ rev = '([patch][//[bottom | top]]) | <tree-ish> | base'
If neither bottom nor top are given but a '//' is present, the command
shows the specified patch (defaulting to the current one)."""
-directory = DirectoryHasRepository()
+directory = DirectoryHasRepository(log = False)
options = [make_option('-r', '--range',
metavar = 'rev1[..[rev2]]', dest = 'revs',
help = 'show the diff between revisions'),
diff --git a/stgit/commands/files.py b/stgit/commands/files.py
index b43b12f..7844f8d 100644
--- a/stgit/commands/files.py
+++ b/stgit/commands/files.py
@@ -34,7 +34,7 @@ given patch. Note that this command doesn't show the files modified in
the working tree and not yet included in the patch by a 'refresh'
command. Use the 'diff' or 'status' commands for these files."""
-directory = DirectoryHasRepository()
+directory = DirectoryHasRepository(log = False)
options = [make_option('-s', '--stat',
help = 'show the diff stat',
action = 'store_true'),
diff --git a/stgit/commands/id.py b/stgit/commands/id.py
index 94b0229..5bb1ad2 100644
--- a/stgit/commands/id.py
+++ b/stgit/commands/id.py
@@ -33,7 +33,7 @@ the standard GIT id's like heads and tags, this command also accepts
'top' or 'bottom' are passed and <patch> is a valid patch name, 'top'
will be used by default."""
-directory = DirectoryHasRepository()
+directory = DirectoryHasRepository(log = False)
options = [make_option('-b', '--branch',
help = 'use BRANCH instead of the default one')]
diff --git a/stgit/commands/log.py b/stgit/commands/log.py
index 52d55a5..13e0baa 100644
--- a/stgit/commands/log.py
+++ b/stgit/commands/log.py
@@ -44,7 +44,7 @@ represent the changes to the entire base of the current
patch. Conflicts reset the patch content and a subsequent 'refresh'
will show the entire patch."""
-directory = DirectoryHasRepository()
+directory = DirectoryHasRepository(log = False)
options = [make_option('-b', '--branch',
help = 'use BRANCH instead of the default one'),
make_option('-p', '--patch',
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index b4d4e18..4027170 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -90,7 +90,7 @@ the following:
%(prefix)s - 'prefix ' string passed on the command line
%(shortdescr)s - the first line of the patch description"""
-directory = DirectoryHasRepository()
+directory = DirectoryHasRepository(log = False)
options = [make_option('-a', '--all',
help = 'e-mail all the applied patches',
action = 'store_true'),
diff --git a/stgit/commands/patches.py b/stgit/commands/patches.py
index 140699d..c95c40f 100644
--- a/stgit/commands/patches.py
+++ b/stgit/commands/patches.py
@@ -33,7 +33,7 @@ it shows the patches affected by the local tree modifications. The
'--diff' option also lists the patch log and the diff for the given
files."""
-directory = DirectoryHasRepository()
+directory = DirectoryHasRepository(log = False)
options = [make_option('-d', '--diff',
help = 'show the diff for the given files',
action = 'store_true'),
diff --git a/stgit/commands/show.py b/stgit/commands/show.py
index b77a9c8..dd2a3a3 100644
--- a/stgit/commands/show.py
+++ b/stgit/commands/show.py
@@ -30,7 +30,7 @@ Show the commit log and the diff corresponding to the given
patches. The output is similar to that generated by the 'git show'
command."""
-directory = DirectoryHasRepository()
+directory = DirectoryHasRepository(log = False)
options = [make_option('-b', '--branch',
help = 'use BRANCH instead of the default one'),
make_option('-a', '--applied',
diff --git a/stgit/commands/status.py b/stgit/commands/status.py
index a5b2f88..4d13112 100644
--- a/stgit/commands/status.py
+++ b/stgit/commands/status.py
@@ -40,7 +40,7 @@ under revision control. The files are prefixed as follows:
A 'refresh' command clears the status of the modified, new and deleted
files."""
-directory = DirectoryHasRepository(needs_current_series = False)
+directory = DirectoryHasRepository(needs_current_series = False, log = False)
options = [make_option('-m', '--modified',
help = 'show modified files only',
action = 'store_true'),
@@ -106,6 +106,7 @@ def func(parser, options, args):
directory.cd_to_topdir()
if options.reset:
+ directory.log = True
if args:
conflicts = git.get_conflicts()
git.resolved([fn for fn in args if fn in conflicts])
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index a8881f4..35e03d2 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -139,6 +139,7 @@ class Person(Immutable, Repr):
assert isinstance(self.__date, Date) or self.__date in [None, NoValue]
name = property(lambda self: self.__name)
email = property(lambda self: self.__email)
+ name_email = property(lambda self: '%s <%s>' % (self.name, self.email))
date = property(lambda self: self.__date)
def set_name(self, name):
return type(self)(name = name, defaults = self)
@@ -147,7 +148,7 @@ class Person(Immutable, Repr):
def set_date(self, date):
return type(self)(date = date, defaults = self)
def __str__(self):
- return '%s <%s> %s' % (self.name, self.email, self.date)
+ return '%s %s' % (self.name_email, self.date)
@classmethod
def parse(cls, s):
m = re.match(r'^([^<]*)<([^>]*)>\s+(\d+\s+[+-]\d{4})$', s)
diff --git a/stgit/lib/log.py b/stgit/lib/log.py
new file mode 100644
index 0000000..8646e08
--- /dev/null
+++ b/stgit/lib/log.py
@@ -0,0 +1,254 @@
+r"""This module contains functions and classes for manipulating
+I{patch stack logs} (or just I{stack logs}).
+
+A stack log is a git branch. Each commit contains the complete state
+of the stack at the moment it was written; the most recent commit has
+the most recent state.
+
+For a branch C{I{foo}}, the stack log is stored in C{I{foo}.stgit}.
+
+Stack log format (version 4)
+============================
+
+I{This describes version 4 of the stack log format. Versions 0 throgh
+3 were earlier development versions that are no longer supported.}
+
+Commit message
+--------------
+
+The commit message is meant for human consumption; in most cases it is
+just a subject line: the stg subcommand name and possibly some
+important command-line flag.
+
+The exception is log commits for undo and redo. Their subject line is
+"C{undo I{n}}" and "C{redo I{n}}"; the positive integer I{n} says how
+many steps were undone or redone.
+
+Tree
+----
+
+The top-level tree object has the following entries:
+
+ - C{version}: a blob containing the string "C{5\n}".
+
+ - C{head}: a blob containing the ASCII hex sha1 of the current HEAD,
+ followed by a newline.
+
+ - C{applied}, C{unapplied}: blobs containing one line for each
+ applied or unapplied patch, in order. Each line consists of
+
+ - the ASCII hex sha1 of the patch's commit object,
+
+ - a space, and
+
+ - the patch's name.
+
+ - C{patches}: a tree containing one subtree for each patch, named
+ after that patch. Each such subtree contains:
+
+ - C{a}, C{b}: the patch's I{bottom} and I{top} trees.
+
+ - C{info}: a blob containing::
+
+ Author: <author name and e-mail>
+ Date: <patch timestamp>
+
+ <commit message>
+
+Parents
+-------
+
+ - The first parent is the I{simplified log}, described below.
+
+ - The second and third parents are the branch head and the stack
+ top, in any order; if they are identical (which is always the case
+ unless StGit's metadata is messed up by use of a non-StGit tool),
+ just one of them is listed.
+
+ - The last parent is the previous log entry. (If there is no
+ previous log entry, it is omitted.) This log entry must be version
+ E{4}.
+
+Simplified log
+--------------
+
+The simplified log looks exactly like the normal, or I{full}, log,
+except for the following:
+
+ - Instead of having a tree per patch in the C{patches} subtree, it
+ has a blob per patch. This blob contains::
+
+ Bottom: <sha1 of patch's bottom tree>
+ Top: <sha1 of patch's top tree>
+ Author: <author name and e-mail>
+ Date: <patch timestamp>
+
+ <commit message>
+
+ ---
+
+ <patch's diff>
+
+ - Its only parent is the simplified version of the previous log
+ entry.
+
+The simplified log contains no information not in the full log; its
+purpose is ease of visualization."""
+
+from stgit.lib import git, stack
+from stgit import exception
+from stgit.out import out
+
+class LogException(exception.StgException):
+ pass
+
+_current_version = 4
+
+def commit_info(cd):
+ return 'Author: %s\nDate: %s\n\n%s' % (cd.author.name_email,
+ cd.author.date, cd.message)
+
+def patch_tree(repository, cd):
+ return repository.commit(git.TreeData({
+ 'a': cd.parent.data.tree, 'b': cd.tree,
+ 'info': repository.commit(git.BlobData(commit_info(cd))) }))
+
+def patch_file(repository, cd):
+ return repository.commit(git.BlobData(''.join(s + '\n' for s in [
+ 'Bottom: %s' % cd.parent.data.tree.sha1,
+ 'Top: %s' % cd.tree.sha1,
+ 'Author: %s' % cd.author.name_email,
+ 'Date: %s' % cd.author.date,
+ '',
+ cd.message,
+ '',
+ '---',
+ '',
+ repository.diff_tree(cd.parent.data.tree, cd.tree, ['-M']
+ ).strip()])))
+
+def order_blob(repository, stack, kind):
+ return repository.commit(git.BlobData(''.join(
+ '%s %s\n' % (stack.patches.get(pn).commit.sha1, pn)
+ for pn in getattr(stack.patchorder, kind))))
+
+def log_entry_trees(repository, stack):
+ full_patches, short_patches = [repository.commit(git.TreeData(dict(
+ (pn, x(repository, stack.patches.get(pn).commit.data))
+ for pn in stack.patchorder.all)))
+ for x in [patch_tree, patch_file]]
+ version = repository.commit(git.BlobData('%d\n' % _current_version))
+ head = repository.commit(git.BlobData('%s\n' % stack.head.sha1))
+ applied = order_blob(repository, stack, 'applied')
+ unapplied = order_blob(repository, stack, 'unapplied')
+ return [repository.commit(git.TreeData({
+ 'version': version,
+ 'head': head,
+ 'patches': patches,
+ 'applied': applied,
+ 'unapplied': unapplied,
+ })) for patches in [full_patches, short_patches]]
+
+def log_ref(branch):
+ return 'refs/heads/%s.stgit' % branch
+
+def log_entry(stack, msg):
+ """Write a new log entry for the stack."""
+ ref = log_ref(stack.name)
+ try:
+ last_log = [FullLog(stack.repository, ref,
+ stack.repository.refs.get(ref))]
+ except KeyError:
+ last_log = []
+ except LogException, e:
+ out.warn(str(e), 'No log entry written.')
+ return
+ full_log_tree, short_log_tree = log_entry_trees(stack.repository, stack)
+ stack_log = stack.repository.commit(
+ git.CommitData(tree = short_log_tree, message = msg,
+ parents = [ll.stack_log for ll in last_log]))
+ refs = list(set([stack.head, stack.top]))
+ full_log = stack.repository.commit(
+ git.CommitData(tree = full_log_tree, message = msg,
+ parents = ([stack_log] + refs
+ + [ll.full_log for ll in last_log])))
+ stack.repository.refs.set(ref, full_log, msg)
+
+def compat_log_entry(msg):
+ """Write a new log entry. (Convenience function intended for use by
+ code not yet converted to the new infrastructure.)"""
+ repo = default_repo()
+ stack = repo.get_stack(repo.current_branch_name)
+ log_entry(stack, msg)
+
+class Log(object):
+ """Class used to read the stack log. Each instance represents one log
+ entry."""
+ def __init__(self, repo, ref, commit):
+ """Create a L{Log} object representing the log entry C{commit}. In any
+ error messages, say C{ref} when we mean C{commit}."""
+ self.commit = commit
+ mode, vblob = self.commit.data.tree.data.entries.get(
+ 'version', (None, None))
+ if not isinstance(vblob, git.Blob):
+ raise LogException('%s does not contain a valid log' % ref)
+ try:
+ version = int(vblob.data.str)
+ except ValueError:
+ raise LogException('"%s": invalid version number' % vblob.data.str)
+ if version < _current_version:
+ raise LogException(
+ '%s contains a stack log older than version %d;'
+ ' please delete it' % (ref, _current_version))
+ elif version > _current_version:
+ raise LogException(
+ 'Log contains a stack log newer than version %d;'
+ ' this version of StGit cannot read it' % _current_version)
+
+ # TODO: read the rest of the log lazily.
+
+ def pl(name):
+ patches = [x.split() for x in
+ self.commit.data.tree.data.entries[name][1]
+ .data.str.strip().split('\n') if x]
+ # TODO: handle case where we don't have the commit object.
+ return ([pn for sha1, pn in patches],
+ dict((pn, repo.get_commit(sha1)) for sha1, pn in patches))
+ self.patches = {}
+ self.applied, patches = pl('applied')
+ self.patches.update(patches)
+ self.unapplied, patches = pl('unapplied')
+ self.patches.update(patches)
+ self.head = repo.get_commit(
+ self.commit.data.tree.data.entries['head'][1].data.str.strip())
+ if self.applied:
+ self.base = self.patches[self.applied[0]].data.parent
+ else:
+ self.base = self.head
+
+class FullLog(Log):
+ full_log = property(lambda self: self.commit)
+ """Commit of the full log."""
+ stack_log = property(lambda self: self.commit.data.parents[0])
+ """Commit of the simplified log."""
+
+def delete_log(repo, branch):
+ ref = log_ref(branch)
+ if repo.refs.exists(ref):
+ repo.refs.delete(ref)
+
+def rename_log(repo, old_branch, new_branch, msg):
+ old_ref = log_ref(old_branch)
+ new_ref = log_ref(new_branch)
+ if repo.refs.exists(old_ref):
+ repo.refs.set(new_ref, repo.refs.get(old_ref), msg)
+ repo.refs.delete(old_ref)
+
+def copy_log(repo, src_branch, dst_branch, msg):
+ src_ref = log_ref(src_branch)
+ dst_ref = log_ref(dst_branch)
+ if repo.refs.exists(src_ref):
+ repo.refs.set(dst_ref, repo.refs.get(src_ref), msg)
+
+def default_repo():
+ return stack.Repository.default()
diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
index 9cb3967..62a1ec2 100644
--- a/stgit/lib/stack.py
+++ b/stgit/lib/stack.py
@@ -165,6 +165,15 @@ class Stack(git.Branch):
).commit.data.parent
else:
return self.head
+ @property
+ def top(self):
+ """Commit of the topmost patch, or the stack base if no patches are
+ applied."""
+ if self.patchorder.applied:
+ return self.patches.get(self.patchorder.applied[-1]).commit
+ else:
+ # When no patches are applied, base == head.
+ return self.head
def head_top_equal(self):
if not self.patchorder.applied:
return True
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index e47997e..4c4da1a 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -4,7 +4,7 @@ updates to an StGit stack in a safe and convenient way."""
from stgit import exception, utils
from stgit.utils import any, all
from stgit.out import *
-from stgit.lib import git
+from stgit.lib import git, log
class TransactionException(exception.StgException):
"""Exception raised when something goes wrong with a
@@ -170,6 +170,7 @@ class StackTransaction(object):
_print_current_patch(self.__stack.patchorder.applied, self.__applied)
self.__stack.patchorder.applied = self.__applied
self.__stack.patchorder.unapplied = self.__unapplied
+ log.log_entry(self.__stack, self.__msg)
if self.__error:
return utils.STGIT_CONFLICT
diff --git a/stgit/main.py b/stgit/main.py
index aa1f8ef..ec0e840 100644
--- a/stgit/main.py
+++ b/stgit/main.py
@@ -277,6 +277,7 @@ def main():
ret = command.func(parser, options, args)
except (StgException, IOError, ParsingError, NoSectionError), err:
+ directory.write_log(cmd)
out.error(str(err), title = '%s %s' % (prog, cmd))
if debug_level > 0:
traceback.print_exc()
@@ -292,4 +293,5 @@ def main():
traceback.print_exc()
sys.exit(utils.STGIT_BUG_ERROR)
+ directory.write_log(cmd)
sys.exit(ret or utils.STGIT_SUCCESS)
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 04/14] Add utility function for reordering patches
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (2 preceding siblings ...)
2008-06-12 5:34 ` [StGit PATCH 03/14] Write to a stack log when stack is modified Karl Hasselström
@ 2008-06-12 5:34 ` Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 05/14] New command: stg reset Karl Hasselström
` (9 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:34 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/lib/transaction.py | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 4c4da1a..16f5a4b 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -1,6 +1,8 @@
"""The L{StackTransaction} class makes it possible to make complex
updates to an StGit stack in a safe and convenient way."""
+import itertools as it
+
from stgit import exception, utils
from stgit.utils import any, all
from stgit.out import *
@@ -274,3 +276,15 @@ class StackTransaction(object):
self.__allow_conflicts = lambda trans: True
self.__halt('Merge conflict')
+
+ def reorder_patches(self, applied, unapplied, iw = None):
+ """Push and pop patches to attain the given ordering."""
+ common = len(list(it.takewhile(lambda (a, b): a == b,
+ zip(self.applied, applied))))
+ to_pop = set(self.applied[common:])
+ self.pop_patches(lambda pn: pn in to_pop)
+ for pn in applied[common:]:
+ self.push_patch(pn, iw)
+ assert self.applied == applied
+ assert set(self.unapplied) == set(unapplied)
+ self.unapplied = unapplied
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 05/14] New command: stg reset
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (3 preceding siblings ...)
2008-06-12 5:34 ` [StGit PATCH 04/14] Add utility function for reordering patches Karl Hasselström
@ 2008-06-12 5:34 ` Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 06/14] Log conflicts separately Karl Hasselström
` (8 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:34 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Given a commit object from the log, resets the stack (or just the
named patches) to the state given by that log entry.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/reset.py | 116 ++++++++++++++++++++++++++++++++++++
stgit/main.py | 2 +
t/t3100-reset.sh | 151 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 269 insertions(+), 0 deletions(-)
create mode 100644 stgit/commands/reset.py
create mode 100755 t/t3100-reset.sh
diff --git a/stgit/commands/reset.py b/stgit/commands/reset.py
new file mode 100644
index 0000000..b2643b1
--- /dev/null
+++ b/stgit/commands/reset.py
@@ -0,0 +1,116 @@
+# -*- coding: utf-8 -*-
+
+__copyright__ = """
+Copyright (C) 2008, Karl Hasselström <kha@treskal.com>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License version 2 as
+published by the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+"""
+
+from stgit.commands import common
+from stgit.lib import git, log, transaction
+from stgit.out import out
+
+help = 'reset the patch stack to an earlier state'
+usage = """%prog <state> [<patchnames>]
+
+Reset the patch stack to an earlier state. The state is specified with
+a commit from a stack log; for a branch foo, StGit stores the stack
+log in foo.stgit^. So to undo the last N StGit commands (or rather,
+the last N log entries; there is not an exact one-to-one
+relationship), you would say
+
+ stg reset foo.stgit^~N
+
+or, if you are not sure how many steps to undo, you can view the log
+with "git log" or gitk
+
+ gitk foo.stgit^
+
+and then reset to any sha1 you see in the log.
+
+If one or more patch names are given, reset only those patches, and
+leave the rest alone."""
+
+directory = common.DirectoryHasRepositoryLib()
+options = []
+
+def reset_stack(stack, iw, state, only_patches):
+ only_patches = set(only_patches)
+ def mask(s):
+ if only_patches:
+ return s & only_patches
+ else:
+ return s
+ patches_to_reset = mask(set(state.applied + state.unapplied))
+ existing_patches = set(stack.patchorder.all)
+ to_delete = mask(existing_patches - patches_to_reset)
+ trans = transaction.StackTransaction(stack, 'reset')
+
+ # If we have to change the stack base, we need to pop all patches
+ # first.
+ if not only_patches and trans.base != state.base:
+ trans.pop_patches(lambda pn: True)
+ out.info('Setting stack base to %s' % state.base.sha1)
+ trans.base = state.base
+
+ # In one go, do all the popping we have to in order to pop the
+ # patches we're going to delete or modify.
+ def mod(pn):
+ if only_patches and not pn in only_patches:
+ return False
+ if pn in to_delete:
+ return True
+ if stack.patches.get(pn).commit != state.patches.get(pn, None):
+ return True
+ return False
+ trans.pop_patches(mod)
+
+ # Delete and modify/create patches. We've previously popped all
+ # patches that we touch in this step.
+ trans.delete_patches(lambda pn: pn in to_delete)
+ for pn in patches_to_reset:
+ if pn in existing_patches:
+ if trans.patches[pn] == state.patches[pn]:
+ continue
+ else:
+ out.info('Resetting %s' % pn)
+ else:
+ trans.unapplied.append(pn)
+ out.info('Resurrecting %s' % pn)
+ trans.patches[pn] = state.patches[pn]
+
+ # Push/pop patches as necessary.
+ try:
+ if only_patches:
+ # Push all the patches that we've popped, if they still
+ # exist.
+ pushable = set(trans.unapplied)
+ for pn in stack.patchorder.applied:
+ if pn in pushable:
+ trans.push_patch(pn, iw)
+ else:
+ # Recreate the exact order specified by the goal state.
+ trans.reorder_patches(state.applied, state.unapplied, iw)
+ except transaction.TransactionHalted:
+ pass
+ return trans.run(iw)
+
+def func(parser, options, args):
+ stack = directory.repository.current_stack
+ if len(args) >= 1:
+ ref, patches = args[0], args[1:]
+ state = log.Log(stack.repository, ref, stack.repository.rev_parse(ref))
+ else:
+ raise common.CmdException('Wrong number of arguments')
+ return reset_stack(stack, stack.repository.default_iw, state, patches)
diff --git a/stgit/main.py b/stgit/main.py
index ec0e840..83e6b08 100644
--- a/stgit/main.py
+++ b/stgit/main.py
@@ -89,6 +89,7 @@ commands = Commands({
'refresh': 'refresh',
'rename': 'rename',
'repair': 'repair',
+ 'reset': 'reset',
'resolved': 'resolved',
'series': 'series',
'show': 'show',
@@ -122,6 +123,7 @@ stackcommands = (
'push',
'rebase',
'repair',
+ 'reset',
'series',
'sink',
'top',
diff --git a/t/t3100-reset.sh b/t/t3100-reset.sh
new file mode 100755
index 0000000..1805091
--- /dev/null
+++ b/t/t3100-reset.sh
@@ -0,0 +1,151 @@
+#!/bin/sh
+
+test_description='Simple test cases for "stg reset"'
+
+. ./test-lib.sh
+
+# Ignore our own output files.
+cat > .git/info/exclude <<EOF
+/expected.txt
+EOF
+
+test_expect_success 'Initialize StGit stack with three patches' '
+ stg init &&
+ echo 000 >> a &&
+ git add a &&
+ git commit -m a &&
+ echo 111 >> a &&
+ git commit -a -m p1 &&
+ echo 222 >> a &&
+ git commit -a -m p2 &&
+ echo 333 >> a &&
+ git commit -a -m p3 &&
+ stg uncommit -n 3 &&
+ stg pop
+'
+
+cat > expected.txt <<EOF
+000
+111
+EOF
+test_expect_success 'Pop one patch ...' '
+ stg pop &&
+ test "$(echo $(stg applied))" = "p1" &&
+ test "$(echo $(stg unapplied))" = "p2 p3" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success '... and undo it' '
+ stg reset master.stgit^~1 &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "p3" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+333
+EOF
+test_expect_success 'Push one patch ...' '
+ stg push &&
+ test "$(echo $(stg applied))" = "p1 p2 p3" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success '... and undo it' '
+ stg reset master.stgit^~1 &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "p3" &&
+ test_cmp expected.txt a
+'
+
+test_expect_success 'Commit one patch ...' '
+ stg commit &&
+ test "$(echo $(stg applied))" = "p2" &&
+ test "$(echo $(stg unapplied))" = "p3"
+'
+
+test_expect_success '... and undo it' '
+ stg reset master.stgit^~1 &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "p3"
+'
+
+cat > expected.txt <<EOF
+000
+111
+EOF
+test_expect_success 'Delete two patches ...' '
+ stg delete p2 p3 &&
+ test "$(echo $(stg applied))" = "p1" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+test_expect_success '... and undo one of the deletions ...' '
+ stg reset master.stgit^~1 p3 &&
+ test "$(echo $(stg applied))" = "p1" &&
+ test "$(echo $(stg unapplied))" = "p3" &&
+ test_cmp expected.txt a
+'
+
+test_expect_success '... then undo the first undo ...' '
+ stg reset master.stgit^~1 &&
+ test "$(echo $(stg applied))" = "p1" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success '... and undo the other deletion' '
+ stg reset master.stgit^~3 p2 &&
+ stg push p2 &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+ggg
+EOF
+test_expect_success 'Refresh a patch ...' '
+ echo ggg >> a &&
+ stg refresh &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success '... and undo the refresh' '
+ stg reset master.stgit^~1 &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+test_done
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 06/14] Log conflicts separately
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (4 preceding siblings ...)
2008-06-12 5:34 ` [StGit PATCH 05/14] New command: stg reset Karl Hasselström
@ 2008-06-12 5:34 ` Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 07/14] Log conflicts separately for all commands Karl Hasselström
` (7 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:34 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
This patch makes commands that produce a conflict log that final
conflicting push separately from the rest of the command's effects.
This makes it possible for the user to roll back just the final
conflicting push if she desires. (Rolling back the whole operation is
of course still possible, by resetting to the state yet another step
back in the log.)
This change only applies to the new-infrastructure commands.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/lib/transaction.py | 47 +++++++++++++++++++++++++++++++---------------
1 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 16f5a4b..6347b14 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -79,6 +79,7 @@ class StackTransaction(object):
self.__patches = _TransPatchMap(stack)
self.__applied = list(self.__stack.patchorder.applied)
self.__unapplied = list(self.__stack.patchorder.unapplied)
+ self.__conflicting_push = None
self.__error = None
self.__current_tree = self.__stack.head.data.tree
self.__base = self.__stack.base
@@ -160,19 +161,26 @@ class StackTransaction(object):
out.error(self.__error)
# Write patches.
- for pn, commit in self.__patches.iteritems():
- if self.__stack.patches.exists(pn):
- p = self.__stack.patches.get(pn)
- if commit == None:
- p.delete()
+ def write(msg):
+ for pn, commit in self.__patches.iteritems():
+ if self.__stack.patches.exists(pn):
+ p = self.__stack.patches.get(pn)
+ if commit == None:
+ p.delete()
+ else:
+ p.set_commit(commit, msg)
else:
- p.set_commit(commit, self.__msg)
- else:
- self.__stack.patches.new(pn, commit, self.__msg)
- _print_current_patch(self.__stack.patchorder.applied, self.__applied)
- self.__stack.patchorder.applied = self.__applied
- self.__stack.patchorder.unapplied = self.__unapplied
- log.log_entry(self.__stack, self.__msg)
+ self.__stack.patches.new(pn, commit, msg)
+ self.__stack.patchorder.applied = self.__applied
+ self.__stack.patchorder.unapplied = self.__unapplied
+ log.log_entry(self.__stack, msg)
+ old_applied = self.__stack.patchorder.applied
+ write(self.__msg)
+ if self.__conflicting_push != None:
+ self.__patches = _TransPatchMap(self.__stack)
+ self.__conflicting_push()
+ write(self.__msg + ' (CONFLICT)')
+ _print_current_patch(old_applied, self.__applied)
if self.__error:
return utils.STGIT_CONFLICT
@@ -264,18 +272,27 @@ class StackTransaction(object):
cd = cd.set_tree(tree)
if any(getattr(cd, a) != getattr(orig_cd, a) for a in
['parent', 'tree', 'author', 'message']):
- self.patches[pn] = self.__stack.repository.commit(cd)
+ comm = self.__stack.repository.commit(cd)
else:
+ comm = None
s = ' (unmodified)'
- del self.unapplied[self.unapplied.index(pn)]
- self.applied.append(pn)
out.info('Pushed %s%s' % (pn, s))
+ def update():
+ if comm:
+ self.patches[pn] = comm
+ del self.unapplied[self.unapplied.index(pn)]
+ self.applied.append(pn)
if merge_conflict:
# We've just caused conflicts, so we must allow them in
# the final checkout.
self.__allow_conflicts = lambda trans: True
+ # Save this update so that we can run it a little later.
+ self.__conflicting_push = update
self.__halt('Merge conflict')
+ else:
+ # Update immediately.
+ update()
def reorder_patches(self, applied, unapplied, iw = None):
"""Push and pop patches to attain the given ordering."""
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 07/14] Log conflicts separately for all commands
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (5 preceding siblings ...)
2008-06-12 5:34 ` [StGit PATCH 06/14] Log conflicts separately Karl Hasselström
@ 2008-06-12 5:34 ` Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 08/14] Add a --hard flag to stg reset Karl Hasselström
` (6 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:34 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
This takes care of the old-infrastructure commands as well. They'll
all be converted to the new infrastructure eventually, but until then
this patch is necessary to make all commands behave consistently.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/lib/log.py | 32 +++++++++++++++++++++++++++++++-
stgit/lib/stack.py | 7 +++++++
2 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/stgit/lib/log.py b/stgit/lib/log.py
index 8646e08..920c261 100644
--- a/stgit/lib/log.py
+++ b/stgit/lib/log.py
@@ -174,12 +174,42 @@ def log_entry(stack, msg):
+ [ll.full_log for ll in last_log])))
stack.repository.refs.set(ref, full_log, msg)
+class Fakestack(object):
+ """Imitates a real L{Stack<stgit.lib.stack.Stack>}, but with the
+ topmost patch popped."""
+ def __init__(self, stack):
+ appl = list(stack.patchorder.applied)
+ unappl = list(stack.patchorder.unapplied)
+ class patchorder(object):
+ applied = appl[:-1]
+ unapplied = [appl[-1]] + unappl
+ all = appl + unappl
+ self.patchorder = patchorder
+ class patches(object):
+ @staticmethod
+ def get(pn):
+ if pn == appl[-1]:
+ class patch(object):
+ commit = stack.patches.get(pn).old_commit
+ return patch
+ else:
+ return stack.patches.get(pn)
+ self.patches = patches
+ self.head = stack.head.data.parent
+ self.top = stack.top.data.parent
+ self.base = stack.base
+ self.name = stack.name
+ self.repository = stack.repository
def compat_log_entry(msg):
"""Write a new log entry. (Convenience function intended for use by
code not yet converted to the new infrastructure.)"""
repo = default_repo()
stack = repo.get_stack(repo.current_branch_name)
- log_entry(stack, msg)
+ if repo.default_index.conflicts():
+ log_entry(Fakestack(stack), msg)
+ log_entry(stack, msg + ' (CONFLICT)')
+ else:
+ log_entry(stack, msg)
class Log(object):
"""Class used to read the stack log. Each instance represents one log
diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
index 62a1ec2..2d686e6 100644
--- a/stgit/lib/stack.py
+++ b/stgit/lib/stack.py
@@ -25,6 +25,13 @@ class Patch(object):
def commit(self):
return self.__stack.repository.refs.get(self.__ref)
@property
+ def old_commit(self):
+ """Return the previous commit for this patch."""
+ fn = os.path.join(self.__compat_dir, 'top.old')
+ if not os.path.isfile(fn):
+ return None
+ return self.__stack.repository.get_commit(utils.read_string(fn))
+ @property
def __compat_dir(self):
return os.path.join(self.__stack.directory, 'patches', self.__name)
def __write_compat_files(self, new_commit, msg):
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 08/14] Add a --hard flag to stg reset
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (6 preceding siblings ...)
2008-06-12 5:34 ` [StGit PATCH 07/14] Log conflicts separately for all commands Karl Hasselström
@ 2008-06-12 5:34 ` Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 09/14] Don't write a log entry if there were no changes Karl Hasselström
` (5 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:34 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
With this flag, reset will overwrite any local changes. Useful e.g.
when undoing a push that has polluted the index+worktree with a heap
of conflicts.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/reset.py | 13 +++++++----
stgit/lib/git.py | 4 +++
stgit/lib/transaction.py | 17 ++++++++++++--
t/t3101-reset-hard.sh | 56 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 82 insertions(+), 8 deletions(-)
create mode 100755 t/t3101-reset-hard.sh
diff --git a/stgit/commands/reset.py b/stgit/commands/reset.py
index b2643b1..a7b5d35 100644
--- a/stgit/commands/reset.py
+++ b/stgit/commands/reset.py
@@ -17,12 +17,13 @@ along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
"""
+from optparse import make_option
from stgit.commands import common
from stgit.lib import git, log, transaction
from stgit.out import out
help = 'reset the patch stack to an earlier state'
-usage = """%prog <state> [<patchnames>]
+usage = """%prog [options] <state> [<patchnames>]
Reset the patch stack to an earlier state. The state is specified with
a commit from a stack log; for a branch foo, StGit stores the stack
@@ -43,9 +44,10 @@ If one or more patch names are given, reset only those patches, and
leave the rest alone."""
directory = common.DirectoryHasRepositoryLib()
-options = []
+options = [make_option('--hard', action = 'store_true',
+ help = 'discard changes in your index/worktree')]
-def reset_stack(stack, iw, state, only_patches):
+def reset_stack(stack, iw, state, only_patches, hard):
only_patches = set(only_patches)
def mask(s):
if only_patches:
@@ -55,7 +57,7 @@ def reset_stack(stack, iw, state, only_patches):
patches_to_reset = mask(set(state.applied + state.unapplied))
existing_patches = set(stack.patchorder.all)
to_delete = mask(existing_patches - patches_to_reset)
- trans = transaction.StackTransaction(stack, 'reset')
+ trans = transaction.StackTransaction(stack, 'reset', discard_changes = hard)
# If we have to change the stack base, we need to pop all patches
# first.
@@ -113,4 +115,5 @@ def func(parser, options, args):
state = log.Log(stack.repository, ref, stack.repository.rev_parse(ref))
else:
raise common.CmdException('Wrong number of arguments')
- return reset_stack(stack, stack.repository.default_iw, state, patches)
+ return reset_stack(stack, stack.repository.default_iw, state, patches,
+ options.hard)
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 35e03d2..4c2605b 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -736,6 +736,10 @@ class IndexAndWorktree(RunWithEnvCwd):
env = property(lambda self: utils.add_dict(self.__index.env,
self.__worktree.env))
cwd = property(lambda self: self.__worktree.directory)
+ def checkout_hard(self, tree):
+ assert isinstance(tree, Tree)
+ self.run(['git', 'read-tree', '--reset', '-u', tree.sha1]
+ ).discard_output()
def checkout(self, old_tree, new_tree):
# TODO: Optionally do a 3-way instead of doing nothing when we
# have a problem. Or maybe we should stash changes in a patch?
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 6347b14..10c9b39 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -73,7 +73,14 @@ class StackTransaction(object):
method. This will either succeed in writing the updated state to
your refs and index+worktree, or fail without having done
anything."""
- def __init__(self, stack, msg, allow_conflicts = False):
+ def __init__(self, stack, msg, discard_changes = False,
+ allow_conflicts = False):
+ """Create a new L{StackTransaction}.
+
+ @param discard_changes: Discard any changes in index+worktree
+ @type discard_changes: bool
+ @param allow_conflicts: Whether to allow pre-existing conflicts
+ @type allow_conflicts: bool or function of L{StackTransaction}"""
self.__stack = stack
self.__msg = msg
self.__patches = _TransPatchMap(stack)
@@ -83,6 +90,7 @@ class StackTransaction(object):
self.__error = None
self.__current_tree = self.__stack.head.data.tree
self.__base = self.__stack.base
+ self.__discard_changes = discard_changes
if isinstance(allow_conflicts, bool):
self.__allow_conflicts = lambda trans: allow_conflicts
else:
@@ -107,7 +115,7 @@ class StackTransaction(object):
'This can happen if you modify a branch with git.',
'"stg repair --help" explains more about what to do next.')
self.__abort()
- if self.__current_tree == tree:
+ if self.__current_tree == tree and not self.__discard_changes:
# No tree change, but we still want to make sure that
# there are no unresolved conflicts. Conflicts
# conceptually "belong" to the topmost patch, and just
@@ -118,7 +126,10 @@ class StackTransaction(object):
out.error('Need to resolve conflicts first')
self.__abort()
assert iw != None
- iw.checkout(self.__current_tree, tree)
+ if self.__discard_changes:
+ iw.checkout_hard(tree)
+ else:
+ iw.checkout(self.__current_tree, tree)
self.__current_tree = tree
@staticmethod
def __abort():
diff --git a/t/t3101-reset-hard.sh b/t/t3101-reset-hard.sh
new file mode 100755
index 0000000..1e02805
--- /dev/null
+++ b/t/t3101-reset-hard.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+test_description='Simple test cases for "stg reset"'
+
+. ./test-lib.sh
+
+# Ignore our own output files.
+cat > .git/info/exclude <<EOF
+/expected.txt
+/actual.txt
+EOF
+
+test_expect_success 'Initialize StGit stack with three patches' '
+ stg init &&
+ echo 000 >> a &&
+ git add a &&
+ git commit -m a &&
+ echo 111 >> a &&
+ git commit -a -m p1 &&
+ echo 222 >> a &&
+ git commit -a -m p2 &&
+ echo 333 >> a &&
+ git commit -a -m p3 &&
+ stg uncommit -n 3
+'
+
+cat > expected.txt <<EOF
+C a
+EOF
+test_expect_success 'Pop middle patch, creating a conflict' '
+ ! stg pop p2 &&
+ stg status a > actual.txt &&
+ test_cmp expected.txt actual.txt &&
+ test "$(echo $(stg applied))" = "p1 p3" &&
+ test "$(echo $(stg unapplied))" = "p2"
+'
+
+test_expect_success 'Try to reset without --hard' '
+ ! stg reset master.stgit^~1 &&
+ stg status a > actual.txt &&
+ test_cmp expected.txt actual.txt &&
+ test "$(echo $(stg applied))" = "p1 p3" &&
+ test "$(echo $(stg unapplied))" = "p2"
+'
+
+cat > expected.txt <<EOF
+EOF
+test_expect_success 'Try to reset with --hard' '
+ stg reset --hard master.stgit^~1 &&
+ stg status a > actual.txt &&
+ test_cmp expected.txt actual.txt &&
+ test "$(echo $(stg applied))" = "p1" &&
+ test "$(echo $(stg unapplied))" = "p3 p2"
+'
+
+test_done
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 09/14] Don't write a log entry if there were no changes
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (7 preceding siblings ...)
2008-06-12 5:34 ` [StGit PATCH 08/14] Add a --hard flag to stg reset Karl Hasselström
@ 2008-06-12 5:35 ` Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 10/14] Move stack reset function to a shared location Karl Hasselström
` (4 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:35 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Some commands end up calling log_entry() without verifying that they
did in fact change anything. (One example of this is a conflicting
push, which will log two entries, everything else and the conflicting
push, with the "everything else" part being empty if there was only
one patch to push.) So before appending to the log, make sure that the
entry we're appending isn't a no-op.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/lib/log.py | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/stgit/lib/log.py b/stgit/lib/log.py
index 920c261..3aec6e7 100644
--- a/stgit/lib/log.py
+++ b/stgit/lib/log.py
@@ -164,6 +164,9 @@ def log_entry(stack, msg):
out.warn(str(e), 'No log entry written.')
return
full_log_tree, short_log_tree = log_entry_trees(stack.repository, stack)
+ if len(last_log) == 1 and full_log_tree == last_log[0].full_log.data.tree:
+ # No changes, so there's no point writing a new log entry.
+ return
stack_log = stack.repository.commit(
git.CommitData(tree = short_log_tree, message = msg,
parents = [ll.stack_log for ll in last_log]))
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 10/14] Move stack reset function to a shared location
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (8 preceding siblings ...)
2008-06-12 5:35 ` [StGit PATCH 09/14] Don't write a log entry if there were no changes Karl Hasselström
@ 2008-06-12 5:35 ` Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 11/14] New command: stg undo Karl Hasselström
` (3 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:35 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Move reset_stack() from commands/reset.py to lib/log.py, so that more
commands besides reset can use it. (No such commands exist currently,
but undo and redo will use it.)
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/reset.py | 70 +++++-----------------------------------------
stgit/lib/log.py | 62 +++++++++++++++++++++++++++++++++++++++++
stgit/lib/transaction.py | 3 +-
3 files changed, 71 insertions(+), 64 deletions(-)
diff --git a/stgit/commands/reset.py b/stgit/commands/reset.py
index a7b5d35..5ad9914 100644
--- a/stgit/commands/reset.py
+++ b/stgit/commands/reset.py
@@ -47,67 +47,6 @@ directory = common.DirectoryHasRepositoryLib()
options = [make_option('--hard', action = 'store_true',
help = 'discard changes in your index/worktree')]
-def reset_stack(stack, iw, state, only_patches, hard):
- only_patches = set(only_patches)
- def mask(s):
- if only_patches:
- return s & only_patches
- else:
- return s
- patches_to_reset = mask(set(state.applied + state.unapplied))
- existing_patches = set(stack.patchorder.all)
- to_delete = mask(existing_patches - patches_to_reset)
- trans = transaction.StackTransaction(stack, 'reset', discard_changes = hard)
-
- # If we have to change the stack base, we need to pop all patches
- # first.
- if not only_patches and trans.base != state.base:
- trans.pop_patches(lambda pn: True)
- out.info('Setting stack base to %s' % state.base.sha1)
- trans.base = state.base
-
- # In one go, do all the popping we have to in order to pop the
- # patches we're going to delete or modify.
- def mod(pn):
- if only_patches and not pn in only_patches:
- return False
- if pn in to_delete:
- return True
- if stack.patches.get(pn).commit != state.patches.get(pn, None):
- return True
- return False
- trans.pop_patches(mod)
-
- # Delete and modify/create patches. We've previously popped all
- # patches that we touch in this step.
- trans.delete_patches(lambda pn: pn in to_delete)
- for pn in patches_to_reset:
- if pn in existing_patches:
- if trans.patches[pn] == state.patches[pn]:
- continue
- else:
- out.info('Resetting %s' % pn)
- else:
- trans.unapplied.append(pn)
- out.info('Resurrecting %s' % pn)
- trans.patches[pn] = state.patches[pn]
-
- # Push/pop patches as necessary.
- try:
- if only_patches:
- # Push all the patches that we've popped, if they still
- # exist.
- pushable = set(trans.unapplied)
- for pn in stack.patchorder.applied:
- if pn in pushable:
- trans.push_patch(pn, iw)
- else:
- # Recreate the exact order specified by the goal state.
- trans.reorder_patches(state.applied, state.unapplied, iw)
- except transaction.TransactionHalted:
- pass
- return trans.run(iw)
-
def func(parser, options, args):
stack = directory.repository.current_stack
if len(args) >= 1:
@@ -115,5 +54,10 @@ def func(parser, options, args):
state = log.Log(stack.repository, ref, stack.repository.rev_parse(ref))
else:
raise common.CmdException('Wrong number of arguments')
- return reset_stack(stack, stack.repository.default_iw, state, patches,
- options.hard)
+ trans = transaction.StackTransaction(stack, 'reset',
+ discard_changes = options.hard)
+ try:
+ log.reset_stack(trans, stack.repository.default_iw, state, patches)
+ except transaction.TransactionHalted:
+ pass
+ return trans.run(stack.repository.default_iw)
diff --git a/stgit/lib/log.py b/stgit/lib/log.py
index 3aec6e7..2449913 100644
--- a/stgit/lib/log.py
+++ b/stgit/lib/log.py
@@ -285,3 +285,65 @@ def copy_log(repo, src_branch, dst_branch, msg):
def default_repo():
return stack.Repository.default()
+
+def reset_stack(trans, iw, state, only_patches):
+ """Reset the stack to a given previous state. If C{only_patches} is
+ not empty, touch only patches whose names appear in it.
+
+ @param only_patches: Reset only these patches
+ @type only_patches: iterable"""
+ only_patches = set(only_patches)
+ def mask(s):
+ if only_patches:
+ return s & only_patches
+ else:
+ return s
+ patches_to_reset = mask(set(state.applied + state.unapplied))
+ existing_patches = set(trans.all_patches)
+ original_applied_order = list(trans.applied)
+ to_delete = mask(existing_patches - patches_to_reset)
+
+ # If we have to change the stack base, we need to pop all patches
+ # first.
+ if not only_patches and trans.base != state.base:
+ trans.pop_patches(lambda pn: True)
+ out.info('Setting stack base to %s' % state.base.sha1)
+ trans.base = state.base
+
+ # In one go, do all the popping we have to in order to pop the
+ # patches we're going to delete or modify.
+ def mod(pn):
+ if only_patches and not pn in only_patches:
+ return False
+ if pn in to_delete:
+ return True
+ if trans.patches[pn] != state.patches.get(pn, None):
+ return True
+ return False
+ trans.pop_patches(mod)
+
+ # Delete and modify/create patches. We've previously popped all
+ # patches that we touch in this step.
+ trans.delete_patches(lambda pn: pn in to_delete)
+ for pn in patches_to_reset:
+ if pn in existing_patches:
+ if trans.patches[pn] == state.patches[pn]:
+ continue
+ else:
+ out.info('Resetting %s' % pn)
+ else:
+ trans.unapplied.append(pn)
+ out.info('Resurrecting %s' % pn)
+ trans.patches[pn] = state.patches[pn]
+
+ # Push/pop patches as necessary.
+ if only_patches:
+ # Push all the patches that we've popped, if they still
+ # exist.
+ pushable = set(trans.unapplied)
+ for pn in original_applied_order:
+ if pn in pushable:
+ trans.push_patch(pn, iw)
+ else:
+ # Recreate the exact order specified by the goal state.
+ trans.reorder_patches(state.applied, state.unapplied, iw)
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 10c9b39..2003105 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -103,6 +103,7 @@ class StackTransaction(object):
def __set_unapplied(self, val):
self.__unapplied = list(val)
unapplied = property(lambda self: self.__unapplied, __set_unapplied)
+ all_patches = property(lambda self: self.__applied + self.__unapplied)
def __set_base(self, val):
assert (not self.__applied
or self.patches[self.applied[0]].data.parent == val)
@@ -136,7 +137,7 @@ class StackTransaction(object):
raise TransactionException(
'Command aborted (all changes rolled back)')
def __check_consistency(self):
- remaining = set(self.__applied + self.__unapplied)
+ remaining = set(self.all_patches)
for pn, commit in self.__patches.iteritems():
if commit == None:
assert self.__stack.patches.exists(pn)
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 11/14] New command: stg undo
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (9 preceding siblings ...)
2008-06-12 5:35 ` [StGit PATCH 10/14] Move stack reset function to a shared location Karl Hasselström
@ 2008-06-12 5:35 ` Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 12/14] New command: stg redo Karl Hasselström
` (2 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:35 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Basically, this is just a user-friendly way to access a subset of the
functionality of "stg reset".
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/undo.py | 40 +++++++---------------
stgit/lib/log.py | 38 +++++++++++++++++++++
stgit/main.py | 2 +
t/t3102-undo.sh | 86 +++++++++++++++++++++++++++++++++++++++++++++++
t/t3103-undo-hard.sh | 10 +++--
5 files changed, 144 insertions(+), 32 deletions(-)
copy stgit/commands/{reset.py => undo.py} (56%)
create mode 100755 t/t3102-undo.sh
copy t/{t3101-reset-hard.sh => t3103-undo-hard.sh} (82%)
diff --git a/stgit/commands/reset.py b/stgit/commands/undo.py
similarity index 56%
copy from stgit/commands/reset.py
copy to stgit/commands/undo.py
index 5ad9914..b1d7de9 100644
--- a/stgit/commands/reset.py
+++ b/stgit/commands/undo.py
@@ -22,42 +22,28 @@ from stgit.commands import common
from stgit.lib import git, log, transaction
from stgit.out import out
-help = 'reset the patch stack to an earlier state'
-usage = """%prog [options] <state> [<patchnames>]
+help = 'undo the last operation'
+usage = """%prog [options]
-Reset the patch stack to an earlier state. The state is specified with
-a commit from a stack log; for a branch foo, StGit stores the stack
-log in foo.stgit^. So to undo the last N StGit commands (or rather,
-the last N log entries; there is not an exact one-to-one
-relationship), you would say
-
- stg reset foo.stgit^~N
-
-or, if you are not sure how many steps to undo, you can view the log
-with "git log" or gitk
-
- gitk foo.stgit^
-
-and then reset to any sha1 you see in the log.
-
-If one or more patch names are given, reset only those patches, and
-leave the rest alone."""
+Reset the patch stack to the previous state. Consecutive invocations
+of "stg undo" will take you ever further into the past."""
directory = common.DirectoryHasRepositoryLib()
-options = [make_option('--hard', action = 'store_true',
+options = [make_option('-n', '--number', type = 'int', metavar = 'N',
+ default = 1,
+ help = 'undo the last N commands'),
+ make_option('--hard', action = 'store_true',
help = 'discard changes in your index/worktree')]
def func(parser, options, args):
stack = directory.repository.current_stack
- if len(args) >= 1:
- ref, patches = args[0], args[1:]
- state = log.Log(stack.repository, ref, stack.repository.rev_parse(ref))
- else:
- raise common.CmdException('Wrong number of arguments')
- trans = transaction.StackTransaction(stack, 'reset',
+ if options.number < 1:
+ raise common.CmdException('Bad number of commands to undo')
+ state = log.undo_state(stack, options.number)
+ trans = transaction.StackTransaction(stack, 'undo %d' % options.number,
discard_changes = options.hard)
try:
- log.reset_stack(trans, stack.repository.default_iw, state, patches)
+ log.reset_stack(trans, stack.repository.default_iw, state, [])
except transaction.TransactionHalted:
pass
return trans.run(stack.repository.default_iw)
diff --git a/stgit/lib/log.py b/stgit/lib/log.py
index 2449913..3b242cd 100644
--- a/stgit/lib/log.py
+++ b/stgit/lib/log.py
@@ -95,6 +95,7 @@ except for the following:
The simplified log contains no information not in the full log; its
purpose is ease of visualization."""
+import re
from stgit.lib import git, stack
from stgit import exception
from stgit.out import out
@@ -258,6 +259,16 @@ class Log(object):
self.base = self.patches[self.applied[0]].data.parent
else:
self.base = self.head
+ @property
+ def top(self):
+ if self.applied:
+ return self.patches[self.applied[-1]]
+ else:
+ return self.head
+ @property
+ def parents(self):
+ num_refs = len(set([self.top, self.head]))
+ return self.commit.data.parents[(1 + num_refs):]
class FullLog(Log):
full_log = property(lambda self: self.commit)
@@ -347,3 +358,30 @@ def reset_stack(trans, iw, state, only_patches):
else:
# Recreate the exact order specified by the goal state.
trans.reorder_patches(state.applied, state.unapplied, iw)
+
+def undo_state(stack, undo_steps):
+ """Find the log entry C{undo_steps} steps in the past. (Successive
+ undo operations are supposed to "add up", so if we find other undo
+ operations along the way we have to add those undo steps to
+ C{undo_steps}.)
+
+ @return: The log entry that is the destination of the undo
+ operation
+ @rtype: L{Log}"""
+ ref = log_ref(stack.name)
+ try:
+ commit = stack.repository.refs.get(ref)
+ except KeyError:
+ raise LogException('Log is empty')
+ log = Log(stack.repository, ref, commit)
+ while undo_steps > 0:
+ msg = log.commit.data.message.strip()
+ m = re.match(r'^undo\s+(\d+)$', msg)
+ if m:
+ undo_steps += int(m.group(1))
+ else:
+ undo_steps -= 1
+ if not log.parents:
+ raise LogException('Not enough undo information available')
+ log = Log(stack.repository, log.parents[0].sha1, log.parents[0])
+ return log
diff --git a/stgit/main.py b/stgit/main.py
index 83e6b08..cf7b404 100644
--- a/stgit/main.py
+++ b/stgit/main.py
@@ -99,6 +99,7 @@ commands = Commands({
'top': 'top',
'unapplied': 'unapplied',
'uncommit': 'uncommit',
+ 'undo': 'undo',
'unhide': 'unhide'
})
@@ -129,6 +130,7 @@ stackcommands = (
'top',
'unapplied',
'uncommit',
+ 'undo',
'unhide',
)
patchcommands = (
diff --git a/t/t3102-undo.sh b/t/t3102-undo.sh
new file mode 100755
index 0000000..1093f70
--- /dev/null
+++ b/t/t3102-undo.sh
@@ -0,0 +1,86 @@
+#!/bin/sh
+
+test_description='Simple test cases for "stg undo"'
+
+. ./test-lib.sh
+
+# Ignore our own output files.
+cat > .git/info/exclude <<EOF
+/expected.txt
+EOF
+
+test_expect_success 'Initialize StGit stack with three patches' '
+ stg init &&
+ echo 000 >> a &&
+ git add a &&
+ git commit -m a &&
+ echo 111 >> a &&
+ git commit -a -m p1 &&
+ echo 222 >> a &&
+ git commit -a -m p2 &&
+ echo 333 >> a &&
+ git commit -a -m p3 &&
+ stg uncommit -n 3 &&
+ stg pop
+'
+
+cat > expected.txt <<EOF
+000
+111
+EOF
+test_expect_success 'Pop one patch ...' '
+ stg pop &&
+ test "$(echo $(stg applied))" = "p1" &&
+ test "$(echo $(stg unapplied))" = "p2 p3" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success '... and undo it' '
+ stg undo &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "p3" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+EOF
+test_expect_success 'Pop two patches ...' '
+ stg pop &&
+ stg pop &&
+ test "$(echo $(stg applied))" = "" &&
+ test "$(echo $(stg unapplied))" = "p1 p2 p3" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success '... and undo it' '
+ stg undo &&
+ stg undo &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "p3" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success 'Undo past end of history' '
+ ! stg undo -n 100 &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "p3" &&
+ test_cmp expected.txt a
+'
+
+test_done
diff --git a/t/t3101-reset-hard.sh b/t/t3103-undo-hard.sh
similarity index 82%
copy from t/t3101-reset-hard.sh
copy to t/t3103-undo-hard.sh
index 1e02805..21412f7 100755
--- a/t/t3101-reset-hard.sh
+++ b/t/t3103-undo-hard.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='Simple test cases for "stg reset"'
+test_description='Simple test cases for "stg undo"'
. ./test-lib.sh
@@ -35,8 +35,8 @@ test_expect_success 'Pop middle patch, creating a conflict' '
test "$(echo $(stg unapplied))" = "p2"
'
-test_expect_success 'Try to reset without --hard' '
- ! stg reset master.stgit^~1 &&
+test_expect_success 'Try to undo without --hard' '
+ ! stg undo &&
stg status a > actual.txt &&
test_cmp expected.txt actual.txt &&
test "$(echo $(stg applied))" = "p1 p3" &&
@@ -45,8 +45,8 @@ test_expect_success 'Try to reset without --hard' '
cat > expected.txt <<EOF
EOF
-test_expect_success 'Try to reset with --hard' '
- stg reset --hard master.stgit^~1 &&
+test_expect_success 'Try to undo with --hard' '
+ stg undo --hard &&
stg status a > actual.txt &&
test_cmp expected.txt actual.txt &&
test "$(echo $(stg applied))" = "p1" &&
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 12/14] New command: stg redo
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (10 preceding siblings ...)
2008-06-12 5:35 ` [StGit PATCH 11/14] New command: stg undo Karl Hasselström
@ 2008-06-12 5:35 ` Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 13/14] Log and undo external modifications Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 14/14] Make "stg log" show stack log instead of patch log Karl Hasselström
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:35 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Command for undoing an undo.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/redo.py | 21 +++++++++------
stgit/lib/log.py | 21 ++++++++++++---
stgit/main.py | 2 +
t/t3104-redo.sh | 66 +++++++++++++++++++++++++++++++++++++-----------
4 files changed, 81 insertions(+), 29 deletions(-)
copy stgit/commands/{undo.py => redo.py} (71%)
copy t/{t3102-undo.sh => t3104-redo.sh} (53%)
diff --git a/stgit/commands/undo.py b/stgit/commands/redo.py
similarity index 71%
copy from stgit/commands/undo.py
copy to stgit/commands/redo.py
index b1d7de9..47221a5 100644
--- a/stgit/commands/undo.py
+++ b/stgit/commands/redo.py
@@ -19,28 +19,31 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
from optparse import make_option
from stgit.commands import common
-from stgit.lib import git, log, transaction
-from stgit.out import out
+from stgit.lib import log, transaction
-help = 'undo the last operation'
+help = 'undo the last undo operation'
usage = """%prog [options]
-Reset the patch stack to the previous state. Consecutive invocations
-of "stg undo" will take you ever further into the past."""
+If the last command was an undo, reset the patch stack to the state it
+had before the undo. Consecutive invocations of "stg redo" will undo
+the effects of consecutive invocations of "stg undo".
+
+It is an error to run "stg redo" if the last command was not an
+undo."""
directory = common.DirectoryHasRepositoryLib()
options = [make_option('-n', '--number', type = 'int', metavar = 'N',
default = 1,
- help = 'undo the last N commands'),
+ help = 'undo the last N undos'),
make_option('--hard', action = 'store_true',
help = 'discard changes in your index/worktree')]
def func(parser, options, args):
stack = directory.repository.current_stack
if options.number < 1:
- raise common.CmdException('Bad number of commands to undo')
- state = log.undo_state(stack, options.number)
- trans = transaction.StackTransaction(stack, 'undo %d' % options.number,
+ raise common.CmdException('Bad number of undos to redo')
+ state = log.undo_state(stack, -options.number)
+ trans = transaction.StackTransaction(stack, 'redo %d' % options.number,
discard_changes = options.hard)
try:
log.reset_stack(trans, stack.repository.default_iw, state, [])
diff --git a/stgit/lib/log.py b/stgit/lib/log.py
index 3b242cd..a8de06b 100644
--- a/stgit/lib/log.py
+++ b/stgit/lib/log.py
@@ -365,6 +365,8 @@ def undo_state(stack, undo_steps):
operations along the way we have to add those undo steps to
C{undo_steps}.)
+ If C{undo_steps} is negative, redo instead of undo.
+
@return: The log entry that is the destination of the undo
operation
@rtype: L{Log}"""
@@ -374,13 +376,22 @@ def undo_state(stack, undo_steps):
except KeyError:
raise LogException('Log is empty')
log = Log(stack.repository, ref, commit)
- while undo_steps > 0:
+ while undo_steps != 0:
msg = log.commit.data.message.strip()
- m = re.match(r'^undo\s+(\d+)$', msg)
- if m:
- undo_steps += int(m.group(1))
+ um = re.match(r'^undo\s+(\d+)$', msg)
+ if undo_steps > 0:
+ if um:
+ undo_steps += int(um.group(1))
+ else:
+ undo_steps -= 1
else:
- undo_steps -= 1
+ rm = re.match(r'^redo\s+(\d+)$', msg)
+ if um:
+ undo_steps += 1
+ elif rm:
+ undo_steps -= int(rm.group(1))
+ else:
+ raise LogException('No more redo information available')
if not log.parents:
raise LogException('Not enough undo information available')
log = Log(stack.repository, log.parents[0].sha1, log.parents[0])
diff --git a/stgit/main.py b/stgit/main.py
index cf7b404..c53abf7 100644
--- a/stgit/main.py
+++ b/stgit/main.py
@@ -86,6 +86,7 @@ commands = Commands({
'pull': 'pull',
'push': 'push',
'rebase': 'rebase',
+ 'redo': 'redo',
'refresh': 'refresh',
'rename': 'rename',
'repair': 'repair',
@@ -123,6 +124,7 @@ stackcommands = (
'pull',
'push',
'rebase',
+ 'redo',
'repair',
'reset',
'series',
diff --git a/t/t3102-undo.sh b/t/t3104-redo.sh
similarity index 53%
copy from t/t3102-undo.sh
copy to t/t3104-redo.sh
index 1093f70..290fc6f 100755
--- a/t/t3102-undo.sh
+++ b/t/t3104-redo.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='Simple test cases for "stg undo"'
+test_description='Simple test cases for "stg redo"'
. ./test-lib.sh
@@ -20,18 +20,18 @@ test_expect_success 'Initialize StGit stack with three patches' '
git commit -a -m p2 &&
echo 333 >> a &&
git commit -a -m p3 &&
- stg uncommit -n 3 &&
- stg pop
+ stg uncommit -n 3
'
cat > expected.txt <<EOF
000
111
+222
EOF
test_expect_success 'Pop one patch ...' '
stg pop &&
- test "$(echo $(stg applied))" = "p1" &&
- test "$(echo $(stg unapplied))" = "p2 p3" &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "p3" &&
test_cmp expected.txt a
'
@@ -39,9 +39,22 @@ cat > expected.txt <<EOF
000
111
222
+333
EOF
-test_expect_success '... and undo it' '
+test_expect_success '... undo it ...' '
stg undo &&
+ test "$(echo $(stg applied))" = "p1 p2 p3" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success '... and redo' '
+ stg redo &&
test "$(echo $(stg applied))" = "p1 p2" &&
test "$(echo $(stg unapplied))" = "p3" &&
test_cmp expected.txt a
@@ -50,7 +63,9 @@ test_expect_success '... and undo it' '
cat > expected.txt <<EOF
000
EOF
-test_expect_success 'Pop two patches ...' '
+test_expect_success 'Pop three patches ...' '
+ stg push &&
+ stg pop &&
stg pop &&
stg pop &&
test "$(echo $(stg applied))" = "" &&
@@ -62,24 +77,45 @@ cat > expected.txt <<EOF
000
111
222
+333
EOF
-test_expect_success '... and undo it' '
+test_expect_success '... undo it ...' '
stg undo &&
stg undo &&
- test "$(echo $(stg applied))" = "p1 p2" &&
- test "$(echo $(stg unapplied))" = "p3" &&
+ stg undo &&
+ test "$(echo $(stg applied))" = "p1 p2 p3" &&
+ test "$(echo $(stg unapplied))" = "" &&
test_cmp expected.txt a
'
cat > expected.txt <<EOF
000
111
-222
EOF
-test_expect_success 'Undo past end of history' '
- ! stg undo -n 100 &&
- test "$(echo $(stg applied))" = "p1 p2" &&
- test "$(echo $(stg unapplied))" = "p3" &&
+test_expect_success '... redo the first two pops ...' '
+ stg redo -n 2 &&
+ test "$(echo $(stg applied))" = "p1" &&
+ test "$(echo $(stg unapplied))" = "p2 p3" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+EOF
+test_expect_success '... and the remaining one' '
+ stg redo &&
+ test "$(echo $(stg applied))" = "" &&
+ test "$(echo $(stg unapplied))" = "p1 p2 p3" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+EOF
+test_expect_success 'Redo past end of history' '
+ ! stg redo &&
+ test "$(echo $(stg applied))" = "" &&
+ test "$(echo $(stg unapplied))" = "p1 p2 p3" &&
test_cmp expected.txt a
'
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 13/14] Log and undo external modifications
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (11 preceding siblings ...)
2008-06-12 5:35 ` [StGit PATCH 12/14] New command: stg redo Karl Hasselström
@ 2008-06-12 5:35 ` Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 14/14] Make "stg log" show stack log instead of patch log Karl Hasselström
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:35 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
At the beginning of every StGit command, quickly check if the branch
head recorded in the log is the same as the actual branch head; if
it's not, conclude that some non-StGit tool has modified the stack,
and record a log entry that says so. (Additionally, if the log doesn't
exist yet, create it.)
This introduces the possibility that a log entry specifies a head and
a top that aren't equal. So teach undo, redo, and reset to deal with
that case.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/common.py | 1
stgit/commands/redo.py | 4 +-
stgit/commands/reset.py | 8 +++-
stgit/commands/undo.py | 4 +-
stgit/lib/log.py | 88 ++++++++++++++++++++++++++++--------------
stgit/lib/transaction.py | 39 ++++++++++++-------
t/t3105-undo-external-mod.sh | 68 ++++++++++++++++++++++++++++++++
7 files changed, 162 insertions(+), 50 deletions(-)
create mode 100755 t/t3105-undo-external-mod.sh
diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index fd02398..2689a42 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -525,6 +525,7 @@ class DirectoryAnywhere(_Directory):
class DirectoryHasRepository(_Directory):
def setup(self):
self.git_dir # might throw an exception
+ log.compat_log_external_mods()
class DirectoryInWorktree(DirectoryHasRepository):
def setup(self):
diff --git a/stgit/commands/redo.py b/stgit/commands/redo.py
index 47221a5..a1075ec 100644
--- a/stgit/commands/redo.py
+++ b/stgit/commands/redo.py
@@ -46,7 +46,7 @@ def func(parser, options, args):
trans = transaction.StackTransaction(stack, 'redo %d' % options.number,
discard_changes = options.hard)
try:
- log.reset_stack(trans, stack.repository.default_iw, state, [])
+ log.reset_stack(trans, stack.repository.default_iw, state)
except transaction.TransactionHalted:
pass
- return trans.run(stack.repository.default_iw)
+ return trans.run(stack.repository.default_iw, allow_bad_head = True)
diff --git a/stgit/commands/reset.py b/stgit/commands/reset.py
index 5ad9914..22d7731 100644
--- a/stgit/commands/reset.py
+++ b/stgit/commands/reset.py
@@ -57,7 +57,11 @@ def func(parser, options, args):
trans = transaction.StackTransaction(stack, 'reset',
discard_changes = options.hard)
try:
- log.reset_stack(trans, stack.repository.default_iw, state, patches)
+ if patches:
+ log.reset_stack_partially(trans, stack.repository.default_iw,
+ state, patches)
+ else:
+ log.reset_stack(trans, stack.repository.default_iw, state)
except transaction.TransactionHalted:
pass
- return trans.run(stack.repository.default_iw)
+ return trans.run(stack.repository.default_iw, allow_bad_head = not patches)
diff --git a/stgit/commands/undo.py b/stgit/commands/undo.py
index b1d7de9..58f1da6 100644
--- a/stgit/commands/undo.py
+++ b/stgit/commands/undo.py
@@ -43,7 +43,7 @@ def func(parser, options, args):
trans = transaction.StackTransaction(stack, 'undo %d' % options.number,
discard_changes = options.hard)
try:
- log.reset_stack(trans, stack.repository.default_iw, state, [])
+ log.reset_stack(trans, stack.repository.default_iw, state)
except transaction.TransactionHalted:
pass
- return trans.run(stack.repository.default_iw)
+ return trans.run(stack.repository.default_iw, allow_bad_head = True)
diff --git a/stgit/lib/log.py b/stgit/lib/log.py
index a8de06b..8c0516b 100644
--- a/stgit/lib/log.py
+++ b/stgit/lib/log.py
@@ -259,6 +259,7 @@ class Log(object):
self.base = self.patches[self.applied[0]].data.parent
else:
self.base = self.head
+ all_patches = property(lambda self: self.applied + self.unapplied)
@property
def top(self):
if self.applied:
@@ -297,34 +298,33 @@ def copy_log(repo, src_branch, dst_branch, msg):
def default_repo():
return stack.Repository.default()
-def reset_stack(trans, iw, state, only_patches):
- """Reset the stack to a given previous state. If C{only_patches} is
- not empty, touch only patches whose names appear in it.
+def reset_stack(trans, iw, state):
+ """Reset the stack to a given previous state."""
+ for pn in trans.all_patches:
+ trans.patches[pn] = None
+ for pn in state.all_patches:
+ trans.patches[pn] = state.patches[pn]
+ trans.applied = state.applied
+ trans.unapplied = state.unapplied
+ trans.base = state.base
+ trans.head = state.head
+
+def reset_stack_partially(trans, iw, state, only_patches):
+ """Reset the stack to a given previous state -- but only the given
+ patches, not anything else.
- @param only_patches: Reset only these patches
+ @param only_patches: Touch only these patches
@type only_patches: iterable"""
only_patches = set(only_patches)
- def mask(s):
- if only_patches:
- return s & only_patches
- else:
- return s
- patches_to_reset = mask(set(state.applied + state.unapplied))
+ patches_to_reset = set(state.applied + state.unapplied) & only_patches
existing_patches = set(trans.all_patches)
original_applied_order = list(trans.applied)
- to_delete = mask(existing_patches - patches_to_reset)
-
- # If we have to change the stack base, we need to pop all patches
- # first.
- if not only_patches and trans.base != state.base:
- trans.pop_patches(lambda pn: True)
- out.info('Setting stack base to %s' % state.base.sha1)
- trans.base = state.base
+ to_delete = (existing_patches - patches_to_reset) & only_patches
# In one go, do all the popping we have to in order to pop the
# patches we're going to delete or modify.
def mod(pn):
- if only_patches and not pn in only_patches:
+ if not pn in only_patches:
return False
if pn in to_delete:
return True
@@ -347,17 +347,12 @@ def reset_stack(trans, iw, state, only_patches):
out.info('Resurrecting %s' % pn)
trans.patches[pn] = state.patches[pn]
- # Push/pop patches as necessary.
- if only_patches:
- # Push all the patches that we've popped, if they still
- # exist.
- pushable = set(trans.unapplied)
- for pn in original_applied_order:
- if pn in pushable:
- trans.push_patch(pn, iw)
- else:
- # Recreate the exact order specified by the goal state.
- trans.reorder_patches(state.applied, state.unapplied, iw)
+ # Push all the patches that we've popped, if they still
+ # exist.
+ pushable = set(trans.unapplied)
+ for pn in original_applied_order:
+ if pn in pushable:
+ trans.push_patch(pn, iw)
def undo_state(stack, undo_steps):
"""Find the log entry C{undo_steps} steps in the past. (Successive
@@ -396,3 +391,36 @@ def undo_state(stack, undo_steps):
raise LogException('Not enough undo information available')
log = Log(stack.repository, log.parents[0].sha1, log.parents[0])
return log
+
+def log_external_mods(stack):
+ ref = log_ref(stack.name)
+ try:
+ log_commit = stack.repository.refs.get(ref)
+ except KeyError:
+ # No log exists yet.
+ log_entry(stack, 'start of log')
+ return
+ try:
+ log = Log(stack.repository, ref, log_commit)
+ except LogException:
+ # Something's wrong with the log, so don't bother.
+ return
+ if log.head == stack.head:
+ # No external modifications.
+ return
+ log_entry(stack, '\n'.join([
+ 'external modifications', '',
+ 'Modifications by tools other than StGit (e.g. git).']))
+
+def compat_log_external_mods():
+ try:
+ repo = default_repo()
+ except git.RepositoryException:
+ # No repository, so we couldn't log even if we wanted to.
+ return
+ try:
+ stack = repo.get_stack(repo.current_branch_name)
+ except exception.StgException:
+ # Stack doesn't exist, so we can't log.
+ return
+ log_external_mods(stack)
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 2003105..0c8d9a5 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -91,6 +91,7 @@ class StackTransaction(object):
self.__current_tree = self.__stack.head.data.tree
self.__base = self.__stack.base
self.__discard_changes = discard_changes
+ self.__bad_head = None
if isinstance(allow_conflicts, bool):
self.__allow_conflicts = lambda trans: allow_conflicts
else:
@@ -109,8 +110,22 @@ class StackTransaction(object):
or self.patches[self.applied[0]].data.parent == val)
self.__base = val
base = property(lambda self: self.__base, __set_base)
- def __checkout(self, tree, iw):
- if not self.__stack.head_top_equal():
+ @property
+ def top(self):
+ if self.__applied:
+ return self.__patches[self.__applied[-1]]
+ else:
+ return self.__base
+ def __get_head(self):
+ if self.__bad_head:
+ return self.__bad_head
+ else:
+ return self.top
+ def __set_head(self, val):
+ self.__bad_head = val
+ head = property(__get_head, __set_head)
+ def __checkout(self, tree, iw, allow_bad_head):
+ if not (allow_bad_head or self.__stack.head_top_equal()):
out.error(
'HEAD and top are not the same.',
'This can happen if you modify a branch with git.',
@@ -143,26 +158,22 @@ class StackTransaction(object):
assert self.__stack.patches.exists(pn)
else:
assert pn in remaining
- @property
- def __head(self):
- if self.__applied:
- return self.__patches[self.__applied[-1]]
- else:
- return self.__base
def abort(self, iw = None):
# The only state we need to restore is index+worktree.
if iw:
- self.__checkout(self.__stack.head.data.tree, iw)
- def run(self, iw = None):
+ self.__checkout(self.__stack.head.data.tree, iw,
+ allow_bad_head = True)
+ def run(self, iw = None, allow_bad_head = False):
"""Execute the transaction. Will either succeed, or fail (with an
exception) and do nothing."""
self.__check_consistency()
- new_head = self.__head
+ log.log_external_mods(self.__stack)
+ new_head = self.head
# Set branch head.
if iw:
try:
- self.__checkout(new_head.data.tree, iw)
+ self.__checkout(new_head.data.tree, iw, allow_bad_head)
except git.CheckoutException:
# We have to abort the transaction.
self.abort(iw)
@@ -257,7 +268,7 @@ class StackTransaction(object):
cd = orig_cd.set_committer(None)
s = ['', ' (empty)'][cd.is_nochange()]
oldparent = cd.parent
- cd = cd.set_parent(self.__head)
+ cd = cd.set_parent(self.top)
base = oldparent.data.tree
ours = cd.parent.data.tree
theirs = cd.tree
@@ -267,7 +278,7 @@ class StackTransaction(object):
if iw == None:
self.__halt('%s does not apply cleanly' % pn)
try:
- self.__checkout(ours, iw)
+ self.__checkout(ours, iw, allow_bad_head = False)
except git.CheckoutException:
self.__halt('Index/worktree dirty')
try:
diff --git a/t/t3105-undo-external-mod.sh b/t/t3105-undo-external-mod.sh
new file mode 100755
index 0000000..289a42a
--- /dev/null
+++ b/t/t3105-undo-external-mod.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+test_description='Undo external modifications of the stack'
+
+. ./test-lib.sh
+
+# Ignore our own output files.
+cat > .git/info/exclude <<EOF
+/expected.txt
+/head?.txt
+EOF
+
+test_expect_success 'Initialize StGit stack' '
+ stg init &&
+ echo 000 >> a &&
+ git add a &&
+ git commit -m p0 &&
+ echo 111 >> a &&
+ git add a &&
+ git commit -m p1 &&
+ stg uncommit -n 1
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success 'Make a git commit and turn it into a patch' '
+ git rev-parse HEAD > head0.txt &&
+ echo 222 >> a &&
+ git add a &&
+ git commit -m p2 &&
+ git rev-parse HEAD > head1.txt &&
+ stg repair &&
+ test "$(echo $(stg applied))" = "p1 p2" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success 'Undo the patchification' '
+ stg undo &&
+ git rev-parse HEAD > head2.txt &&
+ test_cmp head1.txt head2.txt &&
+ test "$(echo $(stg applied))" = "p1" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+EOF
+test_expect_success 'Undo the commit' '
+ stg undo &&
+ git rev-parse HEAD > head3.txt &&
+ test_cmp head0.txt head3.txt &&
+ test "$(echo $(stg applied))" = "p1" &&
+ test "$(echo $(stg unapplied))" = "" &&
+ test_cmp expected.txt a
+'
+
+test_done
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [StGit PATCH 14/14] Make "stg log" show stack log instead of patch log
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
` (12 preceding siblings ...)
2008-06-12 5:35 ` [StGit PATCH 13/14] Log and undo external modifications Karl Hasselström
@ 2008-06-12 5:35 ` Karl Hasselström
13 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-12 5:35 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Make "stg log" show the new stack log instead of the old patch logs,
which is now obsolete. Delete t1400-patch-history, which is specific
to the old "stg log".
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/log.py | 169 ++++++++++++++--------------------------------
stgit/commands/reset.py | 15 +---
stgit/lib/log.py | 3 +
t/t1400-patch-history.sh | 103 ----------------------------
4 files changed, 58 insertions(+), 232 deletions(-)
delete mode 100755 t/t1400-patch-history.sh
diff --git a/stgit/commands/log.py b/stgit/commands/log.py
index 13e0baa..cf15c7d 100644
--- a/stgit/commands/log.py
+++ b/stgit/commands/log.py
@@ -1,5 +1,8 @@
+# -*- coding: utf-8 -*-
+
__copyright__ = """
Copyright (C) 2006, Catalin Marinas <catalin.marinas@gmail.com>
+Copyright (C) 2008, Karl Hasselström <kha@treskal.com>
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License version 2 as
@@ -15,133 +18,67 @@ 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, time
-from optparse import OptionParser, make_option
-from pydoc import pager
-from stgit.commands.common import *
-from stgit import stack, git
-from stgit.out import *
-from stgit.run import Run
+import os.path
+from optparse import make_option
+from stgit import run
+from stgit.commands import common
+from stgit.lib import log
+from stgit.out import out
help = 'display the patch changelog'
-usage = """%prog [options] [patch]
-
-List all the current and past commit ids of the given patch. The
---graphical option invokes gitk instead of printing. The changelog
-commit messages have the form '<action> <new-patch-id>'. The <action>
-can be one of the following:
+usage = """%prog [options] [<patchnames>]
- new - new patch created
- refresh - local changes were added to the patch
- push - the patch was cleanly pushed onto the stack
- push(m) - the patch was pushed onto the stack with a three-way merge
- push(f) - the patch was fast-forwarded
- undo - the patch boundaries were restored to the old values
+List the history of the patch stack: the stack log. If one or more
+patch names are given, limit the list to the log entries that touch
+the named patches.
-Note that only the diffs shown in the 'refresh', 'undo' and 'sync'
-actions are meaningful for the patch changes. The 'push' actions
-represent the changes to the entire base of the current
-patch. Conflicts reset the patch content and a subsequent 'refresh'
-will show the entire patch."""
+"stg undo" and "stg redo" let you step back and forth in the patch
+stack. "stg reset" lets you go directly to any state."""
-directory = DirectoryHasRepository(log = False)
+directory = common.DirectoryHasRepositoryLib()
options = [make_option('-b', '--branch',
help = 'use BRANCH instead of the default one'),
- make_option('-p', '--patch',
- help = 'show the refresh diffs',
- action = 'store_true'),
+ make_option('-d', '--diff', action = 'store_true',
+ help = 'show diff to previous state'),
make_option('-n', '--number', type = 'int',
- help = 'limit the output to NUMBER commits'),
- make_option('-f', '--full',
- help = 'show the full commit ids',
- action = 'store_true'),
- make_option('-g', '--graphical',
- help = 'run gitk instead of printing',
- action = 'store_true')]
-
-def show_log(log, options):
- """List the patch changelog
- """
- commit = git.get_commit(log)
- if options.number != None:
- n = options.number
- else:
- n = -1
- diff_list = []
- while commit:
- if n == 0:
- # limit the output
- break
-
- log = commit.get_log().split('\n')
-
- cmd_rev = log[0].split()
- if len(cmd_rev) >= 2:
- cmd = cmd_rev[0]
- rev = cmd_rev[1]
- elif len(cmd_rev) == 1:
- cmd = cmd_rev[0]
- rev = ''
- else:
- cmd = rev = ''
-
- if options.patch:
- if cmd in ['refresh', 'undo', 'sync', 'edit']:
- diff_list.append(git.pretty_commit(commit.get_id_hash()))
-
- # limiter decrement
- n -= 1
- else:
- if len(log) >= 3:
- notes = log[2]
- else:
- notes = ''
- author_name, author_email, author_date = \
- name_email_date(commit.get_author())
- secs, tz = author_date.split()
- date = '%s %s' % (time.ctime(int(secs)), tz)
-
- if options.full:
- out.stdout('%-7s %-40s %s' % (cmd[:7], rev[:40], date))
- else:
- out.stdout('%-8s [%-7s] %-28s %s' % \
- (rev[:8], cmd[:7], notes[:28], date))
-
- # limiter decrement
- n -= 1
-
- parent = commit.get_parent()
- if parent:
- commit = git.get_commit(parent)
- else:
- commit = None
-
- if options.patch and diff_list:
- pager('\n'.join(diff_list).rstrip())
+ help = 'limit the output to NUMBER entries'),
+ make_option('-f', '--full', action = 'store_true',
+ help = 'show the log in more detail'),
+ make_option('-g', '--graphical', action = 'store_true',
+ help = 'run gitk instead of printing')]
+
+def show_log(stacklog, pathlim, num, full, show_diff):
+ cmd = ['git', 'log']
+ if num != None and num > 0:
+ cmd.append('-%d' % num)
+ if show_diff:
+ cmd.append('-p')
+ elif not full:
+ cmd.append('--pretty=format:%h %aD %s')
+ run.Run(*(cmd + [stacklog.sha1, '--'] + pathlim)).run()
def func(parser, options, args):
- """Show the patch changelog
- """
- if len(args) == 0:
- name = crt_series.get_current()
- if not name:
- raise CmdException, 'No patches applied'
- elif len(args) == 1:
- name = args[0]
- if not name in crt_series.get_applied() + crt_series.get_unapplied() + \
- crt_series.get_hidden():
- raise CmdException, 'Unknown patch "%s"' % name
+ if options.branch:
+ stack = directory.repository.get_stack(options.branch)
else:
- parser.error('incorrect number of arguments')
-
- patch = crt_series.get_patch(name)
-
- log = patch.get_log()
- if not log:
- raise CmdException, 'No changelog for patch "%s"' % name
+ stack = directory.repository.current_stack
+ patches = common.parse_patches(args, list(stack.patchorder.all))
+ logref = log.log_ref(stack.name)
+ try:
+ logcommit = stack.repository.refs.get(logref)
+ except KeyError:
+ out.info('Log is empty')
+ return
+ stacklog = log.Log(stack.repository, logref, logcommit)
+ pathlim = [os.path.join('patches', pn) for pn in patches]
if options.graphical:
- # discard the exit codes generated by SIGINT, SIGKILL, SIGTERM
- Run('gitk', log).returns([0, -2, -9, -15]).run()
+ for o in ['diff', 'number', 'full']:
+ if getattr(options, o):
+ parser.error('cannot combine --graphical and --%s' % o)
+ # Discard the exit codes generated by SIGINT, SIGKILL, and SIGTERM.
+ run.Run(*(['gitk', stacklog.pretty.sha1, '--'] + pathlim)
+ ).returns([0, -2, -9, -15]).run()
else:
- show_log(log, options)
+ show_log(stacklog.pretty, pathlim,
+ options.number, options.full, options.diff)
diff --git a/stgit/commands/reset.py b/stgit/commands/reset.py
index 22d7731..bbe5dda 100644
--- a/stgit/commands/reset.py
+++ b/stgit/commands/reset.py
@@ -26,19 +26,8 @@ help = 'reset the patch stack to an earlier state'
usage = """%prog [options] <state> [<patchnames>]
Reset the patch stack to an earlier state. The state is specified with
-a commit from a stack log; for a branch foo, StGit stores the stack
-log in foo.stgit^. So to undo the last N StGit commands (or rather,
-the last N log entries; there is not an exact one-to-one
-relationship), you would say
-
- stg reset foo.stgit^~N
-
-or, if you are not sure how many steps to undo, you can view the log
-with "git log" or gitk
-
- gitk foo.stgit^
-
-and then reset to any sha1 you see in the log.
+a commit id from a stack log; "stg log" lets you view this log, and
+"stg reset" lets you reset to any state you see in the log.
If one or more patch names are given, reset only those patches, and
leave the rest alone."""
diff --git a/stgit/lib/log.py b/stgit/lib/log.py
index 8c0516b..61e9cf2 100644
--- a/stgit/lib/log.py
+++ b/stgit/lib/log.py
@@ -270,6 +270,9 @@ class Log(object):
def parents(self):
num_refs = len(set([self.top, self.head]))
return self.commit.data.parents[(1 + num_refs):]
+ @property
+ def pretty(self):
+ return self.commit.data.parents[0]
class FullLog(Log):
full_log = property(lambda self: self.commit)
diff --git a/t/t1400-patch-history.sh b/t/t1400-patch-history.sh
deleted file mode 100755
index a693e75..0000000
--- a/t/t1400-patch-history.sh
+++ /dev/null
@@ -1,103 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Catalin Marinas
-#
-
-test_description='Test the patch history generation.
-
-'
-
-. ./test-lib.sh
-
-test_expect_success \
- 'Initialize the StGIT repository' \
- '
- stg init
- '
-
-test_expect_success \
- 'Create the first patch' \
- '
- stg new foo -m "Foo Patch" &&
- echo foo > test && echo foo2 >> test &&
- git add test &&
- stg refresh --annotate="foo notes"
- '
-
-test_expect_success \
- 'Create the second patch' \
- '
- stg new bar -m "Bar Patch" &&
- echo bar >> test &&
- stg refresh
- '
-
-test_expect_success \
- 'Check the "new" and "refresh" logs' \
- '
- stg log --full foo | grep -q -e "^refresh" &&
- stg log --full | grep -q -e "^refresh"
- '
-
-test_expect_success \
- 'Check the log annotation' \
- '
- stg log foo | grep -q -e "\[refresh\] foo notes " &&
- stg log bar | grep -q -e "\[refresh\] " &&
- stg refresh -p foo --annotate="foo notes 2" &&
- stg log foo | grep -q -v -e "\[refresh\] foo notes " &&
- stg log foo | grep -q -e "\[refresh\] foo notes 2"
- '
-
-test_expect_success \
- 'Check the "push" log' \
- '
- stg pop &&
- echo foo > test2 && git add test2 && stg refresh &&
- stg push &&
- stg log --full | grep -q -e "^push "
- '
-
-test_expect_success \
- 'Check the "push(f)" log' \
- '
- stg pop &&
- stg edit -m "Foo2 Patch" &&
- stg push &&
- stg log --full | grep -q -e "^push(f) "
- '
-
-test_expect_success \
- 'Check the "push(m)" log' \
- '
- stg pop &&
- echo foo2 > test && stg refresh &&
- stg push &&
- stg log --full | grep -q -e "^push(m) "
- '
-
-test_expect_success \
- 'Check the "push(c)" log' \
- '
- echo bar > test && stg refresh &&
- stg pop &&
- echo foo > test && stg refresh &&
- ! stg push &&
- stg log --full | grep -q -e "^push(c) "
- '
-
-test_expect_success \
- 'Check the push "undo" log' \
- '
- stg push --undo &&
- stg log --full bar | grep -q -e "^undo "
- '
-
-test_expect_success \
- 'Check the refresh "undo" log' \
- '
- stg refresh --undo &&
- stg log --full | grep -q -e "^undo "
- '
-
-test_done
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-12 5:34 ` [StGit PATCH 03/14] Write to a stack log when stack is modified Karl Hasselström
@ 2008-06-17 10:24 ` Catalin Marinas
2008-06-17 12:31 ` Karl Hasselström
0 siblings, 1 reply; 32+ messages in thread
From: Catalin Marinas @ 2008-06-17 10:24 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
Hi Karl,
There are some comments below but I think I haven't fully understood this.
2008/6/12 Karl Hasselström <kha@treskal.com>:
> Create a log branch (called <branchname>.stgit) for each StGit branch,
> and write to it whenever the stack is modified.
>
> Commands using the new infrastructure write to the log when they
> commit a transaction. Commands using the old infrastructure get a log
> entry write written for them when they exit, unless they explicitly
> ask for this not to happen.
I was more thinking for this to be the default for backwards API compatibility.
> class _Directory(object):
> - def __init__(self, needs_current_series = True):
> + def __init__(self, needs_current_series = True, log = True):
i.e. we make log = False here by default.
> --- /dev/null
> +++ b/stgit/lib/log.py
> @@ -0,0 +1,254 @@
> +r"""This module contains functions and classes for manipulating
Why does this start with an 'r'? I thought this is for regular expressions.
> +A stack log is a git branch. Each commit contains the complete state
> +of the stack at the moment it was written; the most recent commit has
> +the most recent state.
> +
> +For a branch C{I{foo}}, the stack log is stored in C{I{foo}.stgit}.
The main question. Is this history preserved after a git-gc?
> +Tree
> +----
> +
> +The top-level tree object has the following entries:
> +
> + - C{version}: a blob containing the string "C{5\n}".
> +
> + - C{head}: a blob containing the ASCII hex sha1 of the current HEAD,
> + followed by a newline.
> +
> + - C{applied}, C{unapplied}: blobs containing one line for each
> + applied or unapplied patch, in order. Each line consists of
> +
> + - the ASCII hex sha1 of the patch's commit object,
> +
> + - a space, and
> +
> + - the patch's name.
> +
> + - C{patches}: a tree containing one subtree for each patch, named
> + after that patch. Each such subtree contains:
> +
> + - C{a}, C{b}: the patch's I{bottom} and I{top} trees.
> +
> + - C{info}: a blob containing::
> +
> + Author: <author name and e-mail>
> + Date: <patch timestamp>
> +
> + <commit message>
I might not fully understand this but can we not store just the commit
object if the patch, which would have the bottom/top information.
> +Simplified log
> +--------------
> +
> +The simplified log looks exactly like the normal, or I{full}, log,
> +except for the following:
> +
> + - Instead of having a tree per patch in the C{patches} subtree, it
> + has a blob per patch. This blob contains::
> +
> + Bottom: <sha1 of patch's bottom tree>
> + Top: <sha1 of patch's top tree>
> + Author: <author name and e-mail>
> + Date: <patch timestamp>
Can we not point to the original commit corresponding to the patch? It
already has this information.
> +
> + <commit message>
> +
> + ---
> +
> + <patch's diff>
What is the patch's diff here? Cannot it be determined from the trees?
> +The simplified log contains no information not in the full log; its
> +purpose is ease of visualization."""
Ah, OK. But I think it would be more useful to see the diff between
subsequent revisions of a stack rather than the full patch diff.
Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-17 10:24 ` Catalin Marinas
@ 2008-06-17 12:31 ` Karl Hasselström
2008-06-17 12:55 ` Karl Hasselström
2008-06-17 14:11 ` Catalin Marinas
0 siblings, 2 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-17 12:31 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-17 11:24:53 +0100, Catalin Marinas wrote:
> There are some comments below but I think I haven't fully understood
> this.
I've done my best to answer. Though no doubt you'll have follow-up
questions.
> 2008/6/12 Karl Hasselström <kha@treskal.com>:
>
> > Create a log branch (called <branchname>.stgit) for each StGit
> > branch, and write to it whenever the stack is modified.
> >
> > Commands using the new infrastructure write to the log when they
> > commit a transaction. Commands using the old infrastructure get a
> > log entry write written for them when they exit, unless they
> > explicitly ask for this not to happen.
>
> I was more thinking for this to be the default for backwards API
> compatibility.
>
> > class _Directory(object):
> > - def __init__(self, needs_current_series = True):
> > + def __init__(self, needs_current_series = True, log = True):
>
> i.e. we make log = False here by default.
I might not have understood precisely what you meant; but I don't
think API backwards compatibilty should be an issue here? I simply fix
all callers. If log should default to true or false is immaterial --
it just means some extra text in one or the other of two equally
common cases.
> > --- /dev/null
> > +++ b/stgit/lib/log.py
> > @@ -0,0 +1,254 @@
> > +r"""This module contains functions and classes for manipulating
>
> Why does this start with an 'r'? I thought this is for regular
> expressions.
"r" in front of a string literal means "raw" (or some such). Escape
sequences aren't recognized inside a raw string -- e.g., r'\n' ==
'\\n'. They are useful when you have to write strings with embedded
backslashes, such as regexes -- or this string, which has \n in it.
> > +A stack log is a git branch. Each commit contains the complete state
> > +of the stack at the moment it was written; the most recent commit has
> > +the most recent state.
> > +
> > +For a branch C{I{foo}}, the stack log is stored in C{I{foo}.stgit}.
>
> The main question. Is this history preserved after a git-gc?
Yes. It's stored in a regular git branch. (The design is such that it
should even be possible to pull a stack log from another repository
and _still_ get everything you need.)
This should probably be in the description text.
> > + - C{patches}: a tree containing one subtree for each patch, named
> > + after that patch. Each such subtree contains:
> > +
> > + - C{a}, C{b}: the patch's I{bottom} and I{top} trees.
> > +
> > + - C{info}: a blob containing::
> > +
> > + Author: <author name and e-mail>
> > + Date: <patch timestamp>
> > +
> > + <commit message>
>
> I might not fully understand this but can we not store just the
> commit object if the patch, which would have the bottom/top
> information.
You can't store a commit object in a tree. (Well, with submodules you
can, but said commit object isn't protected from gc and won't be
included when pulling.) The idea with this format is that with the two
trees and the info file, you can recreate the patch's commit -- not
exactly, but close enough as makes no difference.
> > +Simplified log
> > +--------------
> > +
> > +The simplified log looks exactly like the normal, or I{full}, log,
> > +except for the following:
> > +
> > + - Instead of having a tree per patch in the C{patches} subtree, it
> > + has a blob per patch. This blob contains::
> > +
> > + Bottom: <sha1 of patch's bottom tree>
> > + Top: <sha1 of patch's top tree>
> > + Author: <author name and e-mail>
> > + Date: <patch timestamp>
>
> Can we not point to the original commit corresponding to the patch?
> It already has this information.
>
> > +
> > + <commit message>
> > +
> > + ---
> > +
> > + <patch's diff>
>
> What is the patch's diff here? Cannot it be determined from the
> trees?
>
> > +The simplified log contains no information not in the full log; its
> > +purpose is ease of visualization."""
>
> Ah, OK. But I think it would be more useful to see the diff between
> subsequent revisions of a stack rather than the full patch diff.
Have you tried looking at a patch stack log (in gitk, say)? That is,
"stg log -g" in this patch series. It shows you diffs between
subsequent revisions of the simplified log. I'm sure it's far from
perfect, but I think it's actually quite useful.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-17 12:31 ` Karl Hasselström
@ 2008-06-17 12:55 ` Karl Hasselström
2008-06-17 14:11 ` Catalin Marinas
1 sibling, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-17 12:55 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-17 14:31:38 +0200, Karl Hasselström wrote:
> Have you tried looking at a patch stack log (in gitk, say)? That is,
> "stg log -g" in this patch series. It shows you diffs between
> subsequent revisions of the simplified log. I'm sure it's far from
> perfect, but I think it's actually quite useful.
Oh -- just be sure to use a colorized diff (which you get by default
in gitk, but not in git log, which is what stg log without -g ends up
calling). Without colors, a diff of two diffs is too hard to read, at
least for me.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-17 12:31 ` Karl Hasselström
2008-06-17 12:55 ` Karl Hasselström
@ 2008-06-17 14:11 ` Catalin Marinas
2008-06-17 15:32 ` Karl Hasselström
1 sibling, 1 reply; 32+ messages in thread
From: Catalin Marinas @ 2008-06-17 14:11 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
2008/6/17 Karl Hasselström <kha@treskal.com>:
> On 2008-06-17 11:24:53 +0100, Catalin Marinas wrote:
>> 2008/6/12 Karl Hasselström <kha@treskal.com>:
>> > class _Directory(object):
>> > - def __init__(self, needs_current_series = True):
>> > + def __init__(self, needs_current_series = True, log = True):
>>
>> i.e. we make log = False here by default.
>
> I might not have understood precisely what you meant; but I don't
> think API backwards compatibilty should be an issue here? I simply fix
> all callers. If log should default to true or false is immaterial --
> it just means some extra text in one or the other of two equally
> common cases.
Not an issue, I just favour the existing one when the two cases are
almost equal.
>> > --- /dev/null
>> > +++ b/stgit/lib/log.py
>> > @@ -0,0 +1,254 @@
>> > +r"""This module contains functions and classes for manipulating
>>
>> Why does this start with an 'r'? I thought this is for regular
>> expressions.
>
> "r" in front of a string literal means "raw" (or some such). Escape
> sequences aren't recognized inside a raw string -- e.g., r'\n' ==
> '\\n'. They are useful when you have to write strings with embedded
> backslashes, such as regexes -- or this string, which has \n in it.
Thanks, I didn't know this.
>> > +A stack log is a git branch. Each commit contains the complete state
>> > +of the stack at the moment it was written; the most recent commit has
>> > +the most recent state.
>> > +
>> > +For a branch C{I{foo}}, the stack log is stored in C{I{foo}.stgit}.
>>
>> The main question. Is this history preserved after a git-gc?
>
> Yes. It's stored in a regular git branch. (The design is such that it
> should even be possible to pull a stack log from another repository
> and _still_ get everything you need.)
But how are the patches recreated when undoing (the
refs/patches/<branch>/* files)? Using the Bottom/Top tree ids that a
patch had in the past? Are these trees still present after a git-gc?
>> > + - C{patches}: a tree containing one subtree for each patch, named
>> > + after that patch. Each such subtree contains:
>> > +
>> > + - C{a}, C{b}: the patch's I{bottom} and I{top} trees.
>> > +
>> > + - C{info}: a blob containing::
>> > +
>> > + Author: <author name and e-mail>
>> > + Date: <patch timestamp>
>> > +
>> > + <commit message>
>>
>> I might not fully understand this but can we not store just the
>> commit object if the patch, which would have the bottom/top
>> information.
>
> You can't store a commit object in a tree. (Well, with submodules you
> can, but said commit object isn't protected from gc and won't be
> included when pulling.) The idea with this format is that with the two
> trees and the info file, you can recreate the patch's commit -- not
> exactly, but close enough as makes no difference.
What I meant is the SHA1 value of the patch commit instead of
Bottom/Top, Author and Date. The corresponding commit object has all
this information.
>> > +The simplified log contains no information not in the full log; its
>> > +purpose is ease of visualization."""
>>
>> Ah, OK. But I think it would be more useful to see the diff between
>> subsequent revisions of a stack rather than the full patch diff.
>
> Have you tried looking at a patch stack log (in gitk, say)?
I tried "gitk master.stgit" and got scared :-)
> That is, "stg log -g" in this patch series.
This is more readable.
> It shows you diffs between
> subsequent revisions of the simplified log. I'm sure it's far from
> perfect, but I think it's actually quite useful.
It is useful, though it might take a bit of time to get used to it. It
might also be a bit difficult if you want to revert some changes to a
single patch but not do a full stack undo which would affect other
patches.
--
Catalin
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-17 14:11 ` Catalin Marinas
@ 2008-06-17 15:32 ` Karl Hasselström
2008-06-18 13:03 ` Catalin Marinas
2008-07-01 20:13 ` Karl Hasselström
0 siblings, 2 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-06-17 15:32 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-17 15:11:42 +0100, Catalin Marinas wrote:
> 2008/6/17 Karl Hasselström <kha@treskal.com>:
>
> > On 2008-06-17 11:24:53 +0100, Catalin Marinas wrote:
> >
> > > 2008/6/12 Karl Hasselström <kha@treskal.com>:
> > >
> > > > class _Directory(object):
> > > > - def __init__(self, needs_current_series = True):
> > > > + def __init__(self, needs_current_series = True, log = True):
> > >
> > > i.e. we make log = False here by default.
> >
> > I might not have understood precisely what you meant; but I don't
> > think API backwards compatibilty should be an issue here? I simply
> > fix all callers. If log should default to true or false is
> > immaterial -- it just means some extra text in one or the other of
> > two equally common cases.
>
> Not an issue, I just favour the existing one when the two cases are
> almost equal.
Fair enough. I'll change it.
> > > > +A stack log is a git branch. Each commit contains the complete state
> > > > +of the stack at the moment it was written; the most recent commit has
> > > > +the most recent state.
> > > > +
> > > > +For a branch C{I{foo}}, the stack log is stored in C{I{foo}.stgit}.
> > >
> > > The main question. Is this history preserved after a git-gc?
> >
> > Yes. It's stored in a regular git branch. (The design is such that it
> > should even be possible to pull a stack log from another repository
> > and _still_ get everything you need.)
>
> But how are the patches recreated when undoing (the
> refs/patches/<branch>/* files)? Using the Bottom/Top tree ids that a
> patch had in the past? Are these trees still present after a git-gc?
The log actually _contains_ those trees, so there is no problem.
(Relatedly: I've still not written the code that recreates a patch
from the trees and info if its old commit object is no longer present,
but it should be straightforward -- there's even a TODO at the right
place in the code.)
> > > > + - C{patches}: a tree containing one subtree for each patch, named
> > > > + after that patch. Each such subtree contains:
> > > > +
> > > > + - C{a}, C{b}: the patch's I{bottom} and I{top} trees.
> > > > +
> > > > + - C{info}: a blob containing::
> > > > +
> > > > + Author: <author name and e-mail>
> > > > + Date: <patch timestamp>
> > > > +
> > > > + <commit message>
> > >
> > > I might not fully understand this but can we not store just the
> > > commit object if the patch, which would have the bottom/top
> > > information.
> >
> > You can't store a commit object in a tree. (Well, with submodules
> > you can, but said commit object isn't protected from gc and won't
> > be included when pulling.) The idea with this format is that with
> > the two trees and the info file, you can recreate the patch's
> > commit -- not exactly, but close enough as makes no difference.
>
> What I meant is the SHA1 value of the patch commit instead of
> Bottom/Top, Author and Date. The corresponding commit object has all
> this information.
I think this question is settled by my answer above: the log actually
contains the trees in question, which it cannot do with the commit
object.
The applied and unapplied files do contain the sha1s of the patches'
commit objects, since most of the time they will still be there, and
we can just re-use them. But gc and pull don't "see" this kind of
pointer, so we can't rely on it.
> > > > +The simplified log contains no information not in the full log; its
> > > > +purpose is ease of visualization."""
> > >
> > > Ah, OK. But I think it would be more useful to see the diff
> > > between subsequent revisions of a stack rather than the full
> > > patch diff.
> >
> > Have you tried looking at a patch stack log (in gitk, say)?
>
> I tried "gitk master.stgit" and got scared :-)
You want gitk master.stgit^, which is the simplified log. Can you
guess why I added it? :-)
The full log unfortunately has to look like hell, because it needs
both the stack top and the previous log entry as ancestors (since
ancestry is the only way to point to other commit objects that
survives gc and cloning).
> > That is, "stg log -g" in this patch series.
>
> This is more readable.
(This is just gitk <branch>.stgit^.)
> > It shows you diffs between subsequent revisions of the simplified
> > log. I'm sure it's far from perfect, but I think it's actually
> > quite useful.
>
> It is useful, though it might take a bit of time to get used to it.
Yes, much like diffs take some time to get used to if you haven't seen
them before.
If you have ideas for a better way to visualize this, I'm all ears.
> It might also be a bit difficult if you want to revert some changes
> to a single patch but not do a full stack undo which would affect
> other patches.
The reset command can already reset just a single patch and not all of
them.
I agree that a "revert"-style command, which undoes just one update
and not everything that happened since then (for the entire stack or
just a single patch), is an intriguing idea. I haven't thought about
it much, but I'm sure it's doable. (In fact, it should be equivalent
to patch stack merging, which I wrote a post about some time ago if
you recall.)
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-17 15:32 ` Karl Hasselström
@ 2008-06-18 13:03 ` Catalin Marinas
2008-06-18 14:36 ` Karl Hasselström
2008-07-01 20:13 ` Karl Hasselström
1 sibling, 1 reply; 32+ messages in thread
From: Catalin Marinas @ 2008-06-18 13:03 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
2008/6/17 Karl Hasselström <kha@treskal.com>:
> On 2008-06-17 15:11:42 +0100, Catalin Marinas wrote:
>
>> 2008/6/17 Karl Hasselström <kha@treskal.com>:
>>
>> > On 2008-06-17 11:24:53 +0100, Catalin Marinas wrote:
>> > > The main question. Is this history preserved after a git-gc?
>> >
>> > Yes. It's stored in a regular git branch. (The design is such that it
>> > should even be possible to pull a stack log from another repository
>> > and _still_ get everything you need.)
>>
>> But how are the patches recreated when undoing (the
>> refs/patches/<branch>/* files)? Using the Bottom/Top tree ids that a
>> patch had in the past? Are these trees still present after a git-gc?
>
> The log actually _contains_ those trees, so there is no problem.
OK, I begin to understand. It is a generic solution for storing
metadata but IMHO it is too overweight when we only need a list of
applied and unapplied files (we can remove hidden) that could be
easily stored in a commit log. It would be useful to run a quick
benchmark with many (hundreds) of patches and compare it with no
logging variant (or the current patch logging, which isn't as
advanced).
Could we not make this (much) simpler? I.e. <branch>.stgit is a commit
object whose tree is <branch>^{tree} and the message contains the
command followed by list of patches in the form "<commit> <patch>"?
This commit can have two parents - the previous <branch>.stgit and
current <branch> head. All the patches are referred via the <branch>
head or the previous <branch> heads if unapplied (assuming that when a
patch is created it is first applied and then popped, maybe this needs
a bit of thinking). This way, a diff between subsequent <branch>.stgit
commits would show the tree changes. The 'stg log' command could be
made to show differences in the commit messages.
>> > It shows you diffs between subsequent revisions of the simplified
>> > log. I'm sure it's far from perfect, but I think it's actually
>> > quite useful.
>>
>> It is useful, though it might take a bit of time to get used to it.
>
> Yes, much like diffs take some time to get used to if you haven't seen
> them before.
>
> If you have ideas for a better way to visualize this, I'm all ears.
The diff of diffs is generally useful but for 'refresh' logs, you can
show the diff between the old top tree and the new one (the current
patch log implementation does something like this but it doesn't show
anything for commands like 'push' where a diff of diffs would be more
appropriate).
--
Catalin
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-18 13:03 ` Catalin Marinas
@ 2008-06-18 14:36 ` Karl Hasselström
2008-06-18 16:16 ` Catalin Marinas
0 siblings, 1 reply; 32+ messages in thread
From: Karl Hasselström @ 2008-06-18 14:36 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-18 14:03:51 +0100, Catalin Marinas wrote:
> 2008/6/17 Karl Hasselström <kha@treskal.com>:
>
> > The log actually _contains_ those trees, so there is no problem.
>
> OK, I begin to understand. It is a generic solution for storing
> metadata
I don't know about "generic" -- it's made specifically for storing an
StGit stack. You could certainly store just about anything in a git
tree, but that's also true for a git commit message ...
> but IMHO it is too overweight when we only need a list of applied
> and unapplied files (we can remove hidden) that could be easily
> stored in a commit log.
Do you mean remove hidden patches from StGit altogether, or just not
store them in the log?
> It would be useful to run a quick benchmark with many (hundreds) of
> patches and compare it with no logging variant (or the current patch
> logging, which isn't as advanced).
Will do, as soon as I've done some basic optimization. (A simple
"dirty" flag on each patch will enable us to only write out the log
for the patches that have actually changed, and reuse the rest from
the previous log entry.)
> Could we not make this (much) simpler? I.e. <branch>.stgit is a
> commit object whose tree is <branch>^{tree} and the message contains
> the command followed by list of patches in the form "<commit>
> <patch>"? This commit can have two parents - the previous
> <branch>.stgit and current <branch> head. All the patches are
> referred via the <branch> head or the previous <branch> heads if
> unapplied (assuming that when a patch is created it is first applied
> and then popped, maybe this needs a bit of thinking).
I considered a scheme much like this, but besides the problem with
(the very few) patches that are created unapplied, it fails badly in a
very important corner case: when you start logging in a pre-existing
stack. A similar failure will occur if we ever build some sort of log
pruning (without which the log will grow without limit).
I suppose it would be possible to special-case the very first log
entry: let it have all patches as ancestors. But I don't really see
this format as being simpler than the one I use now: all you've done,
basically, is cram all the data into the commit message instead of
storing it in the tree. (Benchmarking could certainly prove your
design superior to mine, however.)
> This way, a diff between subsequent <branch>.stgit commits would
> show the tree changes. The 'stg log' command could be made to show
> differences in the commit messages.
Tree changes are only really useful for refresh. (More on this below.)
> > If you have ideas for a better way to visualize this, I'm all
> > ears.
>
> The diff of diffs is generally useful but for 'refresh' logs, you
> can show the diff between the old top tree and the new one (the
> current patch log implementation does something like this but it
> doesn't show anything for commands like 'push' where a diff of diffs
> would be more appropriate).
The diff-of-diffs view is actually very useful for refresh as well.
The diff of old and new top tree is useful only for refresh (but it
_is_ useful -- I'm not arguing against it).
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-18 14:36 ` Karl Hasselström
@ 2008-06-18 16:16 ` Catalin Marinas
2008-06-18 17:32 ` Karl Hasselström
0 siblings, 1 reply; 32+ messages in thread
From: Catalin Marinas @ 2008-06-18 16:16 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
2008/6/18 Karl Hasselström <kha@treskal.com>:
> On 2008-06-18 14:03:51 +0100, Catalin Marinas wrote:
>
>> 2008/6/17 Karl Hasselström <kha@treskal.com>:
>>
>> > The log actually _contains_ those trees, so there is no problem.
>>
>> OK, I begin to understand. It is a generic solution for storing
>> metadata
>
> I don't know about "generic" -- it's made specifically for storing an
> StGit stack. You could certainly store just about anything in a git
> tree, but that's also true for a git commit message ...
By generic I meant that it is easily extensible to store other blobs
of whatever you need. As you say, a commit message could be extensible
as well.
>> but IMHO it is too overweight when we only need a list of applied
>> and unapplied files (we can remove hidden) that could be easily
>> stored in a commit log.
>
> Do you mean remove hidden patches from StGit altogether, or just not
> store them in the log?
Remove them altogether. Is there anyone using them apart from me? I
could create a "rotten" branch and pick those patches with
--unapplied.
>> It would be useful to run a quick benchmark with many (hundreds) of
>> patches and compare it with no logging variant (or the current patch
>> logging, which isn't as advanced).
>
> Will do, as soon as I've done some basic optimization. (A simple
> "dirty" flag on each patch will enable us to only write out the log
> for the patches that have actually changed, and reuse the rest from
> the previous log entry.)
It gets too complicated, really. A single commit with the proper
parents could do the job. We could also easily use the commit message
directly for metadata instead of .git/patches/<branch>/.
>> Could we not make this (much) simpler? I.e. <branch>.stgit is a
>> commit object whose tree is <branch>^{tree} and the message contains
>> the command followed by list of patches in the form "<commit>
>> <patch>"? This commit can have two parents - the previous
>> <branch>.stgit and current <branch> head. All the patches are
>> referred via the <branch> head or the previous <branch> heads if
>> unapplied (assuming that when a patch is created it is first applied
>> and then popped, maybe this needs a bit of thinking).
>
> I considered a scheme much like this, but besides the problem with
> (the very few) patches that are created unapplied, it fails badly in a
> very important corner case: when you start logging in a pre-existing
> stack. A similar failure will occur if we ever build some sort of log
> pruning (without which the log will grow without limit).
>
> I suppose it would be possible to special-case the very first log
> entry: let it have all patches as ancestors. But I don't really see
> this format as being simpler than the one I use now: all you've done,
> basically, is cram all the data into the commit message instead of
> storing it in the tree. (Benchmarking could certainly prove your
> design superior to mine, however.)
The time to create that tree and blobs worries me a bit, plus the (in
my view) complicated structure.
Making the first log entry special gets difficult with log pruning
(unless you prune the whole log rather than entries older than a
chosen time or number) since you might have to re-create all the
chained log entries as the first log's sha1 will probably change.
The applied patches are chained automatically via HEAD. For unapplied
patches, we could add the correponding commits as parents of the
logging commit (starting with the third parent as the first two are
used for log chaining and applied patches). Do we hit any OS limit
with the number of arguments?
Since we only need the unapplied commits tracking for gc and pull
reasons, we could only create that commit that references them when we
prune the stack log and link it from the top one (but it won't be used
by stgit).
--
Catalin
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-18 16:16 ` Catalin Marinas
@ 2008-06-18 17:32 ` Karl Hasselström
2008-06-19 9:24 ` Catalin Marinas
0 siblings, 1 reply; 32+ messages in thread
From: Karl Hasselström @ 2008-06-18 17:32 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-18 17:16:10 +0100, Catalin Marinas wrote:
> 2008/6/18 Karl Hasselström <kha@treskal.com>:
>
> > Do you mean remove hidden patches from StGit altogether, or just
> > not store them in the log?
>
> Remove them altogether. Is there anyone using them apart from me? I
> could create a "rotten" branch and pick those patches with
> --unapplied.
I've never heard about anyone else using them. So fine by me!
> It gets too complicated, really. A single commit with the proper
> parents could do the job.
Yes. The problem is that you need quite a lot of parents. Every system
I could come up with that got all the corner cases right was even more
complex (in my eyes) than the one I ended up implementing.
> We could also easily use the commit message directly for metadata
> instead of .git/patches/<branch>/.
Yes. This is true for any log scheme, though. (But I agree -- longer
term, it would be very nice to have _only_ the log.)
> The time to create that tree and blobs worries me a bit, plus the
> (in my view) complicated structure.
You're right to worry. My log format makes things feel slightly slower
when logging a 20-30 deep stack. If I can't make it faster, it's not
viable. But I'm pretty sure I can -- it should be simple to reuse all
the trees and blobs for the patches that weren't touched. (And
operations that touch lots of patches, like rebase, have to write a
lot of git objects anyway as part of their operation, so the relative
slowdown should not be large.)
Anyway, benchmarking is the way to go here. Talk will only get us that
far.
> Making the first log entry special gets difficult with log pruning
> (unless you prune the whole log rather than entries older than a
> chosen time or number) since you might have to re-create all the
> chained log entries as the first log's sha1 will probably change.
You have to re-create all the commits anyway, since they all are
immutable, and all have a back pointer.
> The applied patches are chained automatically via HEAD. For
> unapplied patches, we could add the correponding commits as parents
> of the logging commit (starting with the third parent as the first
> two are used for log chaining and applied patches). Do we hit any OS
> limit with the number of arguments?
Not until long after we hit git limits to the number of parents of a
commit. I believe the octopus merge refuses to create merges with more
than about 25 parents, and we probably shouldn't do more than that
either. We'll have to do a tree of octopuses.
This is why my log format looks the way it does. :-)
> Since we only need the unapplied commits tracking for gc and pull
> reasons, we could only create that commit that references them when
> we prune the stack log and link it from the top one (but it won't be
> used by stgit).
Yes, we need to create an "unapplied octopus" if and only if we have
unapplied patches that we can't prove are reachable from the stack top
or the branch head (we have to save both, in case the user has done
something such as git-committing on topp of the stack and caused head
!= top). Which is for the first log entry, and in situations such as
"stg pick --unapplied", but not for "stg pop" and the like.
I do agree that we shouldn't try to use the octopuses to get hold of
the commits, though -- just to keep them reachable. We save the sha1
along with the patch name elsewhere in a more convenient form. (My
proposed format does precisely this.)
So. If I got it right, your proposal is:
* Tree: just take the HEAD tree.
* Commit message: list the applied and unapplied patches with their
commit sha1s.
* Parents: the previous log entry; branch head; something that
(recusively) points to all unapplied commits, if necessary.
I'd add to that:
* The stack top must be a parent too if head != top.
* The commit message must include a version number, and the branch
head sha1.
* I'm pretty sure we want the kind of "simplified" log I have in my
proposal. The full log in your proposal is going to look every bit
as ugly as the one in mine.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-18 17:32 ` Karl Hasselström
@ 2008-06-19 9:24 ` Catalin Marinas
2008-06-19 10:07 ` Karl Hasselström
0 siblings, 1 reply; 32+ messages in thread
From: Catalin Marinas @ 2008-06-19 9:24 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
2008/6/18 Karl Hasselström <kha@treskal.com>:
> On 2008-06-18 17:16:10 +0100, Catalin Marinas wrote:
>> We could also easily use the commit message directly for metadata
>> instead of .git/patches/<branch>/.
>
> Yes. This is true for any log scheme, though. (But I agree -- longer
> term, it would be very nice to have _only_ the log.)
It would make collaboration using stgit stacks easier. I can start
looking at this once we agree on the log structure.
>> The time to create that tree and blobs worries me a bit, plus the
>> (in my view) complicated structure.
>
> You're right to worry. My log format makes things feel slightly slower
> when logging a 20-30 deep stack. If I can't make it faster, it's not
> viable. But I'm pretty sure I can -- it should be simple to reuse all
> the trees and blobs for the patches that weren't touched. (And
> operations that touch lots of patches, like rebase, have to write a
> lot of git objects anyway as part of their operation, so the relative
> slowdown should not be large.)
>
> Anyway, benchmarking is the way to go here. Talk will only get us that
> far.
In the past, I ran some tests because people complained that stgit was
slow compared to quilt:
http://article.gmane.org/gmane.comp.version-control.git/9670
After profiling (the stg-prof file), as expected, most of the time was
spent in external Git calls which I tried to keep to a minimum. With
your approach, you probably add at least 4-5 calls to Git (just a
guess, I haven't counted) and, with a big repository, it will be
visible (I have about 15 branches on my Linux kernel clone going back
to the 2.6.12 kernel and the .git size is over 1/2GB after git-gc).
>> Making the first log entry special gets difficult with log pruning
>> (unless you prune the whole log rather than entries older than a
>> chosen time or number) since you might have to re-create all the
>> chained log entries as the first log's sha1 will probably change.
>
> You have to re-create all the commits anyway, since they all are
> immutable, and all have a back pointer.
Ah, OK. So, at least initially, we should only support the full log pruning.
>> The applied patches are chained automatically via HEAD. For
>> unapplied patches, we could add the correponding commits as parents
>> of the logging commit (starting with the third parent as the first
>> two are used for log chaining and applied patches). Do we hit any OS
>> limit with the number of arguments?
>
> Not until long after we hit git limits to the number of parents of a
> commit. I believe the octopus merge refuses to create merges with more
> than about 25 parents, and we probably shouldn't do more than that
> either. We'll have to do a tree of octopuses.
For the first log only, we could chain the unapplied patches using
commits with 2 parents. We just need to warn people not to stare at
the <branch>.stgit directly :-)
>> Since we only need the unapplied commits tracking for gc and pull
>> reasons, we could only create that commit that references them when
>> we prune the stack log and link it from the top one (but it won't be
>> used by stgit).
>
> Yes, we need to create an "unapplied octopus" if and only if we have
> unapplied patches that we can't prove are reachable from the stack top
> or the branch head (we have to save both, in case the user has done
> something such as git-committing on topp of the stack and caused head
> != top). Which is for the first log entry, and in situations such as
> "stg pick --unapplied", but not for "stg pop" and the like.
Yes, but just chain unapplied commits rather than using octopus (you
could try to the -mm series with 500+ patches and see what happens).
> I do agree that we shouldn't try to use the octopuses to get hold of
> the commits, though -- just to keep them reachable. We save the sha1
> along with the patch name elsewhere in a more convenient form. (My
> proposed format does precisely this.)
Yes, but you still have to refer the tree objects corresponding to a
patch and extra work to generate the simplified log.
> So. If I got it right, your proposal is:
>
> * Tree: just take the HEAD tree.
Yes.
> * Commit message: list the applied and unapplied patches with their
> commit sha1s.
We don't even need to differentiate between applied and unapplied in
the commit message as long as they are listed in order since one of
the parents would represent the boundary between them. Since lib.stack
is pretty well structured, we could later modify PatchOrder to use the
log.
> * Parents: the previous log entry; branch head; something that
> (recusively) points to all unapplied commits, if necessary.
As you pointed below, "branch head" should probably be the "stack
top". We don't need to track the "branch head" if different, just need
to fix up the error and add the patches to the stack. And, anyway, if
one modifies the HEAD using git directly, the log will still point to
the top of the stack.
The third head would only be needed for the first log entry or when we
use pick --unapplied (in the latter, it only points to the unapplied
commit). The third head shouldn't be used at all in the log, just
created when needed.
> I'd add to that:
>
> * The stack top must be a parent too if head != top.
See above, branch head would actually be the stack top.
> * The commit message must include a version number, and the branch
> head sha1.
OK with the version number but the branch head (or stack top) is one
of the parents already.
> * I'm pretty sure we want the kind of "simplified" log I have in my
> proposal. The full log in your proposal is going to look every bit
> as ugly as the one in mine.
I agree it will look ugly but the simplified log adds an extra
overhead on any stgit action. If we don't use stg log -g, a text only
log command could show the diff. We can add it afterwards though if it
is fast enough.
--
Catalin
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-19 9:24 ` Catalin Marinas
@ 2008-06-19 10:07 ` Karl Hasselström
2008-06-20 9:14 ` Catalin Marinas
0 siblings, 1 reply; 32+ messages in thread
From: Karl Hasselström @ 2008-06-19 10:07 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-19 10:24:28 +0100, Catalin Marinas wrote:
> 2008/6/18 Karl Hasselström <kha@treskal.com>:
>
> > On 2008-06-18 17:16:10 +0100, Catalin Marinas wrote:
> >
> > > We could also easily use the commit message directly for
> > > metadata instead of .git/patches/<branch>/.
> >
> > Yes. This is true for any log scheme, though. (But I agree --
> > longer term, it would be very nice to have _only_ the log.)
>
> It would make collaboration using stgit stacks easier. I can start
> looking at this once we agree on the log structure.
Yes. Collaboration was one of the things I was thinking of when
starting all this. (If not for that, there would be little reason to
try to store the entire log under a _single_ ref.)
> After profiling (the stg-prof file), as expected, most of the time
> was spent in external Git calls which I tried to keep to a minimum.
> With your approach, you probably add at least 4-5 calls to Git (just
> a guess, I haven't counted) and, with a big repository, it will be
> visible (I have about 15 branches on my Linux kernel clone going
> back to the 2.6.12 kernel and the .git size is over 1/2GB after
> git-gc).
The extra git calls take constant time and don't depend on the size of
the repository. But they _are_ a problem. (Which could be solved by
making a new git command specifically for users like StGit that want
to make a small bunch of simple object read/write commands.)
> > > Making the first log entry special gets difficult with log
> > > pruning (unless you prune the whole log rather than entries
> > > older than a chosen time or number) since you might have to
> > > re-create all the chained log entries as the first log's sha1
> > > will probably change.
> >
> > You have to re-create all the commits anyway, since they all are
> > immutable, and all have a back pointer.
>
> Ah, OK. So, at least initially, we should only support the full log
> pruning.
How do you imagine we'd do anything except a "full" pruning? There are
grafts and shallow clones, I guess, but both of them have drawbacks.
> > > The applied patches are chained automatically via HEAD. For
> > > unapplied patches, we could add the correponding commits as
> > > parents of the logging commit (starting with the third parent as
> > > the first two are used for log chaining and applied patches). Do
> > > we hit any OS limit with the number of arguments?
> >
> > Not until long after we hit git limits to the number of parents of
> > a commit. I believe the octopus merge refuses to create merges
> > with more than about 25 parents, and we probably shouldn't do more
> > than that either. We'll have to do a tree of octopuses.
>
> For the first log only, we could chain the unapplied patches using
> commits with 2 parents. We just need to warn people not to stare at
> the <branch>.stgit directly :-)
We could make chains (or trees) of 16-parent commits -- that'd speed
it up by a factor of 15. :-)
> > > Since we only need the unapplied commits tracking for gc and
> > > pull reasons, we could only create that commit that references
> > > them when we prune the stack log and link it from the top one
> > > (but it won't be used by stgit).
> >
> > Yes, we need to create an "unapplied octopus" if and only if we
> > have unapplied patches that we can't prove are reachable from the
> > stack top or the branch head (we have to save both, in case the
> > user has done something such as git-committing on topp of the
> > stack and caused head != top). Which is for the first log entry,
> > and in situations such as "stg pick --unapplied", but not for "stg
> > pop" and the like.
>
> Yes, but just chain unapplied commits rather than using octopus (you
> could try to the -mm series with 500+ patches and see what happens).
If there are a lot of unapplied patches, you don't want to throw that
factor of 15 away I think.
> > I do agree that we shouldn't try to use the octopuses to get hold
> > of the commits, though -- just to keep them reachable. We save the
> > sha1 along with the patch name elsewhere in a more convenient
> > form. (My proposed format does precisely this.)
>
> Yes, but you still have to refer the tree objects corresponding to a
> patch and extra work to generate the simplified log.
True. But only for patches that have changed. Which will rarely be a
lot of them, as I've said before.
> > * Commit message: list the applied and unapplied patches with
> > their commit sha1s.
>
> We don't even need to differentiate between applied and unapplied in
> the commit message as long as they are listed in order since one of
> the parents would represent the boundary between them.
Not if top != head (which I'm convinced we want to allow the design to
handle). See below.
> Since lib.stack is pretty well structured, we could later modify
> PatchOrder to use the log.
Certainly. This has been one of my evil master plans all along. ;-)
Though it would have to be restructured a bit since we can't just
write to a part of the log -- we have to write a complete log entry
all at once.
> > * Parents: the previous log entry; branch head; something that
> > (recusively) points to all unapplied commits, if necessary.
>
> As you pointed below, "branch head" should probably be the "stack
> top". We don't need to track the "branch head" if different, just
> need to fix up the error and add the patches to the stack. And,
> anyway, if one modifies the HEAD using git directly, the log will
> still point to the top of the stack.
If we ever want to be able to undo "stg repair", we have to be able to
represent an inconsistent state where head != top. And it's actually
not difficult -- the patch series I posted recently does this. All it
takes is that we save both the branch head and the stack top, instead
of assuming that they are the same.
> The third head would only be needed for the first log entry or when
> we use pick --unapplied (in the latter, it only points to the
> unapplied commit).
Actually, except for the previous log entry, all the parents are just
there for gc's benefit. So we could just put all of them in the same
bucket -- branch head, stack top, and unapplied patches.
( By "bucket" I mean something like: if there are just a few of them,
have them as direct parents of the log commit; otherwise, refer to
them using a tree of octopuses. But in any case, just treat them as
a set of sha1s that we need to have as ancestors but don't otherwise
care about. )
> The third head shouldn't be used at all in the log, just created
> when needed.
Yes. And as I said, we could do the same for the head and top commits.
> > * I'm pretty sure we want the kind of "simplified" log I have in
> > my proposal. The full log in your proposal is going to look
> > every bit as ugly as the one in mine.
>
> I agree it will look ugly but the simplified log adds an extra
> overhead on any stgit action. If we don't use stg log -g, a text
> only log command could show the diff. We can add it afterwards
> though if it is fast enough.
I'd actually say the opposite: until we have a good visualizer that
doesn't need the simplified log, we need to have the simplified log.
If I actually have to look at the diffs in the log, I find gitk
indispensible.
Plus, once we try to support merging logs, gitk on a simplified log
will be truly indispensible.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-19 10:07 ` Karl Hasselström
@ 2008-06-20 9:14 ` Catalin Marinas
2008-06-23 12:36 ` Karl Hasselström
0 siblings, 1 reply; 32+ messages in thread
From: Catalin Marinas @ 2008-06-20 9:14 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
2008/6/19 Karl Hasselström <kha@treskal.com>:
> On 2008-06-19 10:24:28 +0100, Catalin Marinas wrote:
>
>> 2008/6/18 Karl Hasselström <kha@treskal.com>:
>>
>> > On 2008-06-18 17:16:10 +0100, Catalin Marinas wrote:
>> > > Making the first log entry special gets difficult with log
>> > > pruning (unless you prune the whole log rather than entries
>> > > older than a chosen time or number) since you might have to
>> > > re-create all the chained log entries as the first log's sha1
>> > > will probably change.
>> >
>> > You have to re-create all the commits anyway, since they all are
>> > immutable, and all have a back pointer.
>>
>> Ah, OK. So, at least initially, we should only support the full log
>> pruning.
>
> How do you imagine we'd do anything except a "full" pruning? There are
> grafts and shallow clones, I guess, but both of them have drawbacks.
Recreate the log but we I wouldn't spend time on this.
>> > > The applied patches are chained automatically via HEAD. For
>> > > unapplied patches, we could add the correponding commits as
>> > > parents of the logging commit (starting with the third parent as
>> > > the first two are used for log chaining and applied patches). Do
>> > > we hit any OS limit with the number of arguments?
>> >
>> > Not until long after we hit git limits to the number of parents of
>> > a commit. I believe the octopus merge refuses to create merges
>> > with more than about 25 parents, and we probably shouldn't do more
>> > than that either. We'll have to do a tree of octopuses.
>>
>> For the first log only, we could chain the unapplied patches using
>> commits with 2 parents. We just need to warn people not to stare at
>> the <branch>.stgit directly :-)
>
> We could make chains (or trees) of 16-parent commits -- that'd speed
> it up by a factor of 15. :-)
Yes, good idea.
>> > * Parents: the previous log entry; branch head; something that
>> > (recusively) points to all unapplied commits, if necessary.
>>
>> As you pointed below, "branch head" should probably be the "stack
>> top". We don't need to track the "branch head" if different, just
>> need to fix up the error and add the patches to the stack. And,
>> anyway, if one modifies the HEAD using git directly, the log will
>> still point to the top of the stack.
>
> If we ever want to be able to undo "stg repair", we have to be able to
> represent an inconsistent state where head != top.
I wouldn't bother with this feature. Why would one want to break the
stack again after repairing? If they merge patches and git commits,
they either repair the stack or commit all the patches and continue
with using Git only.
>> The third head would only be needed for the first log entry or when
>> we use pick --unapplied (in the latter, it only points to the
>> unapplied commit).
>
> Actually, except for the previous log entry, all the parents are just
> there for gc's benefit. So we could just put all of them in the same
> bucket -- branch head, stack top, and unapplied patches.
>
> ( By "bucket" I mean something like: if there are just a few of them,
> have them as direct parents of the log commit; otherwise, refer to
> them using a tree of octopuses. But in any case, just treat them as
> a set of sha1s that we need to have as ancestors but don't otherwise
> care about. )
Yes.
>> > * I'm pretty sure we want the kind of "simplified" log I have in
>> > my proposal. The full log in your proposal is going to look
>> > every bit as ugly as the one in mine.
>>
>> I agree it will look ugly but the simplified log adds an extra
>> overhead on any stgit action. If we don't use stg log -g, a text
>> only log command could show the diff. We can add it afterwards
>> though if it is fast enough.
>
> I'd actually say the opposite: until we have a good visualizer that
> doesn't need the simplified log, we need to have the simplified log.
> If I actually have to look at the diffs in the log, I find gitk
> indispensible.
And what would the simplified log contain if we decide to go with a
new scheme? In your proposal, it points to the tree of main log and
you get the diff of diffs (which also means that the diffs must be
generated for every modification of a patch). Would this be the same?
Again, I worry a bit about the overhead to generate the patch diff for
every push (with refresh I'm OK). It can be optimised as in the stable
branch where we try git-apply followed by a three-way merge (which,
BTW, I'd like added before 0.15). If git-apply succeeds, there is no
need to re-generate the diff.
--
Catalin
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-20 9:14 ` Catalin Marinas
@ 2008-06-23 12:36 ` Karl Hasselström
2008-07-12 10:09 ` Catalin Marinas
0 siblings, 1 reply; 32+ messages in thread
From: Karl Hasselström @ 2008-06-23 12:36 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-20 10:14:29 +0100, Catalin Marinas wrote:
> 2008/6/19 Karl Hasselström <kha@treskal.com>:
>
> > If we ever want to be able to undo "stg repair", we have to be
> > able to represent an inconsistent state where head != top.
>
> I wouldn't bother with this feature. Why would one want to break the
> stack again after repairing? If they merge patches and git commits,
> they either repair the stack or commit all the patches and continue
> with using Git only.
Sometimes, stg repair isn't what you want. Say e.g. that you've done
git merge. stg repair will in this case stop reading your history once
it sees the merge commit, and consider all your patches unapplied. But
what you really want is to undo the git merge. Or that you've done git
rebase -- in that case, stg repair will realize that your patches are
unapplied, which is correct but probably not what you want.
For those of us who know what stg repair does, realizing when not to
run it and what to do instead is easy. But for very little extra
effort in the stack log, we can make the generic "stg undo" able to
fix these mistakes automatically. _And_ give the user assurance that
whatever she does with stg reset and stg undo, it can be undone.
> > I'd actually say the opposite: until we have a good visualizer
> > that doesn't need the simplified log, we need to have the
> > simplified log. If I actually have to look at the diffs in the
> > log, I find gitk indispensible.
>
> And what would the simplified log contain if we decide to go with a
> new scheme? In your proposal, it points to the tree of main log and
> you get the diff of diffs (which also means that the diffs must be
> generated for every modification of a patch). Would this be the
> same? Again, I worry a bit about the overhead to generate the patch
> diff for every push (with refresh I'm OK). It can be optimised as in
> the stable branch where we try git-apply followed by a three-way
> merge (which, BTW, I'd like added before 0.15). If git-apply
> succeeds, there is no need to re-generate the diff.
Yes, I was imagining a simplified log precisely like the one I've
currently implemented (a tree with one blob per patch, which contains
the message, the diff, and some other odds and ends such as the
author). Two optimizations would hopefully make it fast:
1. If the patch's sha1 hasn't changed, we don't have to regenerate
the diff.
2. If the patch's sha1 has changed, but git apply was sufficient
during the merge stage, we can just reuse that patch. We do have
to write it to a blob, but we have already generated the diff and
don't need to do so again. (I've shamelessly stolen your idea
here.)
In most cases, (1) would make sure that only a small handful of
patches would need to be considered. In the cases where a lot of
patches are touched, such as rebase, (2) would provide a good speedup
(except for the cases where we had to call merge-recursive, and those
are slow anyway).
( By the way: Yes, I agree that we want to try git apply before
merge-recursive. It's on my TODO list. )
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-17 15:32 ` Karl Hasselström
2008-06-18 13:03 ` Catalin Marinas
@ 2008-07-01 20:13 ` Karl Hasselström
2008-07-03 22:05 ` Catalin Marinas
1 sibling, 1 reply; 32+ messages in thread
From: Karl Hasselström @ 2008-07-01 20:13 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-17 17:32:47 +0200, Karl Hasselström wrote:
> On 2008-06-17 15:11:42 +0100, Catalin Marinas wrote:
>
> > 2008/6/17 Karl Hasselström <kha@treskal.com>:
> >
> > > On 2008-06-17 11:24:53 +0100, Catalin Marinas wrote:
> > >
> > > > 2008/6/12 Karl Hasselström <kha@treskal.com>:
> > > >
> > > > > class _Directory(object):
> > > > > - def __init__(self, needs_current_series = True):
> > > > > + def __init__(self, needs_current_series = True, log = True):
> > > >
> > > > i.e. we make log = False here by default.
> > >
> > > I might not have understood precisely what you meant; but I
> > > don't think API backwards compatibilty should be an issue here?
> > > I simply fix all callers. If log should default to true or false
> > > is immaterial -- it just means some extra text in one or the
> > > other of two equally common cases.
> >
> > Not an issue, I just favour the existing one when the two cases
> > are almost equal.
>
> Fair enough. I'll change it.
I had an even better idea: no default value. Every caller gets to say
either log = True or log = False, which makes it immediately obvious
to the reader. (That is, every caller still using the old
infrastructure; with the new infrastructure, we log if and only if a
transaction is run.)
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-07-01 20:13 ` Karl Hasselström
@ 2008-07-03 22:05 ` Catalin Marinas
0 siblings, 0 replies; 32+ messages in thread
From: Catalin Marinas @ 2008-07-03 22:05 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
2008/7/1 Karl Hasselström <kha@treskal.com>:
> On 2008-06-17 17:32:47 +0200, Karl Hasselström wrote:
>
>> On 2008-06-17 15:11:42 +0100, Catalin Marinas wrote:
>>
>> > 2008/6/17 Karl Hasselström <kha@treskal.com>:
>> >
>> > > On 2008-06-17 11:24:53 +0100, Catalin Marinas wrote:
>> > >
>> > > > 2008/6/12 Karl Hasselström <kha@treskal.com>:
>> > > >
>> > > > > class _Directory(object):
>> > > > > - def __init__(self, needs_current_series = True):
>> > > > > + def __init__(self, needs_current_series = True, log = True):
>> > > >
>> > > > i.e. we make log = False here by default.
>> > >
>> > > I might not have understood precisely what you meant; but I
>> > > don't think API backwards compatibilty should be an issue here?
>> > > I simply fix all callers. If log should default to true or false
>> > > is immaterial -- it just means some extra text in one or the
>> > > other of two equally common cases.
>> >
>> > Not an issue, I just favour the existing one when the two cases
>> > are almost equal.
>>
>> Fair enough. I'll change it.
>
> I had an even better idea: no default value. Every caller gets to say
> either log = True or log = False, which makes it immediately obvious
> to the reader. (That is, every caller still using the old
> infrastructure; with the new infrastructure, we log if and only if a
> transaction is run.)
Fair enough.
--
Catalin
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-06-23 12:36 ` Karl Hasselström
@ 2008-07-12 10:09 ` Catalin Marinas
2008-07-14 6:32 ` Karl Hasselström
0 siblings, 1 reply; 32+ messages in thread
From: Catalin Marinas @ 2008-07-12 10:09 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
2008/6/23 Karl Hasselström <kha@treskal.com>:
> On 2008-06-20 10:14:29 +0100, Catalin Marinas wrote:
>
>> 2008/6/19 Karl Hasselström <kha@treskal.com>:
>> > I'd actually say the opposite: until we have a good visualizer
>> > that doesn't need the simplified log, we need to have the
>> > simplified log. If I actually have to look at the diffs in the
>> > log, I find gitk indispensible.
>>
>> And what would the simplified log contain if we decide to go with a
>> new scheme? In your proposal, it points to the tree of main log and
>> you get the diff of diffs (which also means that the diffs must be
>> generated for every modification of a patch). Would this be the
>> same? Again, I worry a bit about the overhead to generate the patch
>> diff for every push (with refresh I'm OK). It can be optimised as in
>> the stable branch where we try git-apply followed by a three-way
>> merge (which, BTW, I'd like added before 0.15). If git-apply
>> succeeds, there is no need to re-generate the diff.
>
> Yes, I was imagining a simplified log precisely like the one I've
> currently implemented (a tree with one blob per patch, which contains
> the message, the diff, and some other odds and ends such as the
> author).
At a first look, I'm OK with a simplified log.
> Two optimizations would hopefully make it fast:
>
> 1. If the patch's sha1 hasn't changed, we don't have to regenerate
> the diff.
>
> 2. If the patch's sha1 has changed, but git apply was sufficient
> during the merge stage, we can just reuse that patch. We do have
> to write it to a blob, but we have already generated the diff and
> don't need to do so again. (I've shamelessly stolen your idea
> here.)
It can be optimised a bit more to actually apply the diff in the blob
directly rather than the current way of generating the diff (since we
don't store the diff).
> In most cases, (1) would make sure that only a small handful of
> patches would need to be considered. In the cases where a lot of
> patches are touched, such as rebase, (2) would provide a good speedup
> (except for the cases where we had to call merge-recursive, and those
> are slow anyway).
I think it should work. Rebase is indeed my worry but it might be even
faster for most of the patches to apply the blob than computing the
diff. In my experience with the Linux kernel, full merge is rarely
needed.
--
Catalin
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [StGit PATCH 03/14] Write to a stack log when stack is modified
2008-07-12 10:09 ` Catalin Marinas
@ 2008-07-14 6:32 ` Karl Hasselström
0 siblings, 0 replies; 32+ messages in thread
From: Karl Hasselström @ 2008-07-14 6:32 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-07-12 11:09:18 +0100, Catalin Marinas wrote:
> 2008/6/23 Karl Hasselström <kha@treskal.com>:
>
> > Two optimizations would hopefully make it fast:
> >
> > 1. If the patch's sha1 hasn't changed, we don't have to
> > regenerate the diff.
> >
> > 2. If the patch's sha1 has changed, but git apply was sufficient
> > during the merge stage, we can just reuse that patch. We do
> > have to write it to a blob, but we have already generated the
> > diff and don't need to do so again. (I've shamelessly stolen
> > your idea here.)
>
> It can be optimised a bit more to actually apply the diff in the
> blob directly rather than the current way of generating the diff
> (since we don't store the diff).
Ah. Yes, that might be faster. While git is supposedly very fast in
diffing two trees, it should be even faster in just reading a blob.
> > In most cases, (1) would make sure that only a small handful of
> > patches would need to be considered. In the cases where a lot of
> > patches are touched, such as rebase, (2) would provide a good
> > speedup (except for the cases where we had to call
> > merge-recursive, and those are slow anyway).
>
> I think it should work. Rebase is indeed my worry but it might be
> even faster for most of the patches to apply the blob than computing
> the diff. In my experience with the Linux kernel, full merge is
> rarely needed.
Yes. Now all we need is a good benchmarking system so that we know
we're heading in the right direction. I need to fix up and post that
script I used.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2008-07-14 6:34 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-12 5:34 [StGit PATCH 00/14] Undo series Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 01/14] Fix typo Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 02/14] Library functions for tree and blob manipulation Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 03/14] Write to a stack log when stack is modified Karl Hasselström
2008-06-17 10:24 ` Catalin Marinas
2008-06-17 12:31 ` Karl Hasselström
2008-06-17 12:55 ` Karl Hasselström
2008-06-17 14:11 ` Catalin Marinas
2008-06-17 15:32 ` Karl Hasselström
2008-06-18 13:03 ` Catalin Marinas
2008-06-18 14:36 ` Karl Hasselström
2008-06-18 16:16 ` Catalin Marinas
2008-06-18 17:32 ` Karl Hasselström
2008-06-19 9:24 ` Catalin Marinas
2008-06-19 10:07 ` Karl Hasselström
2008-06-20 9:14 ` Catalin Marinas
2008-06-23 12:36 ` Karl Hasselström
2008-07-12 10:09 ` Catalin Marinas
2008-07-14 6:32 ` Karl Hasselström
2008-07-01 20:13 ` Karl Hasselström
2008-07-03 22:05 ` Catalin Marinas
2008-06-12 5:34 ` [StGit PATCH 04/14] Add utility function for reordering patches Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 05/14] New command: stg reset Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 06/14] Log conflicts separately Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 07/14] Log conflicts separately for all commands Karl Hasselström
2008-06-12 5:34 ` [StGit PATCH 08/14] Add a --hard flag to stg reset Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 09/14] Don't write a log entry if there were no changes Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 10/14] Move stack reset function to a shared location Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 11/14] New command: stg undo Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 12/14] New command: stg redo Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 13/14] Log and undo external modifications Karl Hasselström
2008-06-12 5:35 ` [StGit PATCH 14/14] Make "stg log" show stack log instead of patch log Karl Hasselström
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).