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

Update NatTable to the latest dependencies #95

Closed
merks opened this issue Jul 2, 2024 · 12 comments
Closed

Update NatTable to the latest dependencies #95

merks opened this issue Jul 2, 2024 · 12 comments
Labels
enhancement New feature or request
Milestone

Comments

@merks
Copy link
Contributor

merks commented Jul 2, 2024

While investing eclipse-simrel/simrel.build#438 the first bundle I tracked was this one:

  • ca.odell.glazedlists/1.9.0.v201303080712

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:

eclipse-pde/eclipse.pde#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
Copy link
Contributor

fipro78 commented Jul 3, 2024

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 (92073a9). 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
Copy link
Contributor Author

merks commented Jul 3, 2024

@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
Copy link
Contributor

fipro78 commented Jul 3, 2024

@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
Copy link
Contributor Author

merks commented Jul 3, 2024

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
Copy link
Contributor

fipro78 commented Jul 3, 2024

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
Copy link
Contributor Author

merks commented Jul 3, 2024

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
Copy link
Contributor

fipro78 commented Jul 3, 2024

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

@fipro78
Copy link
Contributor

fipro78 commented Jul 9, 2024

@merks
I am trying now to cleanup a bit, update the NatTable version and correct some versions in other places. I'll keep you updated once I am done so you can have a second look.

merks added a commit to eclipse-oomph/oomph that referenced this issue Jul 9, 2024
fipro78 added a commit that referenced this issue Jul 10, 2024
Updated examples build, removed e4.rcp.feature, updated BREE to
JavaSE-11, corrected deprecated warnings, updated version to 2.5.0
@merks
Copy link
Contributor Author

merks commented Jul 10, 2024

FYI, I updated this above too to actually work with the real clone now:

Create Eclipse Development Environment for Eclipse Nebula

fipro78 added a commit that referenced this issue Jul 11, 2024
Updated examples build, removed e4.rcp.feature, updated BREE to
JavaSE-11, corrected deprecated warnings, updated version to 2.5.0
@fipro78 fipro78 added this to the 2.5.0 milestone Jul 15, 2024
@fipro78 fipro78 added the enhancement New feature or request label Jul 15, 2024
@fipro78
Copy link
Contributor

fipro78 commented Aug 25, 2024

@merks
To finalize this issue I was trying to update the DEPENDENCIES and also try to start creating a SBOM in the maven build. The DEPENDENCIES file now contains these entries that could be problematic

p2/orbit/p2.eclipse.plugin/junit-jupiter-migrationsupport/5.11.0, unknown, restricted, none
p2/orbit/p2.eclipse.plugin/junit-platform-runner/1.11.0, unknown, restricted, none
p2/orbit/p2.eclipse.plugin/junit-platform-suite-api/1.11.0, unknown, restricted, none
p2/orbit/p2.eclipse.plugin/junit-vintage-engine/5.11.0, unknown, restricted, none

This raises two questions on my side:

  1. Why are there artefacts in Orbit for which the license is unknown?
  2. Is it necessary to add the test dependencies also to the DEPENDENCIES file? For the SBOM generation I think the test dependencies are excluded?

@waynebeaton
What are the rules for the DEPENDENCIES generation? Should the test dependencies be included? Should I skip the test plugins or try the includeScope setting?

@merks
Copy link
Contributor Author

merks commented Aug 25, 2024

There often appear to be synonyms for the same artifact:

https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/?sort=created_date&state=closed&search=maven%2Fmavencentral%2Forg.junit.jupiter&first_page_size=100

The Orbit builds use dash and it issues for new libraries:

https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/?sort=created_date&state=closed&search=maven%2Fmavencentral%2Forg.junit.jupiter&first_page_size=100

The builds such as https://ci.eclipse.org/orbit/job/orbit-simrel-maven-osgi/lastBuild/consoleFull log information like this:

[INFO] --- license-tool-plugin:1.1.1-SNAPSHOT:license-check (license-check) @ org.eclipse.orbit.maven.osgi.parent ---
[INFO] Querying Eclipse Foundation for license data for 308 items.
[INFO] Found 265 items.
[INFO] Querying ClearlyDefined for license data for 43 items.
[INFO] Found 43 items.
[INFO] Vetted license information was found for all content. No further investigation is required.
[INFO] Summary file was written to: /home/jenkins/agent/workspace/orbit-simrel-maven-osgi/maven-osgi/target/dash/summary

So I assume that dash has reviewed and approved everything.

@waynebeaton
Copy link

As @merks says, p2 dependencies are often synonyms for similar artifacts. These may have already been approved in their maven form.

@fipro78 The easiest thing to do is to just create the review issues so that the system can learn about this particular synonym. In the case of JUnit, the tool is pretty good at figuring out the association between the p2 ID and the corresponding maven artifact. I've initiated scans of some of the outstanding dependencies.

Strictly speaking, all dependencies should be vetted unless there is some compelling reason not to. An argument could be made that it is probably okay to exclude the test scope, but my strong preference for now is that we get good licence information for all dependencies, including test content.

@fipro78 fipro78 closed this as completed Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants