eclipse-nattable / nattable

High performance SWT data grid
https://eclipse.dev/nattable/
Eclipse Public License 2.0
11 stars 6 forks source link

Update NatTable to the latest dependencies #95

Open merks opened 3 days ago

merks commented 3 days ago

While investing https://github.com/eclipse-simrel/simrel.build/issues/438 the first bundle I tracked was this one:

It's present in Orbit indirectly because of NatTable's dependency:

image

Orbit provides version 1.11.0 of ca.odell.glazedlists:

So I'd like to help update NatTable to use only newer Orbit dependencies.

The next digression is that NatTable doesn't have an Oomph setup which makes it much more difficult to contribute to the project than necessary. I think it would be nice for NatTable to have a project setup and a setup configuration so that the setup is fully automated:

https://github.com/eclipse/nebula/tree/master?tab=readme-ov-file#setup-a-development-enviroment

I've already done most of the authoring for that and tested that it's working:

image

Ideally the project setup and the setup configuration would be hosted in this GitHub repository.

I've made a bunch of changes to update the target platform and update the dependencies to use only newer Orbit dependencies:

image

I really dislike authoring target files by hand, so the version I am using is generated automatically by the setup:

image

It's generated by this part of the setup:

image

I created an m2e run configuration so I can validate the that build actually works:

image

At this point I wonder how best to proceed? In small steps starting with the Oomph setup or all at once to include all these changes in a big bang approach?

Also, I wonder how you manage versions. I see there has been a 2.4.0 release, After setting up the API baseline (via the setup), there are these errors:

image

They're actually bogus errors being address by this issue:

https://github.com/eclipse-pde/eclipse.pde/pull/1302

But I assume you want to produce either a 2.4.1 release or a 2.5.0 release next. Do you generally just update all the versions?

fipro78 commented 3 days ago

Hi @merks,

thanks a lot for the offer to support.

Some comments from my side:

  1. We have updated to GlazedLists 1.11 about 5 years ago (https://github.com/eclipse-nattable/nattable/commit/92073a92f84c18d4767585caf5cf059e2f367a69). It seems that Papyrus is still using a quite old NatTable version, which is causing the transitive dependency issue.
  2. Target Definition Generation
    I did something similar in the past, but learned the hard way that this might have negative impact on our consumers. While I really like to update the third-party-dependencies of the extensions (GlazedLists, POI, Nebula to some extend), and probably even Eclipse Collections and other Apache Third Party dependencies, updating the Eclipse Core dependency to its latest could be problematic. This way things could sneek in, that would break the usage for consumers that are not on the latest Eclipse version. And from my experience, the Target Definition is not only a build artefact, it is also used to ensure that you develop against an older version of Eclipse, while developing in the newest version. If we can ensure that we do not simply update everything all the time, but keep a little consistence, I am fine with such an approach.
    BTW, the simple update of the Target Definition, would not automatically update the pom.xml files, which I try to keep in sync manually for the Maven Central publishing.
  3. Oomph Setup
    I tried to create one several years ago and failed miserably. And I never tried it again, because of missing time and missing need on my side. But the contribution of an Oomph setup is very welcome. :)
  4. API baseline
    I currently even see an issue with the API baseline if I try to set it manually in the IDE. If I try to configure a baseline via preferences, selecting the api-basline folder in the repository, I get an error that there are no matching VMs present. Although I have JDKs for every LTS version configured.
  5. Versioning
    What is the question about how we manage versions? We try to use API tooling, which seems to be broken somehow (see 4.). Apart from that I typically update all versions to be the same on a release. The past showed that it is hard to understand the difference between a product version and a bundle version. And for the maintenance it is also quite hard. So while I personally think you should only update a version of a bundle if you change something in it, for NatTable I decided to go the easier way and have the version of Core and the Extensions aligned. Mainly because there is no "NatTable Product". And actually for quite a while there were always changes in all bundles when I published a new release. ;)
  6. How to proceed?
    Well, what would be the "big bang"? The Oomph setup and the updated Target Definition? What else?
merks commented 3 days ago

@fipro78

In order to let you preview the changes and how this works, I've created everything on my fork such that you can test it before we create a pull request.

