Fix jdt.feature version in P build and add commonmark bundles to the patch #2341

jarthana · 2024-09-12T10:48:10Z

No description provided.

patch

jarthana · 2024-09-12T10:50:04Z

@MohananRahul Please take a look at this and merge. Can you also kick off a new P build after that? TIA!

stephan-herrmann · 2024-09-12T10:55:50Z

eclipse.platform.releng.tychoeclipsebuilder/java23patch/org.eclipse.jdt.java23patch/feature.xml

+   <plugin
+         id="org.commonmark"
+         version="0.0.0"/>
+   <plugin
+         id="org.commonmark-gfm-tables"
+         version="0.0.0"/>


Is this saying that commonmark plugins should be available in the P-build already due to ..prereqs.sdk? That would be very cool!

Is the include of these 3rd party bundles necessary? I expect the bundles that require them are sufficient requirements to ensure they get installed.

@akurtakov

With all the effort to make upgrades to 3rd party bundles easier, I hope we don’t move in the opposite direction at the same time.

As bundles require them, features should not list them at all.

Is the include of these 3rd party bundles necessary? I expect the bundles that require them are sufficient requirements to ensure they get installed.

I think this is more about provisioning than about installing: without these inclusions, installation simply has no chance to find those plugins. Ergo, the P-build update site must somehow "contain" those plugins, either physically, or using a composite site mentioning also orbit.

also note, that the patch feature is a bit of a short lived animal, where upgrades of individual ingredients is less likely.

Is this saying that commonmark plugins should be available in the P-build already due to ..prereqs.sdk? That would be very cool!

That's right as per Sravan.

If it’s in the category.xml it must end up in the update site built from that at least as built by tycho.

As said I tried it using the export wizard, which did not work.

Next I tried to build using maven / tycho, but I seem to be too stupid for that. Current failure:

Missing requirement: org.eclipse.jdt.core 3.39.50.qualifier requires 'osgi.bundle; org.eclipse.core.runtime [3.29.0,4.0.0)' but it could not be found

Is it possible to build just the patch feature, or must I perform a full SDK build with all submodules materialized?

Please try mvn -f eclipse.platform.releng.tychoeclipsebuilder/java23patch/pom.xml clean verify -Pjava23patch -DskipTests=true

This is the command we use for patch build

So perhaps this will be my last question: Where can I find org.eclipse:eclipse-platform-parent:4.33.0 ? repo.eclipse.org doesn't seem to have it, and also the snapshot version seems to have gone missing.

It is there https://repo.eclipse.org/content/repositories/eclipse/org/eclipse/eclipse-platform-parent/4.33.0-SNAPSHOT/ . What you hit is infrastructure issue at EF.

It is not that hard to add it to category.xml and move on but do as you please. I am more and more convinced that PDE/JDT/Equinox should be their own separate releng procedures and scripts which projects can drive according to their taste and needs.

In the past, the P builds also had PDE in them. So, it probably made sense at that time to have the releng scripts in the releng project. Would be nice if we can get them into jdt feature itself.

Also, what can we, the JDT committers, do to get permissions to kick off P builds and Y builds?

Move them to https://ci.eclipse.org/jdt/ . This is not a different issue from Platform committers not being able to run https://ci.eclipse.org/jdt/job/copyAndDeployJDTCompiler/ .
It's a possibility to add 1-2 JDT committers manually to some jobs (IIRC that gives the permission to edit the cron pattern but doesn't give direct run permission) but this would be overriden whenever https://ci.eclipse.org/releng/job/Create%20Jobs/ is run (and it's the only way to edit all jenkinsfiles and redeploy in one go and there are so many different scripts) and the manually added committers will have to be readded.

jarthana · 2024-09-12T13:57:43Z

I rolled back the changes in feature.xml and made similar changes in category.xml as @merks suggested. Don't know what I need to make the PR pass, though. Is this a result of trying build off the R4_33 branch?

jarthana · 2024-09-12T14:29:36Z

@MohananRahul Any idea what's going on here? Is this because jdt.core.binaries and jdt feature are pulled from master? Shouldn't we pull everything from the R4_33_maintenance branch?

jarthana · 2024-09-12T14:55:10Z

OK, that last hack didn't work either. I give up and hopefully someone with more releng expertise chip in here.

Also, I see this, which explains why my change didn't take effect:
Loading trusted files from base branch R4_33_maintenance at 1683e62084aaeff0545baf8e6b6324eae691d067 rather than 60a3b57e9ec654c4e5bb4e49a9c8dc2e72b953cb Obtained Jenkinsfile from 1683e62084aaeff0545baf8e6b6324eae691d067 ‘Jenkinsfile’ has been modified in an untrusted revision

stephan-herrmann · 2024-09-12T21:28:44Z

Looking at https://ci.eclipse.org/platform/job/eclipse.platform.releng.aggregator/view/change-requests/ I'm not confident that the PR build is likely to succeed any time soon.

OTOH, P-builds have been working to some degree as can be seen at https://download.eclipse.org/eclipse/updates/4.33-P-builds/

All that's needed to make progress are the changes in this PR.

If changes to jenkinsfile are reverted none of the changes here should in any way affect master build, right?

So I kindly ask that someone with commit rights to this repo simply push Jay's changes - minus jenkinsfile, and the next P-build will tell us success / failure better then a broken PR build.

stephan-herrmann · 2024-09-12T21:37:08Z

After Java 23 GA I'm happy to join discussions of unburdening platform.releng from these tasks, but I don't think it's humanly possible to make this move between today and 17 Sept!

JDT doesn't yet have any aggregator build, do we?
I don't see JDT prepared to sign artifacts on short notice.

MohananRahul · 2024-09-13T08:05:48Z

After applying this fix locally PBuild fails.

Adding in category.xml didn't work. but if add as suggested earlier in feature.xml , able to generate it.

akurtakov · 2024-09-13T08:55:02Z

After applying this fix locally PBuild fails.

Adding in category.xml didn't work. but if add as suggested earlier in feature.xml , able to generate it.

Well, @jarthana added commonmark as feature (which it clearly isn't) while the example and what @stephan-herrmann was using bundle. Please change.

merks · 2024-09-13T09:09:40Z

FYI, I am currently trying to replicate a build locally so that I can help more effectively and see the problems. But it's a bit of a learning curve.

MohananRahul · 2024-09-13T09:14:10Z

After applying this fix locally PBuild fails.

Adding in category.xml didn't work. but if add as suggested earlier in feature.xml , able to generate it.

Well, @jarthana added commonmark as feature (which it clearly isn't) while the example and what @stephan-herrmann was using bundle. Please change.

Yes, as bundle works category.xml, Let me create PR

sravanlakkimsetti · 2024-09-13T09:27:54Z

Please hold on for some time. I am looking into this one.

there are three new bundles that are required. We need to be able to install these new bundles. For installation we use eclipse p2. because of these the dependencies needs to be present in the feature patch. That will also add the bundles to repo.

I am currently building locally first.

merks · 2024-09-13T09:32:22Z

I get much farther doing as @akurtakov suggests:

I keep needing more magic properties to be set:

mvn clean verify -Pbuild-individual-bundles -Duser.home=D:/Users/test35 -DfeatureToPatchVersion=3.19.600.v20240903-0240 -DfeatureToPatch=org.eclipse.jdt

merks · 2024-09-13T10:26:19Z

This worked for me:

$mvn clean verify -Pbuild-individual-bundles -Duser.home=D:/Users/test35 -DfeatureToPatchVersion=3.19.600.v20240903-0240 -DfeatureToPatch=org.eclipse.jdt -DversionRangeForPatch="[3.19.600.v20240903-0240,3.19.600.v20240903-0240]"

But in the real build, what sets the magic properties?

sravanlakkimsetti · 2024-09-13T10:27:02Z

These changes have been replaced by #2344

merks · 2024-09-13T10:29:36Z

The contents are correct too:

merks · 2024-09-13T10:30:54Z

It seems helping is not very effective when folks ignore what is being done. 😱

I can certainly find other ways to waste my time. 😭

sravanlakkimsetti · 2024-09-13T10:51:29Z

@merks I completely agree your approach is much better than the existing approach. Since the java23 release in next week and jdt team here went into panic mode I went ahead and fixed the patch build using older approach which I am quite familiar with. That said, I am open if someone else can take up the alternative approach to provide a P build.

btw, for the record, I also do not think highly of the current patch build process as it is too fragile with too many hacks. Lets migrate the patch build to the better one you suggested if someone else can pick it up now or for the next (Java24) release atleast.

merks · 2024-09-13T11:03:36Z

I know everyone means well. Also, while the includes of the plugins in the feature are not such a big deal for a temporary patch feature. I don't think it's necessary as you stated. The bundles just need to be available in the update site.

My big concern is the problem commons logging which basically destroys the users ability to work with EGit, p2 is broken, and marketplace is broken. An installation with that problem cannot be repaired except maybe by rollback:

#2344 (comment)

It just seems so unnecessary given the bundle is already in the 4.33 update site but best to let package imports choose the 1.2 version that's used other Apache components to the exclusion of 1.3.

mpalat · 2024-09-13T11:18:59Z

@merks Due to the limited availability of team today and since this was a blocker for the release, I was literally sitting next to Sravan though he had moved out of the project and requested him to get a P Build up while I was doing some sanity testing. We can remove the apache.commons and upload a P build as you suggested for now. @stephan-herrmann @jarthana any comments here?

akurtakov · 2024-09-13T11:21:34Z

A suggestion for next cycle - start P-builds early in the cycle not as a last minute thing.

stephan-herrmann · 2024-09-13T11:26:01Z

@merks Due to the limited availability of team today and since this was a blocker for the release, I was literally sitting next to Sravan though he had moved out of the project and requested him to get a P Build up while I was doing some sanity testing. We can remove the apache.commons and upload a P build as you suggested for now. @stephan-herrmann @jarthana any comments here?

I'm sorry if commons-logging was a red herring. I believed that was the reason why at one point p2 claimed the feature were not applicable although commonmark was already installed.

If we have a patch build that is installable in pristine SDK 4.33 without stuffing commons-logging into the patch: much better!

Many thanks!!

I'm sure you'll let us know once a new P-build is available for testing, right? :)

merks · 2024-09-13T11:32:28Z

@mpalat

It's all fine. As I said, everyone means well and there was quite a bit of pressure due to deadlines (that would be best avoided in the future). I still haven't completely learned not to vent my frustration, so apologies for that. No offense intended to the hard working people!

Let's just get to the bottom of the logging library issue. I'm here to help.

For background, most EPP packages have this version installed:

That installed in combination with the 1.3.4 version of commons-logging version appears to be really bad with class loader problems. The 1.2.0 version of that bundle doesn't appear to cause problems in combination with 1.3.4, though I'm not exactly sure why or what's the difference. In any case, I worked hard to eliminate the older 1.2.0.v20180409-1502 version of org.apache.commons.logging from the release train to prevent future problems as much as possible.

As such, I'm highly doubtful that anything in the patch feature's transitive closure should require this specific bundle by name. I expect there are only package requirements and that the 1.2.0 version satisfies those requirements. Best that be the case!

stephan-herrmann · 2024-09-13T11:39:13Z

I'm sure you'll let us know once a new P-build is available for testing, right? :)

