git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* stgit error on status command
@ 2008-07-07 15:10 Jon Smirl
  2008-07-08  7:02 ` Karl Hasselström
  0 siblings, 1 reply; 10+ messages in thread
From: Jon Smirl @ 2008-07-07 15:10 UTC (permalink / raw)
  To: Git Mailing List

jonsmirl@terra:~$ stg --version
Stacked GIT 0.14.3.163.g06f9
git version 1.5.6.1
Python version 2.5.2 (r252:60911, Apr 21 2008, 11:17:30)
[GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu7)]
jonsmirl@terra:~$

jonsmirl@terra:~/fs$ stg status
Error: Unhandled exception:
Traceback (most recent call last):
  File "/usr/local/lib/python2.5/site-packages/stgit/main.py", line 278, in main
    ret = command.func(parser, options, args)
  File "/usr/local/lib/python2.5/site-packages/stgit/commands/status.py",
line 119, in func
    options.diff_flags)
  File "/usr/local/lib/python2.5/site-packages/stgit/commands/status.py",
line 75, in status
    diff_flags = diff_flags)
  File "/usr/local/lib/python2.5/site-packages/stgit/git.py", line
255, in tree_status
    for t, fn in parse_git_ls(GRun('diff-index', '-z', *args).raw_output()):
  File "/usr/local/lib/python2.5/site-packages/stgit/git.py", line
201, in parse_git_ls
    mode_a, mode_b, sha1_a, sha1_b, t = line.split(' ')
ValueError: need more than 1 value to unpack
jonsmirl@terra:~/fs$

jonsmirl@terra:~/fs$ git status
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       renamed:    arch/powerpc/boot/dts/digispeaker-alpha.dts ->
arch/powerpc/boot/dts/dspeak01.dts
#       renamed:    arch/powerpc/platforms/52xx/digispeaker_alpha.c ->
arch/powerpc/platforms/52xx/dspeak0
#
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   arch/powerpc/platforms/52xx/dspeak01.c


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: stgit error on status command
  2008-07-07 15:10 stgit error on status command Jon Smirl
@ 2008-07-08  7:02 ` Karl Hasselström
  2008-07-08 12:06   ` Jon Smirl
  0 siblings, 1 reply; 10+ messages in thread
From: Karl Hasselström @ 2008-07-08  7:02 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Git Mailing List, Catalin Marinas

Thanks for the report.

The problem is that we mis-parse the output of git diff-index when
rename detection is on (and it prints more than one filename on one
line). This happens if you give stg status the --diff-opts=-M flag,
but you didn't -- but it could also happen if you have the
stgit.diff-opts config variable set.

I'll try to get a patch out tonight. In the mean time, if you like you
should be able to work around the problem by unsetting
stgit.diff-opts.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: stgit error on status command
  2008-07-08  7:02 ` Karl Hasselström
@ 2008-07-08 12:06   ` Jon Smirl
  2008-07-08 12:25     ` Karl Hasselström
  0 siblings, 1 reply; 10+ messages in thread
From: Jon Smirl @ 2008-07-08 12:06 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Git Mailing List, Catalin Marinas

On 7/8/08, Karl Hasselström <kha@treskal.com> wrote:
> Thanks for the report.
>
>  The problem is that we mis-parse the output of git diff-index when
>  rename detection is on (and it prints more than one filename on one
>  line). This happens if you give stg status the --diff-opts=-M flag,
>  but you didn't -- but it could also happen if you have the
>  stgit.diff-opts config variable set.

I have -M in my config file. If I don't use people on lkml complain
about renames generating piles of output in the patches. I'm able to
work around the problem for the moment.

>
>  I'll try to get a patch out tonight. In the mean time, if you like you
>  should be able to work around the problem by unsetting
>  stgit.diff-opts.
>
>
>  --
>  Karl Hasselström, kha@treskal.com
>       www.treskal.com/kalle
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: stgit error on status command
  2008-07-08 12:06   ` Jon Smirl
@ 2008-07-08 12:25     ` Karl Hasselström
  2008-07-08 19:54       ` [StGit PATCH 0/2] stg status rename fix (stable) Karl Hasselström
  2008-07-08 19:57       ` [StGit PATCH] stg status rename fix (safe+experimental) Karl Hasselström
  0 siblings, 2 replies; 10+ messages in thread
From: Karl Hasselström @ 2008-07-08 12:25 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Git Mailing List, Catalin Marinas

On 2008-07-08 08:06:04 -0400, Jon Smirl wrote:

> On 7/8/08, Karl Hasselström <kha@treskal.com> wrote:
>
> > Thanks for the report.
> >
> > The problem is that we mis-parse the output of git diff-index when
> > rename detection is on (and it prints more than one filename on
> > one line). This happens if you give stg status the --diff-opts=-M
> > flag, but you didn't -- but it could also happen if you have the
> > stgit.diff-opts config variable set.
>
> I have -M in my config file. If I don't use people on lkml complain
> about renames generating piles of output in the patches. I'm able to
> work around the problem for the moment.

Oh, I'm certainly not suggesting that you shouldn't have -M in your
stgit.diff-opts -- just that dropping it for the moment is a
workaround to your problem.

Hmm. Another workaround is to pass an explicit --diff-opts="" to stg
status; that'll override your stgit.diff-opts, and avoid triggering
the bug.

But with any luck, I'll have this fixed within 4-6 hours or so.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [StGit PATCH 0/2] stg status rename fix (stable)
  2008-07-08 12:25     ` Karl Hasselström
@ 2008-07-08 19:54       ` Karl Hasselström
  2008-07-08 19:54         ` [StGit PATCH 1/2] Test "stg status" with renames Karl Hasselström
                           ` (2 more replies)
  2008-07-08 19:57       ` [StGit PATCH] stg status rename fix (safe+experimental) Karl Hasselström
  1 sibling, 3 replies; 10+ messages in thread
From: Karl Hasselström @ 2008-07-08 19:54 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Jon Smirl

This fixes the bug on the stable branch. (stgit.diff-opts doesn't
exist there, so it's less of an issue, but still.)

Also available at

  git://repo.or.cz/stgit/kha.git safe

---

Karl Hasselström (2):
      Don't allow extra diff options with "stg status"
      Test "stg status" with renames


 stgit/commands/status.py |   16 +++-------------
 stgit/git.py             |    9 ++++++---
 t/t0002-status.sh        |   11 +++++++++++
 3 files changed, 20 insertions(+), 16 deletions(-)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [StGit PATCH 1/2] Test "stg status" with renames
  2008-07-08 19:54       ` [StGit PATCH 0/2] stg status rename fix (stable) Karl Hasselström
@ 2008-07-08 19:54         ` Karl Hasselström
  2008-07-08 19:54         ` [StGit PATCH 2/2] Don't allow extra diff options with "stg status" Karl Hasselström
  2008-07-09  4:05         ` [StGit PATCH 0/2] stg status rename fix (stable) Karl Hasselström
  2 siblings, 0 replies; 10+ messages in thread
From: Karl Hasselström @ 2008-07-08 19:54 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Jon Smirl

Currently, it only works if -M is not passed to git diff-files, so the
second of the two tests fails.

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

---

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


diff --git a/t/t0002-status.sh b/t/t0002-status.sh
index 43e1ca0..69c29a0 100755
--- a/t/t0002-status.sh
+++ b/t/t0002-status.sh
@@ -171,4 +171,20 @@ test_expect_success 'Status of disappeared newborn' '
     diff -u expected.txt output.txt
 '
 
+cat > expected.txt <<EOF
+A fay
+D fie
+EOF
+test_expect_success 'Status after renaming a file' '
+    git rm foo/bar &&
+    git mv fie fay &&
+    stg status > output.txt &&
+    diff -u expected.txt output.txt
+'
+
+test_expect_failure 'Status after renaming a file (with rename detection)' '
+    stg status --diff-opts=-M > output.txt &&
+    diff -u expected.txt output.txt
+'
+
 test_done

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [StGit PATCH 2/2] Don't allow extra diff options with "stg status"
  2008-07-08 19:54       ` [StGit PATCH 0/2] stg status rename fix (stable) Karl Hasselström
  2008-07-08 19:54         ` [StGit PATCH 1/2] Test "stg status" with renames Karl Hasselström
@ 2008-07-08 19:54         ` Karl Hasselström
  2008-07-09  4:05         ` [StGit PATCH 0/2] stg status rename fix (stable) Karl Hasselström
  2 siblings, 0 replies; 10+ messages in thread
From: Karl Hasselström @ 2008-07-08 19:54 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Jon Smirl

The only extra diff options (given either with -O/--diff-opts) that
would affect "stg status" were -C and -M, and those made it crash
because it couldn't handle them. So remove those options.

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

---

 stgit/commands/status.py |   16 +++-------------
 stgit/git.py             |    9 ++++++---
 t/t0002-status.sh        |    5 -----
 3 files changed, 9 insertions(+), 21 deletions(-)


diff --git a/stgit/commands/status.py b/stgit/commands/status.py
index 20614b0..94d0b57 100644
--- a/stgit/commands/status.py
+++ b/stgit/commands/status.py
@@ -59,22 +59,18 @@ options = [make_option('-m', '--modified',
            make_option('-x', '--noexclude',
                        help = 'do not exclude any files from listing',
                        action = 'store_true'),
-           make_option('-O', '--diff-opts',
-                       help = 'options to pass to git-diff'),
            make_option('--reset',
                        help = 'reset the current tree changes',
                        action = 'store_true')]
 
 
 def status(files = None, modified = False, new = False, deleted = False,
-           conflict = False, unknown = False, noexclude = False,
-           diff_flags = []):
+           conflict = False, unknown = False, noexclude = False):
     """Show the tree status
     """
     cache_files = git.tree_status(files,
                                   unknown = (not files),
-                                  noexclude = noexclude,
-                                  diff_flags = diff_flags)
+                                  noexclude = noexclude)
     filtered = (modified or new or deleted or conflict or unknown)
 
     if filtered:
@@ -116,11 +112,5 @@ def func(parser, options, args):
             resolved_all()
             git.reset()
     else:
-        if options.diff_opts:
-            diff_flags = options.diff_opts.split()
-        else:
-            diff_flags = []
-
         status(args, options.modified, options.new, options.deleted,
-               options.conflict, options.unknown, options.noexclude,
-               diff_flags = diff_flags)
+               options.conflict, options.unknown, options.noexclude)
diff --git a/stgit/git.py b/stgit/git.py
index 8e6bdf4..35579d4 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -192,6 +192,9 @@ def ls_files(files, tree = 'HEAD', full_name = True):
             'Some of the given paths are either missing or not known to GIT'
 
 def parse_git_ls(output):
+    """Parse the output of git diff-index, diff-files, etc. Doesn't handle
+    rename/copy output, so don't feed it output generated with the -M
+    or -C flags."""
     t = None
     for line in output.split('\0'):
         if not line:
@@ -205,7 +208,7 @@ def parse_git_ls(output):
             t = None
 
 def tree_status(files = None, tree_id = 'HEAD', unknown = False,
-                  noexclude = True, verbose = False, diff_flags = []):
+                  noexclude = True, verbose = False):
     """Get the status of all changed files, or of a selected set of
     files. Returns a list of pairs - (status, filename).
 
@@ -252,7 +255,7 @@ def tree_status(files = None, tree_id = 'HEAD', unknown = False,
     # specified when calling the function (i.e. report all files) or
     # files were specified but already found in the previous step
     if not files or files_left:
-        args = diff_flags + [tree_id]
+        args = [tree_id]
         if files_left:
             args += ['--'] + files_left
         for t, fn in parse_git_ls(GRun('diff-index', '-z', *args).raw_output()):
@@ -268,7 +271,7 @@ def tree_status(files = None, tree_id = 'HEAD', unknown = False,
     # function (i.e. report all files) or files were specified but
     # already found in the previous step
     if not files or files_left:
-        args = list(diff_flags)
+        args = []
         if files_left:
             args += ['--'] + files_left
         for t, fn in parse_git_ls(GRun('diff-files', '-z', *args).raw_output()):
diff --git a/t/t0002-status.sh b/t/t0002-status.sh
index 69c29a0..a030739 100755
--- a/t/t0002-status.sh
+++ b/t/t0002-status.sh
@@ -182,9 +182,4 @@ test_expect_success 'Status after renaming a file' '
     diff -u expected.txt output.txt
 '
 
-test_expect_failure 'Status after renaming a file (with rename detection)' '
-    stg status --diff-opts=-M > output.txt &&
-    diff -u expected.txt output.txt
-'
-
 test_done

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [StGit PATCH] stg status rename fix (safe+experimental)
  2008-07-08 12:25     ` Karl Hasselström
  2008-07-08 19:54       ` [StGit PATCH 0/2] stg status rename fix (stable) Karl Hasselström
@ 2008-07-08 19:57       ` Karl Hasselström
  2008-07-08 19:57         ` [StGit PATCH] Test "stg status" with -M in stgit.diff-opts Karl Hasselström
  1 sibling, 1 reply; 10+ messages in thread
From: Karl Hasselström @ 2008-07-08 19:57 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Jon Smirl

This is just a patch to make sure the bug is gone; the bugfix consists
of a merge of the updated stable branch.

Both safe and experimental at git://repo.or.cz/stgit/kha.git has said
merge, and this test.

---

Karl Hasselström (1):
      Test "stg status" with -M in stgit.diff-opts


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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [StGit PATCH] Test "stg status" with -M in stgit.diff-opts
  2008-07-08 19:57       ` [StGit PATCH] stg status rename fix (safe+experimental) Karl Hasselström
