-
Notifications
You must be signed in to change notification settings - Fork 701
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
Warning: "debug build with assertions" interferes with --numeric-version #8645
Comments
I suppose normal verbosity is needed to help people avoid surprises. However, |
We have:
This topic has a history...
Why do we need this warning preemptively at all? Couldn't we just add it as explanation if an actual assertion is thrown? In >99% of my invocations of I'd think the best path of action here (if we want to avoid the thickets of stdout vs stderr) is to remove the warning and only utter it when an assertion failed. |
Ok, to not start a revolution, I submitted a modest PR that shifts the warning to |
I like the idea of printing it only when an assertion actually fails. |
I think that the main purpose of the warning is to warn about decreased performance. |
#8647) * Fix #8654: no assertion warning on special modes (help, version, etc.) This patch moves the `warnIfAssertionsAreEnabled` until after the main command line argument parsing, and then only prints it on regular operation, i.e. not when just the help or version of cabal was demanded, or the command line could not be parsed. Further, the warning is printed on stderr. The previous attempt to use `warn` does not work as expected, as the verbosity aperatshnik isn't in place yet, so the warning cannot be suppressed by -v0. Drive-by-shooting: straighten the convoluted control flow involving `maybeScriptAndArgs`. This IO computation was invoked "brutto" before, when it is actually only needed in *one* branch of the case distinction. * PR #8647: changelog, fix text of warning Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@grayjay wrote:
Digging the history I saw that once this warning had some text about decreased performance. Do you know why this part was dropped? |
Just as a historical note, there was certainly a case I found at some point where we were shipping binaries that accidentally had assertions on, leading to vastly decreased solver performance. I think having some warning in at least some cases is a good guard against this. |
Which warning mentioned decreased performance? Are you referring to the expensive assertions mentioned in #8240? They were only enabled with a custom build flag, so they were not easy to enable unintentionally. |
@grayjay: Ah, it was in a PR that didn't get merged as such: |
The warning printed by the
cabal
development version interferes with collecting output, e.g.CABAL_VERSION=$(cabal --numeric-version)
will usually set
CABAL_VERSION
to something like3.8.1.0
, but for the development version it is the warning-string followed by the version:The problem is that this warning is printed to
stdout
withnormal
verbosity:cabal/cabal-install/main/Main.hs
Lines 192 to 198 in 399cf64
I think this warning should only be printed with increased verbosity, e.g.
-v2
.What confuses me is that
-v0
does not seem to have any effect:I still see this well-meant but silly warning.
The text was updated successfully, but these errors were encountered: