patchPatch to allow import of compressed files

> This patch allows StGit to directly import compressed (.gz and .bz2)
> files with reasonable patch names.
>
> I do a lot of work on modified kernel trees and usually the first
> two things imported are a stable update patch followed immediately
> by an -rt patch, both of which are compressed. With this patch I can
> just copy the files down directly from kernel.org and import them,
> rather than having to keep uncompressed copies around.
>
> Hey, I'm lazy... :)

> +        if filename.endswith(".gz"):
> +            import gzip
> +            f = gzip.open(filename)
> +            pname = filename.replace(".gz", "")
> +        elif filename.endswith(".bz2"):
> +            import bz2
> +            f = bz2.BZ2File(filename, 'r')
> +            pname = filename.replace(".bz2", "")

> On 2008-06-09 13:38:55 -0500, Clark Williams wrote:
>
>> This patch allows StGit to directly import compressed (.gz and .bz2)
>> files with reasonable patch names.
>>
>> I do a lot of work on modified kernel trees and usually the first
>> two things imported are a stable update patch followed immediately
>> by an -rt patch, both of which are compressed. With this patch I can
>> just copy the files down directly from kernel.org and import them,
>> rather than having to keep uncompressed copies around.
>>
>> Hey, I'm lazy... :)
>
> Lazy is good. Thanks for the patch!
>
>> +        if filename.endswith(".gz"):
>> +            import gzip
>> +            f = gzip.open(filename)
>> +            pname = filename.replace(".gz", "")
>> +        elif filename.endswith(".bz2"):
>> +            import bz2
>> +            f = bz2.BZ2File(filename, 'r')
>> +            pname = filename.replace(".bz2", "")
>
> Some comments here:
>
>  * By my reading of the docs, the second argument to BZ2File defaults
>    to 'r' anyway, so you could omit it.

> Peanut gallery question: Why not just always try these methods and catch
> some format exception if they fail, proceeding to the next possible
> decompressor (proceeding on to no decompressor)?
>
> That way if a file is called .GZ, it will still be handled properly; in
> fact, all files would still be handled properly.  And these formats leave
> notes in the first few bytes of the file as to if they should be tried, so
> it's not as if it would come at some performance cost.

> On 2008-06-10 10:07:25 +0200, Sverre Rabbelier wrote:
>
>> How about adding in '.tar' decompression as well, -after- the '.gz'
>> decompression? That way you can have .tar.gz's and still be fine.
>
> A tar file would presumably contain more than one patch -- it'd be
> more like a directory of patches than a single patch file.

> On Tue, Jun 10, 2008 at 11:53 AM, Karl Hasselström <kha@treskal.com> wrote:
>
> > A tar file would presumably contain more than one patch -- it'd be
> > more like a directory of patches than a single patch file.
>
> Why?
> $ tar czvf mypatch.patch.tar.gz mypatch.patch

> > I'm not saying it wouldn't be nice to support it, but it's not
> > what the original poster needed, and building it would be a bunch
> > of extra work.
>
> A bunch of extra work?
> http://docs.python.org/lib/module-tarfile.html I say it'd be about
> as much work as the original patch ;).

> On Tue, Jun 10, 2008 at 12:28 PM, Karl Hasselström <kha@treskal.com> wrote:
>> If there's just one patch in the tar file, why did you use a tar file
>> in the first place instead of just gzipping?
> 
> I guess mostly habbit :P. Whenever I zip something I create a gzipped
> tarball because that's how I usually do it.
> 
>> I'm pretty sure that anyone who really has use for the tar-file
>> capability would be using tar files with multiple patches in them.
> 
> Yeah, I guess that's true for most people indeed.
> 
>> I was refering to the fact that due to tar-files in the general case
>> containing more than one patch, you'd have to modify the parts of
>> imprt.py that deal with importing multiple patches at once, in
>> addition to the parts the current patch touches.
> 
> Mhhh, yeah, but should be something like
> for patch in patches
>   applyPatch(patch)
> 

>> But you're probably right that the amount of additional work would not
>> be much more than what went into the current patch.
> 
> To just support .tar, yeah, but let's see what the author has to say
> about this ;).
> 

> On Tue, Jun 10, 2008 at 8:38 AM, Asheesh Laroia <asheesh@asheesh.org> wrote:
>> Peanut gallery question: Why not just always try these methods and catch
>> some format exception if they fail, proceeding to the next possible
>> decompressor (proceeding on to no decompressor)?
>>
>> That way if a file is called .GZ, it will still be handled properly; in
>> fact, all files would still be handled properly.  And these formats leave
>> notes in the first few bytes of the file as to if they should be tried, so
>> it's not as if it would come at some performance cost.
> 
> How about adding in '.tar' decompression as well, -after- the '.gz'
> decompression? That way you can have .tar.gz's and still be fine.
> 

> 
> Peanut gallery question: Why not just always try these methods and catch
> some format exception if they fail, proceeding to the next possible
> decompressor (proceeding on to no decompressor)?
> 
> That way if a file is called .GZ, it will still be handled properly; in
> fact, all files would still be handled properly.  And these formats
> leave notes in the first few bytes of the file as to if they should be
> tried, so it's not as if it would come at some performance cost.

