[darcs-users] darcs patch: Adding test for setpref predist bug 1269 (and 1 more)

[Reinier Lamers]

Hi,

Spontaneous review!

On Wednesday 04 March 2009 07:55:32 Ben Moseley wrote:
> hunk ./bugs/issue1269_setpref_predist.sh 1
> +#!/bin/sh
> +
> +set -ev
> +
> +not () { "$@" && exit 1 || :; }
> +
> +rm -rf temp
> +rm -rf dist1269.tar.gz

I suppose that doing 'rm -f' is enough to get rid of a regular file.

> +mkdir temp
> +cd temp
> +
> +darcs init
> +printf "Line1\nLine2\nLine3\n" > foo
> +darcs record -alm Base
> +
> +darcs setpref predist false
> +darcs dist -d dist1269
> +
> +# It is a bug in darcs if the dist file has been created
> +# it should /not/ be created if the predist cmd returned
> +# a non-zero exit code
> +not test -f dist1269.tar.gz
> +
> +cd ..
> +rm -rf temp
> [resolve issue1269: setpref predist - exitcode ignored bug
> ben at moseley.name**20090301122629
> Ignore-this: 90a63a90eeadc0764f413e784d3cec6d
> ] move ./bugs/issue1269_setpref_predist.sh ./tests/
> issue1269_setpref_predist.sh
> hunk ./src/Darcs/Commands/Dist.lhs 45
>                            createPartialsPristineDirectoryTree )
> import Darcs.Repository.Prefs ( get_prefval )
> import Darcs.Lock ( withTemp, withTempDir, readBinFile )
> -import Darcs.RepoPath ( toFilePath )
> +import Darcs.RepoPath ( AbsolutePath, toFilePath )
> import Darcs.Utils ( withCurrentDirectory )
> import Exec ( exec, Redirect(..) )

AbsolutePath is needed for the type signature of do_dist below

> hunk ./src/Darcs/Commands/Dist.lhs 119
>          if have_nonrange_match opts
>            then withCurrentDirectory ddir $ get_nonrange_match
> repository opts
>            else createPartialsPristineDirectoryTree repository
> path_list (toFilePath ddir)
> -        case predist of Nothing -> return ExitSuccess
> -                        Just pd -> system pd
> -        setCurrentDirectory (toFilePath tempdir)
> -        exec "tar" ["-cf", "-", safename $ takeFileName $ toFilePath
> ddir]
> -                   (Null, File tarfile, AsIs)
> -        when verb $ withTemp $ \tar_listing -> do
> -                      exec "tar" ["-tf", "-"]
> -                           (File tarfile, File tar_listing, Stdout)
> -                      to <- readBinFile tar_listing
> -                      putStr to
> -        exec "gzip" ["-c"]
> -             (File tarfile, File resultfile, AsIs)
> -        putStrLn $ "Created dist as "++resultfile
> +        ec <- case predist of Nothing -> return ExitSuccess
> +                              Just pd -> system pd
> +        if (ec == ExitSuccess) then do_dist verb tarfile tempdir ddir
> resultfile
> +            else
> +                putStrLn "Dist aborted due to predist failure"
> +
> +do_dist :: Bool -> FilePath -> AbsolutePath -> AbsolutePath ->
> FilePath -> IO ()
> +do_dist verb tarfile tempdir ddir resultfile = do
> +  setCurrentDirectory (toFilePath tempdir)
> +  exec "tar" ["-cf", "-", safename $ takeFileName $ toFilePath ddir]
> +             (Null, File tarfile, AsIs)
> +  when verb $ withTemp $ \tar_listing -> do
> +                exec "tar" ["-tf", "-"]
> +                     (File tarfile, File tar_listing, Stdout)
> +                to <- readBinFile tar_listing
> +                putStr to
> +  exec "gzip" ["-c"]
> +       (File tarfile, File resultfile, AsIs)
> +  putStrLn $ "Created dist as "++resultfile
>    where
>      safename n@(c:_) | isAlphaNum c  = n
>      safename n = "./" ++ n

This does two things. First, it adds a check for the return value of the 
predist command. Second, it factors out the code for what to do if the return 
value is 0/true to a separate function do_dist.

Reinier
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20090304/341dea72/attachment.pgp>