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

Improve javac error output parsing - Fix #336 #337

Merged
merged 3 commits into from
Dec 20, 2023

Conversation

kriegaex
Copy link
Contributor

@kriegaex kriegaex commented Dec 17, 2023

Fixes #336 and generalises stack trace parsing.

@slawekjaranowski, all unit and integration tests are passing. If you generally like the implementation, I can add some more unit tests.

@slawekjaranowski
Copy link
Member

It looks ok for me but unit or even IT with different locale should be added

@kriegaex
Copy link
Contributor Author

It looks ok for me but unit or even IT with different locale should be added

I am going to add more unit tests. IT coverage is so low, they are nothing more than smoke tests for now. To improve that is out of scope for this PR. BTW, it is somewhat difficult to reproduce internal compiler errors and annotation errors across JDKs 8-21, because those are the ones that get typically fixed in new update releases. I even had to install a very old version of JDK one (u45) to reproduce any compiler error that prompts users to raise a JDK bug. But I did, just to be sure.

The unit tests, however, are meant to reproduce error outputs for both cases (compiler and annotation processor errors) in several locales.

Copy link
Member

@slawekjaranowski slawekjaranowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So waiting for a test.

Instead of two separate 'while' loops, one inline and one in an extra
method, parsing stack traces after finding error messages specific to
the English locale of the JDK, now there is generic stack trace parsing,
which works in these two cases independent of locale and also in others
previously not covered.

For backward compatibility, the Javac localized message 'javac.msg.bug'
("An exception has occurred in the compiler ... Please file a bug") is
salvaged in a locale-independent way, matching on error reporting URLs
always occurring in all JDK locales (English, Japanese, Chinese, German
as of JDK 21).

Fixes codehaus-plexus#336.
@kriegaex
Copy link
Contributor Author

@slawekjaranowski, done. Sorry, yesterday I was busy otherwise.

@slawekjaranowski
Copy link
Member

Thanks for unit test ... is it possible to prepare a IT for it ...
I will try with exemple from https://issues.apache.org/jira/browse/MCOMPILER-424

@kriegaex
Copy link
Contributor Author

It is always possible to prepare an IT. But I already explained why I won't:

IT coverage is so low, they are nothing more than smoke tests for now. To improve that is out of scope for this PR. BTW, it is somewhat difficult to reproduce internal compiler errors and annotation errors across JDKs 8-21, because those are the ones that get typically fixed in new update releases. I even had to install a very old version of JDK one (u45) to reproduce any compiler error that prompts users to raise a JDK bug. But I did, just to be sure.

Moreover, in this comment in MCOMPILER-424 I also said, that I have already created an OpenJDK bug report for this particular problem. Chances are, that someone will fix it, and then the IT will no longer run, unless you make a complex setup with toolchains, installing specific older JDK update versions in which the bug still exists, even if in the next update of the same JDK it was fixed already. Why set up an IT with a half life like this, if you cannot rely on it still working in three months? If you want to do that and volunteer to maintain it, always finding new examples of these types of JDK bugs in the future, be my guest.

@slawekjaranowski slawekjaranowski changed the title GitHub 336 Improve javac error output parsing - Fix #336 Dec 19, 2023
@slawekjaranowski slawekjaranowski merged commit b5631da into codehaus-plexus:master Dec 20, 2023
32 checks passed
@kriegaex kriegaex deleted the github-336 branch December 20, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Forked javac annotation processing error stack trace not logged for non-English locales
3 participants