berkus / include-what-you-use

Automatically exported from code.google.com/p/include-what-you-use
Other
1 stars 0 forks source link

Please return different exit codes #157

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
iwyu always returns with a non-zero exit status so to not accidentally be used 
to compile something. I understand this is by design.

Unfortunately this makes iwyu hard to use if one just wants to check whether or 
not the existing includes of a file are sufficient or should be changed. I 
would like to be able to use iwyu as part of my test suite for my project.

The problem could be solved by iwyu returning different exit codes depending on 
the outcome of the check. Exit code 1 could mean "all includes are fine" and 
exit code 2 could mean "some includes are missing or superfluous".

What do you think?

Original issue reported on code.google.com by j.scha...@email.de on 20 Sep 2014 at 7:58

GoogleCodeExporter commented 9 years ago
Sounds reasonable. How fine-grained do you think it needs to be; is it just 1 
or 2, or do we need to be more particular?

Any chance you could work on a patch? You should be able to start in iwyu.cc, 
HandleTranslationUnit and collect the return values from 
CalculateAndReportIwyuViolations, and then pass the aggregated result to exit().

Original comment by kim.gras...@gmail.com on 20 Sep 2014 at 1:48

GoogleCodeExporter commented 9 years ago
Thank you for being willing to accept a patch but I don't think I currently 
have enough time at my hands to dive into another project even for a simple 
patch, sorry :(

For my purposes, I only need to know whether the includes are sufficient or 
not. So only exit values 1 and 2 would be enough.

Original comment by j.scha...@email.de on 21 Sep 2014 at 6:15

GoogleCodeExporter commented 9 years ago
I had to take a break from all the other difficult issues, so I took some time 
to create a patch for this.

I revised IWYU's exit code policy to be:
1 => Invalid arguments
2 => IWYU found nothing to do, all includes are good
>2 => ((Number of suggested edits) + 2)

Let me know if this makes sense.

Original comment by kim.gras...@gmail.com on 27 Sep 2014 at 8:27

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks a lot! This would fix my problem :)

Original comment by j.scha...@email.de on 27 Sep 2014 at 8:28

GoogleCodeExporter commented 9 years ago
Here's an updated patch where I pull exit codes out into constants.

I don't know a way to get test coverage with our current framework, so I'll 
just note that no tests failed after this change.

Original comment by kim.gras...@gmail.com on 27 Sep 2014 at 10:04

Attachments:

GoogleCodeExporter commented 9 years ago
ping

Original comment by kim.gras...@gmail.com on 18 Nov 2014 at 9:02

GoogleCodeExporter commented 9 years ago
Sorry, I haven't realised this is a pending code review. Plan to look into it 
the next week (November 24-30).

Original comment by vsap...@gmail.com on 24 Nov 2014 at 4:40

GoogleCodeExporter commented 9 years ago
The patch looks good to me. Maybe add something like CHECK_(diff_output && 
"Must provide diff_output") in PrintableDiffs.

Original comment by vsap...@gmail.com on 30 Nov 2014 at 7:23

GoogleCodeExporter commented 9 years ago
Thanks, r592 with the additional CHECK_!

Original comment by kim.gras...@gmail.com on 30 Nov 2014 at 9:30