-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Output Swift Build PIF JSON for Graphviz visualization #8539
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
Conversation
/// | ||
/// * [DOT command line](https://graphviz.org/doc/info/command.html) | ||
/// * [DOT language specs](https://graphviz.org/doc/info/lang.html) | ||
func writePIF(_ workspace: PIF.Workspace, toDOT outputStream: OutputByteStream) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main entry point for construction the Graphviz compatible PIF graph.
@swift-ci test |
@swift-ci please test |
7932159
to
4a5319b
Compare
@swift-ci please test |
@swift-ci test windows |
@swift-ci test macOS |
@swift-ci test linux |
@swift-ci please test |
@swift-ci test self hosted windows |
1 similar comment
@swift-ci test self hosted windows |
@swift-ci test windows |
Sources/SwiftBuildSupport/PIF.swift
Outdated
@@ -205,6 +206,7 @@ public enum PIF { | |||
} | |||
} | |||
|
|||
// FIXME: Delete this (rdar://149003797). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: Maybe this can be a GH issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point Chris. Done: #8552
|
||
if printPIFManifestGraphviz { | ||
// Print dot graph to stdout. | ||
writePIF(topLevelObject.workspace, toDOT: stdoutStream) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: How will the output look if there are other things going to stdout, like the build logs, and error messages? Maybe it will be better to have the option specify a file where the dot should go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was under the impression that all build logs, warnings, and errors go to stderr now, right? I mean when using --build-system swiftbuild
option.
I think sending out to stdout provides a slightly better Unix-ish DX, following what we already do in the existing --print-manifest-job-graph
option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, this is how stdout looks like if do not abort the build process (ie, right after printing out the Graphiz):
$ swift build --build-system swiftbuild --vv --print-pif-manifest-graph 2> /dev/null
digraph PIF {
...
}
Building for debugging...
100%:
Build complete! (0.49424425 seconds)
Even on --very-verbose
mode all stdout "noise" only happens after printing out the Graphiz, so aborting the build there provides a nice workaround for this now. Luckily, all build output before digraph
goes instead to stderr, which won't mess up with dot
parsing.
With the build abort in place, it then looks like this:
$ swift build --build-system swiftbuild --vv --print-pif-manifest-graph 2> /dev/null
digraph PIF {
...
}
PS. The existing --print-manifest-job-graph
option follows a very similar logic.
@@ -969,6 +969,7 @@ extension ProjectModel.BuildSettings.Platform { | |||
case .windows: .windows | |||
case .wasi: .wasi | |||
case .openbsd: .openbsd | |||
case .freebsd: .freebsd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question Does FreeBSD support belong as a separate PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's already merged and the diff is just showing this because this PR is branched off an older main.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had that in another PR... but never landed it. Anyway, I will send out a dedicated PR for this ;)
This how the file looks like in main
:
default: preconditionFailure("Unexpected platform: \(platform.name)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you go: #8550
@swift-ci test windows |
dc33b1f
to
2b263cd
Compare
@swift-ci test |
@swift-ci test self hosted windows |
@swift-ci test windows |
### Motivation: Fix a minor cmake build issue I introduced in pull request #8539. ### Modifications: Add missing Swift file to `Sources/SwiftBuildSupport/CMakeLists.txt`.
Motivation:
Improve visualization/debuggability of the PIF JSON sent over to Swift Build (when using the new
--build-system swiftbuild
option).Modifications:
This PR introduces the
--print-pif-manifest-graph
option to write out the PIF JSON sent to Swift Build as a Graphviz file file. This follows a similar design we used for the existing--print-manifest-job-graph
option.All the serialization happens in the new
DotPIFSerializer.swift
file, with this function as the entry point:Result:
With Graphviz tool installed (i.e., provides the
dot
command below), we can then run commands like this:A few quick examples:
Sample Executable
Sample Library