Bootstrapping fix #8085
Bootstrapping fix #8085
Conversation
| @@ -215,7 +215,8 @@ instance A.ToJSON SrcType where | |||
| ------------------------------------------------------------------------------- | |||
|
|
|||
| info :: String -> IO () | |||
| info msg = hPutStrLn stderr $ "INFO: " ++ msg | |||
| info _msg = return () | |||
There was a problem hiding this comment.
Yes... The bootstrap generation was uselessly verbose (dumping the whole json output and this list of all dependencies), part of what made finding this bug difficult because it kept getting hidden in the output.
And then I couldn't think of a better way to do this, and thought it might be fine for a tool such as this -- if someone is interested in the debug output they can easily change this.
Other options I've considered:
- leave this in, but redirect stderr from the Makefile -- hard to get right since stderr vs stdout is already tricky with using
cabal run(we don't want to hidecabal's stderr) - remove the logging entirely
- disable the logging by default, add a
-vflag to turn it on (seemed like overcomplicating things)
Happy for alternate suggestions (and also happy to just revert this part if preferred).
There was a problem hiding this comment.
i would go for 2 or 3, maybe using a env var would be slightly simple?
but current version would be fine too, it is a script at the end
There was a problem hiding this comment.
I'd be quite happy to remove the logging entirely -- will wait a while for further input then maybe make that change.
There was a problem hiding this comment.
I'd be fine with an extensive code comment that says put hPutStrLn stderr $ "INFO: " ++ msg there to get detailed debug info. Basically comment out the line and say why and when it's useful.
There was a problem hiding this comment.
sounds good, I'll make that change
There was a problem hiding this comment.
Turns out I'm not completely sure I understood your suggestion. I pushed a doc comment for the disabled info function, is this good?
There was a problem hiding this comment.
Yes, this is perfect, thank you.
| -- | ||
| -- Disabled by default to keep the output tidy, replace by | ||
| -- the version with 'hPutStrLn' when debugging. | ||
| -- |
There was a problem hiding this comment.
@Mikolaj this is the new comment.
| @@ -215,7 +215,8 @@ instance A.ToJSON SrcType where | |||
| ------------------------------------------------------------------------------- | |||
|
|
|||
| info :: String -> IO () | |||
| info msg = hPutStrLn stderr $ "INFO: " ++ msg | |||
| info _msg = return () | |||
There was a problem hiding this comment.
Yes, this is perfect, thank you.
Mikolaj
added
merge me
`cabal run -vnormal+err cabal-bootstrap-gen` would output ghc build output on stdout, messing up the expected JSON output and causing empty linux-*.json. Instead, call `cabal run -v0` which silences ghc as well as cabal, unless something goes wrong.
This removes the need for passing dedicated project and build directory names and cleans up the top level directory a little bit.
- remove tee output - disable debug logging in cabal-bootstrap-gen by default
andreabedini
force-pushed
the
bootstrap-fix
branch
from
3f74ad6 to
a564ed7
Compare
I found a bug in the bootstrap generation Makefile targets while checking them against #8079:
cabal v2-run -vnormal+stderrwas (sometimes) writing GHC output to stdout, messing withthe boostrapping JSON. This fixes that, and makes things a tiny bit tidier.
Please include the following checklist in your PR:
Please also shortly describe how you tested your change. Bonus points for added tests!