GraphViz migration to ValueGraph

[fortmarek]

Short description 📝

Migrate GraphViz to ValueGraph

I'd just ask someone to test this locally, please 🙏

I can't get tuist graph to work on my machine as I always get:

I have no idea what is wrong (I keep getting even after I made fresh install two weeks ago, so maybe my machine is fundamentally and irreparably flawed), so if anyone has an idea how to fix this/debug this, that'd be great.

Checklist ✅

• The code architecture and patterns are consistent with the rest of the codebase.
• The changes have been tested following the documented guidelines.
• The CHANGELOG.md has been updated to reflect the changes. In case of a breaking change, it's been flagged as such.
• In case the PR introduces changes that affect users, the documentation has been updated.

[kwridan]

Nice work @fortmarek - thanks for migrating this over!

I can't get tuist graph to work on my machine as I always get:

I wonder if it's Big Sur or maybe an installation issue with graphviz? Worth trying removing and re-installing graphviz.

brew uninstall graphviz
brew install graphviz

If that still fails, you can try changing the output format to be a .dot file and examining the results (text based).

tuist graph --format dot

---

While testing I noticed a few discrepancies:

When it comes to filtering targets (by explicitly specifying their name), there's a mismatch between the released version of Tuist and this PR where transitive dependencies are not included:

cd fixtures/ios_app_with_transitive_framework
tuist graph AppTests

Released Version:

This PR:

Framework2 isn't included.

Unrelated to those changes, I also noticed the tuist graph --skip-external-dependencies (even the realised version) produces a slightly odd graph (unless I misunderstood what it's meant to do)

cd fixtures/ios_app_with_transitive_framework
tuist graph --skip-external-dependencies

[bolismauro]

@fortmarek I had the very same issue when fixing the graph command. I feel it is a permission issue.
The way I checked the result, besides testing, is by outputting the textual description with po and then use an online service to render the graph.
I haven't been able to understand the reason of the issue though (I'm not very familiar with mac development and permissions)

[fortmarek]

I have a question regarding ValueGraphTraverser - what's the reason for directTargetDependencies returning only local target dependencies (ie it does not return direct target dependencies that are part of a different project)? I believe that naming is misleading and it lead to the bug where tuist graph AppTests does not include Framework1-iOS. If there is a valid reason, I am for renaming directTargetDependencies to directLocalTargetDependencies and keeping method directTargetDependencies which would actually include all direct target dependencies - even if they are a part of a different project. cc @kwridan @pepibumur

[fortmarek]

thanks @bolismauro!

It might be related to permissions but I am not really sure how to fix it/where to look.
I have tried reinstalling + running brew install graphviz --HEAD to install graphviz from source, neither of those helped, unfortunately. So far I have resorted to opening .dot file with a VS Code extension to visualize the graph. But this might be worth investigating more as it seems this is not an issue applicable only to my setup but might be more widespread than we have previously thought.

[kwridan]

what's the reason for directTargetDependencies returning only local target dependencies (ie it does not return direct target dependencies that are part of a different project)

I believe there are some cases like searching for host apps (for tests or extensions) where only local targets need to be searched.

I am for renaming directTargetDependencies to directLocalTargetDependencies and keeping method directTargetDependencies which would actually include all direct target dependencies

+1 this makes sense to clarify the intent

[fortmarek]

all of the troubling scenarios you have listed should be fixed. I tested that locally leveraging dot format + added appropriate unit tests to catch any possible future regressions. I think this is worth another look @kwridan, thanks for the thorough review 🙇

[kwridan]

awesome work @fortmarek

[kwridan]

👍

[kwridan]

nicely done