Nevermind, I found the patch to be available already. Installs fine and passes smoke test.

GREAT!

So, we have a release candidate!

In case we had some last minute code changes, would it require more than a push of one button to create a new P-build? Who has the permissions to push that button?

merks · 2024-09-13T11:51:39Z

Can someone share the URL of the patch?

stephan-herrmann · 2024-09-13T11:57:00Z

Can someone share the URL of the patch?

Sure: here's the composite: https://download.eclipse.org/eclipse/updates/4.33-P-builds/

merks · 2024-09-13T12:23:40Z

I tested installing 2024-06 Java IDE, updating to 2024-09, and then installing the patches. Interestingly the common-logging bundle is not actually installed in the end result:

Probably because a patch is different from a regular feature and expresses the requirement like this:

    <change>
      <from>
        <required namespace='org.eclipse.equinox.p2.iu' name='org.apache.commons.commons-logging' range='0.0.0'/>
      </from>
      <to>
        <required namespace='org.eclipse.equinox.p2.iu' name='org.apache.commons.commons-logging' range='[1.3.4,1.3.4]'/>
      </to>
    </change>

I.e., it's more like update instructions. So probably this instructs to update org.apache.commons.commons-logging to 1.3.4 only if it's present. Probably that even applies for the commonmark bundles, which are only actually installed because some bundle actually requires them to be installed, not because of the inclusion in the patch feature:

So the org.apache.commons.commons-logging thing looks to be a red herring, but one that doesn't cause any problems. Also, even the inclusion of the commonmark bundles in the feature wasn't strictly necessary, just their presence in the update site, via category.xml, would suffice.

In any case, it looks like a case of don't worry be happy. 🥳

mpalat · 2024-09-14T08:29:06Z

