Skip to content

Conversation

@merks
Copy link
Contributor

@merks merks commented May 22, 2025

@merks
Copy link
Contributor Author

merks commented May 22, 2025

FYI, I can see that this works because Oomph's repository explorer can explore the contents of the workspace projects published as IUs by p2's publisher:

image

@merks merks requested review from akurtakov and jonahgraham May 22, 2025 12:11
# https://github.com/eclipse-packaging/packages/issues/307
requires.100.namespace=org.eclipse.equinox.p2.iu
requires.100.name=com.genuitec.eclipse.theming.core.feature.feature.group
requires.100.range = [0.0.1,2024.4.0.202412101111]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know that they don't auto push things with new qualifier? IMO it's best this to become [0.0.1, 2024.4.1).

Copy link
Contributor Author

@merks merks May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever they do, it must have a higher number to update. Note the upper bound is inclusive. And moreover, note that 2024.4.1) doesn't include 2024.4.0.202412101111 and because we want a negative requirement we must include the thing we want to exclude...

Given the comments here:

eclipse-cdt/cdt#1164 (comment)

Maybe we can hope for the best instead?

Or maybe better safe than sorry?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we don't have any clue in what version it will be fixed we better should exclude any version unless there is a proven fixed one available...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would exclude everything below 2025.0.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really think folks are missing the point and are now split hairs. We are excluding the broken version and are assuming that any new version will not be broken because that's what folks have promised.

@github-actions
Copy link
Contributor

Test Results

   545 files  ±0     545 suites  ±0   30m 13s ⏱️ +51s
 4 398 tests ±0   4 380 ✅ ±0   18 💤 ±0  0 ❌ ±0 
16 719 runs  ±0  16 578 ✅ ±0  141 💤 ±0  0 ❌ ±0 

Results for commit 25210e4. ± Comparison against base commit 9d3227d.

@laeubi
Copy link
Contributor

laeubi commented May 22, 2025

@merks adding this to SWT bundle really seems odd. Isn't this a problem of jface?

@jonahgraham
Copy link
Contributor

The thing they are code weaving badly is org.eclipse.ui.internal.BrandingProperties - within their modifications they are referring to now deleted org.eclipse.jface.resource.FileImageDescriptor

So I think there are numerous bundles that this negative req can go on, but I agree that JFace makes sense. But SWT may work too, at least for EPP because we don't really support updating JFace without updating SWT too.

@laeubi
Copy link
Contributor

laeubi commented May 22, 2025

at least for EPP

If only EPP is of a concern it could probably better added there... SWT really has nothing to do with that as the File was deleted from JFace.

I really find it ugly to add this to a bundle as we effectively should not maintain a list of bad third party plugins, but if we do so it should at least be on the right place.

@merks
Copy link
Contributor Author

merks commented May 22, 2025

So you guys think the right place is the JFace bundle? It's ugly no matter where we put it. 😢

@merks
Copy link
Contributor Author

merks commented May 22, 2025

Here instead:

eclipse-platform/eclipse.platform.ui#3002

@merks merks closed this May 22, 2025
@merks
Copy link
Contributor Author

merks commented May 22, 2025

If only EPP is of a concern it could probably better added there... SWT really has nothing to do with that as the File was deleted from JFace.

I really find it ugly to add this to a bundle as we effectively should not maintain a list of bad third party plugins, but if we do so it should at least be on the right place.

Nothing in EPP prevents the Platform from updating. The updated Platform is the problem so this is also a problem for the Eclipse SDK with darkest dark installed. So this is a problem that darkest dark does not work with the latest Platform...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants