StGIT cannot import properly from stdin

StGIT cannot import properly from stdin

[Pierre Ossman]

The import command of stgit does not handle stdin in a very sane way.
The logic expects to be able to read the patch several times which isn't
possible with stdin. So for it to work you have to give it the patch twice.

A simple solution would be to store it to disk and then handle it the
same way as if a file had been given on the command line.

Rgds
Pierre

Re: StGIT cannot import properly from stdin

[Catalin Marinas]

On 21/09/05, Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> The import command of stgit does not handle stdin in a very sane way.
> The logic expects to be able to read the patch several times which isn't
> possible with stdin. So for it to work you have to give it the patch twice.

It works for me with the latest snapshot. What might happen in your
case is a missing "---" line after the patch description. The import
command reads the sys.stdin file descriptor until the first "---"
line. After that, the git.apply() function will read the rest of the
lines and pass them to git-apply. You can put some prints in the
git.apply() function to check where it starts reading from.

Patches without description cannot be read from stdin at the moment.
Maybe I could add another option to prevent it from reading the
description.

-- 
Catalin

Re: StGIT cannot import properly from stdin

[Pierre Ossman]

Catalin Marinas wrote:

>
>It works for me with the latest snapshot. What might happen in your
>case is a missing "---" line after the patch description. The import
>command reads the sys.stdin file descriptor until the first "---"
>line. After that, the git.apply() function will read the rest of the
>lines and pass them to git-apply. You can put some prints in the
>git.apply() function to check where it starts reading from.
>
>  
>

Hmm... it seems you're kind of right. It doesn't need the patch twice.
But it needs EOF more than once. The for-loop in __parse_patch() doesn't
start until the EOF is recieved. And the second for-loop in _input()
requires a new EOF.

>Patches without description cannot be read from stdin at the moment.
>Maybe I could add another option to prevent it from reading the
>description.
>
>  
>

If you would cache the input to a file (or memory) you would remove both
of these problems.

Rgds
Pierre

Re: StGIT cannot import properly from stdin

[Catalin Marinas]

Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>>It works for me with the latest snapshot. What might happen in your
>>case is a missing "---" line after the patch description. The import
>>command reads the sys.stdin file descriptor until the first "---"
>>line. After that, the git.apply() function will read the rest of the
>>lines and pass them to git-apply. You can put some prints in the
>>git.apply() function to check where it starts reading from.
>
> Hmm... it seems you're kind of right. It doesn't need the patch twice.
> But it needs EOF more than once. The for-loop in __parse_patch() doesn't
> start until the EOF is recieved. And the second for-loop in _input()
> requires a new EOF.

The line below in __parse_patch() ensures that the 'for' loop does not
consume all the input lines:

        if re.match('---\s*$', line) or re.match('diff -', line):
            break

Unfortunately, if the '---' line is missing, it consumes all the input
lines. Is this the case with your patch?

> If you would cache the input to a file (or memory) you would remove both
> of these problems.

True, but it requires more work than simply writing the patch to a
file and reading it twice.

-- 
Catalin

Re: StGIT cannot import properly from stdin

[Pierre Ossman]

Catalin Marinas wrote:

>
>Unfortunately, if the '---' line is missing, it consumes all the input
>lines. Is this the case with your patch?
>
>  
>

Nope. It consumes the correct ammount of lines. The problem is that I
have to give it a kick (^D) to start things twice. The first for
__parse_patch() and the second for _input(). I put some debug-prints in
there and the for-loops will not start executing on the first complete
line, only EOF.

>>If you would cache the input to a file (or memory) you would remove both
>>of these problems.
>>    
>>
>
>True, but it requires more work than simply writing the patch to a
>file and reading it twice.
>
>  
>

If it supports everything when given a filename then shouldn't adding a
preprocessing step to dump stdin to disk give the same code path?

Rgds
Pierre

Re: StGIT cannot import properly from stdin

[Catalin Marinas]

Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>>Unfortunately, if the '---' line is missing, it consumes all the input
>>lines. Is this the case with your patch?
>
> Nope. It consumes the correct ammount of lines. The problem is that I
> have to give it a kick (^D) to start things twice. The first for
> __parse_patch() and the second for _input(). I put some debug-prints in
> there and the for-loops will not start executing on the first complete
> line, only EOF.

I tried it with reading from the terminal directly and you are right.

