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

Add --external flag to stack dot #437

Merged
merged 3 commits into from
Jul 1, 2015

Conversation

markus1189
Copy link
Contributor

When given --external, stack dot will include external dependencies.

By default it keeps the old behavior and only includes local packages and their interdependencies.
If --external is activated local packages will have the "dashed" line attribute so that one can distinguish local packages from external dependencies.

As an example, given a tree like this with two projects one and two:

├── [4.0K]  one
│   ├── [ 686]  one.cabal
│   └── [  46]  Setup.hs
├── [  78]  stack.yaml
└── [4.0K]  two
    ├── [  46]  Setup.hs
    └── [ 710]  two.cabal

stack dot will produce:
foo

while stack dot --external will produce:
foo

This would also fix #431.

@rvion
Copy link
Contributor

rvion commented Jun 27, 2015

as discussed in #431, should we make --external the default (without command option) and come up with an other option name (like --local-only) for current behaviour ?

anyway, that's quite minor.
thanks @markus1189

@snoyberg
Copy link
Contributor

I'd rather have the current behavior as default, with flags to force it either direction. Then we can change the default in the future and people can keep backwards compatibility easily. Let me review this patch. I can make the addition of a --no-external or equivalent myself after review unless @markus1189 beats me to it :).

@snoyberg
Copy link
Contributor

I don't think this is working as intended: it currently only goes one level deep. For example, when run on the stack repo, it tells you all of the packages that stack depends on, but not the packages that they depend on as well. Am I correct in understanding that that was the desired behavior?

@snoyberg snoyberg added this to the 0.2.0.0 milestone Jun 27, 2015
@markus1189
Copy link
Contributor Author

I did not find an easy way to resolve the PackageName into something we can use to extract further dependencies without changing even more of the original version. So currently the intended behavior is that only one level is shown.

But I agree that it would be very cool to get a full dependency graph generated. Maybe that could be done as an extension though I don't know how much work it will be.

@snoyberg
Copy link
Contributor

I can probably either add that myself tomorrow or, if you'd prefer, add information to this issue explaining where to get the missing information.

@markus1189
Copy link
Contributor Author

If you want to explain it that's great because then I can give it a shot
myself :)
On Jun 27, 2015 11:24 PM, "Michael Snoyman" notifications@github.com
wrote:

I can probably either add that myself tomorrow or, if you'd prefer, add
information to this issue explaining where to get the missing information.


Reply to this email directly or view it on GitHub
#437 (comment)
.

@snoyberg
Copy link
Contributor

OK, I'll give more details tomorrow when I'm at my computer instead of on my phone. Probably the Stack.Build.Sources module is the right place to start.

@snoyberg
Copy link
Contributor

Alright, a bit more information:

  • loadSourceMap will give you a Map from PackageName to some information on package in question
  • For local packages, you can just grab the Package value with lpPackage like the code does now
  • For non-local packages, you'll want to use withLoadPackage in Stack.Build.

I hope that helps, let me know if you have any questions

@markus1189
Copy link
Contributor Author

Yes thanks that definitely helps. I think I will have something working soon, I'll let you know.

@markus1189
Copy link
Contributor Author

Okay so I got something working.
I added two more flags, --[no-]include-base and --depth DEPTH

Usage: stack dot ([--external] | [--no-external]) ([--include-base] |
                 [--no-include-base]) [--depth DEPTH]
  Visualize your project's dependency graph using Graphviz dot

Available options:
  --external               Enable inclusion of external dependencies
  --no-external            Disable inclusion of external dependencies
  --include-base           Enable inclusion of dependencies on base
  --no-include-base        Disable inclusion of dependencies on base
  --depth DEPTH            Limit the depth of dependency resolution (Default: No
                           limit)

The reason for --include-base is that most (all?) dependencies will have a edge to base which makes the graph harder to read.

In addition --depth N limits the depth of the shown dependencies, as it will become very big for larger projects.

As an example take two cabal projects one and two

Dependencies for one:

build-depends:       base >=4.8 && <4.9, free

