# [PATCH - stgit] Patch to allow import of compressed files 18 messages from 2008-06-09 to 2008-06-19. Participants: Clark Williams, Karl Hasselström, Asheesh Laroia, Sverre Rabbelier, David Kågedal. Thread: https://gitlist.dev/t/13876 ## Clark Williams, 2008-06-09 18:38 Subject: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <484D78BF.6030504@gmail.com> URL: https://gitlist.dev/e/484D78BF.6030504%40gmail.com ``` -----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----- From: Clark Williams Patch to allow import from compressed (.gz and .bz2) files Signed-off-by: Clark Williams --- 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) ``` ## Karl Hasselström, 2008-06-10 06:33 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <20080610063328.GB26965@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20080610063328.GB26965%40diana.vm.bytemark.co.uk In-Reply-To: <484D78BF.6030504@gmail.com> ``` 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 ``` ## Asheesh Laroia, 2008-06-10 06:38 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.10.0806092335420.17180%40swallowtail In-Reply-To: <20080610063328.GB26965@diana.vm.bytemark.co.uk> ``` 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. ``` ## Sverre Rabbelier, 2008-06-10 08:07 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: URL: https://gitlist.dev/e/bd6139dc0806100107y415a292eqe22af7a7b2215e65%40mail.gmail.com In-Reply-To: ``` On Tue, Jun 10, 2008 at 8:38 AM, 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. 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 ``` ## Karl Hasselström, 2008-06-10 09:53 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <20080610095349.GA30119@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20080610095349.GA30119%40diana.vm.bytemark.co.uk In-Reply-To: ``` 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 ``` ## Sverre Rabbelier, 2008-06-10 09:57 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: URL: https://gitlist.dev/e/bd6139dc0806100257k4700d364occ08457279e8a735%40mail.gmail.com In-Reply-To: <20080610095349.GA30119@diana.vm.bytemark.co.uk> ``` On Tue, Jun 10, 2008 at 11:53 AM, Karl Hasselström 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 ``` ## Karl Hasselström, 2008-06-10 10:28 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <20080610102800.GB30119@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20080610102800.GB30119%40diana.vm.bytemark.co.uk In-Reply-To: ``` On 2008-06-10 11:57:05 +0200, Sverre Rabbelier wrote: > On Tue, Jun 10, 2008 at 11:53 AM, Karl Hasselström 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 ``` ## Sverre Rabbelier, 2008-06-10 10:33 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: URL: https://gitlist.dev/e/bd6139dc0806100333s4395cca4g52b58679d7fc5c25%40mail.gmail.com In-Reply-To: <20080610102800.GB30119@diana.vm.bytemark.co.uk> ``` On Tue, Jun 10, 2008 at 12:28 PM, Karl Hasselström 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 ``` ## Clark Williams, 2008-06-10 13:54 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <484E877E.801@gmail.com> URL: https://gitlist.dev/e/484E877E.801%40gmail.com In-Reply-To: <20080610063328.GB26965@diana.vm.bytemark.co.uk> ``` -----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----- ``` ## Clark Williams, 2008-06-10 13:54 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <484E87B2.2090506@gmail.com> URL: https://gitlist.dev/e/484E87B2.2090506%40gmail.com In-Reply-To: <20080610063328.GB26965@diana.vm.bytemark.co.uk> ``` -----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----- Patch to allow import from compressed (.gz and .bz2) files From: Clark Williams Signed-off-by: Clark Williams --- 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) ``` ## Clark Williams, 2008-06-10 13:57 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <484E8833.9030302@gmail.com> URL: https://gitlist.dev/e/484E8833.9030302%40gmail.com In-Reply-To: ``` -----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----- ``` ## Clark Williams, 2008-06-10 14:01 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <484E8947.5070508@gmail.com> URL: https://gitlist.dev/e/484E8947.5070508%40gmail.com In-Reply-To: ``` -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Sverre Rabbelier wrote: > On Tue, Jun 10, 2008 at 8:38 AM, 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. > > 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----- ``` ## Asheesh Laroia, 2008-06-10 14:04 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0806100703460.22017%40alchemy.localdomain In-Reply-To: <484E8833.9030302@gmail.com> ``` 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. ``` ## Clark Williams, 2008-06-10 14:06 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <484E8A6F.1060602@gmail.com> URL: https://gitlist.dev/e/484E8A6F.1060602%40gmail.com In-Reply-To: ``` -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Sverre Rabbelier wrote: > On Tue, Jun 10, 2008 at 12:28 PM, Karl Hasselström 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----- ``` ## Karl Hasselström, 2008-06-11 06:27 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <20080611062753.GB15034@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20080611062753.GB15034%40diana.vm.bytemark.co.uk In-Reply-To: <484E87B2.2090506@gmail.com> ``` 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 ``` ## Clark Williams, 2008-06-11 17:28 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <48500B28.1050505@gmail.com> URL: https://gitlist.dev/e/48500B28.1050505%40gmail.com In-Reply-To: <20080611062753.GB15034@diana.vm.bytemark.co.uk> ``` -----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----- ``` ## Karl Hasselström, 2008-06-11 19:14 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <20080611191408.GB23670@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20080611191408.GB23670%40diana.vm.bytemark.co.uk In-Reply-To: <48500B28.1050505@gmail.com> ``` 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 ``` ## David Kågedal, 2008-06-19 14:17 Subject: Re: [PATCH - stgit] Patch to allow import of compressed files Message-ID: <878wx14iga.fsf@lysator.liu.se> URL: https://gitlist.dev/e/878wx14iga.fsf%40lysator.liu.se In-Reply-To: <20080610063328.GB26965@diana.vm.bytemark.co.uk> ``` Karl Hasselström 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 ```