How to make shake return an exitcode > 0 whenever one rule instance "has failed"

[Pierre R]

Hi,

I am trying a simple shake file that have to parse for each node its associated puppet catalog. I first build a file "build.last" that serves as the build report. It needs a result for each node. Result is computed using an haskell library (not a shell call). In there I can ask what files have been parsed and included them as needed dependencies:

  buildDir <> "/build.last" %> \out -> do
    alwaysRerun
    Right nx <- liftIO $ runExceptT $ getNodes pdbapi QEmpty
    let deps = [ buildDir <> "/" <> (Text.unpack n) <> ".node" | n <- nx ^.. traverse.nodeInfoName]
    need deps
    Stdout stdout <- cmd ("cat"::String) deps
    putNormal stdout
    writeFileChanged out stdout

  buildDir <> "//*.node" %> \out -> do
    let node = dropDirectory1 (dropExtension out)
    facts <- liftIO $ mergeFacts (pref ^. prefFactsDefault) (pref ^. prefFactsOverride) <$> F.puppetDBFacts (Text.pack node) pdbapi
    r <- liftIO $ getCatalog daemon (Text.pack node) facts
    deps <- liftIO $ Set.fromList .HM.keys <$> getStats (parserStats daemon)
    need $ Text.unpack <$> Set.toList deps
    case r of
      S.Right _ -> do
        liftIO $ withFile out WriteMode (\h -> hPutDoc h ( dullgreen "Passed" <+> text node <> line))
      S.Left msg -> do
        liftIO $ withFile out WriteMode (\h -> hPutDoc h ( red "Failed" <+> text node <> line <> indent 7 (getError msg) <> line))

  "test" ~>
    need [ buildDir <> "/build.last"]



Everything works nicely except that I would like `shake` to return a exit code > 0 in case of a failing build. What would be the best way to do this ? I can parse  build.last and fail at the end of the top rule in case there is a line starting with "Failed" but it does not feel so right.

Is there any quick improve that you would immediately recommend ?

Thanks

[Neil Mitchell]

Hi Pierre,

Interesting question, and an interesting domain.

The only place that has structured information about pass/fail is the
*.node rules. If you want the information to be preserved in a
structured manner then it has to come through there. You could store
it in a file, or pass it through a custom rules, but that's getting
more complex.

Personally, I'd just look for Failed and raise a non-exit code in that
case. It's pragmatic and probably works nicely in practice.

Thanks, Neil

[Pierre R]

So following your advice, I have ended up with the following. Is this idiomatic enough ?
It all works nicely except for one little nitpick: When failing, shake output is rather verbose and I would love if `quietly $ fail "Build has failed"` would make a difference (as it does in front of `cat`)

  buildDir <> "/build.last" %> \out -> do

    Right nx <- liftIO $ runExceptT $ getNodes pdbapi QEmpty
    let deps = [ buildDir <> "/" <> (Text.unpack n) <> ".node" | n <- nx ^.. traverse.nodeInfoName]
    need deps

    Stdout stdout <- quietly $ cmd ("cat"::String) deps

    writeFileChanged out stdout

  buildDir <> "//*.node" %> \out -> do
    let node = dropDirectory1 (dropExtension out)
    facts <- liftIO $ mergeFacts (pref ^. prefFactsDefault) (pref ^. prefFactsOverride) <$> F.puppetDBFacts (Text.pack node) pdbapi
    r <- liftIO $ getCatalog daemon (Text.pack node) facts
    deps <- liftIO $ Set.fromList .HM.keys <$> getStats (parserStats daemon)
    need $ Text.unpack <$> Set.toList deps
    case r of
      S.Right _ -> do

        liftIO $ withFile out WriteMode (\h -> hPutDoc h ( dullgreen "✓" <+> text node <> line))
      S.Left msg -> do
        liftIO $ withFile out WriteMode (\h -> hPutDoc h ( char 'x' <> space <> red (text node) <> line <> indent 2 (getError msg) <> line))

  "test" ~> do
    content <- readFile' (buildDir <> "/build.last")
    putNormal content
    let hasFailure = any (\i -> head i == 'x') (lines content)
    if hasFailure
      then fail("The build has failed !")
      else liftIO $ putDoc (dullgreen "All green." <> line)

[Neil Mitchell]

One minor issue, you should probably use ("x" `isPrefixOf`) instead of
(\i -> head i == 'x'), since the second crashes on any blank lines.

If you call exitFailure do you avoid the verbosity of the error
message? If not, Shake should probably avoid capturing that error
message, to allow people to exit from within Shake.

As to whether it is idiomatic - it looks reasonable to me. Tracking
the values directly from the *.node files would be a bit more
principled, and hopefully will be a bit easier in the next release of
Shake - but it's still probably not actually necessary.

Thanks, Neil

[Pierre R]

Thanks for your help  ! It is very much appreciated.

Wish you well for your upcoming talk in London. Hope it will be recorded.

Cheers