Hi,

I haven't had a very thorough look, but I'm willing to apply patches
to the (currently) alternative build system without too much review...

On Tue, Nov 11, 2008 at 11:02:18 +0100, Petr Rockai wrote:
> this bundle should add a bunch of missing functionality to the
> (slated-to-be-default) Cabal build method, namely running the testsuite and
> producing source tarballs.

So keeping our usual attitude that changes should be slow, cautious and
deliberate (but determined!), I think we will be making this transition
over the span of at least one year.  The idea is to keep the Makefile
around as default for at least one more major release, and then keep it
optional (but deprecated) for a release after that.

Hopefully, the Cabal Team will have made some progress on our wishlist
in that year :-D

   http://wiki.darcs.net/index.html/CabalWishlist

Otherwise, there are some things for which we may have to keep
the Makefile around for even if we no longer use it for building:
- documentation (depending on wishlist progress)
- make continuous (? maybe an egg we'll have to accept breaking)
- maintainery things (automatically updating the webpage, make
  hoogleindex, etc)

Otherwise, I am applying most of these patches...

> Tue Nov 11 10:54:53 CET 2008  Petr Rockai <[EMAIL PROTECTED]>
>   * Relax the dependencies on regex-compat and mtl.

Except this one ;-)

Implement runTests hook for Cabal.
----------------------------------
This just calls the test harness almost the same way the Makefile did.
One possible objections is that we no longer have the tests_to_run
mechanism, which I found handy (although I am biased in that) because
it lets me tweak something and just re-run the same tests as last time.

But if the goal isn't to replace the Makefile test suite running right
away, we don't need to match functionality just yet.

> +harness :: String
> +harness = "perl ../tests/shell_harness"

> +isTest :: FilePath -> Bool
> +isTest = (".sh" `isSuffixOf`)
> +
> +execTests' :: TestKind -> IO ()
> +execTests' k =
> +    do fs <- getDirectoryContents "."
> +       let run = filter isTest fs
> +           cmd = harness ++ concatMap (' ':) run
> +       res <- system cmd
> +       case res of
> +         ExitSuccess -> return ()
> +         _ -> if k /= Bug then fail "Tests failed" else return ()

Perhaps soon we will be able to integrate the shell harness
directly

> +execTests :: TestKind -> String -> IO ()
> +execTests k fmt = do
> +  copyFile "dist/build/darcs/darcs" "darcs"

Hmm, I guess this makes sense if we're going to keep the Makefile
around for a while

> +  let dir = (flat k) ++ "-" ++ fmt ++ ".dir"

No parens needed here, for what it's worth.   We could also
tweak these to use the same names as the Makefile currently
does, test-hashed, test-darcs-2, etc

> +  rmRf dir

I don't think the Makefile does this, btw... (which is not to say
that we should slavishly follow the makefile...)

> +-------------------------------------------------------
> +-- More utility functions (FIXME)
> +-- copy & paste & edit: darcs wants to share these

I assumed these were just copy and paste, so I did not review them.

Bundle version and context data in Cabal source distribution.
-------------------------------------------------------------
> +  sDistHook = \ pkg lbi hooks flags -> do
> +    let pkgVer = packageVersion pkg
> +        verb = fromFlag $ sDistVerbosity flags
> +    x <- versionPatches verb pkgVer
> +    y <- context verb
> +    rewriteFile "release/distributed-version" $ show x
> +    rewriteFile "release/distributed-context" $ show y

So if I understand correctly, the purpose of shipping a
release/distributed-version and release/distributed-context is to allow
people to get a darcs --exact-version and a (+ N patches) count from a
tarball-built darcs.

Alternatively we could just bundle ThisVersion.hs but then people
would not be able to cabal build from the darcs repo.

> +    versionStateString :: Maybe Int -> Version -> String
> +    versionStateString Nothing  _ = "unknown" 
...
> +    versionStateString (Just n) _ = "+ " ++ show n ++ " patches"

hunk move...

> +  numPatchesDist <- do
> +    exist <- doesFileExist versionFile
> +    if exist then read `fmap` readFile versionFile else return Nothing

We should probably use something like maybeRead idiom here
http://www.haskell.org/pipermail/libraries/2008-February/009213.html
 
> +  contextDist <- do
> +      exist <- doesFileExist contextFile
> +      if exist then read `fmap` readFile contextFile else return Nothing

Another maybeRead

> +  return $ case (contextDarcs, contextDist) of
> +             (Just x, _) -> x
> +             (Nothing, Just x) -> x
> +             (Nothing, Nothing) -> "context not available"

Maybe we could give some advice on how to build darcs with
context

Fixes to the release/distributed-* file generation in Cabal sDistHook.
----------------------------------------------------------------------
> hunk ./Setup.lhs 226
> +      -- FIXME currently we run changes --from-tag to at least assert that 
> the
> +      -- requested version is tagged in this repository... it is a weak 
> check,
> +      -- but otherwise, my ~/_darcs context tends to gets used when running
> +      -- from an unpacked distribution
> +      rawSystemStdout verbosity "darcs"
> +                          ["changes", "--from-tag", display version ]

I'm not sure I understand this... we're trying to account for building a
tarball darcs which is unpacked in a darcs directory?  

> +parseFile :: (Read a) => String -> IO (Maybe a)
> +parseFile f = do
> +  exist <- doesFileExist f
> +  if exist then do
> +             content <- readFile f
> +             case reads content of
> +               ((s,_):_) -> return s

Hah! I was confused by this line until I actually ran the thing
and it said 'Just 554' (which is a bigger number than my release
STATE, 479, but that's probably because configure.ac is hard-coded to
2.1.1 whereas darcs.cabal is hard-coded to 2.1.0)

I'm guessing we write it as a Maybe to account for people doing cabal
sdist in a darcs with an unknown version?  It may be good to write a
small table showing are all the cases are in trying to figure out our
darcs version number, and what behaviour we expect... I'm not sure
what all variables are.  One of them is building from tarball vs
building from darcs.  Another is building from clean vs building after
making some changes or pulling some changes, maybe...

Relax the dependencies on regex-compat and mtl.
-----------------------------------------------
> Petr Rockai <[EMAIL PROTECTED]>**20081111095453] hunk ./darcs.cabal 260

> -                   regex-compat == 0.71.*,
> -                   mtl          == 1.1.*,
> +                   regex-compat >= 0.71,
> +                   mtl          >= 1.1,

Thanks, but not applying this one, because if I understand correctly,
we're supposed to put upper bounds on package dependencies (because
future packages may have non-backward compatible API changes).

See http://www.haskell.org/haskellwiki/Package_versioning_policy

I might accept more relaxed upper bounds, on the the other hand

-----------------------------------------------------------------------------
Update Cabal file lists to match reality.
Bundle the version/context data (produced by Setup.lhs sdist) in the tarball.
Include the testsuite in Cabal sdist.
-----------------------------------------------------------------------------
No comments needed :-)


-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

Attachment: signature.asc
Description: Digital signature

_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users

Reply via email to