> Asheesh Laroia wrote:
>>
>> Peanut gallery question: Why not just always try these methods and catch
>> some format exception if they fail, proceeding to the next possible
>> decompressor (proceeding on to no decompressor)?
>>
>> That way if a file is called .GZ, it will still be handled properly; in
>> fact, all files would still be handled properly.  And these formats
>> leave notes in the first few bytes of the file as to if they should be
>> tried, so it's not as if it would come at some performance cost.
>
> Interesting thought. Do all the decompressors throw an error if the input format
> isn't recognized?

>> +        if filename.endswith(".gz"):
>> +            import gzip
>> +            f = gzip.open(filename)
>> +            pname = filename.replace(".gz", "")
>> +        elif filename.endswith(".bz2"):
>> +            import bz2
>> +            f = bz2.BZ2File(filename, 'r')
>> +            pname = filename.replace(".bz2", "")
> 
> Some comments here:
> 
>   * By my reading of the docs, the second argument to BZ2File defaults
>     to 'r' anyway, so you could omit it.

> 
> Some comments here:
> 
>   * By my reading of the docs, the second argument to BZ2File defaults
>     to 'r' anyway, so you could omit it.
> 
>   * We try to use single quotes wherever possible (except when triple
>     quoting). You're using a mix ...
> 
>   * .replace() will happily replace anywhere in the string. Please
>     consider using stgit.util.strip_suffix() instead.
> 
> And last but not least, it'd be terrific if you'd let me bully you
> into adding .gz and .bz2 test cases for t1800-import. :-)
> 

> --- a/stgit/commands/imprt.py
> +++ b/stgit/commands/imprt.py
> @@ -178,8 +178,22 @@ def __create_patch(filename, message, author_name, author_email,
>  def __import_file(filename, options, patch = None):
>      """Import a patch from a file or standard input
>      """
> +    if patch:
> +        pname = patch
> +    else:
> +        pname = filename
> +
>      if filename:
> -        f = file(filename)
> +        if filename.endswith('.gz'):
> +            import gzip
> +            f = gzip.open(filename)
> +            pname = strip_suffix('.gz', filename)
> +        elif filename.endswith('.bz2'):
> +            import bz2
> +            f = bz2.BZ2File(filename)
> +            pname = strip_suffic('.bz2', filename)

> +        else:
> +            f = file(filename)
>      else:
>          f = sys.stdin
>  
> @@ -197,11 +211,6 @@ def __import_file(filename, options, patch = None):
>      if filename:
>          f.close()
>  
> -    if patch:
> -        pname = patch
> -    else:
> -        pname = filename
> -

> On 2008-06-10 08:54:58 -0500, Clark Williams wrote:
> 
>> --- a/stgit/commands/imprt.py
>> +++ b/stgit/commands/imprt.py
>> @@ -178,8 +178,22 @@ def __create_patch(filename, message, author_name, author_email,
>>  def __import_file(filename, options, patch = None):
>>      """Import a patch from a file or standard input
>>      """
>> +    if patch:
>> +        pname = patch
>> +    else:
>> +        pname = filename
>> +
>>      if filename:
>> -        f = file(filename)
>> +        if filename.endswith('.gz'):
>> +            import gzip
>> +            f = gzip.open(filename)
>> +            pname = strip_suffix('.gz', filename)
>> +        elif filename.endswith('.bz2'):
>> +            import bz2
>> +            f = bz2.BZ2File(filename)
>> +            pname = strip_suffic('.bz2', filename)
>                                   ^
> Here's why I keep blathering about tests! In Python, you don't have a
> compiler to catch these for you ...
> 

>> +        else:
>> +            f = file(filename)
>>      else:
>>          f = sys.stdin
>>  
>> @@ -197,11 +211,6 @@ def __import_file(filename, options, patch = None):
>>      if filename:
>>          f.close()
>>  
>> -    if patch:
>> -        pname = patch
>> -    else:
>> -        pname = filename
>> -
> 
> I just realized a problem with this that was already present in your
> first version: if patch != None, so that you set pname = patch, you
> overwrite pname since you strip the .gz/.bz2 suffixes _later_.
> 

> On 2008-06-09 13:38:55 -0500, Clark Williams wrote:
>
>> This patch allows StGit to directly import compressed (.gz and .bz2)
>> files with reasonable patch names.
>>
>> I do a lot of work on modified kernel trees and usually the first
>> two things imported are a stable update patch followed immediately
>> by an -rt patch, both of which are compressed. With this patch I can
>> just copy the files down directly from kernel.org and import them,
>> rather than having to keep uncompressed copies around.
>>
>> Hey, I'm lazy... :)
>
> Lazy is good. Thanks for the patch!
>
>> +        if filename.endswith(".gz"):
>> +            import gzip
>> +            f = gzip.open(filename)
>> +            pname = filename.replace(".gz", "")
>> +        elif filename.endswith(".bz2"):
>> +            import bz2
>> +            f = bz2.BZ2File(filename, 'r')
>> +            pname = filename.replace(".bz2", "")
>
> Some comments here:
>
>   * By my reading of the docs, the second argument to BZ2File defaults
>     to 'r' anyway, so you could omit it.
>
>   * We try to use single quotes wherever possible (except when triple
>     quoting). You're using a mix ...
>
>   * .replace() will happily replace anywhere in the string. Please
>     consider using stgit.util.strip_suffix() instead.