* [StGIT PATCH] Implement a new patch identification scheme and id command
@ 2008-06-14 7:28 Catalin Marinas
2008-06-14 7:32 ` Catalin Marinas
2008-06-14 9:47 ` Karl Hasselström
0 siblings, 2 replies; 6+ messages in thread
From: Catalin Marinas @ 2008-06-14 7:28 UTC (permalink / raw)
To: git
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 7913 bytes --]
The new scheme allows '[<branch>:]<patch>' and '[<branch>:]{base}'
(the latter showing the base of a stack). The former format allows
symbols like ^, ^{...} etc.
Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---
stgit/commands/common.py | 32 ++++++++++++++++++++++++++++++++
stgit/commands/id.py | 25 +++++++++++--------------
stgit/lib/git.py | 4 ++--
t/t0001-subdir-branches.sh | 26 +++++++++++---------------
t/t1200-push-modified.sh | 2 +-
t/t1201-pull-trailing.sh | 2 +-
t/t2200-rebase.sh | 2 +-
7 files changed, 59 insertions(+), 34 deletions(-)
diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index 029ec65..d1545e4 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 git as libgit
# Command exception class
class CmdException(StgException):
@@ -116,6 +117,37 @@ def git_id(crt_series, rev):
raise CmdException, 'Unknown patch or revision: %s' % rev
+def git_sha1(repository, branch, name):
+ """Return the SHA1 value if 'name' is a patch name or Git commit.
+ The patch names allowed are in the form '<branch>:<patch>' and can be
+ followed by standard symbols used by git-rev-parse. If <patch> is '{base}',
+ it represents the bottom of the stack.
+ """
+ # Try a Git commit first
+ try:
+ return repository.rev_parse(name, discard_stderr = True).sha1
+ except libgit.RepositoryException:
+ pass
+
+ # Git didn't recognise it, try a [branch:]patch name
+ try:
+ branch, patch = name.split(':', 1)
+ except ValueError:
+ patch = name
+ if not branch:
+ branch = repository.current_branch_name
+
+ # the stack base
+ if patch == '{base}':
+ return repository.get_stack(branch).base.sha1
+
+ # any other combination of branch and patch
+ try:
+ return repository.rev_parse('patches/%s/%s' % (branch, patch),
+ discard_stderr = True).sha1
+ except libgit.RepositoryException:
+ raise CmdException('%s: Unknown patch or revision name' % name)
+
def check_local_changes():
if git.local_changes():
raise CmdException('local changes in the tree. Use "refresh" or'
diff --git a/stgit/commands/id.py b/stgit/commands/id.py
index 94b0229..50f6e8a 100644
--- a/stgit/commands/id.py
+++ b/stgit/commands/id.py
@@ -15,29 +15,26 @@ 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
from optparse import OptionParser, make_option
-from stgit.commands.common import *
-from stgit.utils import *
-from stgit.out import *
-from stgit import stack, git
-
+from stgit.out import out
+from stgit.commands import common
+from stgit.lib import stack
help = 'print the GIT hash value of a StGIT reference'
usage = """%prog [options] [id]
-Print the hash value of a GIT id (defaulting to HEAD). In addition to
-the standard GIT id's like heads and tags, this command also accepts
-'base[@<branch>]' and '[<patch>[@<branch>]][//[bottom | top]]'. If no
-'top' or 'bottom' are passed and <patch> is a valid patch name, 'top'
-will be used by default."""
+Print the SHA1 value of a Git id (defaulting to HEAD). In addition to
+the standard Git id's like heads and tags, this command also accepts
+'[<branch>:]<patch>]' and '[<branch>:]{base}]' showing the id of a patch
+or the base of the stack. If no branch is specified, it defaults to the
+current one. The bottom of a patch is accessible with the
+'[<branch>:]<patch>]^' format."""
-directory = DirectoryHasRepository()
+directory = common.DirectoryHasRepositoryLib()
options = [make_option('-b', '--branch',
help = 'use BRANCH instead of the default one')]
-
def func(parser, options, args):
"""Show the applied patches
"""
@@ -48,4 +45,4 @@ def func(parser, options, args):
else:
parser.error('incorrect number of arguments')
- out.stdout(git_id(crt_series, id_str))
+ out.stdout(common.git_sha1(directory.repository, options.branch, id_str))
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 0e0cccb..379fb52 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -422,11 +422,11 @@ class Repository(RunWithEnv):
refs = property(lambda self: self.__refs)
def cat_object(self, sha1):
return self.run(['git', 'cat-file', '-p', sha1]).raw_output()
- def rev_parse(self, rev):
+ def rev_parse(self, rev, discard_stderr = False):
try:
return self.get_commit(self.run(
['git', 'rev-parse', '%s^{commit}' % rev]
- ).output_one_line())
+ ).discard_stderr(discard_stderr).output_one_line())
except run.RunException:
raise RepositoryException('%s: No such revision' % rev)
def get_tree(self, sha1):
diff --git a/t/t0001-subdir-branches.sh b/t/t0001-subdir-branches.sh
index 0eed3a4..cf2059a 100755
--- a/t/t0001-subdir-branches.sh
+++ b/t/t0001-subdir-branches.sh
@@ -1,6 +1,6 @@
#!/bin/sh
#
-# Copyright (c) 2006 Karl Hasselström
+# Copyright (c) 2006 Karl Hasselstr�m
#
test_description='Branch names containing slashes
@@ -18,25 +18,21 @@ test_expect_success 'Create a patch' \
stg new foo -m "Add foo.txt" &&
stg refresh'
-test_expect_success 'Old and new id with non-slashy branch' \
- 'stg id foo &&
- stg id foo// &&
- stg id foo/ &&
- stg id foo//top &&
- stg id foo/top &&
- stg id foo@master &&
- stg id foo@master//top &&
- stg id foo@master/top'
+test_expect_success 'Try id with non-slashy branch' \
+ 'stg id &&
+ stg id foo &&
+ stg id foo^ &&
+ stg id master:foo &&
+ stg id master:foo^'
test_expect_success 'Clone branch to slashier name' \
'stg branch --clone x/y/z'
-test_expect_success 'Try new form of id with slashy branch' \
+test_expect_success 'Try new id with slashy branch' \
'stg id foo &&
- stg id foo// &&
- stg id foo//top &&
- stg id foo@x/y/z &&
- stg id foo@x/y/z//top'
+ stg id foo^ &&
+ stg id x/y/z:foo &&
+ stg id x/y/z:foo^'
test_expect_success 'Try old id with slashy branch' '
! stg id foo/ &&
diff --git a/t/t1200-push-modified.sh b/t/t1200-push-modified.sh
index ba4f70c..e3c6425 100755
--- a/t/t1200-push-modified.sh
+++ b/t/t1200-push-modified.sh
@@ -36,7 +36,7 @@ test_expect_success \
(
cd foo &&
GIT_DIR=../bar/.git git-format-patch --stdout \
- $(cd ../bar && stg id base@master)..HEAD | git-am -3 -k
+ $(cd ../bar && stg id master:{base})..HEAD | git-am -3 -k
)
'
diff --git a/t/t1201-pull-trailing.sh b/t/t1201-pull-trailing.sh
index 9d70fe0..8a74873 100755
--- a/t/t1201-pull-trailing.sh
+++ b/t/t1201-pull-trailing.sh
@@ -30,7 +30,7 @@ test_expect_success \
'Port those patches to orig tree' \
'(cd foo &&
GIT_DIR=../bar/.git git-format-patch --stdout \
- $(cd ../bar && stg id base@master)..HEAD |
+ $(cd ../bar && stg id master:{base})..HEAD |
git-am -3 -k
)
'
diff --git a/t/t2200-rebase.sh b/t/t2200-rebase.sh
index ec2a104..cd43c41 100755
--- a/t/t2200-rebase.sh
+++ b/t/t2200-rebase.sh
@@ -27,7 +27,7 @@ test_expect_success \
'Rebase to previous commit' \
'
stg rebase master~1 &&
- test `stg id base@stack` = `git rev-parse master~1` &&
+ test `stg id stack:{base}` = `git rev-parse master~1` &&
test `stg applied | wc -l` = 1
'
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [StGIT PATCH] Implement a new patch identification scheme and id command
2008-06-14 7:28 [StGIT PATCH] Implement a new patch identification scheme and id command Catalin Marinas
@ 2008-06-14 7:32 ` Catalin Marinas
2008-06-14 9:47 ` Karl Hasselström
1 sibling, 0 replies; 6+ messages in thread
From: Catalin Marinas @ 2008-06-14 7:32 UTC (permalink / raw)
To: git; +Cc: Karl Hasselström
2008/6/14 Catalin Marinas <catalin.marinas@gmail.com>:
> --- a/t/t0001-subdir-branches.sh
> +++ b/t/t0001-subdir-branches.sh
> @@ -1,6 +1,6 @@
> #!/bin/sh
> #
> -# Copyright (c) 2006 Karl Hasselström
> +# Copyright (c) 2006 Karl Hasselstr�m
I'll remove this hunk :-) (not sure how it got here).
--
Catalin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [StGIT PATCH] Implement a new patch identification scheme and id command
2008-06-14 7:28 [StGIT PATCH] Implement a new patch identification scheme and id command Catalin Marinas
2008-06-14 7:32 ` Catalin Marinas
@ 2008-06-14 9:47 ` Karl Hasselström
2008-06-16 10:00 ` Catalin Marinas
1 sibling, 1 reply; 6+ messages in thread
From: Karl Hasselström @ 2008-06-14 9:47 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-14 08:28:33 +0100, Catalin Marinas wrote:
> The new scheme allows '[<branch>:]<patch>' and '[<branch>:]{base}'
> (the latter showing the base of a stack). The former format allows
> symbols like ^, ^{...} etc.
I like your choices.
> +def git_sha1(repository, branch, name):
> + """Return the SHA1 value if 'name' is a patch name or Git commit.
> + The patch names allowed are in the form '<branch>:<patch>' and can be
> + followed by standard symbols used by git-rev-parse. If <patch> is '{base}',
> + it represents the bottom of the stack.
> + """
Why not return the Commit directly, and let the caller extract its
sha1 if that's what it wants?
You don't remove the old parse_rev() and git_id(), and particularly
the latter has a lot of callers. Meaning that the rest of StGit still
speaks the old syntax.
> + # Try a Git commit first
> + try:
> + return repository.rev_parse(name, discard_stderr = True).sha1
> + except libgit.RepositoryException:
> + pass
What if you have a branch or tag with the same name as a patch? This
will prefer the branch, which might not be the best choice.
> +current one. The bottom of a patch is accessible with the
> +'[<branch>:]<patch>]^' format."""
You have an extra ] here.
> -directory = DirectoryHasRepository()
> +directory = common.DirectoryHasRepositoryLib()
> options = [make_option('-b', '--branch',
> help = 'use BRANCH instead of the default one')]
Couldn't we kill this option? (And in the process, the branch argument
to git_sha1.)
> -test_expect_success 'Try new form of id with slashy branch' \
> +test_expect_success 'Try new id with slashy branch' \
Strictly speaking, this isn't so new anymore.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [StGIT PATCH] Implement a new patch identification scheme and id command
2008-06-14 9:47 ` Karl Hasselström
@ 2008-06-16 10:00 ` Catalin Marinas
2008-06-16 13:30 ` Catalin Marinas
0 siblings, 1 reply; 6+ messages in thread
From: Catalin Marinas @ 2008-06-16 10:00 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
2008/6/14 Karl Hasselström <kha@treskal.com>:
> On 2008-06-14 08:28:33 +0100, Catalin Marinas wrote:
>
>> The new scheme allows '[<branch>:]<patch>' and '[<branch>:]{base}'
>> (the latter showing the base of a stack). The former format allows
>> symbols like ^, ^{...} etc.
>
> I like your choices.
I think the initial idea belongs to Yann.
>> +def git_sha1(repository, branch, name):
>> + """Return the SHA1 value if 'name' is a patch name or Git commit.
>> + The patch names allowed are in the form '<branch>:<patch>' and can be
>> + followed by standard symbols used by git-rev-parse. If <patch> is '{base}',
>> + it represents the bottom of the stack.
>> + """
>
> Why not return the Commit directly, and let the caller extract its
> sha1 if that's what it wants?
OK, I was thinking about that. I'll convert it to git_commit(...) and
return a commit since I think it's only "stg id" that needs the SHA1
value.
> You don't remove the old parse_rev() and git_id(), and particularly
> the latter has a lot of callers. Meaning that the rest of StGit still
> speaks the old syntax.
I thought about removing them when we convert the commands to the new
infrastructure. In the meantime, I can rewrite git_id to use
git_commit directly. The parse_rev is only used by the 'pick' command
(and git_id). I'll have a look at these functions.
>> + # Try a Git commit first
>> + try:
>> + return repository.rev_parse(name, discard_stderr = True).sha1
>> + except libgit.RepositoryException:
>> + pass
>
> What if you have a branch or tag with the same name as a patch? This
> will prefer the branch, which might not be the best choice.
I can swap them and have the patch as preferred. People can be more
precise if they want the branch or tag by passing heads/... or
tags/...
>> -directory = DirectoryHasRepository()
>> +directory = common.DirectoryHasRepositoryLib()
>> options = [make_option('-b', '--branch',
>> help = 'use BRANCH instead of the default one')]
>
> Couldn't we kill this option? (And in the process, the branch argument
> to git_sha1.)
No problem with the option but I would like to keep the branch
argument to git_sha1. There might be cases where it is used like
picking multiple patches and I only specify one -B option to 'pick'. I
don't want to build the qualified patch name for every patch. I can
change the prototype of git_sha1 though (to git_commit):
def git_commit(name, repository, branch = None):
and branch needs to be passed explicitly and used instead of the
default one (when the patch name doesn't contain any).
>> -test_expect_success 'Try new form of id with slashy branch' \
>> +test_expect_success 'Try new id with slashy branch' \
>
> Strictly speaking, this isn't so new anymore.
The id format is new.
--
Catalin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [StGIT PATCH] Implement a new patch identification scheme and id command
2008-06-16 10:00 ` Catalin Marinas
@ 2008-06-16 13:30 ` Catalin Marinas
2008-06-16 14:08 ` Karl Hasselström
0 siblings, 1 reply; 6+ messages in thread
From: Catalin Marinas @ 2008-06-16 13:30 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
2008/6/16 Catalin Marinas <catalin.marinas@gmail.com>:
> 2008/6/14 Karl Hasselström <kha@treskal.com>:
>> On 2008-06-14 08:28:33 +0100, Catalin Marinas wrote:
>> You don't remove the old parse_rev() and git_id(), and particularly
>> the latter has a lot of callers. Meaning that the rest of StGit still
>> speaks the old syntax.
>
> I thought about removing them when we convert the commands to the new
> infrastructure. In the meantime, I can rewrite git_id to use
> git_commit directly. The parse_rev is only used by the 'pick' command
> (and git_id). I'll have a look at these functions.
Done that - git_id/parse_rev now use the new id mechanism. Eventually
we'll drop git_id once all the commands are converted.
>>> -directory = DirectoryHasRepository()
>>> +directory = common.DirectoryHasRepositoryLib()
>>> options = [make_option('-b', '--branch',
>>> help = 'use BRANCH instead of the default one')]
>>
>> Couldn't we kill this option? (And in the process, the branch argument
>> to git_sha1.)
>
> No problem with the option but I would like to keep the branch
> argument to git_sha1. There might be cases where it is used like
> picking multiple patches and I only specify one -B option to 'pick'.
What about supporting patch ranges with the new id format, something like:
branch:patch1..patch2,patch3
or
branch:patch1..patch2 branch:patch3
The following can be valid as well but patch3 may be on a different branch:
branch:patch1..patch2 patch3
This way we could get rid of many --branch options.
--
Catalin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [StGIT PATCH] Implement a new patch identification scheme and id command
2008-06-16 13:30 ` Catalin Marinas
@ 2008-06-16 14:08 ` Karl Hasselström
0 siblings, 0 replies; 6+ messages in thread
From: Karl Hasselström @ 2008-06-16 14:08 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
On 2008-06-16 14:30:32 +0100, Catalin Marinas wrote:
> What about supporting patch ranges with the new id format, something
> like:
>
> branch:patch1..patch2,patch3
>
> or
>
> branch:patch1..patch2 branch:patch3
Yes, that's a good idea; the endpoints of a range have to be on the
same branch no matter what, so having the branch: prefix apply to both
of the endpoint patches in "branch:patch1..patch2" is a good idea.
I'm not sure if the comma notation is worth it. And if it turns out to
have been useful, we can just advise users to write
$ stg foo branch:{p1..p2,p3}
which the shell will expand to
$ stg foo branch:p1..p2 branch:p3
> This way we could get rid of many --branch options.
Indeed.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-06-16 14:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-06-14 7:28 [StGIT PATCH] Implement a new patch identification scheme and id command Catalin Marinas
2008-06-14 7:32 ` Catalin Marinas
2008-06-14 9:47 ` Karl Hasselström
2008-06-16 10:00 ` Catalin Marinas
2008-06-16 13:30 ` Catalin Marinas
2008-06-16 14:08 ` Karl Hasselström
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).