Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix c2hs handling in Cabal #6233

Draft
wants to merge 40 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
fbef0e3
Add sorting mechanism for chs files
vmchale Sep 11, 2019
c3e9b87
Fix nested parser bug + remove dead code
vmchale Sep 11, 2019
e74115e
Style improvements + slimming
vmchale Sep 11, 2019
cca903b
Reorder c2hs files in a library
vmchale Sep 11, 2019
2290218
Prettify
vmchale Sep 11, 2019
625423e
Update ChangeLog
vmchale Sep 11, 2019
71b6d32
Compatibility with older GHCs
vmchale Sep 11, 2019
16b00a2
Disbale warning since we're using a wrapper/template
vmchale Sep 11, 2019
8358fdf
Use Cabal prelude
vmchale Sep 11, 2019
6338732
Fix build for older GHCs
vmchale Sep 11, 2019
c018b57
Check Cabal/Distribution/C2Hs/Lexer.hs into version control for sake …
vmchale Sep 11, 2019
f721560
Use bootstraph script for Alex
vmchale Sep 11, 2019
64c7d1a
Use Makefile for lexer
vmchale Sep 11, 2019
6281535
Use makefile approach
vmchale Sep 11, 2019
bc1ed2c
Modify makefile
vmchale Sep 11, 2019
0dc1c71
Try to reduce time on Mac
vmchale Sep 11, 2019
a4a2448
Travis don't install happy
vmchale Sep 11, 2019
f71b5af
Pass verbosity appropriately + use cabal's graph/sort capabilities
vmchale Sep 13, 2019
a8c95ae
Add comments to functions
vmchale Sep 13, 2019
5f2814f
Clean up + consistent indentation
vmchale Sep 13, 2019
746ae57
Polish up lexer module w.r.t. documentation
vmchale Sep 13, 2019
2154c2c
Use revTopSort instead of reverse . topSort
vmchale Sep 13, 2019
eb3d592
Merge branch 'master' of github.com:vmchale/cabal
vmchale Sep 13, 2019
b616f0e
Polish/rewrite Distribution.C2Hs
vmchale Sep 13, 2019
243e6b8
Add a few unit tests
vmchale Sep 13, 2019
e9cb223
Add tests of c2hs import lexer
vmchale Sep 13, 2019
7b2c729
Add tests of reorderC2Hs + example source files
vmchale Sep 13, 2019
0d0ec9e
Fix documentation + escape correctly
vmchale Sep 13, 2019
0812063
Don't bother supporting .chs main file
vmchale Sep 14, 2019
34f3ff0
Don't use $> to preserve GHC 7.6.3 compatibility
vmchale Sep 14, 2019
280e647
Merge branch 'master' of github.com:haskell/cabal
vmchale Sep 16, 2019
f4c354a
Merge branch 'master' of github.com:haskell/cabal
vmchale Sep 24, 2019
d6f6feb
Merge branch 'master' of github.com:haskell/cabal
vmchale Sep 25, 2019
6b461de
Fix block comments
vmchale Sep 28, 2019
6ae37b0
Add unit tests for block comments
vmchale Sep 28, 2019
fe760c5
Move test modules
vmchale Sep 28, 2019
6031b43
Expand module-level comment
vmchale Sep 28, 2019
ca7537b
Add a module-level comment to Cabal.Distribution.C2Hs
vmchale Sep 28, 2019
5bc4c03
Merge branch 'master' of github.com:haskell/cabal
vmchale Oct 13, 2019
0bba20f
merge
vmchale Nov 22, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cabal/Cabal.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,9 @@ library
Distribution.Version
Language.Haskell.Extension
Distribution.Compat.Binary
other-modules:
Distribution.C2Hs
Distribution.C2Hs.Lexer

-- Parsec parser-related modules
build-depends:
Expand Down
1 change: 1 addition & 0 deletions Cabal/ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# 3.1.0.0 (current development version)
* TODO
* Fix dependency resolution for preprocessing `chs` files.

----

