eclipse-platform / eclipse.platform.releng.aggregator

Aggregated repository for Eclipse Java IDE
https://www.eclipse.org/eclipse/
Eclipse Public License 2.0
29 stars 74 forks source link

Don't use API-Tools from I-Builds for API checks in verification builds #2327

Closed laeubi closed 3 weeks ago

HeikoKlare commented 4 weeks ago

To better understand the proposal: Why shouldn't I-Builds perform API checks? That sounds unintuitive to me.

And note that (in my understanding) this change will not fix the issues metioned in https://github.com/eclipse-pde/eclipse.pde/pull/1396

laeubi commented 4 weeks ago

Why shouldn't I-Builds perform API checks?

I builds perform API check (but in a different way), but we are currently using the I-Build Results to run the api-checks in verification checks. No there is a binary incompatibility, this does not work as Tycho was compiled against the previous code-base.

HeikoKlare commented 4 weeks ago

Thank you for clarification. Now I understand the purpose. Then only the commit message is misleading, since you can read it as "with this change, we do not make API checks in I-Builds (anymore)" (like I did).

mickaelistria commented 3 weeks ago

For our fork of JDT in https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/ , which has PR rebased on top of master, we see some related failures. Are all platform builds currently broken too?

HeikoKlare commented 3 weeks ago

Yes, all platform builds are currently broken.

laeubi commented 3 weeks ago

Yes, all platform builds are currently broken.

... if the use the api-check profile :-)

HannesWell commented 3 weeks ago

Thank you for clarification. Now I understand the purpose. Then only the commit message is misleading, since you can read it as "with this change, we do not make API checks in I-Builds (anymore)" (like I did).

I just updated the commit message of this change to hopefully make this more clear.

With that I think we should apply this because the current situation there is a very tight dependency loop between Tycho and the PDE API-tools that make it hard to evolve methods that are internal in API-Tools (in defense of Tycho the API tools currently don't provide a proper API just an internal provisional API).

So lets have this and see if this solves the issue.

HannesWell commented 3 weeks ago

The latest build of that PDE PR https://github.com/eclipse-pde/eclipse.pde/pull/1397, which uses the api-checks profile, succeeded without a problem. So I think that issue is resolved.