Build Profiling and Dynamic Ways for libraries

[chitrak7]

We need Profiling and Dynamic Ways to pass validation. These ways are required for ghci, base and rts.
@snowleopard @alpmestan

[chitrak7]

I tried doing this but this is not working.
needfile stage pkg | isLibrary pkg = pkgConfFile (Context stage pkg profilingDynamic)

[chitrak7]

@angerman @alpmestan @snowleopard

I have made some changes and obtained promising results.

SUMMARY for test run started at Sat Jun 16 00:37:07 2018 IST
0:19:36 spent to go through
6418 total tests, which gave rise to
23606 test cases, of which
17251 were skipped

28 had missing libraries
6178 expected passes
91 expected failures

0 caused framework failures
0 caused framework warnings
0 unexpected passes
56 unexpected failures
2 unexpected stat failures
The missing libraries are due to dynamic and profiling and maybe threaded libraries. Resolving this issue will hopefully bring the failures below 20. I think this will be sufficient to pass validation.

[chitrak7]

/home/chitrak/ghc/_build/stage1/lib/../lib/x86_64-linux-ghc-8.5.20180616/rts-1.0/libHSrts-1.0_thr_l.a(GC.thr_l_o)(.text+0xb): error: undefined reference to 'evacuate' /home/chitrak/ghc/_build/stage1/lib/../lib/x86_64-linux-ghc-8.5.20180616/rts-1.0/libHSrts-1.0_thr_l.a(GC.thr_l_o)(.text+0x43): error: undefined reference to 'scavenge_loop' /home/chitrak/ghc/_build/stage1/lib/../lib/x86_64-linux-ghc-8.5.20180616/rts-1.0/libHSrts-1.0_thr_l.a(GC.thr_l_o):function GarbageCollect: error: undefined reference to 'scavenge_capability_mut_lists' /home/chitrak/ghc/_build/stage1/lib/../lib/x86_64-linux-ghc-8.5.20180616/rts-1.0/libHSrts-1.0_thr_l.a(GC.thr_l_o):function GarbageCollect: error: undefined reference to 'scavenge_capability_mut_lists' /home/chitrak/ghc/_build/stage1/lib/../lib/x86_64-linux-ghc-8.5.20180616/rts-1.0/libHSrts-1.0_thr_l.a(GC.thr_l_o):function gcWorkerThread: error: undefined reference to 'scavenge_capability_mut_lists' /home/chitrak/ghc/_build/stage1/lib/../lib/x86_64-linux-ghc-8.5.20180616/rts-1.0/libHSrts-1.0_thr_l.a(MarkWeak.thr_l_o):function markWeakPtrList: error: undefined reference to 'evacuate' /home/chitrak/ghc/_build/stage1/lib/../lib/x86_64-linux-ghc-8.5.20180616/rts-1.0/libHSrts-1.0_thr_l.a(MarkWeak.thr_l_o):function scavengeLiveWeak: error: undefined reference to 'evacuate' /home/chitrak/ghc/_build/stage1/lib/../lib/x86_64-linux-ghc-8.5.20180616/rts-1.0/libHSrts-1.0_thr_l.a(MarkWeak.thr_l_o):function scavengeLiveWeak: error: undefined reference to 'evacuate' collect2: error: ld returned 1 exit status gcc' failed in phase `Linker'. (Exit code: 1)

