Preliminary nofib rule #599
Conversation
|
@alpmestan Thanks! Perhaps you can capture https://hackage.haskell.org/package/shake-0.16.4/docs/Development-Shake.html#v:cmd |
src/Rules/Nofib.hs
Outdated
| nofibRules = do | ||
| "nofib" ~> do | ||
| needNofibBuilders | ||
| makePath <- builderPath (Make "") |
There was a problem hiding this comment.
I guess Make "nofib" would be a bit more appropriate (but equivalent).
So you mean something like: (Stdout out, Stderr err) <- cmd ...? |
|
@alpmestan Yes! This might require introducing a new |
|
Oh, there even is an $ make 2>&1 | tee nofib-logthat is required to run I can then write the resulting bytestring to a file under $ cd nofib
$ make clean
$ make boot # Generates Makefile dependencies
$ make 2>&1 | tee nofib-log # Compiles and runs benchmarks one by oneSo I suppose I would be better off with a dedicated builder for |
|
@alpmestan Sounds good :) Please ping me when you think this PR is ready. |
|
@snowleopard This PR in its current state lets me run nofib to completion with CI seems to be failing for an unrelated reason (and fails with the same error in the validation related PR). This patch is ready for review & possibly landing if you're satisfied. |
| -- by running the nofib suite and capturing | ||
| -- the relevant output. | ||
| root -/- nofibLogFile %> \fp -> do | ||
| needNofibDeps |
There was a problem hiding this comment.
I'm confused: why don't you simply execute all required commands here? What is the benefit of having a separate builder?
In the rest of Hadrian, we use builders primarily to have a unified way to track builder-specific command line arguments. But you are not using Args here, so it seems that defining a new builder is not necessary.
There was a problem hiding this comment.
OK, I'll simplify this.
|
@alpmestan The PR looks good to me now -- do you think it is ready to be merged? |
|
Yes, I'm happy with this patch as it is now. Merge when you want :-) |
|
Done, thank you :-) |
|
@alpmestan Wait, it looks like this PR introduced recursive rules -- could you please have a look and fix? |
|
Once again, we are hit by now working CI -- we should give the highest priority to fixing it! |
|
@snowleopard I think I was seeing recursive rule errors on other PRs before I made this one, and I'm definitely not seeing anything like this locally. Could you give me commands + git commit for reproducing the issue? |
|
@alpmestan Well, both AppVeyor and Travis bots fail in the same way, and I think this is the first PR where this happened, but I may be mistaken, of course. I think it is sufficient to run |
| @@ -9,8 +10,16 @@ makeBuilderArgs = do | |||
| threads <- shakeThreads <$> expr getShakeOptions | |||
| gmpPath <- expr gmpBuildPath | |||
| libffiPath <- expr libffiBuildPath | |||
| ghcPath <- expr $ | |||
| (-/-) <$> topDirectory <*> builderPath (Ghc CompileHs Stage2) | |||
There was a problem hiding this comment.
@alpmestan This could be the reason? You might want to move this into the nofib subexpression.
There was a problem hiding this comment.
Here is the error, which mentions ghc-source-path:
Build system error - indirect recursion detected:
Key value 1: OracleQ (KeyValue ("hadrian/cfg/system.config","host-os"))
Key value 2: hadrian/cfg/system.config
Key value 3: hadrian/cfg/system.config settings mk/config.h compiler/ghc.cabal
Key value 4: OracleQ (KeyValue ("hadrian/cfg/system.config","ghc-source-path"))
Rules may not be recursive
There was a problem hiding this comment.
I don't even need this anymore. Let me try getting rid of the changes to this file altogether.
|
@alpmestan To be more precise, you should try |
|
I managed to reproduce with |
* first draft of a nofib rule * address some of Andrey's feedback * refactor nofib into a proper Builder, now runs but one of the programs fails * more subtle error handling, docs * get rid of RunNofib builder, invoke commands directly
* first draft of a nofib rule * address some of Andrey's feedback * refactor nofib into a proper Builder, now runs but one of the programs fails * more subtle error handling, docs * get rid of RunNofib builder, invoke commands directly
This doesn't yet capture stdout/stderr (nor does it
cleanorboot, even though I think & hope that the latter is done automatically), so we can't really usenofib-analyseyet (see the nofib trac page).Still a WIP. Meant to address #594.
@snowleopard Any guidance for the output capture bit? I would have thought the code in Nofib.hs would accomplish what I want (well, eventually I'll want stdout and stderr in the same file), but I can't see any
nofib.stdout/nofib.stderrunder my build root.