and for two

build-depends:       base >=4.8 && <4.9, free, mtl, transformers, one

stack dot gives
default

stack dot --external --depth 0 --no-include-base gives
ext0

stack dot --external --depth 1 --no-include-base gives
ext1

stack dot --external --depth 2 --no-include-base gives
onetwo

As an example why --no-include-base is handy, compare
stack dot --external --include-base
with_base
with stack dot --external --no-include-base
without_base

However I noticed that for packages that come with ghc it seems like I have to do something different to get the dependencies, as they are not in the SourceMap, is that correct?
That would be something I would like to change before this can be merged in addition to some tests so this is only a preview :)

@snoyberg
Copy link
Contributor

This is looking awesome. On my phone so I can't review properly, I'll provide feedback later.

@snoyberg
Copy link
Contributor

It looks like this doesn't build with GHC 7.8 due to lack of Applicative => Monad.

@markus1189
Copy link
Contributor Author

Yeah that trips me up every time because I currently only have ghc 7.10 installed :)

@snoyberg
Copy link
Contributor

You could just use stack setup ;)

@markus1189
Copy link
Contributor Author

Did I mention that my internet is SUPER slow? About 30 kB/s is the normal rate :P

@@ -87,7 +88,13 @@ mkBaseConfigOpts bopts = do
}

-- | Provide a function for loading package information from the package index
withLoadPackage :: M env m
withLoadPackage :: ( MonadIO m
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@snoyberg
Copy link
Contributor

Ouch, I think I'd go crazy at that rate :(

The patch looks good, barring the 7.8 issues I'd be happy to merge it. I don't mind fixing those up myself, would you like me to?

And thanks again for the contributions, not sure if you noticed, but:

Achievement Unlocked: Commit Bit

@markus1189
Copy link
Contributor Author

The 7.8 issue should be fixed. There is still a problem with packages included with ghc, like template-haskell, ghc-prim and others. I guess I have to use something else to get those dependencies, any ideas?

And I would like to add some tests for the graph building before this is merged.

@snoyberg
Copy link
Contributor

I'm not sure how useful it is to include dependencies of wired-in packages (those that are tied into the compiler), since their dependencies are somewhat irrelevant (they can never be reinstalled). I'd say showing the ghc package as a leaf is a good call.

I'll hold off on the merge then.

@markus1189
Copy link
Contributor Author

Is there an easy way to check whether it is wired-in? Or is checking for an empty dependency set enough?

@snoyberg
Copy link
Contributor

It's not exactly the same thing, but I'd just trust that empty dependency set means it should be displayed as a leaf. This will apply to some non-wired-in packages that are in the global database as well if I'm not mistaken.

But to answer your question much more directly: look at Stack.Constraints.wiredInPackages.

- replace type alias `M` with minimal set of required constraints
@markus1189 markus1189 force-pushed the dot-command branch 4 times, most recently from d0aa685 to 98ea4de Compare June 30, 2015 19:00
- new flag --[no-]external to include external dependencies
- new flag --[no-]include-base to toggle edges to base package
- new flag --depth to limit depth of external dependencies shown
@markus1189
Copy link
Contributor Author

Okay I think this can be merged. Packages from Stack.Constants.wiredInPackages are drawn as rectangle nodes, though there are more leaf nodes if they are in the global package db I think.
I chose not to add a "ghc" node because the graph already is complicated enough for large projects and adding this pseudo node just hinders readability.

I also added some tests although not as many as I would like to have because for that I would need to also create LocalPackages which is a little unwieldy manually :)

I split the pr into 3 commits but if you want I can also just squash them into one.

Best,
Markus

mboes added a commit that referenced this pull request Jul 1, 2015
Add --external flag to `stack dot`
@mboes mboes merged commit bf086c6 into commercialhaskell:master Jul 1, 2015
@mboes
Copy link
Contributor

mboes commented Jul 1, 2015

Thanks!

@markus1189 markus1189 deleted the dot-command branch July 1, 2015 09:47
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.

stack dot seems broken
4 participants