Using 'stg import -n patch < patch-file' is fine since the stdin is
not re-opened and the _input() loop starts automatically.

'cat | stg import -n patch' also works fine since cat exits when stdin
receives EOF.

It looks like a problem with Python since it re-opens sys.stdin once
it receives EOF.

>>>If you would cache the input to a file (or memory) you would remove both
>>>of these problems.
>>
>>True, but it requires more work than simply writing the patch to a
>>file and reading it twice.
>
> If it supports everything when given a filename then shouldn't adding a
> preprocessing step to dump stdin to disk give the same code path?

I was referring to the 'cache the input to memory' variant, which is a
bit more complicated. Caching to a file is simple but do you need to
read patches from the terminal directly?

-- 
Catalin

Re: StGIT cannot import properly from stdin

[Pierre Ossman]

Catalin Marinas wrote:

>It looks like a problem with Python since it re-opens sys.stdin once
>it receives EOF.
>
>  
>

Bummer. I checked with the local Python guru here and he suggested
changing the for loop to an infinite while and f.readline().

>I was referring to the 'cache the input to memory' variant, which is a
>bit more complicated. Caching to a file is simple but do you need to
>read patches from the terminal directly?
>
>  
>

I don't explicitly need it, but it keeps things simple for me. I can
copy and paste the patch from my mailer without having to bother with
saving it to a temporary file.

Re: StGIT cannot import properly from stdin

[Catalin Marinas]

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

On 22/09/05, Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> Bummer. I checked with the local Python guru here and he suggested
> changing the for loop to an infinite while and f.readline().

It works, thanks for this. I attached a patch if you want to try.

--
Catalin

[-- Attachment #2: import-stdin --]
[-- Type: application/octet-stream, Size: 1313 bytes --]

Fix importing from stdin

The current stdin patch importing expects two EOFs since the 'for' loop
doesn't start before one EOF is received. As suggested, this patch changes
the 'for' loop with a 'while True' loop.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---

 stgit/commands/imprt.py |    6 +++++-
 stgit/git.py            |    5 ++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py
--- a/stgit/commands/imprt.py
+++ b/stgit/commands/imprt.py
@@ -134,7 +134,11 @@ def __parse_patch(filename = None):
     authname = authemail = authdate = None
 
     descr = ''
-    for line in f:
+    while True:
+        line = f.readline()
+        if not line:
+            break
+
         # the first 'Signed-of-by:' is the author
         if not authname and re.match('signed-off-by:\s+', line, re.I):
             auth = re.findall('^.*?:\s+(.*)$', line)[0]
diff --git a/stgit/git.py b/stgit/git.py
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -113,7 +113,10 @@ def get_conflicts():
 
 def _input(cmd, file_desc):
     p = popen2.Popen3(cmd)
-    for line in file_desc:
+    while True:
+        line = file_desc.readline()
+        if not line:
+            break
         p.tochild.write(line)
     p.tochild.close()
     if p.wait():

Re: StGIT cannot import properly from stdin

[Catalin Marinas]

Catalin Marinas <catalin.marinas@gmail.com> wrote:
> It works, thanks for this. I attached a patch if you want to try.

Oops, gmail attached it as a binary. It is inserted below (as
text/plain):

======================================================
Fix importing from stdin

The current stdin patch importing expects two EOFs since the 'for' loop
doesn't start before one EOF is received. As suggested, this patch changes
the 'for' loop with a 'while True' loop.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---

 stgit/commands/imprt.py |    6 +++++-
 stgit/git.py            |    5 ++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py
--- a/stgit/commands/imprt.py
+++ b/stgit/commands/imprt.py
@@ -134,7 +134,11 @@ def __parse_patch(filename = None):
     authname = authemail = authdate = None
 
     descr = ''
-    for line in f:
+    while True:
+        line = f.readline()
+        if not line:
+            break
+
         # the first 'Signed-of-by:' is the author
         if not authname and re.match('signed-off-by:\s+', line, re.I):
             auth = re.findall('^.*?:\s+(.*)$', line)[0]
diff --git a/stgit/git.py b/stgit/git.py
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -113,7 +113,10 @@ def get_conflicts():
 
 def _input(cmd, file_desc):
     p = popen2.Popen3(cmd)
-    for line in file_desc:
+    while True:
+        line = file_desc.readline()
+        if not line:
+            break
         p.tochild.write(line)
     p.tochild.close()
     if p.wait():