On Tue, Oct 24, 2006 at 11:38:20PM -0700, Wim Lewis wrote:
> This more or less handles issue 323 --- the first patch makes sure darcs
> emits a valid tar file, without intermixing error messages; and the second
> allows more flexibility in the dist-name argument.
>
> Dist should really check the exit status of the commands it runs, though,
> and not create the tarfile if tar or the predist command has an error.
Content-Description: A patch for Darcs
>
> New patches:
>
> [Avoid mixing stderr into tar files (issue323)
> Wim Lewis <[EMAIL PROTECTED]>**20061025052202] {
> hunk ./Dist.lhs 94
> - (Null, File tarfile, Stdout)
> + (Null, File tarfile, AsIs)
> hunk ./Dist.lhs 97
> - (File tarfile, File tar_listing, Stdout)
> + (File tarfile, File tar_listing, AsIs)
This redirection should probably not be changed. Darcs is
reading the file listing of the tar file and later prints it (if
verbose mode is on). Redirecting errors to Stdout will make them
show up in the right place in what darcs prints. Redirecting to
AsIs will send them to the terminal before darcs prints the
(rest of the) output, where they might scroll away unnoticed.
> hunk ./Dist.lhs 101
> - (File tarfile, File (formerdir++"/"++dn++".tar.gz"), Stdout)
> + (File tarfile, File (formerdir++"/"++dn++".tar.gz"), AsIs)
> }
I haven't pushed my own patch that does the above to stable yet,
because it doesn't do any good before darcs can really detect
the error. Leaving the error output in the broken tar file gives
the user a chance to find out what happened when she examines
it. But it is of course arguable that printing the errors to the
terminal even if darcs exits with success would be better.
> [Tidy filenames before invoking tar
> Wim Lewis <[EMAIL PROTECTED]>**20061025061809
> Only use the last path component of --dist-name for the distribution
> name; the rest is still used when creating the final tar file. (issue323)
> ] {
> hunk ./Dist.lhs 22
> +import Data.Char ( isAlphaNum )
> hunk ./Dist.lhs 31
> +import FileName ( own_name, fn2fp, fp2fn )
> hunk ./Dist.lhs 84
> - dn <- get_dist_name opts
> + distname <- get_dist_name opts
> hunk ./Dist.lhs 88
> + resultfile <- return (formerdir++"/"++distname++".tar.gz")
> hunk ./Dist.lhs 92
> - withRecorded (withTempDir (tempdir++"/"++dn)) $ \ddir -> do
> + withRecorded (withTempDir (tempdir++"/"++(basename distname))) $ \ddir
> -> do
> hunk ./Dist.lhs 96
> - exec "tar" ["-cf", "-", reverse $ takeWhile (/='/') $ reverse ddir]
> + exec "tar" ["-cf", "-", safename $ basename ddir]
> hunk ./Dist.lhs 104
> - (File tarfile, File (formerdir++"/"++dn++".tar.gz"), AsIs)
> - putStrLn $ "Created dist as "++dn++".tar.gz"
> + (File tarfile, File resultfile, AsIs)
> + putStrLn $ "Created dist as "++resultfile
> + where
> + basename = fn2fp . own_name . fp2fn
> + safename n@(c:_) | isAlphaNum c = n
> + safename n = "./" ++ n
> }
Just a slightly off-topic nit pick. I find unified patch bundles
much easier to review. Putting "send unified" in
~/.darcs/defaults make this automatic. I'm a bit short of time,
so I haven't reviewed this second patch, but Eric did anyway.
Just time for nit-picking... ;-)
> Context:
[... ..................................................]
Wow! You might want to run 'darcs optimize' on your repo. This
will break the inventory at the last tag and the context will be
much reduced.
--
Tommy Pettersson <[EMAIL PROTECTED]>
_______________________________________________
darcs-devel mailing list
[email protected]
http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
