search

eclipse-mpc / epp.mpc

Eclipse Public License 2.0
2 stars 3 forks source link

Remove HC4 site #2

Closed akurtakov closed 8 months ago

akurtakov commented 9 months ago

It's no longer used in MPC (hc5 is used) and even it was it shouldn't be done this way as this would lead to different qualifiers and thus duplicate hc4 in simrel. Delete entirely to reduce possible confusion and other issues.

akurtakov commented 9 months ago

@l3-g5 I plan to push a bunch of "cleanup" to start making sense of mpc. Are you going to review/accept such PRs?

l3-g5 commented 9 months ago

@l3-g5 I plan to push a bunch of "cleanup" to start making sense of mpc. Are you going to review/accept such PRs?

Sure. What kind of cleanups? Maybe a littele late for 2023-12, but we will see.

akurtakov commented 9 months ago

Sure. What kind of cleanups? Maybe a littele late for 2023-12, but we will see.

Target platform cleanup. Build plugins updates. Warnings fixes. Etc.

l3-g5 commented 9 months ago

Sounds great. Note that I already updated Tycho yesterday.

akurtakov commented 9 months ago

Ok, I'll open PRs one by one and wait till the first one is in.

l3-g5 commented 9 months ago

Regarding this PR: There was a reason why this update site was needed in the past. Some test with Windows or something like that. Will ask Carsten if we need to update or could savely remove it as you suggested.

akurtakov commented 9 months ago

Note, that this is not and shouldn't be used at all. This brings another question - userstorage brings Apache HC4 in but it's unclear to me - is it needed, is it developed, if yes where and etc.

l3-g5 commented 9 months ago

The idea is to deprecate the userstorage. See https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/2181 and https://gitlab.eclipse.org/eclipsefdn/it/websites/marketplace.eclipse.org/-/issues/191.

akurtakov commented 9 months ago

So is work started to drop it from MPC ?

l3-g5 commented 9 months ago

Currently the migration is only done on server side. MPC would be the next step. Not yet scheduled.

akurtakov commented 9 months ago

Currently the migration is only done on server side. MPC would be the next step. Not yet scheduled.

Would you please open issue in this repo with description - what/when/how is needed so others can jump in?

akurtakov commented 9 months ago

Regarding this PR: There was a reason why this update site was needed in the past. Some test with Windows or something like that. Will ask Carsten if we need to update or could savely remove it as you suggested.

So what is the outcome ?

akurtakov commented 9 months ago

Now just imagine your efforts being totally neglected like it happens here! And projects complain they don't get help!

creckord commented 9 months ago

Now just imagine your efforts being totally neglected like it happens here! And projects complain they don't get help!

This was my fault, sorry. I'm not around that much these days, and Leif asked me to explain the history of this construct a bunch of times. Sadly, I lost track of it again and again, as life kept intervening. Anyway, here goes:

tl;dr

I think it's okay to remove the hc site. It's the pragmatic solution, although I would personally probably fix it instead.

the history

Disclaimer: I haven't checked if advances in Tycho and target platform handling might have made this approach obsolete.

The reason for the update site was that it wasn't possible to have a cross-platform target platform definition with os-specific bundles alongside the platform definitions in MPC. Adding the httpclient-win bundle directly to the TP would have made it resolve successfully under Windows and fail with missing artifacts under linux and mac.

As a workaround, we created a wrapper feature for the Windows bits and added that to the TP instead. This worked, because now the platform filter was in the feature and not the TP. In order to build it (needing a different target platform setup), we created the httpclient-site, and just pulled the feature from there into MPC.

Also note that this is incorrect:

this would lead to different qualifiers

The site only wrapped the existing bundles from orbit. The qualifiers were not changed. What could happen was that a new version was pushed to orbit and we still had the old one in our site until we did a rebuild. But that would have happened anyay, as long as we included the httpclient bundles in the MPC update site.

current situation

In the move to HttpClient 5, the site was removed and HC5 was only implicitly included in the TP as a transitive dependency of org.eclipse.sdk.feature.group. This works and is probably the pragmatic way to go.

Personally, I still find it a bit ugly. For one, because having org.eclipse.sdk.feature.group in there in the first place is an ugly concession to having something easily launchable from the IDE over having only the required bits in the TP. But mostly because I am a firm believer in including everything that I explicitly use in my bundles in my TP as well. As such, I would probably migrate the site to HC5 and re-include it.

But it is a lot of extra moving parts (and given the length of this explanation, some very non-obvious ones), and as such, we can probably be pragmatic about it and remove the whole thing.

... and the hc bundles?

This leaves me with one question/remark about the change: It used to be best practice to include anything we consumed from Orbit into our own update site. Given that HC5 is now more or less part of the platform, and that this practice could actually cause multiple hc versions in simrel, I'm wondering if we should also go ahead and remove the org.eclipse.epp.mpc.dependencies.feature from the MPC site.

merks commented 8 months ago

In general we've been recommending that features contributed to SimRel not include 3rd party builds. To ensure that your update site is complete, any 3rd party bundles required via package imports (preferred) or via bundle requirements, can be directly specified in the category.xml or via a "3rd" party requirement feature that in turn is included in the category.xml (and not categorized to discourage anyone installing it). Typically duplicate versions in SimRel are caused by features including 3rd party bundles because that locks in a specific version. When those are avoided, the aggregator will generally pick the highest version that satisfies all the constraints of all the bundles using that library...