I recommend that you download and unrestricted installer with a JRE from here:

E.g., this if you are on Windows, this one:

https://download.eclipse.org/oomph/products/latest/eclipse-inst-jre-win64.exe

Run it from the command line like this specifying a folder that will be used as a user.home such that later you can delete that entire folder to delete the entire experiment:

-vmargs -Duser.home=${fake-user-home} -Doomph.setup.user.home.redirect=true

Drag and drop the following onto the installer:

Create Eclipse Development Environment for Eclipse Nebula

Also try opening that link in a separate tab to see what it looks like and for instructions.

Best not to change any of the defaults:

image

You should end up with a workspace like this:

image

No doubt you'll have questions...

fipro78 commented 2 days ago

@merks Thanks, I just tested it out, and looks ok. Things are downloaded, installed and configured. I am able to run the JUnit tests from inside the IDE. The Maven build also succeeded.

But the product is broken now. Only an empty windows opens.

I am also unsure if we really need to update the dependency to Eclipse 4.33 in the build. I mentioned before that we have consumers that are still on older versions. One I know is currently on 2020-06 and plans to update to 2023-03 by end of this year. Having a dependency to the newest version increases the risk that some API sneeks in that is not yet available on the consumer side.

Apart from that, nice to see the third-party updates coming in. And also having an Oomph setup. Of course there is a lot more work to do then (update versions, update pom.xml files, double check if still everything works, etc. etc. etc.).

merks commented 2 days ago

If you build against old dependencies then there is the risk that the stuff doesn't work with the latest, and of course the converse is true as well. Currently the generated target platform is generated to a separate file that is git ignored with the expectation that you could copy it over manually when you decide it's a good time. In the IDE it's probably good to use the latest so you notice problems immediately, but in the target platform for the actual build, indeed it's probably better to use something older. Let me try that out...

I noticed the produce being empty as well, but I wasn't sure it worked before. 😱 I can look if there are exceptions that are providing a clue, or if maybe something is missing. I removed a few things from features that maybe should be replaced and that might be the cause. I'll try to find some time to investigate in the next day or so and force push changes. I just hope that if I take a little while that you won't be commit changes that lead to merge conflicts, because I'm really clueless about how to resolve such things. 😕

--

Did you noticed that the setup creates resources so the root folder is a project and uses filters so that nested projects are present only as placeholders?

image

This makes it easier to search for or editing anything while still avoiding duplicates resources....

fipro78 commented 2 days ago

Did you noticed that the setup creates resources so the root folder is a project and uses filters so that nested projects are present only as placeholders?

I noticed it and wondered. But I simply thought "nice what is possible with an Oomph setup" :)

I just hope that if I take a little while that you won't be commit changes that lead to merge conflicts, because I'm really clueless about how to resolve such things.

Currently I am busy with some other topic and did not plan to work in the NatTable area. There is actually nothing urgent to look after. Maybe I will find the time to check it also. But it will also take a while before I will find the time. I assume there is either a dependency that is not resolved correctly, or missing as you removed quite a lot of the CSS stuff, not sure if that was by intention or if the CSS engine has changed in the meanwhile.

I also noticed that yesterday Apach POI 5.3.0 was released. So probably also good to wait until that one becomes available in Orbit. And then I want to take some time to verify different settings to ensure the dependency update did not break anything in other areas.

That said, don't worry if you need some time. No need to hurry. And I am happy about your support in that area.

merks commented 2 days ago

I just force pushed changes so that the *.target file is generated for 4.24 and the build works with that. Also the resulting product appears to work:

image

Orbit has the latest poi, it's just not in a milestone build yet, but that will be done by early next week:

https://download.eclipse.org/tools/orbit/simrel/orbit-aggregation/table.html

So at this point I think it's maybe ready to create a pull request? I'd just need to delete these two variables from the setup so that it clones the correct repository and branches rather than my fork:

image

Let me know if you are comfortable with a PR. Of course this is not a hit-and-run exercise. I'm here to help after the fact as well.

fipro78 commented 2 days ago

Feel free to create the PR. I will double check it and let you know if if I still see an issue.