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

[systeminfo] Migrate to BND #5455

Merged
merged 2 commits into from
Apr 22, 2019
Merged

Conversation

falkena
Copy link
Contributor

@falkena falkena commented Apr 14, 2019

Signed-off-by: Alexander Falkenstern [email protected]

@falkena falkena requested review from svilenvul and a team as code owners April 14, 2019 14:46
@falkena falkena force-pushed the feature/MigrateToBnd branch from 693e036 to e494bc3 Compare April 14, 2019 14:52
Copy link
Member

@davidgraeff davidgraeff left a comment

Choose a reason for hiding this comment

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

Thanks.
The karaf feature file need to be modified. The libraries need to be added there as well so that openHAB knows of them and can install them during runtime.

bundles/org.openhab.binding.systeminfo/.classpath Outdated Show resolved Hide resolved
@falkena falkena force-pushed the feature/MigrateToBnd branch 5 times, most recently from 4789637 to b2dc0e7 Compare April 15, 2019 15:55
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

otherwise looks good.

@@ -847,7 +847,10 @@

<feature name="openhab-binding-systeminfo" description="System Info Binding" version="${project.version}">
<feature>openhab-runtime-base</feature>
<bundle start-level="80">mvn:org.openhab.binding/org.openhab.binding.systeminfo/${project.version}</bundle>
<bundle dependency="true">wrap:mvn:com.github.oshi/oshi-core/3.13.0</bundle>
<bundle dependency="true">wrap:mvn:net.java.dev.jna/jna/5.2.0</bundle>
Copy link
Member

Choose a reason for hiding this comment

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

wrapping should not be necessary, this is already an OSGi bundle

Copy link
Contributor Author

@falkena falkena Apr 15, 2019

Choose a reason for hiding this comment

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

Played around to solve travis issues. Sadly, still no progress. see below

<bundle start-level="80">mvn:org.openhab.binding/org.openhab.binding.systeminfo/${project.version}</bundle>
<bundle dependency="true">wrap:mvn:com.github.oshi/oshi-core/3.13.0</bundle>
<bundle dependency="true">wrap:mvn:net.java.dev.jna/jna/5.2.0</bundle>
<bundle dependency="true">wrap:mvn:net.java.dev.jna/jna-platform/5.2.0</bundle>
Copy link
Member

Choose a reason for hiding this comment

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

wrapping should not be necessary, this is already an OSGi bundle

Copy link
Contributor Author

@falkena falkena Apr 15, 2019

Choose a reason for hiding this comment

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

Played around to solve travis issues. Sadly, still no progress. See below

Copy link
Member

Choose a reason for hiding this comment

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

I don‘t think Travis failure is related to your changes. Travis fails on all PR.

Copy link
Contributor Author

@falkena falkena Apr 15, 2019

Choose a reason for hiding this comment

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