@mpalat

It's all fine. As I said, everyone means well and there was quite a bit of pressure due to deadlines (that would be best avoided in the future). I still haven't completely learned not to vent my frustration, so apologies for that. No offense intended to the hard working people!

hey @merks - please do not change yourself - your comments are indeed extremely valuable as always. Personally I feel privileged to be involved in this Eclipse family of extremely dedicated and knowledgeable people like you and others, where everyone means well as you mentioned, also everyone being themselves and all working towards a great release. sorry for commenting (that too non-technical one) on a closed issue but could n't resist so that it will not be missed in the release activities.

stephan-herrmann · 2024-09-19T12:05:52Z

The technical aspect of this has been resolved via #2344. So I guess anyone with the necessary permission can safely close this PR.

OTOH, the discussion we started here, should be continued, for which purpose I just created eclipse-jdt/eclipse.jdt.core#2985 . Please join me there.

akurtakov · 2024-09-19T12:40:29Z

I'm closing this one now as there is nothing more to be done here. The change was merged in via #2344

Fix jdt.feature version in P build and add commonmark bundles to the

b61cb66

patch

jarthana mentioned this pull request Sep 12, 2024

Update P build to the release version of 4.33 #2340

Closed

stephan-herrmann reviewed Sep 12, 2024

View reviewed changes

jarthana added 3 commits September 12, 2024 18:49

Review comments

f4569e2

Add dependencies to category.xml

d759fe3

Partial rollback

cc8d441

Use R4_33_maintenance instead of master

60a3b57

sravanlakkimsetti enabled auto-merge (rebase) September 12, 2024 18:11

sravanlakkimsetti disabled auto-merge September 12, 2024 18:11

mpalat mentioned this pull request Sep 13, 2024

Adding markdown lib to P-build #2344

Merged

stephan-herrmann mentioned this pull request Sep 13, 2024

[23] General Release Activities eclipse-jdt/eclipse.jdt.core#2213

Closed

2 tasks

mpalat mentioned this pull request Sep 16, 2024

[23] P Build Creation - JDT placeholder eclipse-jdt/eclipse.jdt.ui#1649

Closed

akurtakov closed this Sep 19, 2024

akurtakov mentioned this pull request Nov 5, 2024

[24] Top Level Bug for Java 24 Support for JDT.Core eclipse-jdt/eclipse.jdt.core#2899

Open

32 tasks

Fix jdt.feature version in P build and add commonmark bundles to the patch #2341

Fix jdt.feature version in P build and add commonmark bundles to the patch #2341

Conversation

jarthana commented Sep 12, 2024

jarthana commented Sep 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sravanlakkimsetti Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarthana commented Sep 12, 2024

jarthana commented Sep 12, 2024

jarthana commented Sep 12, 2024 • edited Loading

stephan-herrmann commented Sep 12, 2024 • edited Loading

stephan-herrmann commented Sep 12, 2024

MohananRahul commented Sep 13, 2024 • edited Loading

akurtakov commented Sep 13, 2024

merks commented Sep 13, 2024

MohananRahul commented Sep 13, 2024

sravanlakkimsetti commented Sep 13, 2024

merks commented Sep 13, 2024

merks commented Sep 13, 2024

sravanlakkimsetti commented Sep 13, 2024

merks commented Sep 13, 2024

merks commented Sep 13, 2024

sravanlakkimsetti commented Sep 13, 2024

merks commented Sep 13, 2024 • edited Loading

mpalat commented Sep 13, 2024 • edited Loading

akurtakov commented Sep 13, 2024

stephan-herrmann commented Sep 13, 2024 • edited Loading

merks commented Sep 13, 2024

stephan-herrmann commented Sep 13, 2024

merks commented Sep 13, 2024

stephan-herrmann commented Sep 13, 2024

merks commented Sep 13, 2024

mpalat commented Sep 14, 2024

stephan-herrmann commented Sep 19, 2024

akurtakov commented Sep 19, 2024

sravanlakkimsetti Sep 12, 2024 •

edited

Loading

jarthana commented Sep 12, 2024 •

edited

Loading

stephan-herrmann commented Sep 12, 2024 •

edited

Loading

MohananRahul commented Sep 13, 2024 •

edited

Loading

merks commented Sep 13, 2024 •

edited

Loading

mpalat commented Sep 13, 2024 •

edited

Loading

stephan-herrmann commented Sep 13, 2024 •

edited

Loading