Patch 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... :)

stgit/commands/imprt.py def __create_patch(filename, message, author_name, author_email, · line 194

Quoted 8 lines

stgit/commands/imprt.py +8 -0

+        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.

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. :-)

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. :-)

stgit/commands/imprt.py +5 -0

@@ -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
+

stgit/commands/imprt.py +2 -0

+        else:
+            f = file(filename)
     else:
         f = sys.stdin
 

Quoted 11 lines · Clark Williams

stgit/commands/imprt.py +5 -0

@@ -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
+

stgit/commands/imprt.py def __create_patch(filename, message, author_name, author_email, · line 194

Quoted 10 lines

stgit/commands/imprt.py +8 -1

     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 ...

Quoted 9 lines

stgit/commands/imprt.py +2 -0

+        else:
+            f = file(filename)
     else:
         f = sys.stdin
 

@@ -197,11 +211,6 @@ def __import_file(filename, options, patch = None):
     if filename:
         f.close()
 

stgit/commands/imprt.py def __import_file(filename, options, patch = None): · line 204

Quoted 5 lines

stgit/commands/imprt.py +0 -5

-    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_.

Other than that, it looks good. But you sounded tempted to go with the idea of just trying the decompressors rather than go by the suffix? I think that'd be an improvement.

As for testing, you'd simply make two copies of one of the subtests in t1800, where you test .gz- and .bz2-compressed versions of the same patch. Should take about five minutes to write.

Seems it would just take a try/except block

with logic to make the patchname right (I still would want to remove a .gz/.bz2 suffix from the patchname).

I'll make sure I have tests to go with the next patch.

Quoted 10 lines · Clark Williams

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!

stgit/commands/imprt.py def __create_patch(filename, message, author_name, author_email, · line 194

Quoted 8 lines

stgit/commands/imprt.py +8 -0

+        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.

Quoted 5 lines · Asheesh Laroia

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)?

Quoted 4 lines

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?

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.

Quoted 2 lines · Sverre Rabbelier

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.

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.

Quoted 2 lines · Karl Hasselström

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

Quoted 3 lines

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 ;).

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'm pretty sure that anyone who really has use for the tar-file capability would be using tar files with multiple patches in them.

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.

But you're probably right that the amount of additional work would not be much more than what went into the current patch.

Quoted 2 lines · Karl Hasselström

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.

Quoted 2 lines

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.

Quoted 4 lines

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)

Quoted 2 lines

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 ;).

Quoted 4 lines · Asheesh Laroia

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)?

Quoted 4 lines

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.

Quoted 10 lines · Clark Williams

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!

stgit/commands/imprt.py def __create_patch(filename, message, author_name, author_email, · line 194

Quoted 8 lines

stgit/commands/imprt.py +8 -0

+        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.

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. :-)