My local build fails too :-( I mean: Build binding only works. I i try to build all addons on top level, then feature verification fails...

@@ -847,7 +847,10 @@

<feature name="openhab-binding-systeminfo" description="System Info Binding" version="${project.version}">
<feature>openhab-runtime-base</feature>
<bundle start-level="80">mvn:org.openhab.binding/org.openhab.binding.systeminfo/${project.version}</bundle>
<bundle dependency="true">wrap:mvn:com.github.oshi/oshi-core/3.13.0</bundle>
Copy link
Member

Choose a reason for hiding this comment

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

wrapping should not be necessary, this is already an OSGi bundle

Copy link
Contributor Author

@falkena falkena Apr 15, 2019

Choose a reason for hiding this comment

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

Played around to solve travis issues. Sadly, still no progress: See below.

@falkena falkena force-pushed the feature/MigrateToBnd branch from b2dc0e7 to 6bbb2ee Compare April 15, 2019 20:05
@falkena
Copy link
Contributor Author

falkena commented Apr 15, 2019

I stuck with karaf feature verification: openHAB Add-ons :: Features :: Karaf :: Add-ons .... FAILURE
The message is

Feature resolution failed for [openhab-binding-systeminfo/2.5.0.SNAPSHOT]
[ERROR] Message: Unable to resolve root: missing requirement [root] osgi.identity; osgi.identity=openhab-binding-systeminfo; type=karaf.feature; version=2.5.0.SNAPSHOT; filter:="(&(osgi.identity=openhab-binding-systeminfo)(type=karaf.feature)(version>=2.5.0.SNAPSHOT))" [caused by: Unable to resolve openhab-binding-systeminfo/2.5.0.SNAPSHOT: missing requirement [openhab-binding-systeminfo/2.5.0.SNAPSHOT] osgi.identity; osgi.identity=org.openhab.binding.systeminfo; type=osgi.bundle; version="[2.5.0.201904152011,2.5.0.201904152011]"; resolution:=mandatory [caused by: Unable to resolve org.openhab.binding.systeminfo/2.5.0.201904152011: missing requirement [org.openhab.binding.systeminfo/2.5.0.201904152011] osgi.wiring.package; filter:="(&(osgi.wiring.package=oshi.hardware)(version>=3.13.0)(!(version>=4.0.0)))" [caused by: Unable to resolve com.github.oshi.oshi-core/3.13.0: missing requirement [com.github.oshi.oshi-core/3.13.0] osgi.wiring.package; filter:="(osgi.wiring.package=com.sun.jna.platform.linux)"]]]

Build runs smoothly. Any hint?

@J-N-K
Copy link
Member

J-N-K commented Apr 15, 2019

Seems to be a bug in JNA 5.2.0: java-native-access/jna#1054

Did this ever work under Linux?

Edit: this probably broke by updating the libs in #4834. We should check if we can revert that.

@J-N-K
Copy link
Member

J-N-K commented Apr 16, 2019

You can try removing the dependency to net.java.dev.jna:.jna-platform, putting jna-platform-5.2.0.jar to /lib and add the following lines to pom.xml.

<properties>
  <bnd.exportpackage>com.sun.jna.platform.*;version="5.2.0"</bnd.exportpackage>
</properties>

I did not try but that should export all packages.

@falkena
Copy link
Contributor Author

falkena commented Apr 16, 2019

Hi @J-N-K , thank you :-) I've traced the dependecies down to jna 4.2.1: still the same problem. It seems, that it's broken for the long time and nobody catched the bug. On Raspberry jna 5.2.0 runs as expected. Therefore it seems, that we must live with workaround.

According to oshi/oshi#752, bugfix is planned for 3.13.1: https://github.com/oshi/oshi/milestone/11

@falkena falkena force-pushed the feature/MigrateToBnd branch from 6bbb2ee to 3a3b625 Compare April 16, 2019 17:36
@J-N-K
Copy link
Member

J-N-K commented Apr 16, 2019

The export works?

@falkena
Copy link
Contributor Author

falkena commented Apr 16, 2019

Build and verification was OK. Currently i rebase my other PR and test all the stuff.

@falkena
Copy link
Contributor Author

falkena commented Apr 16, 2019

Ok. Rebased, updated eclipse, wait till workspace build... And new fail...

openhab> bundle:restart 254
Error executing command: Error restarting bundles:
Unable to start bundle 254: Could not resolve module: org.openhab.binding.systeminfo [254]
Unresolved requirement: Import-Package: com.sun.jna; version="[5.2.0,6.0.0)"

Ok, embed jna-5.2.0.jar the same way as jna-platform-5.2.0.jar. Rebuilt, new attempt...

openhab> bundle:restart 254
Error executing command: Error restarting bundles:
Unable to start bundle 254: Could not resolve module: org.openhab.binding.systeminfo [254]
Unresolved requirement: Import-Package: oshi; version="[3.13.0,4.0.0)"

Ok, embed oshi-core-3.13.0.jar the same way as jna-5.2.0.jar. Rebuilt, new attempt...

openhab> bundle:restart 254
Error executing command: Error restarting bundles:
Unable to start bundle 254: Could not resolve module: org.openhab.binding.systeminfo [254]
Unresolved requirement: Import-Package: com.sun.management

Built on Windows and run on raspberry...

@davidgraeff
Copy link
Member

Did it work with the old buildsystem? If not we are not loosing anything if we merge this PR for now and keep investigating later

@J-N-K
Copy link
Member

J-N-K commented Apr 17, 2019

This is probably due to the missing dependencies which are defined in the feature.

Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

did you check that the itests successfully run?

<attribute name="test" value="true"/>
</attributes>
</classpathentry>
<classpathentry kind="output" path="target/classes"/>
Copy link
Member

Choose a reason for hiding this comment

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

add jna-platform as lib (see e.g. allplay)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in current version

