Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Print dependency graph on failure #906

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Nov 27, 2021

This is my attempt at finding out why I get get “Dependency cycle detected” errors.

  • I am not sure how should I actually output the Graphviz data, currently just printing them to stdout.
    • Maybe make Logger output to stderr.
  • It also increases dependency tree size: Nix package went from 4.5G to 4.8G. So we probably need to allow disabling it.
  • Identifiers with the same path should be grouped together.
  • Also print when the build succeeds.
  • Avoid Orphan instance PrintDot Identifier
  • Listed dependencies incomplete.
  • Ensure graph nodes are escaped.
  • Do not cluster single nodes.
    • Looks worse

      Patch
      --- a/lib/Hakyll/Core/Runtime.hs
      +++ b/lib/Hakyll/Core/Runtime.hs
      @@ -236,9 +237,12 @@ dependencyGraph fullDeps = graphElemsToDot graphParams (map labelNodes nodes) ed
           where
               nodes = S.toList (M.keysSet fullDeps `S.union` foldMap (S.map fst) fullDeps)
               edges = S.toList (M.foldMapWithKey (\ident -> S.map (\(dep, snap) -> (ident, dep, snap))) fullDeps)
      +        clusterElemCounts = foldr (\ident -> M.insertWith (+) (toFilePath ident) 1) M.empty nodes
               graphParams = defaultParams
                   { clusterBy = \(n, nl) -> C (toFilePath n) (N (n, nl))
                   , clusterID = Str . LT.pack
      +            -- Do not make paths without multiple versions into clusters.
      +            , isDotCluster = \cl -> M.findWithDefault 0 cl clusterElemCounts > 1
                   , fmtEdge = \(_, _, el) -> if null el then [] else [textLabel (LT.pack el)]
                   }
               labelNodes ident = (ident, show ident)
  • Show snapshot names.
  • Maybe move it to a separate command?

I recommend xdot for viewing the graphs. Edotor works in the web browser but does not support highlighting adjacent edges. Passing -f fdp to xdot might lay out the graph slightly more manageably.

@jtojnar jtojnar marked this pull request as draft November 27, 2021 00:31
@jtojnar jtojnar force-pushed the graphviz branch 6 times, most recently from 5299469 to 0d40ac1 Compare November 27, 2021 08:09
@jaspervdj
Copy link
Owner

This is really cool work!

I wonder how to integrate this into the existing commands. Maybe one option would be to have something like this.

This is currently the most "low-level" way to run Hakyll:

hakyllWithExitCode :: Config.Configuration -> Rules a -> IO ExitCode

If we wanted to return more diagnostic information, we could do something like:

-- Other possible names: HakyllOutput, Diagnostics?
data HakyllResult = HakyllResult
  { hakyllDependencyGraph :: ...
  , hakyllExitCode :: ExitCode
  , -- Other stuff that people may care about in the future
  }

hakyllWithResult :: Config.Configuration -> Rules a -> IO HakyllResult

And then people could use it however they want in their main:

main = do
  info <- hakyllWithResult $ ...
  writeFile "graph.dot" $ hakyllDependencyGraph info

@Minoru What do you think?

@Minoru
Copy link
Collaborator

Minoru commented Nov 28, 2021

I see how this could be used to debug our dependency-checking code, but I wonder if end users actually need this. It seems to me that it'd be better to improve our error messages instead (and debug the deps checker, of course).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants