-
Notifications
You must be signed in to change notification settings - Fork 112
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 two options, /quiet
and /silent
#694
Conversation
The /quiet option suppresses output other than warnings and errors. The /silent option suppresses all output except the exit code.
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.
LGTM, aside from my comments, but I'm not familiar with boogie code, so another review would be appropriate.
@@ -56,6 +64,10 @@ public virtual void ErrorWriteLine(TextWriter tw, string format, params object[] | |||
public virtual void AdvisoryWriteLine(TextWriter output, string format, params object[] args) | |||
{ | |||
Contract.Requires(format != null); | |||
if (Options.Verbosity < CoreOptions.VerbosityLevel.Quiet) { |
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.
Elsewhere you test with == against Silent. Choose one consistent method of comparison.
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.
Done.
@@ -0,0 +1,7 @@ | |||
// RUN: %parallel-boogie -silent "%s" > "%t" |
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.
Does boogie have an exit code to test?
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 thought it did, but apparently not! So -silent
is not super useful for Boogie alone. The internal option it controls is useful for clients of Boogie as a library, like Dafny, however.
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.
How does dafny know whether boogie is successful or not? I presume it makes some API call rather than spawning a command-line invocation.
I suggest you add an issue for boogie to return a proper exit code (though it is not an issue for me) like all good citizens should
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.
Is the exit code typically spit out on the console along with other output?
It would be great if we could add the standard story for exit code to Boogie also.
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.
In dafny testing, we write things like %exits-with 1 %parallel-boogie ...
to check the output exit code
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.
Dafny uses Boogie as a library, so it inspects the verification results directly. I added #695 to make Boogie emit a non-zero exit code when verification fails, like Dafny does.
Boogie doesn't set a non-zero exit code on verification errors.
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.
Could you say something about the use-case for this change? I'm guessing it was made in support of Dafny. However, I'm surprised this would affect Dafny, because it works through the ConsolePrinter, a class that Dafny will override anyways.
From Boogie's perspective, it seems like a reasonable change, but as you commented the silent
option can't affectively be used right now. Should we fix that?
More generally, I wonder whether Dafny should allow Boogie to write anything to the Console, or whether Dafny should take full ownership of its stdio, just like it new takes full ownership of its options.
I'm guessing that by default Dafny should not allow Boogie to write anything to stdio, and rely purely on an C# API provided by Boogie to get data that it then uses to write output. If Dafny is configured to show debug output, then it could show debug output from Boogie as well.
The key use case for this is in testing, particularly when testing manually. Running with Dafny extends
As I mentioned above, I think there still is a useful role for
This seems like a reasonable approach, on the Dafny side. :) I don't think the answer will affect what we do with Boogie right now, though. |
I like tools that have a quiet option for scripting. This just makes dafny fit in well -- plus I've heard requests for an option to shut off info message. Completely silent is a rarer need, but I've used it in CI on other projects. |
Makes sense, thanks. |
The
/quiet
option suppresses output other than warnings and errors. The/silent
option suppresses all output except the exit code.