<version>3.13.0</version>
<scope>provided</scope>
</dependency>
<dependency>
Copy link
Member

Choose a reason for hiding this comment

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

remove this dependency (always provided if embedded)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -855,7 +855,10 @@

<feature name="openhab-binding-systeminfo" description="System Info Binding" version="${project.version}">
<feature>openhab-runtime-base</feature>
<bundle start-level="80">mvn:org.openhab.binding/org.openhab.binding.systeminfo/${project.version}</bundle>
<bundle dependency="true">mvn:com.github.oshi/oshi-core/3.13.0</bundle>
<bundle dependency="true">mvn:net.java.dev.jna/jna-platform/5.2.0</bundle>
Copy link
Member

Choose a reason for hiding this comment

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

remove feature dependency (always provided if embedded)

@falkena
Copy link
Contributor Author

falkena commented Apr 17, 2019

@davidgraeff : Yes it run bevore @J-N-K I already performed requested changes yesterday, just not pushed them, since it will not run...

@J-N-K
Copy link
Member

J-N-K commented Apr 17, 2019

As I said before: the feature resolution in your production setup will not work because it is using the old feature.xml.

It should work if you install the missing bundles manually. Did you try that?

@Hilbrand Hilbrand added the infrastructure Build system and Karaf related issues and PRs label Apr 17, 2019
Signed-off-by: Alexander Falkenstern <[email protected]>
@falkena falkena force-pushed the feature/MigrateToBnd branch from 3a3b625 to 3af9574 Compare April 17, 2019 21:31
@falkena
Copy link
Contributor Author

falkena commented Apr 17, 2019

I run two environments: production (2.5.0M1) and development with latest snapshot. The strange thing is, i get

Unresolved requirement: Import-Package: oshi; version="[3.13.0,4.0.0)"

if i try to load the binding, but according to oshi-core manifest file, it's provided by oshi-core.jar...

@J-N-K
Copy link
Member

J-N-K commented Apr 19, 2019

I raised a PR against your repo. With those fixes everything compiles and I verified that the binding is working on two systems (both Linux). The tests fail, though. Maybe you can have a look at that.

@J-N-K
Copy link
Member

J-N-K commented Apr 20, 2019

@falkena, thanks to @matthiasblaesing JNA 5.3.0 was released very fast. I have updated my PR to your repo, please merge that, the tests now run fine, too.

@falkena
Copy link
Contributor Author

falkena commented Apr 20, 2019

@falkena, thanks to @matthiasblaesing JNA 5.3.0 was released very fast. I have updated my PR to your repo, please merge that, the tests now run fine, too.

Hi @J-N-K : I've merged your PR. Thank you for help! I'll rebase my another PR and test the stuff this week. If something will go wrong, i'll fix it. May be @dbwiddis can find some time and update the oshi dependency to JNA 5.3.0. Officially OSHI depends now still on JNA 5.2.0

@dbwiddis
Copy link

Hah. I’ve been waiting on JNA 5.3 and checking almost daily. But missed a day.

@J-N-K
Copy link
Member

J-N-K commented Apr 20, 2019

But it works with 5.3.0. so no need to hurry.

@davidgraeff davidgraeff merged commit f101cca into openhab:master Apr 22, 2019
mherwege pushed a commit to mherwege/openhab-addons that referenced this pull request May 10, 2019
* Migrate systeminfo to bnd
* fix dependencies, update JNA, fix tests (openhab#8)

Signed-off-by: Jan N. Klug <[email protected]>
@wborn wborn added this to the 2.5 milestone May 26, 2019
Pshatsillo pushed a commit to Pshatsillo/openhab-addons that referenced this pull request Jun 19, 2019
* Migrate systeminfo to bnd
* fix dependencies, update JNA, fix tests (openhab#8)

Signed-off-by: Jan N. Klug <[email protected]>
Signed-off-by: Pshatsillo <[email protected]>
ne0h pushed a commit to ne0h/openhab-addons that referenced this pull request Sep 15, 2019
* Migrate systeminfo to bnd
* fix dependencies, update JNA, fix tests (openhab#8)

Signed-off-by: Jan N. Klug <[email protected]>
Signed-off-by: Maximilian Hess <[email protected]>
@wborn wborn removed this from the 2.5 milestone Dec 8, 2019
@wborn wborn added this to the 2.5 milestone Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Build system and Karaf related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants