opendylan icon indicating copy to clipboard operation
opendylan copied to clipboard

Make dylan-compiler exit status reflect warnings

Open cgay opened this issue 4 years ago • 4 comments

It would be useful in various automation tasks if the dylan-compiler command would return non-zero exit status when serious warnings are encountered.

$ dylan-compiler -build testworks-run
.../testworks/command-line.dylan:23.20-38: Serious warning - Reference to undefined binding {whatever in %testworks}.
...
Build of 'testworks-run' completed

$ echo $?
0

Currently it's very easy to miss serious warnings in amongst all the other compiler output.

What issues would this raise, if any, for our current processes? Should non-serious warnings be reflected in the exit status as well?

cgay avatar Jan 07 '21 21:01 cgay

Dunno if it's overly complex, but we could use the low three bits of the exit status to indicate

  1. serious warnings
  2. non-serious warnings
  3. other errors

cgay avatar Jan 07 '21 22:01 cgay

I usually just ignore non-serious warnings in the first instance, e.g there are some when compiling the standard library, and also 'binding defined but not referenced or exported' happens all the time during development. But I find serious warnings mean that the program will crash. So it would definitely be useful to change the exit status on serious warnings, then I could do dylan-compiler -build myprog && ./_build/bin/myprog - as you say, if there's a lot of compiler output, a serious warning can be missed. Maybe the compiler needs the equivalent of -Werror for when we want non-serious warnings to affect the exit status.

pedro-w avatar Jan 08 '21 09:01 pedro-w

And since Open Dylan has the property that even serious warnings don't prevent a binary being created, we should have to explicitly ask for the exit status to reflect that as a failure. (Or at least there should be a way to turn it off.)

There's also the question of whether any flag to turn on/off this behavior would apply to the entire build or would mean to stop after compiling any library dependency that generates warnings, or only after the entire build is complete. The former would be good for CI, to reduce overall resource usage. The latter is better for development, or at least is more like current behavior.

Straw man proposal:

  1. For now any options apply to the whole build. Later if we want an early exit for warnings generated by library deps we can add a separate flag for that.
  2. --error-status-on-warnings={all,serious,none}
  3. Default to none, same as today. (This is conservative. I'd be fine with defaulting to serious.)

cgay avatar Jan 08 '21 19:01 cgay

My current thinking is,

  • we don't have any long-term warnings (in dylan, dood, etc) that are serious warnings
  • keep it simple
  • so just make dylan-compiler exit with 1 if there are any serious warnings in any library, otherwise 0
  • this will also give a better signal when compiling in emacs

There might be some interaction with https://github.com/dylan-lang/opendylan/issues/1238 (Always recompile if there were serious warnings).

cgay avatar Feb 28 '22 16:02 cgay

Ended up returning non-zero exit code for serious warnings by default and providing -allow-serious-warnings to disable that behavior.

cgay avatar Sep 19 '22 04:09 cgay