@ 2008-07-08 19:57         ` Karl Hasselström
  0 siblings, 0 replies; 10+ messages in thread
From: Karl Hasselström @ 2008-07-08 19:57 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Jon Smirl

It used to fail, before the recent merge of the -O/--diff-opts
removal, since the default value of that option was taken from
stgit.diff-opts and passed on to stgit.git.tree_status() which
couldn't handle it.

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

---

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


diff --git a/t/t0002-status.sh b/t/t0002-status.sh
index 5e1e8ca..86ff419 100755
--- a/t/t0002-status.sh
+++ b/t/t0002-status.sh
@@ -186,4 +186,10 @@ test_expect_success 'Status after renaming a file' '
     test_cmp expected.txt output.txt
 '
 
+test_expect_success 'Status after renaming a file (with rename detection)' '
+    git config stgit.diff-opts -M &&
+    stg status > output.txt &&
+    test_cmp expected.txt output.txt
+'
+
 test_done

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [StGit PATCH 0/2] stg status rename fix (stable)
  2008-07-08 19:54       ` [StGit PATCH 0/2] stg status rename fix (stable) Karl Hasselström
  2008-07-08 19:54         ` [StGit PATCH 1/2] Test "stg status" with renames Karl Hasselström
  2008-07-08 19:54         ` [StGit PATCH 2/2] Don't allow extra diff options with "stg status" Karl Hasselström
@ 2008-07-09  4:05         ` Karl Hasselström
  2 siblings, 0 replies; 10+ messages in thread
From: Karl Hasselström @ 2008-07-09  4:05 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Jon Smirl

On 2008-07-08 21:54:19 +0200, Karl Hasselström wrote:

> This fixes the bug on the stable branch. (stgit.diff-opts doesn't
> exist there, so it's less of an issue, but still.)
>
> Also available at
>
>   git://repo.or.cz/stgit/kha.git safe

s/safe/stable/ here, of course.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2008-07-09  4:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-07 15:10 stgit error on status command Jon Smirl
2008-07-08  7:02 ` Karl Hasselström
2008-07-08 12:06   ` Jon Smirl
2008-07-08 12:25     ` Karl Hasselström
2008-07-08 19:54       ` [StGit PATCH 0/2] stg status rename fix (stable) Karl Hasselström
2008-07-08 19:54         ` [StGit PATCH 1/2] Test "stg status" with renames Karl Hasselström
2008-07-08 19:54         ` [StGit PATCH 2/2] Don't allow extra diff options with "stg status" Karl Hasselström
2008-07-09  4:05         ` [StGit PATCH 0/2] stg status rename fix (stable) Karl Hasselström
2008-07-08 19:57       ` [StGit PATCH] stg status rename fix (safe+experimental) Karl Hasselström
2008-07-08 19:57         ` [StGit PATCH] Test "stg status" with -M in stgit.diff-opts Karl Hasselström

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).