snowleopard / hadrian

Hadrian: a new build system for the Glasgow Haskell Compiler. Now merged into the GHC tree!
https://gitlab.haskell.org/ghc/ghc/tree/master/hadrian
MIT License
208 stars 39 forks source link

Preliminary nofib rule #599

Closed alpmestan closed 6 years ago

alpmestan commented 6 years ago

This doesn't yet capture stdout/stderr (nor does it clean or boot, even though I think & hope that the latter is done automatically), so we can't really use nofib-analyse yet (see the nofib trac page).

Still a WIP. Meant to address https://github.com/snowleopard/hadrian/issues/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.stderr under my build root.

snowleopard commented 6 years ago

@alpmestan Thanks!

Perhaps you can capture stdout and stderr directly without intermediate files as in the examples below?

https://hackage.haskell.org/package/shake-0.16.4/docs/Development-Shake.html#v:cmd https://hackage.haskell.org/package/shake-0.16.4/docs/Development-Shake.html#v:command

alpmestan commented 6 years ago

Perhaps you can capture stdout and stderr directly without intermediate files as in the examples below?

So you mean something like:

(Stdout out, Stderr err) <- cmd ...

?

snowleopard commented 6 years ago

@alpmestan Yes! This might require introducing a new buildWith primitive, but it will be generally useful.

alpmestan commented 6 years ago

Oh, there even is an Stdouterr newtype with a suitable CmdResult instance. This would mirror the first part of the following command:

$ make 2>&1 | tee nofib-log

that is required to run nofib (see here).

I can then write the resulting bytestring to a file under <build root> myself and achieve what I want. The entire process for running nofib however is:

$ cd nofib
$ make clean
$ make boot                  # Generates Makefile dependencies
$ make 2>&1 | tee nofib-log  # Compiles and runs benchmarks one by one

So I suppose I would be better off with a dedicated builder for nofib that runs all those commands, instead of having to special-case on Make "nofib". Getting this whole set of commands to leave the source tree untouched and to put everything under the build root is not something I'm going to look into right now. :-)

snowleopard commented 6 years ago

@alpmestan Sounds good :) Please ping me when you think this PR is ready.

alpmestan commented 6 years ago

@snowleopard This PR in its current state lets me run nofib to completion with ./build.sh -j4 --build-flavour=_foo nofib from a booted & configured source tree. nofib log available here.

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.

snowleopard commented 6 years ago

@alpmestan The PR looks good to me now -- do you think it is ready to be merged?

alpmestan commented 6 years ago

Yes, I'm happy with this patch as it is now. Merge when you want :-)

snowleopard commented 6 years ago

Done, thank you :-)

snowleopard commented 6 years ago

@alpmestan Wait, it looks like this PR introduced recursive rules -- could you please have a look and fix?

snowleopard commented 6 years ago

Once again, we are hit by now working CI -- we should give the highest priority to fixing it!

alpmestan commented 6 years ago

@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?

snowleopard commented 6 years ago

@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 build selftest to reproduce (but I won't have a chance to try on my machine until evening).

snowleopard commented 6 years ago

@alpmestan To be more precise, you should try build -c selftest to reproduce.

alpmestan commented 6 years ago

I managed to reproduce with build -c selftest. The patch at #606 makes the problem go away for me.