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

org.eclipse.equinox.cm seem to fails some TCKs #190

Open
laeubi opened this issue Jan 20, 2023 · 20 comments · Fixed by #533
Open

org.eclipse.equinox.cm seem to fails some TCKs #190

laeubi opened this issue Jan 20, 2023 · 20 comments · Fixed by #533

Comments

@laeubi
Copy link
Member

laeubi commented Jan 20, 2023

Currently org.eclipse.equinox.cm seem to fail some TCK tests, a report XML that can be opened in Eclipse can be found here:
TEST-org.osgi.test.cases.cm-8.1.0.202212101352.xml.zip

@github-actions
Copy link

This issue has been inactive for 180 days and is therefore labeled as stale.
If this issue became irrelevant in the meantime please close it as completed. If it is still relevant and you think it should be fixed some possibilities are listed below.
Please read https://github.com/eclipse-equinox/.github/blob/main/CONTRIBUTING.md#contributing-to-eclipse-equinox for ways to influence development.

@laeubi
Copy link
Member Author

laeubi commented Dec 18, 2023

@tjwatson can you take a look or have an idea what might causing this?

@tjwatson
Copy link
Contributor

@tjwatson can you take a look or have an idea what might causing this?

It is because of the quick implementation I did to be able to update SCR in https://bugs.eclipse.org/bugs/show_bug.cgi?id=538717 didn't include support for the coordinator.

@laeubi
Copy link
Member Author

laeubi commented Dec 18, 2023

@tjwatson can you take a look or have an idea what might causing this?

It is because of the quick implementation I did to be able to update SCR in https://bugs.eclipse.org/bugs/show_bug.cgi?id=538717 didn't include support for the coordinator.

Do you plan to work on this or can explain what exactly is missing? Is it coordinator support at all or is it incomplete?

@tjwatson
Copy link
Contributor

Do you plan to work on this or can explain what exactly is missing? Is it coordinator support at all or is it incomplete?

The Equinox coodinator implementation is complete. The CM implementation has no integration with the coordinator as described by the specification: https://docs.osgi.org/specification/osgi.cmpn/8.1.0/service.cm.html#service.cm-coordinatorsupport

@tjwatson
Copy link
Contributor

Do you plan to work on this?

No

@laeubi
Copy link
Member Author

laeubi commented Dec 18, 2023

I'm just wondering if felix.cm is compliant and do anything differently than equinox? Maybe we can then better use the felix impl im we have no missing features then?

@tjwatson
Copy link
Contributor

I'm just wondering if felix.cm is compliant and do anything differently than equinox? Maybe we can then better use the felix impl im we have no missing features then?

Is there anything we need CM for? I'm not sure I would pull in another implementation if we don't actually use CM for anything. What would the point be?

@laeubi
Copy link
Member Author

laeubi commented Dec 18, 2023

Is there anything we need CM for?

I don't think the CM is part of any EPP yet (@jonahgraham ?) or required by anything (I didn't find it in platform directly but maybe @merks has a reference in simrel)

I'm not sure I would pull in another implementation if we don't actually use CM for anything.

No the idea would be if equinox.cm does not offer anything more (and plaform/epp does not include it anyways) than felix.cm and

  1. is not compliant and
  2. no one want to work on it

maybe we should drop it and if there is any need for cm impl one can use the felix one instead.

@tjwatson
Copy link
Contributor

maybe we should drop it and if there is any need for cm impl one can use the felix one instead.

I don't have a strong opinion on keeping it. On the other hand, in the past Equinox was meant to be a place to get OSGi specification implementations from, regardless of if they were used by the Eclipse project itself or not. There are many things in Equinox not used by the Eclipse project itself, but I would not want to see a trend that simply removes everything not used by the Eclipse project. Case in point, we could remove the coordinator, then the TCK for CM should pass, assuming it handles the tests to skip things when a coordinator implementation isn't around.

With respect to this coordinator usage in the CM specification. I believe the coordinator to always have been a niche specification. I'm not aware of anyone that uses the coordinator. I could be wrong, but I've not seen any issue reports that would indicate it has any usage. At one point we attempted to use it in our products, but it was not a good fit for our use cases and ultimately we dropped all usage of it.

@laeubi
Copy link
Member Author

laeubi commented Dec 18, 2023

On the other hand, in the past Equinox was meant to be a place to get OSGi specification implementations from, regardless of if they were used by the Eclipse project itself or not.

Yes that's true, but then of course it should be compliant to the specification, that's why I wondering if felix.cm is actually implementing/compliant.

Case in point, we could remove the coordinator, then the TCK for CM should pass, assuming it handles the tests to skip things when a coordinator implementation isn't around.

Last time I checked coordinator was mandatory (the test fail completely if no coordinator service is there), I also can't find in the spec that that part is optional but also not much familiar with that addition.

@tjwatson
Copy link
Contributor

wondering if felix.cm is actually implementing/compliant.

I think so since it is what was used to verify the TCK for R8 release:

https://github.com/osgi/osgi/blob/8ab4f46152fe43176b2a157f32e205a94487a481/cnf/repo/org.osgi.impl.service.cm/org.osgi.impl.service.cm-8.0.0.lib#L2

@HannesWell
Copy link
Member

Can we unpin this issue since it is resolved?

@akurtakov akurtakov unpinned this issue Mar 29, 2024
@akurtakov
Copy link
Member

Done.

@laeubi
Copy link
Member Author

laeubi commented Mar 29, 2024

Can we unpin this issue since it is resolved?

Why? It still fails and the purpose of the pinning is it does not get down the list as it constantly fails the verification...

At least I can't find why it gots automatically closed...

@laeubi
Copy link
Member Author

laeubi commented Mar 29, 2024

@laeubi laeubi reopened this Mar 29, 2024
@laeubi laeubi pinned this issue Mar 29, 2024
@HannesWell
Copy link
Member

Can we unpin this issue since it is resolved?

Why? It still fails and the purpose of the pinning is it does not get down the list as it constantly fails the verification...

Because it was closed. :) But if it is still an issue then re-opening it and pinning it is fine.
Just found in strange to pin a closed issue.

@laeubi
Copy link
Member Author

laeubi commented Mar 29, 2024

I have no clue what closed it :-D

@HannesWell
Copy link
Member

I have no clue what closed it :-D

The history says it was you with #533 :P

grafik

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 a pull request may close this issue.

4 participants