[darcs-users] [patch195] accept issue1427 accept gzipped patch bu... (and 1 more)

Eric Kow kowey at darcs.net
Sun Mar 28 12:11:23 UTC 2010


On Sun, Mar 21, 2010 at 11:01:23 +0000, gh wrote:
> Sun Mar 21 11:56:38 CET 2010  Guillaume Hoffmann <guillaumh at gmail.com>
>   * accept issue1427 accept gzipped patch bundles in darcs apply
> 
> Sun Mar 21 11:59:08 CET 2010  Guillaume Hoffmann <guillaumh at gmail.com>
>   * resolve issue1427 accept gzipped patch bundles in darcs apply

I'll apply these two (thanks!), with a request for minor follow-up work
for the test case and some general commentary for the meat.

accept issue1427 accept gzipped patch bundles in darcs apply
------------------------------------------------------------
> Guillaume Hoffmann <guillaumh at gmail.com>**20100321105638
>  Ignore-this: 4607b02530f8de85e9e9165f993b1080
> ] addfile ./tests/issue1427_apply_gz.sh

Please add license headers as in EXAMPLE.sh

> +#!/usr/bin/env bash
> +set -ev
> +
> +rm -rf temp1 temp2
> +mkdir temp1 temp2
> +
> +cd temp2
> +darcs init
> +
> +cd ../temp1

It's easier to copy and paste blocks of testing script if you
write this out longhand, eg

cd temp2
darcs init
cd ..

cd temp1
...

This makes each block more fully self-contained.

> +cd ../temp2

Might as well do the darcs init here

Also, I was thinking it might be worthwhile to make sure some crazy
stuff still works like sending patches where a gzipped file in them...
but I guess that's somewhat pointless due to the way Darcs encodes
binaries.

But basic spirit is to not only test the straightforward stuff, but
maybe try to think of where the corner cases lie.  [I say this
hypocritically, of course]

resolve issue1427 accept gzipped patch bundles in darcs apply
-------------------------------------------------------------
> +-- | Read standard input, which may or may not be gzip compressed, directly
> +-- into a 'B.ByteString'.
> +gzReadStdin :: IO B.ByteString
> +gzReadStdin = do header <- B.hGet stdin 2

A stdin counterpart to gzReadFile.
It may make sense to just generalise this to hGzGetContents

> +                 rest   <- B.hGetContents stdin
> +                 let allStdin = B.concat [header,rest]
> +                 return $
> +                  if header /= BC.pack "\31\139"

I wonder if it'd be useful to refactor this with isGzFile somehow?  I
imagine that the difficulty is that we have no B.hLookAhead function
that we could use?

> +                   then allStdin
> +                   else let decompress = fst . gzDecompress Nothing
> +                            compressed = BL.fromChunks [allStdin]
> +                        in
> +                        B.concat $ decompress compressed

Trent dislikes these one-time-use variables.  I think he might instead
suggest something like

  B.concat $ fst . gzDecompress Nothing $ BL.fromChunks [allStdin]

I'm not sure how I feel about it either way, myself...

> hunk ./src/Darcs/Commands/Apply.lhs 66

>  applyCmd opts [unfixed_patchesfile] = withRepoLock opts $- \repository -> do
>    patchesfile <- fixFilePathOrStd opts unfixed_patchesfile
> -  ps <- useAbsoluteOrStd (B.readFile . toFilePath) (B.hGetContents stdin) patchesfile
> +  ps <- useAbsoluteOrStd (gzReadFilePS . toFilePath) gzReadStdin patchesfile
>    let from_whom = getFrom ps

This seems right.  Allow both the patch file and the input from stdin to
be gzipped.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20100328/92aff001/attachment.pgp>


More information about the darcs-users mailing list