[PATCH - stgit] Patch to allow import of compressed files

[PATCH - stgit] Patch to allow import of compressed files

[Clark Williams]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

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

Clark

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkhNeL4ACgkQqA4JVb61b9cu0ACdH/Z71xh4gaD5euF3BgYnIhiO
AkUAnipyN/dsTBQDyhc6uzFhxdxPeYvJ
=2G71
-----END PGP SIGNATURE-----

[-- Attachment #2: compressed-input.patch --]
[-- Type: text/plain, Size: 1437 bytes --]

From: Clark Williams <williams@redhat.com>

Patch to allow import from compressed (.gz and .bz2) files

Signed-off-by: Clark Williams <williams@redhat.com>
---

 stgit/commands/imprt.py |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py
index 4a4b792..83dae2f 100644
--- 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 = filename.replace(".gz", "")
+        elif filename.endswith(".bz2"):
+            import bz2
+            f = bz2.BZ2File(filename, 'r')
+            pname = filename.replace(".bz2", "")
+        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
-
     __create_patch(pname, message, author_name, author_email,
                    author_date, diff, options)
 

Re: [PATCH - stgit] Patch to allow import of compressed files

[Karl Hasselström]

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.

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

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

Re: [PATCH - stgit] Patch to allow import of compressed files

[Asheesh Laroia]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1622 bytes --]

On Tue, 10 Jun 2008, Karl Hasselström wrote:

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

-- Asheesh.

-- 
Say "twenty-three-skiddoo" to logout.

Re: [PATCH - stgit] Patch to allow import of compressed files

[Sverre Rabbelier]

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.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH - stgit] Patch to allow import of compressed files

[Karl Hasselström]

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.

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.

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

Re: [PATCH - stgit] Patch to allow import of compressed files

[Sverre Rabbelier]

On Tue, Jun 10, 2008 at 11:53 AM, Karl Hasselström <kha@treskal.com> wrote:
> 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.

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

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH - stgit] Patch to allow import of compressed files

[Karl Hasselström]

On 2008-06-10 11:57:05 +0200, Sverre Rabbelier wrote:

> 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

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

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.

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

Re: [PATCH - stgit] Patch to allow import of compressed files

[Sverre Rabbelier]

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

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH - stgit] Patch to allow import of compressed files

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Karl Hasselström wrote:
>> +        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.

Done.

> 
>   * We try to use single quotes wherever possible (except when triple
>     quoting). You're using a mix ...

I normally use single quotes too, but I've been doing a bunch of C programming
lately, so that's my excuse and I'm sticking with it. Replaced.

> 
>   * .replace() will happily replace anywhere in the string. Please
>     consider using stgit.util.strip_suffix() instead.

Ah, didn't know about strip_suffix(). Done.

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

I'll work on that. Can't do it right now, but I'll look at the test harness and see
what it'll take.

Clark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkhOh34ACgkQqA4JVb61b9ea9gCgoV1MZbT2F62WEkduOfmkgdP3
BwIAnApT1o+VttF4VRHJj4DkPmi/HXfm
=Uwho
-----END PGP SIGNATURE-----

Re: [PATCH - stgit] Patch to allow import of compressed files

[Clark Williams]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Karl Hasselström wrote:
> 
> 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. :-)
> 

Sigh. Never reply before your second cup of coffee.

Updated patch attached.

Clark

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkhOh7EACgkQqA4JVb61b9c4BgCeJH0GUzQJCDdJ8gx5287KE/KO
uTwAoKD7V4JMqHnYmFCg01ij5aBbLDbq
=Yy/n
-----END PGP SIGNATURE-----