Currently, atleast 60% of erros show tis error message. I am stuck at where to go with it.
`

[snowleopard]

@chitrak7 @alpmestan Perhaps, there should be a special test build flavour? This may simplify the logic for setting up the right ways, as well as selecting the right collection of packages to build.

[chitrak7]

But still, by changing the context of libraries, these ways should be built. But it is not.

needfile stage pkg | isLibrary pkg = pkgConfFile (Context stage pkg profilingDynamic)

This is how I modified the rule for libraries needed. But still dynamic ways were not supported.

[snowleopard]

I'm copying my comment in one of the PRs here:

I think someone needs to think about the overall architecture of the testsuite. So far there are a bunch of PRs that seem to introduce various ad hoc solutions, but I do not see a clear design of how everything should be tied together. Can you write up such a design document?

[alpmestan]

Is this really addressed yet? I don't think so.

[snowleopard]

Oops, I think this got auto-closed when merging the PR.

[chitrak7]

@alpmestan @angerman I still couldn't figure out how Hadrian makes flags while running cabal. I believe that the flags are built based on the flavor supplied and not depending on the package context, causing errors.
@snowleopard Can you look into the dead code that you mentioned. Removing it can make the process much simpler.

[alpmestan]

@alpmestan @angerman I still couldn't figure out how Hadrian makes flags while running cabal. I believe that the flags are built based on the flavor supplied and not depending on the package context, causing.

Have you taken a look at Settings.Builders.GhcCabal ?

[chitrak7]

@alpmestan Yes, I did. I made some changes in this file in #628 also. But they didn't seem to work.

[alpmestan]

I need some more details to be able to help you out here. Could you perhaps isolate those changes and show us the patch, telling us what did not work as expected?

[snowleopard]

Have you taken a look at Settings.Builders.GhcCabal?

@alpmestan Isn't this just dead code? We got rid of the dependency on ghc-cabal, so we no longer need this particular file, hence I was planning to remove it, but haven't got around to it yet.

[angerman]

I doubt that GhcCabal in itself is dead. There is some unfortunate(?) reuse of the GhcCabal builder for argument collection. The actual configuration happens in

hadrian/src/Hadrian/Haskell/Cabal/Parse.hs

Lines 118 to 160 in f319243

	-- | This function runs the equivalent of @cabal configure@ using the Cabal
	-- library directly, collecting all the configuration options and flags to be
	-- passed to Cabal before invoking it. It 'need's package database entries for
	-- the dependencies of the package the 'Context' points to.
	configurePackage :: Context -> Action ()
	configurePackage context@Context {..} = do
	putLoud $ "| Configure package " ++ quote (pkgName package)
	Cabal _ _ _ gpd _pd depPkgs <- unsafeReadCabalFile context
	-- Stage packages are those we have in this stage.
	stagePkgs <- stagePackages stage
	-- We'll need those packages in our package database.
	deps <- sequence [ pkgConfFile (context { package = pkg })
	| pkg <- depPkgs, pkg `elem` stagePkgs ]
	need deps
	-- Figure out what hooks we need.
	hooks <- case C.buildType (C.flattenPackageDescription gpd) of
	C.Configure -> pure C.autoconfUserHooks
	-- time has a "Custom" Setup.hs, but it's actually Configure
	-- plus a "./Setup test" hook. However, Cabal is also
	-- "Custom", but doesn't have a configure script.
	C.Custom -> do
	configureExists <- doesFileExist $
	replaceFileName (unsafePkgCabalFile package) "configure"
	pure $ if configureExists then C.autoconfUserHooks else C.simpleUserHooks
	-- Not quite right, but good enough for us:
	_ | package == rts ->
	-- Don't try to do post conf validation for rts. This will simply
	-- not work, due to the ld-options and the Stg.h.
	pure $ C.simpleUserHooks { C.postConf = \_ _ _ _ -> return () }
	| otherwise -> pure C.simpleUserHooks
	-- Compute the list of flags
	-- Compute the Cabal configurartion arguments
	flavourArgs <- args <$> flavour
	flagList <- interpret (target context (CabalFlags stage) [] []) flavourArgs
	argList <- interpret (target context (GhcCabal Conf stage) [] []) flavourArgs
	verbosity <- getVerbosity
	let v = if verbosity >= Loud then "-v3" else "-v0"
	liftIO $ C.defaultMainWithHooksNoReadArgs hooks gpd
	(argList ++ ["--flags=" ++ unwords flagList, v])

In general the ghcCabal package is dead, as well as any ghc-cabal/package-data.mk logic.

[snowleopard]

@angerman I think we should drop the GhcCabal builder and move all Cabal-related arguments to CabalFlags. That would make it easier to understand what's going on.

[angerman]

Well, we could rename GhcCabal to CabalArgs or something, but CabalFlags represents what cabal calls flags, e.g. -fXXX, or -f-YYY.

So we won't be able to flatten both.

[snowleopard]

Yes, we won't be able to flatten expressions, but I don't think we need a separate builder. Anyway, I will look into this -- this has been on my todo list for a while.

[chitrak7]

@alpmestan This is how I tried to change the code for GhcCabal. Earlier, it would only look at flavour ways, but I added contextWay option as well. Looks like unfortunately this is not working.

libraryArgs = do
    flavourWays <- getLibraryWays
    contextWay  <- getWay
    withGhci    <- expr ghcWithInterpreter
    dynPrograms <- dynamicGhcPrograms <$> expr flavour
    let ways = flavourWays ++ [contextWay]
    pure [ if vanilla `elem` ways
           then  "--enable-library-vanilla"
           else "--disable-library-vanilla"
         , if vanilla `elem` ways && withGhci && not dynPrograms
           then  "--enable-library-for-ghci"
           else "--disable-library-for-ghci"
         , if or [Profiling `wayUnit` way | way <- ways]
           then  "--enable-library-profiling"
           else "--disable-library-profiling"
         , if or [Dynamic `wayUnit` way | way <- ways]
           then  "--enable-shared"
           else "--disable-shared" ]

@snowleopard We are only using GhcCabal once, that too only to compute args. It is better to remove the builder and make args separately. Will certainly reduce the confusion. Should I create an issue for this?

[snowleopard]

@chitrak7 Yes, I think the removal of GhcCabal builder deserves a separate issue. Please create one!

[snowleopard]

The issue with GhcCabal and CabalFlags builders has been addressed in #658.

[snowleopard]

I believe this has been fixed.