openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.87k stars 3.58k forks source link

Test all features, if they are correctly installable on the core runtime #386

Closed kaikreuzer closed 8 years ago

kaikreuzer commented 9 years ago

When testing the Karaf distro, I had some trouble with dependencies that were not available for some features (I remember Hue and Sonos). We will have to check all of them once, if they can be installed correctly.

maggu2810 commented 9 years ago

Sure, that should be done. But this is for me not a blocker at all, but a place for improvements. I would also prefer to further separate the runtime features.

kaikreuzer commented 9 years ago

But this is for me not a blocker at all, but a place for improvements.

Well, if you cannot install the Hue or Sonos binding on OH2 anymore, this would be quite a hard regression which would upset quite some people, so I'd rather want to avoid this.

maggu2810 commented 9 years ago

I prefer to solve as much as stuff as possible. But what is the current way to install the Hue or Sonos binding on OH2? I used Karaf a long time, so I am not familiar with the current mechanism. IIRC the bundles have to be dropped to the addon directory. This mechanism (deploy directory for Karaf ATM) should still work. Install stuff using Karaf features is something new, that no person know about. So, I do not see what we will break if some features does not work at day zero. Just to be clear: I do not want to introduce non working stuff. But you asked "Do they make sense to you" and so I want to state, that IMHO this is no blocker for me, just something that could be optimized.

kaikreuzer commented 9 years ago

IIRC the bundles have to be dropped to the addon directory. This mechanism (deploy directory for Karaf ATM) should still work.

Correct, but we just as we would not build a demo distro anymore, we also won't build an addons.zip - hence there is nothing to put into the addons folder :-)

And sticking to the addons would defeat the whole sense of switching to Karaf and maintaining Karaf features for all the addons.

maggu2810 commented 8 years ago

I learned that there is a automatic feature validation that could be used. Here two examples:

kaikreuzer commented 8 years ago

Sounds interesting, it would be cool to have that verified within the build itself.

maggu2810 commented 8 years ago

I started feature verification for shk. It seems to be working, as features could be resolved successfully and some others are complained about missing dependencies (that are really missing). So perhaps you can have a look there.

maggu2810 commented 8 years ago

I started some changed: https://github.com/openhab/openhab2/compare/karaf...maggu2810:karaf-features?expand=1

The verification is already running and identify improvements / TODOs.

maggu2810 commented 8 years ago

Perhaps this helps you

kaikreuzer commented 8 years ago

Or do you want to "hide" all stuff without a function on its own (a comprehensible reason). Don't know why I have choosen runtime before.

Yes, that's what I would like to do! These are dependencies that are installed when installing an "add-on", but they are no valid installable feature on their own. So we should use some other name than "misc". "runtime" sounds like a good choice ;-)

there are bundles that needs the compat1x bundle but are written for / ported to OH2. This should be checked if this is really necessary

Can you name them? I am only aware of astro1, but this is intentional, as the OH1 version has a larger feature set than the OH2 version.

maggu2810 commented 8 years ago

IIRC:

kaikreuzer commented 8 years ago

All these are ${project.version} and not ${oh1.version} - so they are already from OH2. So your point is that they still reference the compat layer as a requirement. Really?

maggu2810 commented 8 years ago

Yes, they are openHAB 2 addons and need the compat layer for openHAB 1 bundles

freebox

rfxcom

cometvisu

myopenhab

kaikreuzer commented 8 years ago

Good catch - I have fixed all of them with https://github.com/openhab/openhab2/pull/510.

kaikreuzer commented 8 years ago

"openhab-binding-dmx" needs "org.eclipse.osgi.framework.console". This should be changed to be optional, so the binding could also be used on other OSGi frameworks

Addressed with https://github.com/openhab/openhab/pull/3503

"openhab-persistence-logging" needs "ch.qos.logback.classic". I am not familar with that binding, but could it use the slf4j-api instead the logback one

This indeed uses the logback classes for its implementation and heavily relies on that. This is definitely not compatible, so we would have to leave that out of the distribution. Should be ok, I don't think that too many people really use it.

maggu2810 commented 8 years ago

I a logback bundle could coexist with the normal logging, we could add a logback artifact for this feature (persistence-logging) only. But don't know if this is relevant.

kaikreuzer commented 8 years ago

Sounds like a possibility we could briefly test at least.

maggu2810 commented 8 years ago

IMHO feature implemented by #519 As stated, it is disabled (but working) until the current master is merged (at least #510).

As stated here that two addons are currently deactivated:

kaikreuzer commented 8 years ago

FTR: #519 is merged. #510 is merged as well.

maggu2810 commented 8 years ago

510 is merged into the master, but not into the karaf branch.

To enable the verification it need to be merged in the karaf branch.

maggu2810 commented 8 years ago

See #521

maggu2810 commented 8 years ago

This should be solved by #523 and #539 now.

kaikreuzer commented 8 years ago

FTR, I just tested the installation of all add-ons and came across a few problems that should be addressed with https://github.com/openhab/openhab/pull/3549.

maggu2810 commented 8 years ago

Missing imports in the manifest file are not checked by the feature verification and this seems correct to me. That should be done at build time (maven-bundle-plugin / bndtools) or by the Eclipse IDE for Manifest-first approach.

kaikreuzer commented 8 years ago

Yes, that's also ok for me, but it means that we need to manually check it for the moment :-)