Expand Down
56 changes: 56 additions & 0 deletions Cabal/Distribution/C2Hs.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
{-# LANGUAGE RankNTypes #-}
{-# LANGUAGE FlexibleContexts #-}

-- Based on https://github.com/gtk2hs/gtk2hs/blob/master/tools/src/Gtk2HsSetup.hs#L414
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's GPLd repository, we cannot "base" anything on that code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good point.

module Distribution.C2Hs ( reorderC2Hs ) where

import Prelude()
import Distribution.Compat.Prelude
import qualified Data.Map as M
import qualified Data.Set as S
import Distribution.C2Hs.Lexer
import Distribution.ModuleName (ModuleName, components)
import Distribution.Parsec (simpleParsec)
import Distribution.Simple.Utils (dieNoVerbosity, findFileWithExtension, withUTF8FileContents)
import System.FilePath (joinPath)

reorderC2Hs :: [FilePath] -> [ModuleName] -> IO [ModuleName]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This need a proper comment, what it does, and why.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

reorderC2Hs dirs preMods = do

let findModule = findFileWithExtension [".chs"] dirs . joinPath . components

mFiles <- traverse findModule preMods

let preDeps = zip (fmap (ModDep []) preMods) mFiles

modDeps <- traverse extractDeps preDeps

let mods = reverse (sortTopological modDeps)

pure (moduleOriginal <$> mods)

data ModDep = ModDep { moduleRequires :: [ModuleName]
, moduleOriginal :: ModuleName
}

extractDeps :: (ModDep, Maybe FilePath) -> IO ModDep
extractDeps (md, Nothing) = pure md
extractDeps (md, (Just f)) = withUTF8FileContents f $ \con -> do
mods <- case getImports con of
Right ms -> case traverse simpleParsec ms of
Just ms' -> pure ms'
Nothing -> dieNoVerbosity ("Cannot parse module name in c2hs file " ++ f)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dieing here feels wrong. There should be way to fail in pure way...

We need tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed the dieNoVerbosity to a warn.

I've added some tests to ensure it works as I need it to.

Left err -> dieNoVerbosity ("Cannot parse c2hs import in " ++ f ++ ": " ++ err)
pure (md { moduleRequires = mods })

sortTopological :: [ModDep] -> [ModDep]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to Distribution.Compat.Graph to not reinvent the wheel?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; I've fixed this.

sortTopological ms = fst $ foldl' visit (([]), S.empty) (moduleOriginal <$> ms)
where
set = M.fromList (fmap (\m -> (moduleOriginal m, m)) ms)
visit (out, visited) m
| m `S.member` visited = (out,visited)
| otherwise = case m `M.lookup` set of
Nothing -> (out, m `S.insert` visited)
Just md -> (md:out', visited')
where
(out',visited') = foldl' visit (out, m `S.insert` visited) (moduleRequires md)
506 changes: 506 additions & 0 deletions Cabal/Distribution/C2Hs/Lexer.hs

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion Cabal/Distribution/Simple/PreProcess.hs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import Prelude ()
import Distribution.Compat.Prelude
import Distribution.Compat.Stack

import Distribution.C2Hs
import Distribution.Simple.PreProcess.Unlit
import Distribution.Backpack.DescribeUnitId
import Distribution.Package
Expand Down Expand Up @@ -168,7 +169,8 @@ preprocessComponent pd comp lbi clbi isSrcDist verbosity handlers = do
(CLib lib@Library{ libBuildInfo = bi }) -> do
let dirs = hsSourceDirs bi ++ [autogenComponentModulesDir lbi clbi
,autogenPackageModulesDir lbi]
for_ (map ModuleName.toFilePath $ allLibModules lib clbi) $
mods <- reorderC2Hs dirs (allLibModules lib clbi)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only for librayr components, shouldn't this be done for all components?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this should be fixed now.

for_ (map ModuleName.toFilePath mods) $
pre dirs (componentBuildDir lbi clbi) (localHandlers bi)
(CFLib flib@ForeignLib { foreignLibBuildInfo = bi, foreignLibName = nm }) -> do
let nm' = unUnqualComponentName nm
Expand Down
12 changes: 9 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
.PHONY : cabal-install-dev cabal-install-prod

LEXER_HS:=Cabal/Distribution/Fields/Lexer.hs
LEXER_CHS:=Cabal/Distribution/C2Hs/Lexer.hs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need to commit in Lexer.hs file too. So the people who do not modify Lexer.x shouldn't need to be aware of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be done.

SPDX_LICENSE_HS:=Cabal/Distribution/SPDX/LicenseId.hs
SPDX_EXCEPTION_HS:=Cabal/Distribution/SPDX/LicenseExceptionId.hs

Expand All @@ -13,21 +14,26 @@ CABALRUN := cabal new-run --enable-tests

all : exe lib

lib : $(LEXER_HS)
lib : $(LEXER_HS) $(LEXER_C2HS)
$(CABALBUILD) Cabal:libs

exe : $(LEXER_HS)
exe : $(LEXER_HS) $(LEXER_C2HS)
$(CABALBUILD) cabal-install:exes

# source generation: Lexer

lexer : $(LEXER_HS)
lexer : $(LEXER_HS) $(LEXER_CHS)

$(LEXER_HS) : boot/Lexer.x
alex --latin1 --ghc -o $@ $^
cat -s $@ > Lexer.tmp
mv Lexer.tmp $@

$(LEXER_CHS) : boot/C2HsLexer.x
alex --latin1 --ghc -o $@ $^
cat -s $@ > C2HsLexer.tmp
mv C2HsLexer.tmp $@

# source generation: SPDX

spdx : $(SPDX_LICENSE_HS) $(SPDX_EXCEPTION_HS)
Expand Down
90 changes: 90 additions & 0 deletions boot/C2HsLexer.x
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
{
{-# OPTIONS_GHC -fno-warn-unused-imports #-}
module Distribution.C2Hs.Lexer ( getImports ) where

import Control.Applicative ((<$>))
}

%wrapper "monad"

$module = [A-Za-z\.]

tokens :-
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file needs comments, what it does. Is there a reason why you chose alex, and not writing something using parsec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used alex because it seems to be the best choice for lexers and I've used it in the past for identifying dependencies in e.g. C.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I don't agree that we need to write any new alex code. It's write-only for almost any other contributor. You should been asked before making tech choice


$white+ ;
<0> "--".* ;

<0> "{-" { \_ _ -> nested_comment }

<chs> "import" { \_ _ -> alex Import }
<chs> "qualified" ;
<chs> "#}" { begin 0 }
<0> "{#" { begin chs }
<chs> $module+ { tok (\_ s -> alex (Module s)) }

<0> [^\{]+ ;
<0> $printable ;
<chs> [^\#$module]+ ;

{

data Token = Import
| Module String
| End

tok f (p,_,_,s) len = f p (take len s)

alex :: a -> Alex a
alex = pure

alexEOF :: Alex Token
alexEOF = pure End

-- | Given a 'String' containing C2Hs, return a list of modules it @{#import#}@s.
getImports :: String -> Either String [FilePath]
getImports = fmap extractDeps . lexC

-- from: https://github.com/simonmar/alex/blob/master/examples/haskell.x#L128
nested_comment :: Alex Token
nested_comment = go 1 =<< alexGetInput

where go :: Int -> AlexInput -> Alex Token
go 0 input = alexSetInput input *> alexMonadScan
go n input =
case alexGetByte input of
Nothing -> err input
Just (c, input') ->
case c of
45 ->
case alexGetByte input' of
Nothing -> err input'
Just (125,input_) -> go (n-1) input_
Just (_,input_) -> go n input_
125 ->
case alexGetByte input' of
Nothing -> err input'
Just (c',input_) -> go (addLevel c' $ n) input_
_ -> go n input'

addLevel c' = if c' == 45 then (+1) else id

err (pos,_,_,_) =
let (AlexPn _ line col) = pos in
alexError ("Error in nested comment at line " ++ show line ++ ", column " ++ show col)

extractDeps :: [Token] -> [FilePath]
extractDeps [] = []
extractDeps (Import:Module s:xs) = s : extractDeps xs
extractDeps (_:xs) = extractDeps xs

lexC :: String -> Either String [Token]
lexC = flip runAlex loop

loop :: Alex [Token]
loop = do
tok' <- alexMonadScan
case tok' of
End -> pure []
_ -> (tok' :) <$> loop

}