[-- Attachment #2: compressed-input.patch --]
[-- Type: text/plain, Size: 1436 bytes --]

Patch to allow import from compressed (.gz and .bz2) files

From: Clark Williams <williams@redhat.com>

Signed-off-by: Clark Williams <williams@redhat.com>
---

 stgit/commands/imprt.py |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py
index 4a4b792..050301a 100644
--- 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
-
     __create_patch(pname, message, author_name, author_email,
                    author_date, diff, options)
 

Re: [PATCH - stgit] Patch to allow import of compressed files

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

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?

Clark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkhOiDMACgkQqA4JVb61b9fixACfZLtxY20tXyZA5oLHTBSm5JPj
ApQAnRsb6RkA2YKB5UXhXiEezMm1j1ZS
=Y7ZK
-----END PGP SIGNATURE-----

Re: [PATCH - stgit] Patch to allow import of compressed files

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sverre Rabbelier wrote:
> 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.
> 

I thought about this, as well as the .zip format. The problem is in multiple files
and how to handle them. Do you look for a series and automagically add '--series'
semantics? What about ignore/replace?  If no series file, do you just try to apply
all of them? In what order? Look for '.patch' suffix and only use those? And what
about the old '.tgz' suffix? Lots of different ways to go here.

I don't think it would be a bad thing to add .tar and .zip handling, but I'd rather
bat it around a little on this list before doing it. That way we can find out how
people would like to use it.

Clark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkhOiUcACgkQqA4JVb61b9cWawCeK+n8uo8XGGlURYG2ImuhUUNY
eZEAn31ZfjC7MgiP1VZ02uksrRJQiQVe
=Tu6s
-----END PGP SIGNATURE-----

Re: [PATCH - stgit] Patch to allow import of compressed files

[Asheesh Laroia]

On Tue, 10 Jun 2008, Clark Williams wrote:

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

Yup!  (Try it!)

-- Asheesh.

-- 
You might have mail.

Re: [PATCH - stgit] Patch to allow import of compressed files

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sverre Rabbelier wrote:
> 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, since patches in general are ordered beasts, you can get into trouble if you
didn't name your patches such that they lexically sort in the proper order *and* the
tar library gives them to you in the right order.

I think if I were going to add this, I'd probably look for a series file and use that
 to apply in the proper order. Same for if we support .zip files. Seems like the only
other thing you could do is read in the list of files and sort them, then apply them.

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

Author. Mmmmmmm, much nicer than "Mad Python patch hacker". I like it!

Clark

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEUEARECAAYFAkhOim8ACgkQqA4JVb61b9eMBACWIoP2sEcIdH8+2R60NN26UB/b
PQCgozvd0Kkonjz7xJlMN5cJqpVzhkY=
=UOWy
-----END PGP SIGNATURE-----

Re: [PATCH - stgit] Patch to allow import of compressed files

[Karl Hasselström]

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

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.

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

Re: [PATCH - stgit] Patch to allow import of compressed files

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Karl Hasselström wrote:
> 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 ...
> 

Yup, caught that right after I sent the updated patch :)

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

Ah, I didn't realize that patchname overrides all (should have). I'll fix that next
go-round.

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

Yeah, it's tempting. I'll play with it a bit and see what it would take. 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).

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

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

Clark


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkhQCygACgkQqA4JVb61b9fckgCfbjep1oC3WT3hxVSo/8y6/FVM
O8AAn1GEGjvbwerEY0U0N1UlJC8Lv37R
=RHjB
-----END PGP SIGNATURE-----

Re: [PATCH - stgit] Patch to allow import of compressed files

[Karl Hasselström]

On 2008-06-11 12:28:08 -0500, Clark Williams wrote:

> Seems it would just take a try/except block

Yes. The only trick here is to catch the specific type of exception
you want, and nothing else. You can easily try it out at an
interactive Python prompt and just see what type of exception is
thrown.

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

Indeed. (And now that's a bit more work, since you'll want to ignore
capitalization -- and you can't be sure the suffix is actually there.)

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

Excellent!

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

Re: [PATCH - stgit] Patch to allow import of compressed files

[David Kågedal]

Karl Hasselström <kha@treskal.com> writes:

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

Or use os.path.splitext(filename) which will save you a couple of
endswith calls as well.

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

-- 
David Kågedal <david@kagedal.org>