Skip to content
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

Make dylan-compiler exit status reflect warnings #1373

Closed
cgay opened this issue Jan 7, 2021 · 5 comments
Closed

Make dylan-compiler exit status reflect warnings #1373

cgay opened this issue Jan 7, 2021 · 5 comments
Assignees

Comments

@cgay
Copy link
Member

cgay commented Jan 7, 2021

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
Copy link
Member Author

cgay commented Jan 7, 2021

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

@pedro-w
Copy link
Contributor

pedro-w commented Jan 8, 2021

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.

@cgay
Copy link
Member Author

cgay commented Jan 8, 2021

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
Copy link
Member Author

cgay commented Feb 28, 2022

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 #1238 (Always recompile if there were serious warnings).

@cgay cgay self-assigned this Feb 28, 2022
cgay added a commit to cgay/opendylan that referenced this issue Aug 7, 2022
Non-serious warnings do not cause an error exit status. We can enable that
behavior later with a flag if desired.

Issue dylan-lang#1373
cgay added a commit to cgay/opendylan that referenced this issue Aug 8, 2022
Non-serious warnings do not cause an error exit status. We can enable that
behavior later with a flag if desired.

Issue dylan-lang#1373
cgay added a commit to cgay/opendylan that referenced this issue Aug 8, 2022
Non-serious warnings do not cause an error exit status. We can enable that
behavior later with a flag if desired.

Issue dylan-lang#1373
cgay added a commit to cgay/opendylan that referenced this issue Aug 19, 2022
Non-serious warnings do not cause an error exit status. We can enable that
behavior later with a flag if desired.

Issue dylan-lang#1373
cgay added a commit to cgay/opendylan that referenced this issue Sep 18, 2022
Non-serious warnings do not cause an error exit status. We can enable that
behavior later with a flag if desired.

Issue dylan-lang#1373
@cgay
Copy link
Member Author

cgay commented Sep 19, 2022

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

@cgay cgay closed this as completed Sep 19, 2022
cgay added a commit to cgay/opendylan that referenced this issue Mar 29, 2023
Non-serious warnings do not cause an error exit status. We can enable that
behavior later with a flag if desired.

Issue dylan-lang#1373
cgay added a commit to cgay/opendylan that referenced this issue Apr 8, 2024
Non-serious warnings do not cause an error exit status. We can enable that
behavior later with a flag if desired.

Issue dylan-lang#1373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants