* [StGit PATCH 2/2] Implement "stg refresh --edit" again
From: Karl Hasselström @ 2008-07-04 6:40 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git, Jakub Narebski
In-Reply-To: <20080704063755.9637.23750.stgit@yoghurt>
The -e/--edit flag to "stg refresh" was dropped between v0.13 and
v0.14, causing severe user dissatisfaction. This patch restores it,
along with -m/--message, -f/--file, --sign, --ack, --author,
--authname, --authemail, and --authdate.
I omitted the --committer options on purpose; I think they are a
mistake. Falsifying the committer info is not a common operation, and
if one wishes to do it for some reason, one can always set the
GIT_COMMITTER_* environment variables.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/coalesce.py | 2 +-
stgit/commands/edit.py | 3 ++-
stgit/commands/new.py | 3 ++-
stgit/commands/refresh.py | 44 +++++++++++++++++++++++++++++++++-----------
stgit/utils.py | 18 ++++++++++++------
5 files changed, 50 insertions(+), 20 deletions(-)
diff --git a/stgit/commands/coalesce.py b/stgit/commands/coalesce.py
index 1a34934..f3954ce 100644
--- a/stgit/commands/coalesce.py
+++ b/stgit/commands/coalesce.py
@@ -35,7 +35,7 @@ done a sequence of pushes and pops yourself."""
directory = common.DirectoryHasRepositoryLib()
options = [make_option('-n', '--name', help = 'name of coalesced patch')
- ] + utils.make_message_options()
+ ] + utils.make_message_options(save_template = True)
class SaveTemplateDone(Exception):
pass
diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py
index a9e8991..a83ec9c 100644
--- a/stgit/commands/edit.py
+++ b/stgit/commands/edit.py
@@ -60,7 +60,8 @@ options = [make_option('-d', '--diff',
action = 'store_true'),
make_option('-e', '--edit', action = 'store_true',
help = 'invoke interactive editor'),
- ] + (utils.make_sign_options() + utils.make_message_options()
+ ] + (utils.make_sign_options()
+ + utils.make_message_options(save_template = True)
+ utils.make_author_committer_options()
+ utils.make_diff_opts_option())
diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index c4ee4e1..f468ab0 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -38,7 +38,8 @@ line of the commit message."""
directory = common.DirectoryHasRepositoryLib()
options = (utils.make_author_committer_options()
- + utils.make_message_options() + utils.make_sign_options())
+ + utils.make_message_options(save_template = True)
+ + utils.make_sign_options())
def func(parser, options, args):
"""Create a new patch."""
diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py
index 7afc55e..37cb559 100644
--- a/stgit/commands/refresh.py
+++ b/stgit/commands/refresh.py
@@ -20,7 +20,7 @@ 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, transaction
+from stgit.lib import git, transaction, edit
from stgit import utils
help = 'generate a new commit for the current patch'
@@ -53,7 +53,11 @@ options = [make_option('-u', '--update', action = 'store_true',
help = ('only include changes from the index, not'
' from the work tree')),
make_option('-p', '--patch',
- help = 'refresh PATCH instead of the topmost patch')]
+ help = 'refresh PATCH instead of the topmost patch'),
+ make_option('-e', '--edit', action = 'store_true',
+ help = 'invoke an editor for the patch description')
+ ] + (utils.make_message_options(save_template = False)
+ + utils.make_sign_options() + utils.make_author_options())
def get_patch(stack, given_patch):
"""Get the name of the patch we are to refresh."""
@@ -116,9 +120,12 @@ def make_temp_patch(stack, patch_name, paths, temp_index):
return trans.run(stack.repository.default_iw,
print_current_patch = False), temp_name
-def absorb_applied(trans, iw, patch_name, temp_name):
+def absorb_applied(trans, iw, patch_name, temp_name, edit_fun):
"""Absorb the temp patch (C{temp_name}) into the given patch
- (C{patch_name}), which must be applied.
+ (C{patch_name}), which must be applied. If the absorption
+ succeeds, call C{edit_fun} on the resulting
+ L{CommitData<stgit.lib.git.CommitData>} before committing it and
+ commit the return value.
@return: C{True} if we managed to absorb the temp patch, C{False}
if we had to leave it for the user to deal with."""
@@ -136,7 +143,7 @@ def absorb_applied(trans, iw, patch_name, temp_name):
temp_cd = trans.patches[temp_name].data
assert trans.patches[patch_name] == temp_cd.parent
trans.patches[patch_name] = trans.stack.repository.commit(
- trans.patches[patch_name].data.set_tree(temp_cd.tree))
+ edit_fun(trans.patches[patch_name].data.set_tree(temp_cd.tree)))
popped = trans.delete_patches(lambda pn: pn == temp_name, quiet = True)
assert not popped # the temp patch was topmost
temp_absorbed = True
@@ -148,9 +155,12 @@ def absorb_applied(trans, iw, patch_name, temp_name):
pass
return temp_absorbed
-def absorb_unapplied(trans, iw, patch_name, temp_name):
+def absorb_unapplied(trans, iw, patch_name, temp_name, edit_fun):
"""Absorb the temp patch (C{temp_name}) into the given patch
- (C{patch_name}), which must be unapplied.
+ (C{patch_name}), which must be unapplied. If the absorption
+ succeeds, call C{edit_fun} on the resulting
+ L{CommitData<stgit.lib.git.CommitData>} before committing it and
+ commit the return value.
@param iw: Not used.
@return: C{True} if we managed to absorb the temp patch, C{False}
@@ -174,7 +184,7 @@ def absorb_unapplied(trans, iw, patch_name, temp_name):
# It worked. Refresh the patch with the new tree, and delete
# the temp patch.
trans.patches[patch_name] = trans.stack.repository.commit(
- patch_cd.set_tree(new_tree))
+ edit_fun(patch_cd.set_tree(new_tree)))
popped = trans.delete_patches(lambda pn: pn == temp_name, quiet = True)
assert not popped # the temp patch was not applied
return True
@@ -183,13 +193,13 @@ def absorb_unapplied(trans, iw, patch_name, temp_name):
# leave the temp patch for the user.
return False
-def absorb(stack, patch_name, temp_name):
+def absorb(stack, patch_name, temp_name, edit_fun):
"""Absorb the temp patch into the target patch."""
trans = transaction.StackTransaction(stack, 'refresh')
iw = stack.repository.default_iw
f = { True: absorb_applied, False: absorb_unapplied
}[patch_name in trans.applied]
- if f(trans, iw, patch_name, temp_name):
+ if f(trans, iw, patch_name, temp_name, edit_fun):
def info_msg(): pass
else:
def info_msg():
@@ -223,4 +233,16 @@ def func(parser, options, args):
stack, patch_name, paths, temp_index = path_limiting)
if retval != utils.STGIT_SUCCESS:
return retval
- return absorb(stack, patch_name, temp_name)
+ def edit_fun(cd):
+ cd, failed_diff = edit.auto_edit_patch(
+ stack.repository, cd, msg = options.message, contains_diff = False,
+ author = options.author, committer = lambda p: p,
+ sign_str = options.sign_str)
+ assert not failed_diff
+ if options.edit:
+ cd, failed_diff = edit.interactive_edit_patch(
+ stack.repository, cd, edit_diff = False,
+ diff_flags = [], replacement_diff = None)
+ assert not failed_diff
+ return cd
+ return absorb(stack, patch_name, temp_name, edit_fun)
diff --git a/stgit/utils.py b/stgit/utils.py
index 2983ea8..c40e666 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -264,13 +264,13 @@ def add_sign_line(desc, sign_str, name, email):
desc = desc + '\n'
return '%s\n%s\n' % (desc, sign_str)
-def make_message_options():
+def make_message_options(save_template):
def no_dup(parser):
if parser.values.message != None:
raise optparse.OptionValueError(
'Cannot give more than one --message or --file')
def no_combine(parser):
- if (parser.values.message != None
+ if (save_template and parser.values.message != None
and parser.values.save_template != None):
raise optparse.OptionValueError(
'Cannot give both --message/--file and --save-template')
@@ -299,15 +299,18 @@ def make_message_options():
parser.values.save_template = w
no_combine(parser)
m = optparse.make_option
- return [m('-m', '--message', action = 'callback', callback = msg_callback,
+ opts = [m('-m', '--message', action = 'callback', callback = msg_callback,
dest = 'message', type = 'string',
help = 'use MESSAGE instead of invoking the editor'),
m('-f', '--file', action = 'callback', callback = file_callback,
dest = 'message', type = 'string', metavar = 'FILE',
- help = 'use FILE instead of invoking the editor'),
+ help = 'use FILE instead of invoking the editor')]
+ if save_template:
+ opts.append(
m('--save-template', action = 'callback', callback = templ_callback,
metavar = 'FILE', dest = 'save_template', type = 'string',
- help = 'save the message template to FILE and exit')]
+ help = 'save the message template to FILE and exit'))
+ return opts
def make_diff_opts_option():
def diff_opts_callback(option, opt_str, value, parser):
@@ -368,8 +371,11 @@ def make_person_options(person, short):
callback_args = (f,), help = 'set the %s %s' % (person, f))
for f in ['name', 'email', 'date']])
+def make_author_options():
+ return make_person_options('author', 'auth')
+
def make_author_committer_options():
- return (make_person_options('author', 'auth')
+ return (make_author_options()
+ make_person_options('committer', 'comm'))
# Exit codes.
^ permalink raw reply related
* [StGit PATCH 1/2] Refactor stgit.commands.edit
From: Karl Hasselström @ 2008-07-04 6:40 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git, Jakub Narebski
In-Reply-To: <20080704063755.9637.23750.stgit@yoghurt>
Reorganize a few existing functions, and break out stuff from the main
function into subroutines.
While we're at it, move one of the old and all of the new functions to
stgit.lib.edit, so that we can use them in a later patch to implement
"stg refresh --edit".
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/commands/common.py | 9 +++-
stgit/commands/edit.py | 79 +++++--------------------------------
stgit/commands/imprt.py | 2 -
stgit/lib/edit.py | 99 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 117 insertions(+), 72 deletions(-)
create mode 100644 stgit/lib/edit.py
diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index 2689a42..c92554d 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -453,12 +453,15 @@ def parse_mail(msg):
return (descr, authname, authemail, authdate, diff)
-def parse_patch(text):
+def parse_patch(text, contains_diff):
"""Parse the input text and return (description, authname,
authemail, authdate, diff)
"""
- descr, diff = __split_descr_diff(text)
- descr, authname, authemail, authdate = __parse_description(descr)
+ if contains_diff:
+ (text, diff) = __split_descr_diff(text)
+ else:
+ diff = None
+ (descr, authname, authemail, authdate) = __parse_description(text)
# we don't yet have an agreed place for the creation date.
# Just return None
diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py
index a8499c6..a9e8991 100644
--- a/stgit/commands/edit.py
+++ b/stgit/commands/edit.py
@@ -22,7 +22,7 @@ from optparse import make_option
from stgit import git, utils
from stgit.commands import common
-from stgit.lib import git as gitlib, transaction
+from stgit.lib import git as gitlib, transaction, edit
from stgit.out import *
help = 'edit a patch description or diff'
@@ -64,45 +64,6 @@ options = [make_option('-d', '--diff',
+ utils.make_author_committer_options()
+ utils.make_diff_opts_option())
-def patch_diff(repository, cd, diff, diff_flags):
- if diff:
- diff = repository.diff_tree(cd.parent.data.tree, cd.tree, diff_flags)
- return '\n'.join([git.diffstat(diff), diff])
- else:
- return None
-
-def patch_description(cd, diff):
- """Generate a string containing the description to edit."""
-
- desc = ['From: %s <%s>' % (cd.author.name, cd.author.email),
- 'Date: %s' % cd.author.date.isoformat(),
- '',
- cd.message]
- if diff:
- desc += ['---',
- '',
- diff]
- return '\n'.join(desc)
-
-def patch_desc(repository, cd, failed_diff, diff, diff_flags):
- return patch_description(cd, failed_diff or patch_diff(
- repository, cd, diff, diff_flags))
-
-def update_patch_description(repository, cd, text):
- message, authname, authemail, authdate, diff = common.parse_patch(text)
- cd = (cd.set_message(message)
- .set_author(cd.author.set_name(authname)
- .set_email(authemail)
- .set_date(gitlib.Date.maybe(authdate))))
- failed_diff = None
- if diff:
- tree = repository.apply(cd.parent.data.tree, diff)
- if tree == None:
- failed_diff = diff
- else:
- cd = cd.set_tree(tree)
- return cd, failed_diff
-
def func(parser, options, args):
"""Edit the given patch or the current one.
"""
@@ -122,44 +83,26 @@ def func(parser, options, args):
cd = orig_cd = stack.patches.get(patchname).commit.data
- # Read patch from user-provided description.
- if options.message == None:
- failed_diff = None
- else:
- cd, failed_diff = update_patch_description(stack.repository, cd,
- options.message)
-
- # Modify author and committer data.
- a, c = options.author(cd.author), options.committer(cd.committer)
- if (a, c) != (cd.author, cd.committer):
- cd = cd.set_author(a).set_committer(c)
-
- # Add Signed-off-by: or similar.
- if options.sign_str != None:
- cd = cd.set_message(utils.add_sign_line(
- cd.message, options.sign_str, gitlib.Person.committer().name,
- gitlib.Person.committer().email))
+ cd, failed_diff = edit.auto_edit_patch(
+ stack.repository, cd, msg = options.message, contains_diff = True,
+ author = options.author, committer = options.committer,
+ sign_str = options.sign_str)
if options.save_template:
options.save_template(
- patch_desc(stack.repository, cd, failed_diff,
- options.diff, options.diff_flags))
+ patch_desc(stack.repository, cd,
+ options.diff, options.diff_flags, failed_diff))
return utils.STGIT_SUCCESS
- # Let user edit the patch manually.
if cd == orig_cd or options.edit:
- fn = '.stgit-edit.' + ['txt', 'patch'][bool(options.diff)]
- cd, failed_diff = update_patch_description(
- stack.repository, cd, utils.edit_string(
- patch_desc(stack.repository, cd, failed_diff,
- options.diff, options.diff_flags),
- fn))
+ cd, failed_diff = edit.interactive_edit_patch(
+ stack.repository, cd, options.diff, options.diff_flags, failed_diff)
def failed():
fn = '.stgit-failed.patch'
f = file(fn, 'w')
- f.write(patch_desc(stack.repository, cd, failed_diff,
- options.diff, options.diff_flags))
+ f.write(patch_desc(stack.repository, cd,
+ options.diff, options.diff_flags, failed_diff))
f.close()
out.error('Edited patch did not apply.',
'It has been saved to "%s".' % fn)
diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py
index 5fb4da3..b958412 100644
--- a/stgit/commands/imprt.py
+++ b/stgit/commands/imprt.py
@@ -220,7 +220,7 @@ def __import_file(filename, options, patch = None):
parse_mail(msg)
else:
message, author_name, author_email, author_date, diff = \
- parse_patch(f.read())
+ parse_patch(f.read(), contains_diff = True)
if filename:
f.close()
diff --git a/stgit/lib/edit.py b/stgit/lib/edit.py
new file mode 100644
index 0000000..6874355
--- /dev/null
+++ b/stgit/lib/edit.py
@@ -0,0 +1,99 @@
+"""This module contains utility functions for patch editing."""
+
+from stgit import utils
+from stgit.commands import common
+from stgit.lib import git
+
+def update_patch_description(repo, cd, text, contains_diff):
+ """Update the given L{CommitData<stgit.lib.git.CommitData>} with the
+ given text description, which may contain author name and time
+ stamp in addition to a new commit message. If C{contains_diff} is
+ true, it may also contain a replacement diff.
+
+ Return a pair: the new L{CommitData<stgit.lib.git.CommitData>};
+ and the diff text if it didn't apply, or C{None} otherwise."""
+ (message, authname, authemail, authdate, diff
+ ) = common.parse_patch(text, contains_diff)
+ a = cd.author
+ for val, setter in [(authname, 'set_name'), (authemail, 'set_email'),
+ (git.Date.maybe(authdate), 'set_date')]:
+ if val != None:
+ a = getattr(a, setter)(val)
+ cd = cd.set_message(message).set_author(a)
+ failed_diff = None
+ if diff:
+ tree = repo.apply(cd.parent.data.tree, diff)
+ if tree == None:
+ failed_diff = diff
+ else:
+ cd = cd.set_tree(tree)
+ return cd, failed_diff
+
+def patch_desc(repo, cd, append_diff, diff_flags, replacement_diff):
+ """Return a description text for the patch, suitable for editing
+ and/or reimporting with L{update_patch_description()}.
+
+ @param cd: The L{CommitData<stgit.lib.git.CommitData>} to generate
+ a description of
+ @param append_diff: Whether to append the patch diff to the
+ description
+ @type append_diff: C{bool}
+ @param diff_flags: Extra parameters to pass to C{git diff}
+ @param replacement_diff: Diff text to use; or C{None} if it should
+ be computed from C{cd}
+ @type replacement_diff: C{str} or C{None}"""
+ desc = ['From: %s <%s>' % (cd.author.name, cd.author.email),
+ 'Date: %s' % cd.author.date.isoformat(),
+ '',
+ cd.message]
+ if append_diff:
+ if replacement_diff:
+ diff = replacement_diff
+ else:
+ just_diff = repo.diff_tree(cd.parent.data.tree, cd.tree, diff_flags)
+ diff = '\n'.join([git.diffstat(just_diff), just_diff])
+ desc += ['---', '', diff]
+ return '\n'.join(desc)
+
+def interactive_edit_patch(repo, cd, edit_diff, diff_flags, replacement_diff):
+ """Edit the patch interactively. If C{edit_diff} is true, edit the
+ diff as well. If C{replacement_diff} is not C{None}, it contains a
+ diff to edit instead of the patch's real diff.
+
+ Return a pair: the new L{CommitData<stgit.lib.git.CommitData>};
+ and the diff text if it didn't apply, or C{None} otherwise."""
+ return update_patch_description(
+ repo, cd, utils.edit_string(
+ patch_desc(repo, cd, edit_diff, diff_flags, replacement_diff),
+ '.stgit-edit.' + ['txt', 'patch'][bool(edit_diff)]),
+ edit_diff)
+
+def auto_edit_patch(repo, cd, msg, contains_diff, author, committer, sign_str):
+ """Edit the patch noninteractively in a couple of ways:
+
+ - If C{msg} is not C{None}, parse it to find a replacement
+ message, and possibly also replacement author and
+ timestamp. If C{contains_diff} is true, also look for a
+ replacement diff.
+
+ - C{author} and C{committer} are two functions that take the
+ original L{Person<stgit.lib.git.Person>} value as argument,
+ and return the new one.
+
+ - C{sign_str}, if not C{None}, is a sign string to append to
+ the message.
+
+ Return a pair: the new L{CommitData<stgit.lib.git.CommitData>};
+ and the diff text if it didn't apply, or C{None} otherwise."""
+ if msg == None:
+ failed_diff = None
+ else:
+ cd, failed_diff = update_patch_description(repo, cd, msg, contains_diff)
+ a, c = author(cd.author), committer(cd.committer)
+ if (a, c) != (cd.author, cd.committer):
+ cd = cd.set_author(a).set_committer(c)
+ if sign_str != None:
+ cd = cd.set_message(utils.add_sign_line(
+ cd.message, sign_str, git.Person.committer().name,
+ git.Person.committer().email))
+ return cd, failed_diff
^ permalink raw reply related
* [StGit PATCH 0/2] stg refresh -e/--edit
From: Karl Hasselström @ 2008-07-04 6:40 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git, Jakub Narebski
Jakub, this one's for you. ;-)
These live in my experimental branch for a little while yet, but the
plan is to include them in v0.15 -- and release that reasonably soon.
---
Karl Hasselström (2):
Implement "stg refresh --edit" again
Refactor stgit.commands.edit
stgit/commands/coalesce.py | 2 -
stgit/commands/common.py | 9 +++-
stgit/commands/edit.py | 82 ++++++------------------------------
stgit/commands/imprt.py | 2 -
stgit/commands/new.py | 3 +
stgit/commands/refresh.py | 44 +++++++++++++++-----
stgit/lib/edit.py | 99 ++++++++++++++++++++++++++++++++++++++++++++
stgit/utils.py | 18 +++++---
8 files changed, 167 insertions(+), 92 deletions(-)
create mode 100644 stgit/lib/edit.py
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* [StGit PATCH] Remove --undo flags from stg commands and docs
From: Karl Hasselström @ 2008-07-04 6:36 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
Now that we have "stg undo" et.al., they aren't needed anymore.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
In this one, I've just removed the --undo flag from sync without
adding anything back. Still undetermined if that's OK.
Documentation/tutorial.txt | 4 +++-
TODO | 2 --
stgit/commands/goto.py | 3 +--
stgit/commands/pop.py | 2 +-
stgit/commands/push.py | 29 +++++-----------------------
stgit/commands/rebase.py | 2 +-
stgit/commands/repair.py | 9 +++++----
stgit/commands/sync.py | 24 +++--------------------
stgit/stack.py | 45 +-------------------------------------------
t/t1200-push-modified.sh | 2 +-
t/t1201-pull-trailing.sh | 2 +-
t/t1202-push-undo.sh | 8 ++++----
12 files changed, 26 insertions(+), 106 deletions(-)
diff --git a/Documentation/tutorial.txt b/Documentation/tutorial.txt
index 6eaa623..0d862c0 100644
--- a/Documentation/tutorial.txt
+++ b/Documentation/tutorial.txt
@@ -178,7 +178,9 @@ patches in a stack.
During a push operation, merge conflicts can occur (especially if you
are changing the order of the patches in your stack). If the push causes
merge conflicts, they need to be fixed and 'stg resolved' run (see
-below). A 'push' operation can also be reverted with 'stg push --undo'.
+below). A 'push' operation can also be reverted with 'stg undo' (you
+will need to give it the --hard flag, since the conflicting push will
+have left your work tree dirty).
A few more stack basics; to rename a patch:
stg rename <old-name> <new-name>
diff --git a/TODO b/TODO
index 884b831..ff52a95 100644
--- a/TODO
+++ b/TODO
@@ -8,8 +8,6 @@ The TODO list before 1.0:
- debian package support
- man page
- document the workflow on the StGIT wiki
-- maybe a separate undo command rather than passing a --undo option to
- push and refresh
- use same configuration file as GIT
- release 1.0
diff --git a/stgit/commands/goto.py b/stgit/commands/goto.py
index b347920..db4a4b6 100644
--- a/stgit/commands/goto.py
+++ b/stgit/commands/goto.py
@@ -23,8 +23,7 @@ help = 'push or pop patches to the given one'
usage = """%prog [options] <name>
Push/pop patches to/from the stack until the one given on the command
-line becomes current. There is no '--undo' option for 'goto'. Use the
-'push --undo' command for this."""
+line becomes current."""
directory = common.DirectoryHasRepositoryLib()
options = []
diff --git a/stgit/commands/pop.py b/stgit/commands/pop.py
index 1d8c203..c7390a1 100644
--- a/stgit/commands/pop.py
+++ b/stgit/commands/pop.py
@@ -33,7 +33,7 @@ not specified).
A series of pop and push operations are performed so that only the
patches passed on the command line are popped from the stack. Some of
-the push operations may fail because of conflicts (push --undo would
+the push operations may fail because of conflicts ("stg undo" would
revert the last push operation)."""
directory = DirectoryGotoToplevel(log = True)
diff --git a/stgit/commands/push.py b/stgit/commands/push.py
index 8f1e400..fabb894 100644
--- a/stgit/commands/push.py
+++ b/stgit/commands/push.py
@@ -30,11 +30,11 @@ usage = """%prog [options] [<patch1>] [<patch2>] [<patch3>..<patch4>]
Push one or more patches (defaulting to the first unapplied one) onto
the stack. The 'push' operation allows patch reordering by commuting
-them with the three-way merge algorithm. If the result of the 'push'
-operation is not acceptable or if there are too many conflicts, the
-'--undo' option can be used to revert the last pushed patch. Conflicts
-raised during the push operation have to be fixed and the 'resolved'
-command run.
+them with the three-way merge algorithm. If there are conflicts while
+pushing a patch, those conflicts are written to the work tree, and the
+command halts. Conflicts raised during the push operation have to be
+fixed and the 'resolved' command run (alternatively, you may undo the
+conflicting push with 'stg undo').
The command also notifies when the patch becomes empty (fully merged
upstream) or is modified (three-way merged) by the 'push' operation."""
@@ -50,9 +50,6 @@ options = [make_option('-a', '--all',
action = 'store_true'),
make_option('-m', '--merged',
help = 'check for patches merged upstream',
- action = 'store_true'),
- make_option('--undo',
- help = 'undo the last patch pushing',
action = 'store_true')]
@@ -60,22 +57,6 @@ def func(parser, options, args):
"""Pushes the given patch or all onto the series
"""
- # If --undo is passed, do the work and exit
- if options.undo:
- patch = crt_series.get_current()
- if not patch:
- raise CmdException, 'No patch to undo'
-
- out.start('Undoing push of "%s"' % patch)
- resolved_all()
- if crt_series.undo_push():
- out.done()
- else:
- out.done('patch unchanged')
- print_crt_patch(crt_series)
-
- return
-
check_local_changes()
check_conflicts()
check_head_top_equal(crt_series)
diff --git a/stgit/commands/rebase.py b/stgit/commands/rebase.py
index 3867b26..c6bbbb4 100644
--- a/stgit/commands/rebase.py
+++ b/stgit/commands/rebase.py
@@ -38,7 +38,7 @@ the rebase by executing the following sequence:
Or if you want to skip that patch:
- $ stg push --undo
+ $ stg undo --hard
$ stg push next-patch..top-patch"""
directory = DirectoryGotoToplevel(log = True)
diff --git a/stgit/commands/repair.py b/stgit/commands/repair.py
index 1993783..80a4dea 100644
--- a/stgit/commands/repair.py
+++ b/stgit/commands/repair.py
@@ -34,8 +34,9 @@ as commit, pull, merge, and rebase -- you will leave the StGit
metadata in an inconsistent state. In that situation, you have two
options:
- 1. Use "git reset" or similar to undo the effect of the git
- command(s).
+ 1. Use "stg undo" to undo the effect of the git commands. (If you
+ know what you are doing and want more control, "git reset" or
+ similar will work too.)
2. Use "stg repair". This will fix up the StGit metadata to
accomodate the modifications to the branch. Specifically, it will
@@ -48,8 +49,8 @@ options:
* However, merge commits cannot become patches; if you have
committed a merge on top of your stack, "repair" will simply
mark all patches below the merge unapplied, since they are no
- longer reachable. If this is not what you want, use "git
- reset" to get rid of the merge and run "stg repair" again.
+ longer reachable. If this is not what you want, use "stg
+ undo" to get rid of the merge and run "stg repair" again.
* The applied patches are supposed to be precisely those that
are reachable from the branch head. If you have used e.g.
diff --git a/stgit/commands/sync.py b/stgit/commands/sync.py
index 5b2c65c..5424cf5 100644
--- a/stgit/commands/sync.py
+++ b/stgit/commands/sync.py
@@ -32,9 +32,7 @@ For each of the specified patches perform a three-way merge with the
same patch in the specified branch or series. The command can be used
for keeping patches on several branches in sync. Note that the
operation may fail for some patches because of conflicts. The patches
-in the series must apply cleanly.
-
-The sync operation can be reverted for individual patches with --undo."""
+in the series must apply cleanly."""
directory = DirectoryGotoToplevel(log = True)
options = [make_option('-a', '--all',
@@ -43,10 +41,7 @@ options = [make_option('-a', '--all',
make_option('-B', '--ref-branch',
help = 'syncronise patches with BRANCH'),
make_option('-s', '--series',
- help = 'syncronise patches with SERIES'),
- make_option('--undo',
- help = 'undo the synchronisation of the current patch',
- action = 'store_true')]
+ help = 'syncronise patches with SERIES')]
def __check_all():
check_local_changes()
@@ -68,18 +63,6 @@ def __series_merge_patch(base, patchdir, pname):
def func(parser, options, args):
"""Synchronise a range of patches
"""
- if options.undo:
- if options.ref_branch or options.series:
- raise CmdException, \
- '--undo cannot be specified with --ref-branch or --series'
- __check_all()
-
- out.start('Undoing the sync of "%s"' % crt_series.get_current())
- crt_series.undo_refresh()
- git.reset()
- out.done()
- return
-
if options.ref_branch:
remote_series = stack.Series(options.ref_branch)
if options.ref_branch == crt_series.get_name():
@@ -157,8 +140,7 @@ def func(parser, options, args):
bottom = patch.get_bottom()
top = patch.get_top()
- # reset the patch backup information. That's needed in case we
- # undo the sync but there were no changes made
+ # reset the patch backup information.
patch.set_top(top, backup = True)
# the actual merging (either from a branch or an external file)
diff --git a/stgit/stack.py b/stgit/stack.py
index 74c2c10..9958e7a 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -740,26 +740,6 @@ class Series(PatchSet):
return commit_id
- def undo_refresh(self):
- """Undo the patch boundaries changes caused by 'refresh'
- """
- name = self.get_current()
- assert(name)
-
- patch = self.get_patch(name)
- old_bottom = patch.get_old_bottom()
- old_top = patch.get_old_top()
-
- # the bottom of the patch is not changed by refresh. If the
- # old_bottom is different, there wasn't any previous 'refresh'
- # command (probably only a 'push')
- if old_bottom != patch.get_bottom() or old_top == patch.get_top():
- raise StackException, 'No undo information available'
-
- git.reset(tree_id = old_top, check_out = False)
- if patch.restore_old_boundaries():
- self.log_patch(patch, 'undo')
-
def new_patch(self, name, message = None, can_edit = True,
unapplied = False, show_patch = False,
top = None, bottom = None, commit = True,
@@ -1019,7 +999,7 @@ class Series(PatchSet):
git.merge_recursive(bottom, head, top)
except git.GitException, ex:
out.error('The merge failed during "push".',
- 'Revert the operation with "push --undo".')
+ 'Revert the operation with "stg undo".')
append_string(self.__applied_file, name)
@@ -1043,29 +1023,6 @@ class Series(PatchSet):
return modified
- def undo_push(self):
- name = self.get_current()
- assert(name)
-
- patch = self.get_patch(name)
- old_bottom = patch.get_old_bottom()
- old_top = patch.get_old_top()
-
- # the top of the patch is changed by a push operation only
- # together with the bottom (otherwise the top was probably
- # modified by 'refresh'). If they are both unchanged, there
- # was a fast forward
- if old_bottom == patch.get_bottom() and old_top != patch.get_top():
- raise StackException, 'No undo information available'
-
- git.reset()
- self.pop_patch(name)
- ret = patch.restore_old_boundaries()
- if ret:
- self.log_patch(patch, 'undo')
-
- return ret
-
def pop_patch(self, name, keep = False):
"""Pops the top patch from the stack
"""
diff --git a/t/t1200-push-modified.sh b/t/t1200-push-modified.sh
index ba4f70c..c56e8cf 100755
--- a/t/t1200-push-modified.sh
+++ b/t/t1200-push-modified.sh
@@ -56,7 +56,7 @@ test_expect_success \
test_expect_success \
'Rollback the push' '
(
- cd bar && stg push --undo &&
+ cd bar && stg undo --hard &&
[ "$(echo $(stg applied))" = "" ] &&
[ "$(echo $(stg unapplied))" = "p1 p2" ]
)
diff --git a/t/t1201-pull-trailing.sh b/t/t1201-pull-trailing.sh
index 9d70fe0..3d4ba14 100755
--- a/t/t1201-pull-trailing.sh
+++ b/t/t1201-pull-trailing.sh
@@ -49,7 +49,7 @@ test_expect_success \
test_expect_success \
'Pull those patches applied upstream' \
- "(cd bar && stg push --undo && stg push --all --merged
+ "(cd bar && stg undo && stg push --all --merged
)
"
diff --git a/t/t1202-push-undo.sh b/t/t1202-push-undo.sh
index b602643..baa986f 100755
--- a/t/t1202-push-undo.sh
+++ b/t/t1202-push-undo.sh
@@ -3,10 +3,10 @@
# Copyright (c) 2006 Catalin Marinas
#
-test_description='Exercise push --undo with missing files.
+test_description='Exercise stg undo with push of missing files.
Test the case where a patch fails to be pushed because it modifies a
-missing file. The "push --undo" command has to be able to revert it.
+missing file. The "stg undo" command has to be able to revert it.
'
. ./test-lib.sh
@@ -49,7 +49,7 @@ test_expect_success \
test_expect_success \
'Undo the previous push' \
'
- stg push --undo
+ stg undo --hard
'
test_expect_success \
@@ -64,7 +64,7 @@ test_expect_success \
touch newfile &&
git add newfile &&
rm newfile &&
- stg push --undo
+ stg undo --hard
'
test_done
^ permalink raw reply related
* Re: Cherry picking instead of merges.
From: Johannes Sixt @ 2008-07-04 6:36 UTC (permalink / raw)
To: David Brown; +Cc: Linus Torvalds, Björn Steinbrink, git
In-Reply-To: <alpine.LFD.1.10.0807032221190.2815@woody.linux-foundation.org>
Linus Torvalds schrieb:
> IOW, let's say that you really do bisect things down to a merge and cannot
> see what the fault in that merge is, you can literally do
>
> # create a test-branch with the 'remote' side of the merge
> git checkout -b test-branch merge^2
>
> # rebase that remote side on top of the local side
> git rebase merge^
>
> and now you've linearized the merge temporarily just to be able to bisect
> in that temporary branch what the bad interaction is. But once you've
> bisected it, the temporary branch is again just junk - there's no real
> value in saving it, because once you know _why_ the bug happened, you're
> just better off going back to the original history and just fixing it (and
> documenting the bug through the fix, rather than by addign extra-ugly
> history).
FWIW, the same thing in different words is written in section
"Why bisecting merge commits can be harder than bisecting linear history"
of Documentation/user-manual.txt.
-- Hannes
^ permalink raw reply
* [StGit PATCH] Discard stderr output from git apply
From: Karl Hasselström @ 2008-07-04 6:35 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
It prints error messages when it fails, and we don't need to see them
since we don't care exactly _why_ it failed.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
With the optimizations that make us start using apply a lot; I noticed
git apply was spamming on stderr.
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 6ccdfa7..9208965 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -556,7 +556,7 @@ class Index(RunWithEnv):
"""In-index patch application, no worktree involved."""
try:
self.run(['git', 'apply', '--cached']
- ).raw_input(patch_text).no_output()
+ ).raw_input(patch_text).discard_stderr().no_output()
except run.RunException:
raise MergeException('Patch does not apply cleanly')
def delete(self):
^ permalink raw reply related
* Re: Cherry picking instead of merges.
From: Linus Torvalds @ 2008-07-04 5:30 UTC (permalink / raw)
To: David Brown; +Cc: Björn Steinbrink, git
In-Reply-To: <20080704044032.GA4445@old.davidb.org>
On Thu, 3 Jul 2008, David Brown wrote:
>
> A cherry-picked tree would allow for an easy bisect, since all of the
> intermediary versions would work. If I somehow knew magical points within
> the other tree I could do some number of merges and the bisect would still
> work. I suppose I could do the merges one at a time, but it would make the
> history rather verbose.
Why do you want to worry so much about bisecting the merge?
You should basically expect that normally, a merge doesn't cause new bugs
in itself. And if it does, the most common case is a fairly obvious
mismerge (we've had a number of those in the kernel, but they weren't all
that mysterious). So _planning_ for a subtle bisection failure sounds a
bit strange and counter-productive.
So don't plan for it, and don't make it a primary issue for your workflow.
Then, *after* you have a merge, and the unlucky thing happens where the
merge is actually the thing that introduces the bug, and it's not obvious
what the exact interaction is, and bisection didn't help you pinpoint it
(because it really was the merge), *that* is when you might then choose to
re-do the rebase in a temporary branch, and then bisect that re-linearized
history.
So basically:
- keep the history clean and obvious
- make it fairly straightforward and don't worry about the unlikely case.
- don't do something painful just because bad things _can_ happen (they
can happen regardless of what you do)
- because in the unlikely situation that bad things happen, you can
always then go back and do things the other way around and pinpoint
where the fault is that way.
IOW, let's say that you really do bisect things down to a merge and cannot
see what the fault in that merge is, you can literally do
# create a test-branch with the 'remote' side of the merge
git checkout -b test-branch merge^2
# rebase that remote side on top of the local side
git rebase merge^
and now you've linearized the merge temporarily just to be able to bisect
in that temporary branch what the bad interaction is. But once you've
bisected it, the temporary branch is again just junk - there's no real
value in saving it, because once you know _why_ the bug happened, you're
just better off going back to the original history and just fixing it (and
documenting the bug through the fix, rather than by addign extra-ugly
history).
Linus
^ permalink raw reply
* Re: Cherry picking instead of merges.
From: David Brown @ 2008-07-04 4:40 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: Linus Torvalds, git
In-Reply-To: <20080704001003.GA19053@atjola.homenet>
On Fri, Jul 04, 2008 at 02:10:03AM +0200, Björn Steinbrink wrote:
>The other one is merging:
>
> A---B---C---M
> \ /
> D---E---/
>
>Of course, you should end up with the same tree either way. It's just a
>different way of getting towards that final state. So commit E' and
>commit M, while different commits, would point to the same tree object.
Ok. I guess I need to explain a little further. The advice I've gotten is
good, BTW.
After we've resolved the merge and committed it, we've then discovered that
it doesn't work. However, there are around 100 commits on both branches of
the merges and it would be nice to come up with some way of doing something
like a bisect.
The trick is that the branch being merged in doesn't actually work on our
platform, so I can't just test the alternate branch.
But, git-bisect isn't being all that helpful here. The problem is that the
only conflicts we resolved is how the two trees were put together. Picking
points in the middle seem to generate lots of similar, but not quite the
same conflicts.
A cherry-picked tree would allow for an easy bisect, since all of the
intermediary versions would work. If I somehow knew magical points within
the other tree I could do some number of merges and the bisect would still
work. I suppose I could do the merges one at a time, but it would make the
history rather verbose.
Thanks,
David
^ permalink raw reply
* [PATCH/RFC] work around recent curl/gcc warnings
From: Junio C Hamano @ 2008-07-04 4:38 UTC (permalink / raw)
To: git
After master.k.org upgrade, I started seeing these warning messages:
transport.c: In function 'get_refs_via_curl':
transport.c:458: error: call to '_curl_easy_setopt_err_write_callback' declared with attribute warning: curl_easy_setopt expects a curl_write_callback argument for this option
It appears that the header wants to enforce the function signature for
fwrite_buffer to be compatible with that of fwrite() or
(*curl_write_callback).
This patch seems to work the issue around.
---
http.c | 13 +++++++------
http.h | 9 +++------
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/http.c b/http.c
index 105dc93..ad14640 100644
--- a/http.c
+++ b/http.c
@@ -30,10 +30,11 @@ static struct curl_slist *pragma_header;
static struct active_request_slot *active_queue_head = NULL;
-size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
- struct buffer *buffer)
+size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb, void *buffer_)
{
size_t size = eltsize * nmemb;
+ struct buffer *buffer = buffer_;
+
if (size > buffer->buf.len - buffer->posn)
size = buffer->buf.len - buffer->posn;
memcpy(ptr, buffer->buf.buf + buffer->posn, size);
@@ -42,17 +43,17 @@ size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
return size;
}
-size_t fwrite_buffer(const void *ptr, size_t eltsize,
- size_t nmemb, struct strbuf *buffer)
+size_t fwrite_buffer(const void *ptr, size_t eltsize, size_t nmemb, void *buffer_)
{
size_t size = eltsize * nmemb;
+ struct strbuf *buffer = buffer_;
+
strbuf_add(buffer, ptr, size);
data_received++;
return size;
}
-size_t fwrite_null(const void *ptr, size_t eltsize,
- size_t nmemb, struct strbuf *buffer)
+size_t fwrite_null(const void *ptr, size_t eltsize, size_t nmemb, void *strbuf)
{
data_received++;
return eltsize * nmemb;
diff --git a/http.h b/http.h
index a04fc6a..905b462 100644
--- a/http.h
+++ b/http.h
@@ -64,12 +64,9 @@ struct buffer
};
/* Curl request read/write callbacks */
-extern size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
- struct buffer *buffer);
-extern size_t fwrite_buffer(const void *ptr, size_t eltsize,
- size_t nmemb, struct strbuf *buffer);
-extern size_t fwrite_null(const void *ptr, size_t eltsize,
- size_t nmemb, struct strbuf *buffer);
+extern size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb, void *strbuf);
+extern size_t fwrite_buffer(const void *ptr, size_t eltsize, size_t nmemb, void *strbuf);
+extern size_t fwrite_null(const void *ptr, size_t eltsize, size_t nmemb, void *strbuf);
/* Slot lifecycle functions */
extern struct active_request_slot *get_active_slot(void);
^ permalink raw reply related
* Re: Can I remove stg sync --undo ?
From: Karl Hasselström @ 2008-07-04 2:09 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0807031502x5f7c4babtd65511d7966c69f6@mail.gmail.com>
On 2008-07-03 23:02:28 +0100, Catalin Marinas wrote:
> Sorry for the delay, I've been really busy recently.
No problem.
> 2008/7/2 Karl Hasselström <kha@treskal.com>:
>
> > I'm preparing a patch that removes all the old --undo flags, and
> > discovered that stg sync has an --undo flag backed by
> > stack.undo_refresh().
>
> The current --undo flag restores the state of the patch before a
> successful sync. If the sync fails with a conflict and it needs a
> refresh after resolving, I think it loses the previous state of the
> patch and just restores to whatever it was before the refresh.
[...]
> The sync performs three operations - push, merge and refresh (if the
> refresh is automatic after merge, it doesn't update the backup
> information since it was done by merge).
>
> If merge fails, the refresh is manual after solving the conflicts. I
> suspect this will be recorded as a separate step for undo
Yeah, the new undo stuff will currently handle sync just like e.g.
push and pop: write one log entry when the command's all done, plus
one extra just before the conflicting push if there is one. So you can
always undo the entire command; and in case of conflicts, you also
have the option of undoing just the conflicting push. Is this enough
for sync?
> (BTW, is resolved take into account for undo?).
Hmmm, what do you mean by "resolved"?
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle
From: Adam Brewster @ 2008-07-04 2:04 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Mark Levedahl, Junio C Hamano, Jakub Narebski
In-Reply-To: <alpine.DEB.1.00.0807040237580.2849@eeepc-johanness>
>>
>> How does everybody feel about the following:
>>
>> - Leave git-basis as a small perl script.
>
> I'd rather not.
>
I'm not exactly sure what your objection is here, but I guess it's
either you don't want a separate tool called git-basis that does all
of the things I've described, or you don't like my choice of perl to
do the work.
If the former, I suggest that my approach is consistent with the
philosophy of doing one thing and doing it well. I acknowledge that
it could do it's one job better (see potential improvements in my last
email), but that doesn't seem to be your complaint.
If the latter, I wonder what practical advantage comes from redoing
what I've already done in C. It would be slightly faster, but I'm not
worried about saving a few milliseconds in a process that takes at
least a couple of minutes (considering of course the time it takes to
walk to whatever remote system).
>> - Add a -b/--basis option in git-bundle that calls git-basis. Any
>> objects mentioned in the output would be excluded from the bundle.
>> Multiple --basis options will call git-basis once with several
>> arguments to generate the intersection of specified bases.
>
> So the only function of -b would be to fork() && exec() a _shell_ script?
> I don't like that at all.
>
Not quite. It would be more like a shortcut for
git-basis --show basis | git-bundle create bundle -all --stdin or
git-bundle create bundle --all <( git-basis --show basis )
(the latter of which of course wouldn't work because git-basis doesn't
take filenames.
The bundle creation is still done by git-bundle. git-basis is just
deciding what should (not) be included in the bundle.
It seems like similar architectures have been accepted to support the
-i/--interactive options or git-add and git-rebase.
>> - (maybe) Add an option "--update-bases" to automatically call git-basis
>> --update after the bundle is created successfully.
>
> Rather, have it as a feature to auto-detect if there is a ".basis" file of
> the same basename (or, rather ".state", a I find "basis" less than
> descriptive), and rewrite it if it was there.
>
> It could be forced by a to-be-introduced "--state" option to git-bundle.
>
Just because I'm creating a bundle, doesn't guarantee that the bundle
will be installed on any particular remote system, so I think that
updating the basis without being told to do so by the user is a bad
idea. For example, when creating a bundle, I find it's best to
exclude objects that I know exist on ALL of my systems, so that I can
pull from it anywhere, but usually I only end up sending it to one
system.
With regard to the use of the word basis, it comes from the
documentation of git-bundle. It's been a while since I took linear
algebra, but if I remember correctly, a basis is a set of vectors that
describe a vector space, such that a combination of those vectors
yields any point in the space. The analogy isn't perfect, but I think
it's pretty close. The word state seems very generic, as the state of
a repository includes much more than a list of objects that are known
to be available.
>> There's still plenty of potential for improvements, like a --gc mode
>> to clean up basis files,
>
> umm, why? "rm" is not simple enough?
>
rm leaves the files a little cleaner than I'd like. A basis is really
a list of objects. --gc should make sure that the objects in the list
actually exist, and aren't redundant (if I know that a remote system
has a given commit, I also know that it has all of the ancestors, so I
could delete them from the basis file without losing (much)
information.
>> a --rewind option to undo an incorrect --update,
>
> Rather hard, would you not think? The information is either not there, or
> you store loads of cruft in the .state file.
>
There's some information that you might describe as cruft. It is
specifically engineered to enable this operation, and the purpose of
--gc is to reduce the volume of that cruft.
>> or improvements in the way it calculates intersections,
>
> Umm. How so?
>
In a tree with three commits, where A (the root) is a parent of X and
Y, if only ommit X is in basisX and only commit Y is in basisY, then
the intersection of basisX and basisY should include A. Currently
git-basis will return nothing, because it doesn't care about ancestry.
I don't consider this a serious flaw, because it results in extra
information being included in the bundle, but should never cause
broken bundles that are missing information.
>> but I think that with these changes the system is as simple as possible
>> while maximizing flexibility, utility, and usability.
>
> I am not convinced. This sort of feature belongs into git-bundle. It
> certainly does not deserve being blessed by yet-another git-* command,
> when we are constantly being bashed for having _way_ too many _already_.
>
I disagree. I think the --basis option seems like a logical addition
to git-bundle, but I don't think git-bundle is the right interface to
update the basis files.
In other news, it seems, the change to make git-bundle accept --stdin
is less controversial, so I'll submit that as a small patch.
> Ciao,
> Dscho
>
>
Adam Brewster
^ permalink raw reply
* Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype
From: Stephan Beyer @ 2008-07-04 1:53 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, git, Daniel Barkalow, Christian Couder
In-Reply-To: <alpine.DEB.1.00.0807040252490.2849@eeepc-johanness>
Hi,
On Fri, Jul 04, 2008 at 03:03:37AM +0200, Johannes Schindelin wrote:
> On Fri, 4 Jul 2008, Stephan Beyer wrote:
> > On Fri, Jul 04, 2008 at 01:53:21AM +0200, Johannes Schindelin wrote:
> > > On Thu, 3 Jul 2008, Stephan Beyer wrote:
> > > > Btw, another root commit problem is btw that it's not possible to
> > > > cherry-pick root commits.
> > >
> > > That is a problem to be fixed in cherry-pick, not in sequencer. Care
> > > to take care of that?
> >
> > Not at the moment but that's one of the things I note down for later ;-)
>
> Well, logically, it should come _before_ you use it in sequencer. And you
> should use it in sequencer.
Yet nobody seems to have asked for a cherry-pick that is able to pick
root commits and sequencer is not closed source after GSoC, so this can
be added whenever there is need and time for it.
That's what I wanted to say with "note down for later". ;-)
> > I don't get the light bulb. You're talking about "the merge", I am
> > talking about fast-forward on picks.
>
> Ooops. I think I was talking about a later comment of Junio's.
>
> The thing is, if you try to pick a commit, and the current revision is
> already the parent of that commit, I think it would be wrong to redo the
> commit, pretending that the current time and committer were applying that
> commit.
Right.
> IMO the --signoff should check if the sign off is present (must be the
> last one, as if we redid that commit), and if it is, fast-forward.
>
> If it is missing, we need to redo the commit.
That's a good point.
Checked that and it's even a bug in the sequencer: on fast-forwards, no
signoff is added.
This should fix that:
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -219,8 +219,6 @@ pick_one () {
fi
;;
esac
- test -n "$SIGNOFF" &&
- what="$what -s"
$use_perform git $what "$@"
}
@@ -973,16 +971,21 @@ insn_pick () {
$use_perform git commit --amend $EDIT $signoff --no-verify \
--author "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" \
--message="$MESSAGE"
- else
+ elif test -n "$AUTHOR"
+ then
# correct author if AUTHOR is set
- test -n "$AUTHOR" &&
- $use_perform git commit --amend $EDIT --no-verify -C HEAD \
- --author "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
- # XXX: a git-cherry-pick --author could be nicer here
+ $use_perform git commit --amend $EDIT --no-verify -C HEAD \
+ --author "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
+ elif test -n "$MESSAGE"
+ then
# correct commit message if MESSAGE is set
- test -n "$MESSAGE" &&
- $use_perform git commit --amend $EDIT $signoff --no-verify \
- -C HEAD --message="$MESSAGE"
+ $use_perform git commit --amend $EDIT $signoff --no-verify \
+ -C HEAD --message="$MESSAGE"
+ elif test -n "$SIGNOFF"
+ then
+ # only add signoff
+ $use_perform git commit --amend $EDIT $signoff --no-verify \
+ -C HEAD
fi
return 0
###
Ah, and that is poorly tested, but not untested. :) See:
$ git checkout -b seq-proto-dev3 HEAD^
Switched to a new branch "seq-proto-dev3"
$ git sequencer
mark :1
pick seq-proto-dev2
ref refs/test/no
reset :1
pick --signoff seq-proto-dev2
Now: seq-proto-dev3 has signoff and seq-proto-dev2 and refs/test/no have the
same SHA1. And the test suite passes.
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: [PATCH 03/15] manpages: fix bogus whitespace
From: Jonathan Nieder @ 2008-07-04 1:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Christian Couder, J. Bruce Fields, Miklos Vajna,
Shawn O. Pearce
In-Reply-To: <7vprpuwmjj.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@uchicago.edu> writes:
>
>> - is successful. This option disables the output shown by
>> + is successful. This option disables the output shown by
>
> How did you find *this* one? It took me a few minutes to finally notice
> that you are talking about the HT. In other words, I had to work hard to
> get distracted by it.
I use vim with listchars=tab:>-,nbsp:~,trail:$ (I was bit too
many times by Makefiles mysteriously breaking) so the tab is
visibile.
If it feels like too much churn for no visible effect, I don't
mind if you drop the patch. I feel bad for sending so many
documentation patches that don't add to the content in any
significant way.
Jonathan
^ permalink raw reply
* Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype
From: Johannes Schindelin @ 2008-07-04 1:11 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Junio C Hamano, git, Daniel Barkalow, Christian Couder
In-Reply-To: <20080704010616.GH6677@leksak.fem-net>
Hi,
On Fri, 4 Jul 2008, Stephan Beyer wrote:
> Johannes Schindelin wrote:
>
> > and you can easily abort a rebase if you explicitely asked for an
> > invalid strategy.
>
> Aborting after fixing a lot of conflicts in the sequencer process is
> really annoying.
I think it should use rerere (if activated, which I have given up
advocating to be the default).
> So I've chosen to never abort automatically.
Who said anything about automatically? Of _course_, the user has to abort
it. Or try to continue with "git sequencer --continue -s ours".
Ciao,
Dscho
^ permalink raw reply
* Re: [KORG] Master downtime
From: J.H. @ 2008-07-04 1:10 UTC (permalink / raw)
To: users, linux-kernel, support, git
In-Reply-To: <486D6B70.7080702@kernel.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hey all,
Sorry about the delay grub decided to be 'helpful' which delayed me a
bit. Anyway the system is up, it's finishing up grabbing all of the
updates but it should be in a reasonably usable state. So if you don't
mind getting kicked off for 10 minutes more sometime later this evening,
to reboot into the latest kernel, master is open for business.
Git has already been reported as non-functioning and that is being
looked into. If you have further problems, report them on the
kernel.org admin IRC channel, or e-mail ftpadmin.
- - John 'Warthog9' Hawley
Chief Kernel.org Admin
J.H. wrote:
| Just a heads up the downtime is running a hair long, be about another
| half hour.
|
| - John 'Warthog9' Hawley
| Chief Kernel.org Admin
|
| J.H. wrote:
| | Afternoon everyone,
| |
| | Just a quick heads up we are going to be taking master down for hardware
| | and software upgrades tomorrow, Thursday July 3rd 2008 at 15:00 UTC.
| | During this time all back-end services, including wiki's, ssh and
| | userweb to name a few things.
| |
| | The hardware portion of the upgrade should be complete within an hour,
| | and the software upgrades will likely take another hour or two - though
| | even when master comes back up expect intermittent service for a few
| | hours after that while we deal with things. Likely ETA for all work
| | completed is Friday July 4th 2008 at 00:00 UTC. If there are
| | complications, I will kick another e-mail off with updates.
| |
| | If you have any questions, comments or concerns please don't hesitate to
| | get ahold of me.
| |
| | - John 'Warthog9' Hawley
| | Chief Kernel.org Admin
- --
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iD8DBQFIbXh+/E3kyWU9dicRAlaMAJ9YO+iMq+lGJwfAUXAjF1aQ47OsDACeMRp4
1/DT/k4BjXKgqNwpNy36ZRY=
=PL28
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype
From: Stephan Beyer @ 2008-07-04 1:06 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, git, Daniel Barkalow, Christian Couder
In-Reply-To: <alpine.DEB.1.00.0807040138090.2849@eeepc-johanness>
Hi again,
Johannes Schindelin wrote:
> > > I'd not check in sequencer for the strategy. Especially given that we
> > > want to support user-written strategies in the future.
> >
> > I don't know how this is planned to look like, but perhaps
> > --list-strategies may make sense here, too.
Funny, a merge with an unknown merge strategy acts somehow like my
proposed --list-strategies (at least on git-merge.sh, haven't checked
on builtin-merge)
But that is just a silly side note.
> No. You just do not check for strategies. Period.
Well, I've seen that my strategy_check (which is now removed) only
produces a warning, so it had no big effect at all.
So I've tested merge --strategy=hours (to simulate a typo):
-- -- snip paste -- --
Testing:
git sequencer todotest1
available strategies are: recur recursive octopus resolve stupid ours subtree
Error merging
* FAIL 33: merge multiple branches and --reuse-commit works
-- -- snap paste -- --
That means, with the information that can be seen, a user should easily be
able to fix that, i.e. run git-sequencer --edit and fix the line.
So I accept your period sign now ;-)
> and you can easily abort a rebase if you explicitely asked for an invalid
> strategy.
Aborting after fixing a lot of conflicts in the sequencer process is
really annoying. So I've chosen to never abort automatically.
(That was one of the first things that I changed after I first used my
own sequencer for real work and not only test cases.)
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype
From: Johannes Schindelin @ 2008-07-04 1:03 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Junio C Hamano, git, Daniel Barkalow, Christian Couder
In-Reply-To: <20080704003857.GG6677@leksak.fem-net>
Hi,
On Fri, 4 Jul 2008, Stephan Beyer wrote:
> On Fri, Jul 04, 2008 at 01:53:21AM +0200, Johannes Schindelin wrote:
> > On Thu, 3 Jul 2008, Stephan Beyer wrote:
> > > Btw, another root commit problem is btw that it's not possible to
> > > cherry-pick root commits.
> >
> > That is a problem to be fixed in cherry-pick, not in sequencer. Care
> > to take care of that?
>
> Not at the moment but that's one of the things I note down for later ;-)
Well, logically, it should come _before_ you use it in sequencer. And you
should use it in sequencer.
> And btw, somehow it is still open for me if builtin sequencer should be
> a git-cherry-pick user (for pick) or if git-cherry-pick should be a
> sequencer user (which would result in a change of usage on cherry-pick
> conflicts).
The thing is, sequencer and cherry-pick _both_ use the same underlying
functionality: cherry-picking a single commit.
So, logically, that part should be moved into libgit.a, and cherry-pick
should use the to-be-introduced sequencer() function.
> > > Johannes Schindelin wrote:
> > > > > > +# Usage: pick_one (cherry-pick|revert) [-*|--edit] sha1
> > > > > > +pick_one () {
> > > > > > + what="$1"
> > > > > > + # we just assume that this is either cherry-pick or revert
> > > > > > + shift
> > > > > > +
> > > > > > + # check for fast-forward if no options are given
> > > > > > + if expr "x$1" : 'x[^-]' >/dev/null
> > > > > > + then
> > > > > > + test "$(git rev-parse --verify "$1^")" = \
> > > > > > + "$(git rev-parse --verify HEAD)" &&
> > > > > > + output git reset --hard "$1" &&
> > > > > > + return
> > > > > > + fi
> > > > > > + test "$1" != '--edit' -a "$what" = 'revert' &&
> > > > > > + what='revert --no-edit'
> > > > >
> > > > > This looks somewhat wrong.
> > > > >
> > > > > When the history looks like ---A---B and we are at A,
> > > > > cherry-picking B can be optimized to just advancing to B, but
> > > > > that optimization has a slight difference (or two) in the
> > > > > semantics.
> > > > >
> > > > > (1) The committer information would not record the user and
> > > > > time of the sequencer operation, which actually may be a
> > > > > good thing.
> > > >
> > > > This is debatable. But I think you are correct, for all the same
> > > > reasons why a merge can result in a fast-forward.
> > >
> > > Dscho, you mean me by referring to 'you' here, right?
> >
> > Nope.
> >
> > > Otherwise I'm a bit confused: "For the same reasons why a merge can
> > > result in a fast-forward we should not do fast forward here" ;-)
> >
> > What I meant: there is no use here to redo it. It has already be
> > done, and redoing just pretends that the girl calling sequencer tried
> > to pretend that she did it.
> >
> > If the merge has been done already, it should not be redone.
> >
> > Only if the user _explicitely_ specified a merge strategy, there _might_
> > be a reason to redo the merge, but I still doubt it.
>
> I don't get the light bulb. You're talking about "the merge", I am
> talking about fast-forward on picks.
Ooops. I think I was talking about a later comment of Junio's.
The thing is, if you try to pick a commit, and the current revision is
already the parent of that commit, I think it would be wrong to redo the
commit, pretending that the current time and committer were applying that
commit.
> I try a simple example just to go sure that we're talking about the
> same.
>
> We have commits
>
> A ---- B ---- C ---- D
> HEAD
>
> A is parent of B, B of C, C of D.
>
> Now we do:
> pick C
> pick --signoff D
> (Assume that the Signed-off-by: line is missing on D)
>
> Without fast-forward, we get
>
> A ---- B ---- C ---- D
> \
> `--- C'---- D'
> HEAD
>
> C' differs in C only in the committer data, perhaps only committer date.
>
> With fast-forward, we get:
>
> A ---- B ---- C ---- D
> \
> `--- D'
> HEAD
IMO the --signoff should check if the sign off is present (must be the
last one, as if we redid that commit), and if it is, fast-forward.
If it is missing, we need to redo the commit.
> > > Johannes Schindelin wrote:
> > > > I'd not check in sequencer for the strategy. Especially given
> > > > that we want to support user-written strategies in the future.
> > >
> > > I don't know how this is planned to look like, but perhaps
> > > --list-strategies may make sense here, too.
> >
> > No. You just do not check for strategies. Period. git-merge does
> > that, and you can easily abort a rebase if you explicitely asked for
> > an invalid strategy.
>
> Hmm, my dream of the "robust sequencing after sanity check passed" is
> dead with your "period".
>
> So I'll have to check what happens, when e.g. "--strategy=hours" is
> used.
Wow. Is that the CVS/SVN merge strategy?
Do not care too much about the rare use cases. Only experts will use -s.
But experts are known to shoot themselves, or inflict other pain on their
bodies, so it is okay to let them suffer from a typo.
Ciao,
Dscho
^ permalink raw reply
* Re: RFC: grafts generalised
From: Jakub Narebski @ 2008-07-04 0:43 UTC (permalink / raw)
To: Stephen R. van den Berg; +Cc: git
In-Reply-To: <20080702173203.GA16235@cuci.nl>
On Wed, 2 July 2008, Stephen R. van den Berg wrote:
> Jakub Narebski wrote:
>>
>> [...] So I think that it would
>> be better to provide generic git-filter-branch filter which can
>> understand this "generalized grafts" file format, or rather
>> 'description of changes' file. Put it in contrib/, and here you
>> go...
>
> The problem is that the process of fixing history is an iterative one,
> which can take many months, and everytime you make a change, the
> correctness needs to be viewed using gitk.
[...]
I wanted to propose that git-filter-branch generic "generalized grafts"
file based filter should be accompanied by extending gitk so it
understand this format to...
...but after reading wonderfull suggestion to create new commits with
corrected contents, and insert them (replace older version by them)
using grafts, thought and brought independently by Dmitry Potapov and
Petr Baudis, I think that you would be best with extending gitk to
support this way instead.
You would have to extend gitk to maintain reverse revision mapping
(from revision to its children), and then you would be able to edit
history interactively from within gitk, with gitk correcting its
internal structures to redisplay changed commits, and creating commits
and doing grafting behind the scenes for later git-filter-branch
run.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle
From: Johannes Schindelin @ 2008-07-04 0:44 UTC (permalink / raw)
To: Adam Brewster; +Cc: git, Mark Levedahl, Junio C Hamano, Jakub Narebski
In-Reply-To: <c376da900807031638l219229bcy983ed994b37512c9@mail.gmail.com>
Hi,
On Thu, 3 Jul 2008, Adam Brewster wrote:
> > Yes, certainly it is more flexible to have them split. I find Adam's
> > argument the most compelling, though. Think about moving commits as a
> > multi-step protocol:
> >
> > 1. Local -> Remote: Here are some new commits, basis..current
> > 2. Remote -> Local: OK, I am now at current.
> > 3. Local: update basis to current
> >
> > git-push has the luxury of asking for "basis" each time, so we know it
> > is correct. But with bundles, we can't do that. And failing to update
> > "basis" means we will send some extra commits next time. But updating
> > "basis" when we shouldn't means that the next bundle will be broken.
> >
> > So I think even if people _do_ want to update "basis" when they create
> > the bundle (because it is more convenient, and they are willing to
> > accept the possibility of losing sync), it is trivial to create that
> > workflow on top of the separate components. But I can see why somebody
> > might prefer the separate components, and it is hard to create them if
> > the feature is lumped into "git-bundle" (meaning in such a way that
> > you cannot perform the steps separately; obviously git-bundle --basis
> > would be equivalent).
> >
> > But I am not a bundle user, so that is just my outsider perspective.
>
> How does everybody feel about the following:
>
> - Leave git-basis as a small perl script.
I'd rather not.
> - Add a -b/--basis option in git-bundle that calls git-basis. Any
> objects mentioned in the output would be excluded from the bundle.
> Multiple --basis options will call git-basis once with several
> arguments to generate the intersection of specified bases.
So the only function of -b would be to fork() && exec() a _shell_ script?
I don't like that at all.
> - (maybe) Add an option "--update-bases" to automatically call git-basis
> --update after the bundle is created successfully.
Rather, have it as a feature to auto-detect if there is a ".basis" file of
the same basename (or, rather ".state", a I find "basis" less than
descriptive), and rewrite it if it was there.
It could be forced by a to-be-introduced "--state" option to git-bundle.
> There's still plenty of potential for improvements, like a --gc mode
> to clean up basis files,
umm, why? "rm" is not simple enough?
> a --rewind option to undo an incorrect --update,
Rather hard, would you not think? The information is either not there, or
you store loads of cruft in the .state file.
> or improvements in the way it calculates intersections,
Umm. How so?
> but I think that with these changes the system is as simple as possible
> while maximizing flexibility, utility, and usability.
I am not convinced. This sort of feature belongs into git-bundle. It
certainly does not deserve being blessed by yet-another git-* command,
when we are constantly being bashed for having _way_ too many _already_.
Ciao,
Dscho
^ permalink raw reply
* Re: Cherry picking instead of merges.
From: Linus Torvalds @ 2008-07-04 0:39 UTC (permalink / raw)
To: David Brown; +Cc: git
In-Reply-To: <20080703223949.GA23092@old.davidb.org>
On Thu, 3 Jul 2008, David Brown wrote:
>
> I'm still not clear how the one-commit-at-a-time resolution gets recorded
> anywhere (except in the cherry-picking branch).
For merges, the _only_ thing that matters from a "resolution" standpoint
is that the history joins together. And it doesn't matter if it joins
together fifty times or once - only the last point is relevant, since that
becomes the last point of shared state.
> It seems to be that I would need to do multiple merges, one at each point
> where there is a conflict that I had to resolved. I would remember this as
> I did each cherry picked change, but after the fact, I would have to
> compare the cherry picked change with the one it came from, and figure out
> where conflicts had to be resolved.
No, because you simply don't care. You only care about the final result.
With a single merge, there is only a single merge-point, and a single
result. The advantage of doing the cherry-pick is just that it splits the
decision on how to merge up into many smaller decisions (that is often the
_dis_advantage of cherry-picking too - it often makes for more work, even
if the individual work may be simpler).
But once you've done all the small cherry-pick decisions, you'd only make
one final merge that takes that final result as-is. Any future work will
then know about the fact that you have a new common base point.
Linus
^ permalink raw reply
* Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype
From: Stephan Beyer @ 2008-07-04 0:38 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, git, Daniel Barkalow, Christian Couder
In-Reply-To: <alpine.DEB.1.00.0807040138090.2849@eeepc-johanness>
Hi,
On Fri, Jul 04, 2008 at 01:53:21AM +0200, Johannes Schindelin wrote:
> On Thu, 3 Jul 2008, Stephan Beyer wrote:
> > Btw, another root commit problem is btw that it's not possible to
> > cherry-pick root commits.
>
> That is a problem to be fixed in cherry-pick, not in sequencer. Care to
> take care of that?
Not at the moment but that's one of the things I note down for later ;-)
And btw, somehow it is still open for me if builtin sequencer should be
a git-cherry-pick user (for pick) or if git-cherry-pick should be a
sequencer user (which would result in a change of usage on cherry-pick
conflicts).
> > Johannes Schindelin wrote:
> > > > > +# Usage: pick_one (cherry-pick|revert) [-*|--edit] sha1
> > > > > +pick_one () {
> > > > > + what="$1"
> > > > > + # we just assume that this is either cherry-pick or revert
> > > > > + shift
> > > > > +
> > > > > + # check for fast-forward if no options are given
> > > > > + if expr "x$1" : 'x[^-]' >/dev/null
> > > > > + then
> > > > > + test "$(git rev-parse --verify "$1^")" = \
> > > > > + "$(git rev-parse --verify HEAD)" &&
> > > > > + output git reset --hard "$1" &&
> > > > > + return
> > > > > + fi
> > > > > + test "$1" != '--edit' -a "$what" = 'revert' &&
> > > > > + what='revert --no-edit'
> > > >
> > > > This looks somewhat wrong.
> > > >
> > > > When the history looks like ---A---B and we are at A, cherry-picking B can
> > > > be optimized to just advancing to B, but that optimization has a slight
> > > > difference (or two) in the semantics.
> > > >
> > > > (1) The committer information would not record the user and time of the
> > > > sequencer operation, which actually may be a good thing.
> > >
> > > This is debatable. But I think you are correct, for all the same reasons
> > > why a merge can result in a fast-forward.
> >
> > Dscho, you mean me by referring to 'you' here, right?
>
> Nope.
>
> > Otherwise I'm a bit confused: "For the same reasons why a merge can
> > result in a fast-forward we should not do fast forward here" ;-)
>
> What I meant: there is no use here to redo it. It has already be done,
> and redoing just pretends that the girl calling sequencer tried to pretend
> that she did it.
>
> If the merge has been done already, it should not be redone.
>
> Only if the user _explicitely_ specified a merge strategy, there _might_
> be a reason to redo the merge, but I still doubt it.
I don't get the light bulb. You're talking about "the merge", I am
talking about fast-forward on picks.
Perhaps I got Junio wrong, too.
I try a simple example just to go sure that we're talking about the
same.
We have commits
A ---- B ---- C ---- D
HEAD
A is parent of B, B of C, C of D.
Now we do:
pick C
pick --signoff D
(Assume that the Signed-off-by: line is missing on D)
Without fast-forward, we get
A ---- B ---- C ---- D
\
`--- C'---- D'
HEAD
C' differs in C only in the committer data, perhaps only committer date.
With fast-forward, we get:
A ---- B ---- C ---- D
\
`--- D'
HEAD
If Junio meant with
> (1) The committer information would not record the user and time of the
> sequencer operation, which actually may be a good thing.
that he thinks the first variant is the way to go, I strongly disagree.
But perhaps I'm getting everyone wrong these days ;)
> > > > (2) When $what is revert, this codepath shouldn't be exercised,
> > > > should it?
> > >
> > > Yes.
> >
> > I haven't done a check intentionally, but there was a stupid thinko.
> > So you're right.
> >
> > But: this will only be a bug if the commit that _comes next in the
> > original history_ is to be reverted.
>
> Does not matter. It's a bug.
>
> A bug is almost always in the details, a corner-case, but it almost always
> needs fixing nevertheless.
Of course ;)
> > Nonetheless, purely tested:
>
> "Nevertheless", maybe? "untested", maybe?
No, I tested it once. ;-)
(For the new single-quoted variant I've changed the author name in
t3350).
> > Johannes Schindelin wrote:
> > > I'd not check in sequencer for the strategy. Especially given that we
> > > want to support user-written strategies in the future.
> >
> > I don't know how this is planned to look like, but perhaps
> > --list-strategies may make sense here, too.
>
> No. You just do not check for strategies. Period. git-merge does that,
> and you can easily abort a rebase if you explicitely asked for an invalid
> strategy.
Hmm, my dream of the "robust sequencing after sanity check passed" is
dead with your "period".
So I'll have to check what happens, when e.g. "--strategy=hours" is used.
(I mean, you should be in a safe state to do git sequencer --edit and
correct "hours" to "ours'.)
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: [KORG] Master downtime
From: J.H. @ 2008-07-04 0:14 UTC (permalink / raw)
To: users, linux-kernel, support, git
In-Reply-To: <486C0877.4070001@eaglescrag.net>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Just a heads up the downtime is running a hair long, be about another
half hour.
- - John 'Warthog9' Hawley
Chief Kernel.org Admin
J.H. wrote:
| Afternoon everyone,
|
| Just a quick heads up we are going to be taking master down for hardware
| and software upgrades tomorrow, Thursday July 3rd 2008 at 15:00 UTC.
| During this time all back-end services, including wiki's, ssh and
| userweb to name a few things.
|
| The hardware portion of the upgrade should be complete within an hour,
| and the software upgrades will likely take another hour or two - though
| even when master comes back up expect intermittent service for a few
| hours after that while we deal with things. Likely ETA for all work
| completed is Friday July 4th 2008 at 00:00 UTC. If there are
| complications, I will kick another e-mail off with updates.
|
| If you have any questions, comments or concerns please don't hesitate to
| get ahold of me.
|
| - John 'Warthog9' Hawley
| Chief Kernel.org Admin
- --
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iD8DBQFIbWtw/E3kyWU9dicRAk15AJ0VJ2H1C6h4LzJTEr5gtR3O/f+n+QCfVjGm
B1B7vbCBeovitXQx1tRZ9+Y=
=NDJ+
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: ':/<oneline prefix>' notation doesn't support full file syntax
From: Johannes Schindelin @ 2008-07-04 0:33 UTC (permalink / raw)
To: Dana How; +Cc: Eric Raible, Junio C Hamano, git
In-Reply-To: <56b7f5510807031127j10e33f3bl516180f7a9b5b5db@mail.gmail.com>
Hi,
On Thu, 3 Jul 2008, Dana How wrote:
> I was surprised to see Dscho advocating removing this feature
> altogether.
Why is everybody surprised when I admit mistakes?
Granted, --grep did not exist when I wrote :/ but now it does, and there
is no good reason to keep an ill-defined construct in Git when we have
something better.
Ciao,
Dscho
^ permalink raw reply
* Re: ':/<oneline prefix>' notation doesn't support full file syntax
From: Johannes Schindelin @ 2008-07-04 0:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dana How, Eric Raible, git
In-Reply-To: <7v7ic2zmjp.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 3 Jul 2008, Junio C Hamano wrote:
> "Dana How" <danahow@gmail.com> writes:
>
> > I was surprised to see Dscho advocating removing this feature
> > altogether. Others proposed other command sequences which avoided :/ .
> > If :/ is now going to be extended and thus perhaps more likely to
> > appear in scripts, is now the time to change it to ? which has no
> > other special meaning to git?
>
> There are number of problems with ":/" notation, but my biggest gripe is
> that it is only slightly better than "give back a random commit".
Well, it _is_ better than that: it gives you the _newest_ matching commit,
provided that the people involved in those commits maintained their NTP
settings correctly.
The _real_ gripe you should have with the notation is what I pointed out
already: it is _ill_-defined. It _could_ match more than one commit, but
matches only _one_.
> As Dscho mentioned, --grep works much better and instead of saying:
>
> $ git diff ':/send-email' HEAD
>
> we can say:
>
> $ git diff \
> $(git log --pretty=format:%H -1 --grep=send-email master next) HEAD
>
> The error behaviour is somewhat different between the two, though.
> When you misspell what to grep, the command substitution will give empty
> and you would get an unexpected result. Being built-in, ':/' syntax can
> say "I do not find anything that match" fairly easily, and the command
> substitution version has to say something ugly like:
>
> $ git diff \
> $(
> x=$(git log --pretty=format:%H -1 --grep=send-email master next)
> case "$x" in
> ('') echo 0000000000000000000000000000000000000000 ;;
> (?) echo $x ;; esac
> ) HEAD
>
> to get a similar effect.
Again, this is the wrong way to think about it. If you grep for things,
you can get 0..infty matches, not necessarily 1.
To assume that you get at least one match is already an error.
Letting that funny "case" syntax slip by, I would suggest this command
line instead:
$ $(git log --pretty=format:'git diff %H..;' --grep=send-email \
master next)
> But the point is that you can extend it easily with the :path suffix if
> you wanted to:
>
> $ git show \
> $(git log --pretty=format:%H -1 \
> --grep=send-email):git-send-email.perl
Again, I would rather suggest pulling the ":<path>" into the format, as I
did _already_ in another mail, robustifying the whole command.
> So in short, ':/' is limited (cannot be suffixed with :path, cannot be
> told to dig down from named revs, etc.) but you can do what ':/' cannot
> do fairly easily with command substitution.
>
> However, $(git pick --all --grep=something), without suffixed modifiers
> such as ~$N and :$path, may still be common enough that it might deserve a
> short-hand ':/' (and that is why we have it).
>
> If people do not find that short-hand useful, I am not strongly opposed to
> the idea of dropping it. I personally find the notation not very useful
> cute hack anyway ;-).
It was a cute hack, and before --grep it was actually useful.
Now it is not any more,
Dscho
^ permalink raw reply
* Re: Cherry picking instead of merges.
From: Björn Steinbrink @ 2008-07-04 0:10 UTC (permalink / raw)
To: David Brown; +Cc: Linus Torvalds, git
In-Reply-To: <20080703223949.GA23092@old.davidb.org>
On 2008.07.03 15:39:49 -0700, David Brown wrote:
> On Thu, Jul 03, 2008 at 02:18:53PM -0700, Linus Torvalds wrote:
>
>> End result: you have a nice merge with nice history that actually
>> converges at a common point, but you effectively did the merge
>> resolution one commit at a time with cherry-picking (or "git rebase",
>> which is obviously just a convenient shorthand for cherry-picking
>> everything).
>
> I'm still not clear how the one-commit-at-a-time resolution gets
> recorded anywhere (except in the cherry-picking branch).
They don't get recorded. Git does not store how merges happened in some
diffs or whatever, it just stores the end result. So what Linus
suggested was to reuse the end result that you already have to create
the merge commit.
Say you have 5 commits like this:
A---B---C
\
D---E
Now there are two ways to get the changes from D and E ontop of C. The
first one is cherry-picking, which leads to:
A---B---C---D'---E'
\
D---E
The other one is merging:
A---B---C---M
\ /
D---E---/
Of course, you should end up with the same tree either way. It's just a
different way of getting towards that final state. So commit E' and
commit M, while different commits, would point to the same tree object.
Now if you want to create that merge commit M, and already have E' (like
you do, as you already did all those cherry-picks), you can use what
Linus suggested. You start the merge, ignore any conflicts and just tell
git to use the tree that E' is pointing to instead.
Björn
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox