Vote on verification build warning reporting

[akurtakov]

#1557 lead to discussion that shows lack of agreement in where/how/why/etc. the community wants to see warnings in verification builds. Please read the comments in the PR for extra information

[mickaelistria]

FWIW, I find warnings are only good in the IDE while developing because they help making code better as you work on it. So I voted for disabling reporting all together at build time as I don't find them helpful at that time.

[laeubi]

FWIW, I find warnings are only good in the IDE while developing because they help making code better as you work on it.

The problem is that

1. JDT does not detect all issues in javadoc in the IDE: Javadoc validation does not take into account implied end tags eclipse-jdt/eclipse.jdt.core#1858
2. If its only a warning often people probably don't care, probably never fix and they accumulate more and more

Because of that it was asked to enable / make them visible in the CI so a reviewer can get aware of it without having to start the IDE and look through thousands of warnings to spot one new item.

[mickaelistria]

While eclipse-jdt/eclipse.jdt.core#1858 is related in the story here, in the end, it's a separate issue/fix to add in JDT (or any other static analyzer we'd like to hook in the IDE or build).

If its only a warning often people probably don't care, probably never fix and they accumulate more and more

Yes, but the very reason why warnings exists instead of errors are that often they're good just being "only a warning" and allowing people to not care too much about them (ie not block). If we want people to fully care and turn some warnings into blockers, then those warnings needs to be reported as errors in the IDE and build.

[laeubi]

And that's where the circle closes, because we getting more and more warnings and "no one" will fix all of them we can't turn them into errors, because people thing they are "just warnings" ;-)

As a side note, these tools does not only report warnings they also report errors and the main benefit is that a user do not need to scan/parse the log itself, so if there is a API error or compile error or whatever it is easily accessible.

[vogella]

Loosely related: I think we should enable that the build fails if the number of warnings increases. IIRC @laeubi had a solution for this, but did dare to enable it, as this may also break the build in other cases, e.g. if the compiler suddenly adds more warnings.

[laeubi]

Just wanted to not that even though the blame is always put on me, it is not that I (personally) have a solution (or demand for them), but these are software industry standard tools (namely Jenkins + Plugins (according to their stats warnings-ng has 23k installs) and yes they offer such (standard) options.

Also in the past it was often complained that it is "too hard to find the issue of the build failure" so we installed this with approval by the project leads of the projects so we can at least show them in a more structured way than a lengthy build log.

[vogella]

My intention was to give you credit for your great work as you learn about and apply the software industry standard tools to the Eclipse project which unfortunately is not using these standard tools. My blame goes usually to people which do not want to apply these tools or apply the the why change things, we always did it like this approach.

[laeubi]

@vogella we are all standing on the shoulders of giants, so my work is just small compared to all the work done before in these tools and plugins, so thanks for the credits but I actually only applied the configuration and I'm happy that is already works to such extend but far from that I crafted a solution I just applied what was already there!

So from my side its now up to us to give feedback to the authors of these great tools if something needs improvement instead of complain its not 100% fit for our purpose... anyone can improve its even open source.

[vogella]

I agree that it is great that these tools are developed as OS.

In the context of Eclipse it is still worth giving gratitude to people which still fighting the fight of applying industrial standards to Eclipse. In the past the "not invented here" and "why change something" mindset was very present and the burden of this mindset is in my opinion still present in the project. Fortunately the project team is much more open to changes these days. But I will stop now posting not closely unrelated topics into @akurtakov voting.

[jukzi]

Now all jdt master builds are rated unstable, because ECJ reports "Quality gate: Unstable". If anybody volunteers to fix that - welcome. otherwise i will even disable ECJ again

pressing "Reset" did not help. The ECJ plugin seems to find but ignore build#30 ff.

14:24:33  [Eclipse ECJ] Obtaining reference build from reference recorder
14:24:33  [Eclipse ECJ] -> Found 'eclipse.jdt.core-Github » master #30'
14:24:33  [Eclipse ECJ] Using reference build 'eclipse.jdt.core-Github/master #27' to compute new, fixed, and outstanding issues

[akurtakov]

@jukzi Which build is that? Have you tried to use the Reset button?
It's really important that we learn to respect each other's opinion more. So far "disable none" seems to be preferred vote and everyone is supposed to respect majority opinion (despite not being yours or mine vote).

[jukzi]

pressing "Reset" did not help.

https://ci.eclipse.org/jdt/job/eclipse.jdt.core-Github/job/master/

[akurtakov]

So

means that next build will use that one as a base for comparison aka warnings that exist in it shouldn't flag the build and indeed https://ci.eclipse.org/jdt/job/eclipse.jdt.core-Github/job/master/32/ is green again.

[jukzi]

"the next build" was not true. we had dozen of PR builds after "Reset" which did not move its reference.
So the workaround was to "Reset"+ manually retrigger master? Someone volunteer for all other repos too, please. Or can that be somehow automated? Like "ignoreQualityGate: true" (for master only?)

[iloveeclipse]

Here is another unrelated error reporting that now fails JDT builds for no obvious reason (PR touched different bundle): eclipse-jdt/eclipse.jdt.core#1928.

So please can we stop this experiments? I don't see an end on unrelated errors reported. They may be valid for some, but they are highly distracting. Every PR fails and very time I check the failure it is something unrelated to the PR.

[akurtakov]

For the record - this vote I started here is for Platform (even UI) as the PR that started the discussion is against this repo.
I would really like to see projects being more standalone as what is best for Platform is not best for JDT or vice-versa, furthermore there is a good amount amount of people that are committers on only one of the projects.
So if Project A comes to a decision that the project wants every possible warning shown and Project B decides that they would like to see no warning at all - it's project's committers decision.

[akurtakov]

If there is disagreement about projects deciding on their own that we should be looking into merging everything into single project (for the record I'm strongly against that!) so there is no case where the need of one subproject puts more/less policies on the other one.

[iloveeclipse]

My comment was not JDT specific, I'm tired to see red state on PR's with unrelated changes, and that is for all repositories we have in SDK. JDT PR was simply the latest ome I saw... Every time there is a different explanation why this particular PR is failing, and that has to find an end.

We should disable all the unstable checks and only enable them if they would provide reasonable and reproducible results.

[akurtakov]

What I try to point is - you're free to do so for JDT (if a committer disagrees he/she should should start a vote in JDT). So far the Platform committers seems to prefer the other way around and see more warnings and look into whether they make sense to them, fix them overtime, ignore and etc.
Such a diversity between projects might actually be best for everyone as it should help us see which one gives better results over some longer(months?) period.

[akurtakov]

Speaking of failing builds - last good aggregator verification master build was on October 31st https://ci.eclipse.org/platform/job/eclipse.platform.releng.aggregator/job/master/ without any of these extra warnings and etc. being enabled and no one noticed! So clearly we need things to change but the disagreement is what - make more things visible everywhere so more people see it and hopefully fix things or hide things and pretend everything is fine.
In the end it's the very same small group of people fixing things and after being the one suffering from javadoc build failures for years, I consider enabling them for everyone so people went and fix them (HUGE THANKS FOR THAT!!!) a total win.

[laeubi]

Here is another unrelated error reporting that now fails JDT builds for no obvious reason (PR touched different bundle)

I really fail to understand how one could make it more obvious whats the problem is

Fixing these 4 API Warnings as part of the PR (or even a separate one if one cares) would probably be much more easier than write the comment and getting mad again. Also @akurtakov has explained already to how reset the quality gate for master if one really things it is to much effort to improve the warnings just a little bit, that might be a bit annoying as JDT has so many different repositories but this is the decision of the team to not act on it and instead higher the releng burden, please note I can't do it because I'm not a committer JDT!

Also as @akurtakov has mentioned JDT is a separate project and can take distinct decisions, but it should then also take the responsibility for the releng work, e.g. this specific issue you mentioned is caused by the fact that JDT still seem not perform the necessary actions (read: ignores the "unreleated" warning!) that a baseline replace has happened on a bundle with different content resulting in

• Build I20240129-1800 is unstable eclipse.platform.releng.aggregator#1757 (would have been discovered directly on PR time)
• I20240126-0330 is unstable eclipse.platform.releng.aggregator#1750 (would have be discovers if not using many different repositories)
• I20240108-1340 build is unstable eclipse.platform.releng.aggregator#1704 (would have been discovered directly on PR time)
• Build: I20231221-1800 Unstable eclipse.platform.releng.aggregator#1686 (would have be discovers if not using many different repositories)
• I20231213-0710 is unstable eclipse.platform.releng.aggregator#1659 (would have been discovered directly on PR time)
• Build I20231201-1800 is unstable eclipse.platform.releng.aggregator#1617 (would have been discovered directly on PR time)

[iloveeclipse]

None of the mentions ibuild failures where needing the "help wanted" ticket, they would have been discovered already right now

eclipse-jdt/eclipse.jdt.core#1917 is required to detect that, and I don't know other solution.

Please share how one can fix a difference between class files compiled with compiler version A that differ from classes compiled with compiler version B for classes that are built in different repositories?

[merks]

Indeed, perhaps you are both expecting the impossible of the other and are not being very nice about it to boot.

I've really, really, really had enough of finger pointing, blaming, and griping. It's beyond tiresome and it is beneath the decorum of what one would expect from such highly-skilled, technical experts who are all working so incredibly hard to make Eclipse excellent. You should all be thanked by the masses every single day. The community respects you folks and appreciate what you do even if so few ever bother to say so.

In any case, the build is not an AI engine that understand related versus unrelated problems. It can check if the problem existed historically and filter those as noise. That is what the releng folks are trying to provide. Also, when we build with a new compiler every day, we will notice when it produces different byte code regularly. That will always be causing us problems. It does little good for me to say, please make that problem go away as if a magic wand could be used. Better to ask, what can we do to mitigate that problem? Is there some additional testing we can do?

Please, please dear goodness, could we focus on what can be improved and how best to improve it without some folks being bad folks and some folks being good folks?

[laeubi]

Please share how one can fix a difference between class files compiled with compiler version A that differ from classes compiled with compiler version B for classes that are built in different repositories?

Please read below just as a hint to give advice how this can be detected as of today, I'm not blaming anyone, and just want to highlight how it works and why it works and what was meant with

this specific issue you mentioned is caused by the fact that JDT still seem not perform the necessary actions (read: ignores the "unreleated" warning!)

and why it is actually bad to qualify anything as "unrelated" just because it get into the way to press submit button, even worse this ticket was blamed there to be "Validation error as usually unrelated" even though this issue where never came up to the integration build at all if the warnings where taken as what they are a hint to the developer that something might go wrong.

Issue that was detected in the I-Build later

Here

• Build I20240129-1800 is unstable eclipse.platform.releng.aggregator#1757

looking into the buildtimeComparatorUnanticipated.log.txt you will see the this message

1.  eclipse.jdt.core/org.eclipse.jdt.core.tests.performance/pom.xml
   no-classifier: different
      org/eclipse/jdt/core/tests/performance/FullSourceWorkspaceBuildTests.class: different

causing change and verifications

You referenced this change

• Parser changes for JEP 443 eclipse-jdt/eclipse.jdt.core#1517

When I look at the build verification it shows

clicking on the details of this new warning it shows

MavenProject: org.eclipse.jdt:org.eclipse.jdt.core.tests.performance:3.12.300-SNAPSHOT @ /home/jenkins/agent/workspace/eclipse.jdt.core-Github_PR-1517/org.eclipse.jdt.core.tests.performance/pom.xml: baseline and build artifacts have same version but different contents

Conclusion

As you can see it is the same project mentioned here (the full context can't be viewed anymore because the build is already deleted), and one could have made the touch of the bundle part of the PR as you have done after the fact here

• Touch bundle affected by compiler implementation change eclipse-jdt/eclipse.jdt.core#1928

then the warning would have been fixed in the verification build, the I build would have run without an issue, one could have looked at the result if it is maybe an undesired side effect before merging.

[iloveeclipse]

Here is another unrelated error reporting that now fails JDT builds for no obvious reason (PR touched different bundle): eclipse-jdt/eclipse.jdt.core#1928.

Once again, the warnings I've mentioned above were unrelated to the PR's in question.
I'm not responsible if anyone else ignores some warnings on other PR's and I can also understand why it happens.

None of the mentions ibuild failures where needing the "help wanted" ticket

This is still wrong, and there is still no solution proposed for that except eclipse-jdt/eclipse.jdt.core#1917.
I'm not sure why you are denying to see the obvious. The only example you've shown was "per occasion" in same repository.
For compiler related SDK build issues I do not see any other solution as to provide a fix for eclipse-jdt/eclipse.jdt.core#1917.

And yes, there might be sometimes "lucky" PR validation errors reported that could be relevant, but they are being simply ignored by almost everyone because there are always some validation errors, and that is the problem we are trying to explain here.

[jukzi]

/home/jenkins/agent/workspace/eclipse.jdt.core-Github_PR-1517/org.eclipse.jdt.core.tests.performance/pom.xml: baseline and build artifacts have same version but different contents

Its cool if we could prevent such errors from to be merged. And this explanation makes clear what should be done.: It would be even better if this error message would

1. explicitly rate it as error - given that it will fail later builds.
2. give an explanation why this is an issue and what has to be done, as a normal java developer does not have the knowledge to understand and solve the error.

[jukzi]

Let's not put mental barrier between different teams. At the end we have the common goal to contribute to the eclipse IDE.
The skillset between the people is very mixed. Not everybody has the skill or time to care about issues in topics like build/javadoc/API/compiler/UI/Multithreading/memory/performance/UX/... .It's a high value to have different skills. The tools should not force everybody to care about the same issue. Please respect that not everybody has the same focus. If anyone is able to fix any issue it is highly welcome in all projects.
For example i don't want to put focus on the releng things and therefore i don't want to get alerted about releng issues, even if i could solve them. I am very happy that people like @akurtakov and @laeubi do work on it. Just please keep the tools in a way that no one else needs to care about, because others can't or just won't. I don't want everybody to need to know how to reset quality gate. Either the people that want that gate enabled please provide the necesessary time to manually monitor and reset it or automate it.

Fixing these 4 API Warnings as part of the PR (or even a separate one if one cares) would probably be much more easier

Please provide a PR - i don't know how to fix that off the cut.

decision of the team to not act on it

Please respect that. Making releng easier is one good purpose, but when even more people suffer from merging repos, for example by higher build times it's also a legit perspective where we need to find a compromise between goals.

[laeubi]

Please provide a PR - i don't know how to fix that off the cut.

Know / accepted problems can be filtered and you already have done this in the past so I can't really believe that but for reference just in case: https://wiki.eclipse.org/PDE/API_Tools/User_Guide#Filtering_Known_Problems

Filters that are no longer being used can simply be delete, in both cases Eclipse offers you a Quickfix you can use.

Just please keep the tools in a way that no one else needs to care about, because others can't or just won't.

or just don't want ... it is the responsibility of a committer to care there is no way to shift responsibilities to a random person.

[merks]

@jukzi

I'm not sure I can agree with this line or reasoning, and certainly not when taken to the extreme. If I don't care about warnings in my Java code and then contribute/modify code that introduces a bunch of new warnings, should someone else who cares be responsible for fixing them? I would hope and expect not. If I don't care that the javadoc *that I do bother to write) is full of errors, should I just submit it anyway and expect someone who cares to fix it? Surely we're all responsible for caring to a significant extent about the overall quality of what we produce collectively. I expect that subsets of people not caring about subsets of issues is what leads to an overall union of problems so numerous that not caring appears to be the only way to cope. After all, one cannot be all too concerned about adding or removing warning from this sea of warnings:

In my projects, warnings are eliminated and new ones are noticed immediately:

One thing that I can say with certainty is that I get floods of email every day and all too often lately I do not like the adversarial tone that's taken. It's negative and depressing. It makes me want to ignore this like ignoring warnings or to go somewhere where there are nice people to work with. When the word "please" takes on the tone of a demand, you know that communication has degraded down the wrong path.

Do you think it's possible that we focus primarily on improving what we have? For example, personally I think it would be great to have a central location where someone who is uninformed can inform themselves on how to interpret the build results and how to find details about problems. Looking at recent example:

eclipse-pde/eclipse.pde#1094

it appears to be working well when it's working well:

Oh wow, and someone saying thank you. How nice is that?

[jukzi]

We agree about the responsibility is on those who introduce warnings. But its complicated: who is responsible for

• code that date back to committers gone
• new warnings introduced by 3rd party changes (deprecation, new features like generics ...)
• new warnings by activating new warning tools, changing existing tools, reconfiguring existing tools?

Does it help to know who is responsible? - I fixed thousands (!) of warnings in the last months that i am not responsible for including those shown by no tool at all. The new tools did not help with that, instead i had to take extra time to fix or ignore those.

Please don't take this discussion personally, its only about the thing not the people. I only try to find way to get happy with the toolchain.

[akurtakov]

You fixing all these warnings allowed us to move ahead and further stabilize a lot - huge thanks!
Responsibility for the state of the codebase is shared between all committers (some do more than others but that's FOSS by nature) and every single one of us is responsible.

The goal with all these extra checks is not to make committer's life harder rather the opposite - catch issues earlier and make them seen earlier and eventually prevent from new ones entering the codebase. Unfortunately, the current state is such that if we just hide warnings more will enter and getting more automation/checks will become even harder to achieve in the future.

The only exit from this vicious cycle seems to be to enable more till we fix, raise from warning to error, require extra approvals for importing new ones and so on and on. It is a painful process for sure but the other option seems to be to accept things degrading .
Github/Jenkins provide us with great way for more checks and automations and we are barely scratching the surface of possibilities now. E.g. see what we are trying in Tycho now (eclipse-tycho/.eclipsefdn@42c0aa8#diff-88861c0250c496da5543bd28a02b2e3cf88129271a4731b2c6dd5e92a76712f4R82) to mandate that verification build and license check are prereqs, hopefully we get to a point when Platform codebase gateway is hardened in such way too so we see less sudden breakage. For me one thing is sure - if we don't try we will never get there and yes, there will be (a lot of!) growing pain.

[jukzi]

From 9 votes 5 votes to disable maven, 4 voted to keep maven.
Only 3 voted to disable javadoc.
So lets disable maven, keep javadoc?

[akurtakov]

Lesson for myself and others for next votes: All votes should be binary so there is no room for interpretation!
The only way multi option votes can work are if we have https://en.wikipedia.org/wiki/Score_voting which I don't think we can achieve via github thus I'll create a new binary vote for disabling Maven so there could be no question about integrity of the voting process and interprecation of results.

[akurtakov]

I've started a new one for the sake of formalizing and preventing any possible miscommunication.

[akurtakov]

#1644

[akurtakov]

Close due to non-clear result. To be clarified in #1644.