add base to migrate to bnd and migrate astro #5018

maggu2810 · 2019-03-02T10:06:14Z

All changes has been made on command line, so I still need to create the Eclipse IDE project settings file. Will add a commit later this day.

openHAB Distro needs to be changed to use the "new" addons Karaf feature (GAV)
Jenkins needs to be changed to execute both builds (already done for Travis)

maggu2810 · 2019-03-02T10:18:33Z

Because of openhab/openhab-distro#882 (comment) we should migrate the bundles with tests first.

I am willing to support you with that mirgations.

WDYT is necessary to get that first migration upstream soon?

J-N-K · 2019-03-02T10:29:17Z

I have no idea how that works, but would be willing to try the DMX binding (which has tests and additionally also rule actions) as a next step. There is only a small PR open for that binding which can easily be adapted if necessary.

maggu2810 · 2019-03-02T10:31:12Z

@J-N-K After this commit has been merged, I will come up with another PR that contains only the changes necessary to migrate one binding.
I assume then it should be more visible what needs to be done.

wborn · 2019-03-02T10:37:25Z

Many thanks for your continued help on getting https://github.com/openhab/openhab2-addons/issues/5005 resolved @maggu2810. 👍

WDYT is necessary to get that first migration upstream soon?

Do you mean to get this PR merged or is there something else that needs to be done first?
I'd sure like @kaikreuzer to also review this PR.

J-N-K

Can we enable the tests again for the maven builds?

maggu2810 · 2019-03-02T11:56:25Z

Tests should be enabled for Maven + Bnd. Did you mean the SAT checks?

wborn · 2019-03-02T12:01:16Z

Perhaps we can also create a new milestone (M2) before we start migrating everything. That way users have a fallback in case of regressions. But we would need openhab/openhab-distro#866 to be fixed before we can create a new milestone.

J-N-K · 2019-03-02T12:01:16Z

I thought that -DskipTests=true in travis.yml disabled tests. Is that wrong?

maggu2810 · 2019-03-02T12:03:43Z

You are right, in Travis is it disabled for Maven + Bnd, too.

I don't know why it has been disabled on Travis and I don't know if it should be enabled on Travis.
I just keep it around as it has been.

maggu2810 · 2019-03-02T12:06:16Z

Perhaps we can also create a new milestone (M2) before we start migrating everything.

The whole mixture of POM first and Maven first is a mess.
Wouldn't it be simpler to proceed the migration ASAP instead of working around several problems that are caused by that mixture?

wborn · 2019-03-02T12:15:19Z

Do you have plans for working on and fixing openhab/openhab-distro#866 @pfink?
We can of course also announce in the community that we've started a migration so users are aware of this.

addons/pom.xml

wborn

Looks like feature validation now fails when building this PR. I see this on a local build as well as Travis:

[ERROR] Failed to execute goal org.apache.karaf.tooling:karaf-maven-plugin:4.2.1:verify (karaf-feature-verification) on project org.openhab.addons.features.karaf.openhab-addons: Feature resolution failed for [openhab-misc-neeo/2.5.0.SNAPSHOT]
[ERROR] Message: Unable to resolve root: missing requirement [root] osgi.identity; osgi.identity=openhab-misc-neeo; type=karaf.feature; version=2.5.0.SNAPSHOT; filter:="(&(osgi.identity=openhab-misc-neeo)(type=karaf.feature)(version>=2.5.0.SNAPSHOT))" [caused by: Unable to resolve openhab-misc-neeo/2.5.0.SNAPSHOT: missing requirement [openhab-misc-neeo/2.5.0.SNAPSHOT] osgi.identity; osgi.identity=org.openhab.io.neeo; type=osgi.bundle; version="[2.5.0.201903021602,2.5.0.201903021602]"; resolution:=mandatory [caused by: Unable to resolve org.openhab.io.neeo/2.5.0.201903021602: missing requirement [org.openhab.io.neeo/2.5.0.201903021602] osgi.wiring.package; filter:="(osgi.wiring.package=org.openhab.ui.dashboard)"]]
[ERROR] Repositories: {
[ERROR] 	file:/home/travis/build/openhab/openhab2-addons/features/karaf/openhab-addons/target/feature/feature.xml
[ERROR] 	mvn:org.apache.karaf.features/framework/4.2.1/xml/features
[ERROR] 	mvn:org.apache.karaf.features/standard/4.2.1/xml/features
[ERROR] 	mvn:org.openhab.core.features.karaf/org.openhab.core.features.karaf.openhab-core/2.5.0-SNAPSHOT/xml/features
[ERROR] 	mvn:org.openhab.core.features.karaf/org.openhab.core.features.karaf.openhab-tp/2.5.0-SNAPSHOT/xml/features
[ERROR] 	mvn:org.ops4j.pax.web/pax-web-features/7.2.3/xml/features
[ERROR] }

poms/bnd/pom.xml

maggu2810 · 2019-03-02T17:09:59Z

Looks like feature validation now fails when building this PR. I see this on a local build as well as Travis:

It has been already reported here, that neeo depends on the dashboard.
Has this already been resolved and I use an old code?

I found this one:

./addons/io/org.openhab.io.neeo/src/main/java/org/openhab/io/neeo/NeeoDashboardTile.java:public class NeeoDashboardTile implements DashboardTile {

kaikreuzer · 2019-03-02T18:54:56Z

features/karaf/pom.xml

+    </pluginManagement>
+  </build>
+
+  <repositories>


We should not add any repos other than the JFrog release and/or snapshot repo - they aggregate all that we need and thus make us flexible if artifacts move around.

You are right, the repos have not been cleaned up.
I just copied the section over to get a working solution.

If they are not necessary to get all things resolved, I will remove them.

kaikreuzer · 2019-03-02T18:59:09Z

bom/pom.xml

+
+  <name>openHAB Add-ons :: BOM</name>
+
+  <modules>


Just for my understanding: Why do we need all these indexes here in the repo? Are they specific to this repo or why can't the bundles/IDE refer to the indexes from openhab-core?
I am especially confused as web-ui seems to live happily without those, so if we can avoid it, I would not want them here either.

The index needs to be used for the integration tests and so needs to be present at build time.
The index contains URLs that points to the location the dependencies are stored in the current run, so I don't know how to reuse them.
If you know a better approach, I am happy to use it.
We could create a new index on-the-fly all the time for every integration test, but I assume it slows down the things.

Has my answer been detailed enough?
If we find a better way later -- perhaps bnd 4.2 or a 4.3 dev build will bring some improvements, or I learn another method to do it more efficient -- we could improve it later easily.

Ok, thanks for the explanation - let's live with it then.

kaikreuzer · 2019-03-02T19:00:40Z

bundles/org.openhab.binding.astro/.classpath

@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>


If there isn't any strong reason, I would prefer to keep the add-ons in separate folders by type (binding/transform/etc...) and not have them all as a flat list under bundles.

What's different from core here?

The new location is bundles/org.openhab.binding.astro, not bundles/binding/org.openhab.binding.astro.

Ah, sorry, you meant the difference to openhab-core?
Well, the difference is that we have a HUGE list of bundles here and that we have well defined types that can be cleanly separated. This is not really the case in openhab-core.

IIRC we had a discussion on the core migration and we agreed to use a flat list there.
openhab/openhab-core#467 (comment)

Now, what's the argument to do it different here?

The question is rather, what is the strong reason to change the current structure of this repo?

There is nothing "strong".

We commented that four days ago:
openhab/openhab-core#589 (comment)

I invested my saturday to implemented what we talked about, just because I would like to move things forward.
Now after the work has done, you suggest to change it to another layout.

I am not strong about the layout, and I am happy to accept PRs that change the layout on my branch.

In openHAB Core all integration tests are part of the itests directory.
Every integration we use contains some configuration that is equal to all the other ones.
I managed it put all that stuff in itests/itests-common.bndrun.
To speed up things the indices for the most bundles are not generated for every integration tests again and again on build time but we reference to a global one.
Please have a look at that file.
To reference that locations I used relative URLs and the URLs needs to be realtive to the integration test that is executed. So, the one that includes the common itests file.
If the integration tests are in separate sub-directories that depth is not always the same, that is not possible anymore.
We need to drop the itests-common and put that stuff in every itests bndrun.
If we would like to change some global stuff in future (e.g. the standalone repos, the used OSGi framework, ...), we cannot change that single file anymore but we have to change all the integration tests' bndrun files.

In the migration phase we need to use two reactors, one for bnd and one for tycho.
If we keep the bundles (not itests) at the old place, it will be very tricky to get this running) as the parents references at least one reactor. So we need also for every parent between the bundle and the main reactor POM a tycho and a bnd version.
We could keep the directory layout for the addons -- just in another base directory (e.g. bundles instead of addons), but then it does not really fit to use a flat layout for the integration tests in itests and a hierachical in bundles.
IMHO the flat layout is very simple. But the addon in bundles and its integration test in itests.

Suggestions?
Are the choosen GA okay? I have added addons in the group ID as we have done it for core in openHAB Core. I did not change the artiact ID (and so the generated BSN) so I assume it is okay.

We commented that four days ago:
openhab/openhab-core#589 (comment)

I invested my saturday to implemented what we talked about, just because I would like to move things forward.
Now after the work has done, you suggest to change it to another layout.

Sorry, I didn't follow the archetype discussion, thus I wasn't aware of that discussion. As you had agreed with @wborn and @davidgraeff already on the flat structure, this is all fine - I did't want to interfere here, sorry!

kaikreuzer · 2019-03-02T19:03:13Z

Has this already been resolved and I use an old code?

At least I haven't yet resolved it, so I guess it is indeed still there. As suggested somewhere else, I'd think we can live with removing this class/dependency and adapt the README (/cc @tmrobert8).

wborn · 2019-03-02T20:11:23Z

The Travis build probably still fails because org.openhab.ui.dashboard is still in the io.neeo manifest.

maggu2810 · 2019-03-02T20:15:35Z

The Travis build probably still fails because org.openhab.ui.dashboard is still in the io.neeo manifest.

I forgot: Manifest first 😉

bundles/org.openhab.binding.astro/pom.xml

Questions have been answered

Signed-off-by: Markus Rathgeb <maggu2810@gmail.com>

maggu2810 · 2019-03-03T20:03:57Z

I rebased the branch (no merge conflict anymore) and squashed all but one commit.
No line of code (or other things) has been changed.

There are now two commits:

remove the Neeo dashboard tile
add base to migrate to bnd and migrate astro

If you use "rebase and merge" instead of "squash and merge" we could keep the dashboard tile removal of Neeo separated. I would prever it because it is not only /really needed for the migration but also discussed on other topics.

I assume because of the rebase, it needs again 2 approving reviews.

cweitkamp · 2019-03-04T07:54:16Z

Do you mind the binding is migrated for demonstration purposes?

No, absolutely not. Feel free to use the avmfritz binding to demonstrate how it works.

kaikreuzer · 2019-03-04T07:55:56Z

org.openhab.addons.features.karaf

Sorry if I am again late and ask stuff that has already been decided upon: But as we are getting rid off p2 features mid-term, all features that we will have are "karaf" features - so I wonder if we should really carry "karaf" in the group and artifact ids... Don't think this is a common scheme that others are following, is it?

maggu2810 · 2019-03-04T08:05:44Z

If there are karaf features only, it would make sense to remove the directory "karaf", place the Karaf features directly under features and don't add karaf to the group and artifact IDs.

But can we wait for that change after we removed all p2 repositories?

kaikreuzer · 2019-03-04T08:14:08Z

Ok to wait, I was just asking because we need to change the distro back and forth, but that should be ok as long as we remember to do so.

maggu2810 · 2019-03-04T08:21:51Z

I assume I remember to note it if I change it.
I already stated it on PR creation in the first comment.

davidgraeff

I'd like if we could go ahead if there's no further objection :)

Related to openhab/openhab2-addons#5018 Signed-off-by: Wouter Born <github@maindrain.net>

wborn · 2019-03-04T21:53:33Z

@kaikreuzer are you OK with the changes in this PR?
If so, we can merge it as well as openhab/openhab-distro#888 and update Jenkins.

openhab-bot · 2019-03-06T08:05:55Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/how-to-run-available-tests-from-ide/69542/2

kaikreuzer · 2019-03-06T20:19:15Z

@kaikreuzer are you OK with the changes in this PR?

Yes, sorry for my late response!

Related to openhab/openhab2-addons#5018 Signed-off-by: Wouter Born <github@maindrain.net>

J-N-K · 2019-03-08T17:08:34Z

Two related issues:

The binding is no longer imported in the IDE (using "perform setup tasks").
It can't be added to the launch config for debugging anymore.

maggu2810 · 2019-03-08T17:59:54Z

It can't be added to the launch config for debugging anymore.

It can't be added to the PDE launch config.
It can be added to the Bnd based launch configuration for debugging (okay, we need to add one manual task, but I can provide the instructions).

maggu2810 · 2019-03-08T19:08:02Z

Have a look at: openhab/openhab-distro#889
IMHO the Web UI and the Addons repository should create a BOM that contains all of its bundles.
It would make it simpler for consumers (like the demo app) the use a up-to-date collection of all provided bundles.

wborn · 2019-03-09T12:13:04Z

The Jenkins openHAB2-Bundles and PR-openHAB2-Addons build jobs have now been updated with some pre-build steps to use the new POMs.

The only issue seems to be that the "Deploy artifacts to Artifactory" post-build action doesn't deploy the artifacts of these pre-build steps. See for instance the Artifactory build information for builds 586 and 589.
So the openHAB-Distribution job can't use these build artifacts.

After looking at the code it seems like it will only deploy artifacts of Maven projects that were build in the main build step (it uses mavenModuleSetBuild).

WDYT would be a good way to fix this @maggu2810, @kaikreuzer? Maybe it can deploy these artifacts by invoking the mvn deploy goal on these projects instead of using this Jenkins plugin?

kaikreuzer · 2019-03-09T12:20:35Z

I didn't follow the details, why do we require pre-build steps? Our builds are supposed to produce everything relevant by a single mvn call. If this isn't the case anymore, we are likely in huge trouble wrt the build & release pipeline as well...

wborn · 2019-03-09T12:35:51Z

There are 2 reactors because we ran into issues when mixing bnd and tycho in the same reactor (dashboard) in openhab-webui (openhab/openhab-distro#879 (comment)). I'm checking if this is actually an issue for openhab2-addons. Maybe can use the same reactor as long as tycho artifacts don't depend on bnd build artifacts in the same reactor.

davidgraeff · 2019-03-09T13:19:25Z

Maybe can use the same reactor as long as tycho artifacts don't depend on bnd build artifacts in the same reactor.

Hm, that will happen when the "io" bundles and transports are ported, so extra care is necessary there.

wborn · 2019-03-09T18:21:41Z

There's finally a working 2.5.0-SNAPSHOT build 1551 containing these changes! 😃

so extra care is necessary there

Yes it's probably best if bindings are migrated before (or simultaneously) their io/transport bundle(s). The same would apply if there were two reactors instead of one. In that case the tycho build was always executed before the bnd build.

maggu2810 requested review from gerrieg and a team as code owners March 2, 2019 10:06

wborn added the work in progress A PR that is not yet ready to be merged label Mar 2, 2019

wborn requested a review from davidgraeff March 2, 2019 10:15

gerrieg removed their request for review March 2, 2019 10:17

J-N-K reviewed Mar 2, 2019

View reviewed changes

wborn reviewed Mar 2, 2019

View reviewed changes

addons/pom.xml Outdated Show resolved Hide resolved

wborn requested a review from kaikreuzer March 2, 2019 13:35

wborn reviewed Mar 2, 2019

View reviewed changes

poms/bnd/pom.xml Outdated Show resolved Hide resolved

poms/bnd/pom.xml Outdated Show resolved Hide resolved

kaikreuzer previously requested changes Mar 2, 2019

View reviewed changes

wborn reviewed Mar 3, 2019

View reviewed changes

bundles/org.openhab.binding.astro/pom.xml Show resolved Hide resolved

wborn removed the work in progress A PR that is not yet ready to be merged label Mar 3, 2019

maggu2810 added 2 commits March 3, 2019 20:59

remove the Neeo dashboard tile

2b46d92

Signed-off-by: Markus Rathgeb <maggu2810@gmail.com>

add base to migrate to bnd and migrate astro

f7ec826

Signed-off-by: Markus Rathgeb <maggu2810@gmail.com>

davidgraeff approved these changes Mar 4, 2019

View reviewed changes

wborn referenced this pull request in wborn/openhab-distro Mar 4, 2019

Update OH2 addons features dependency groupId/artifactId

34c2e32

Related to openhab/openhab2-addons#5018 Signed-off-by: Wouter Born <github@maindrain.net>

wborn mentioned this pull request Mar 4, 2019

Update OH2 addons features dependency groupId/artifactId openhab/openhab-distro#888

Merged

kaikreuzer merged commit b524f60 into openhab:master Mar 6, 2019

kaikreuzer referenced this pull request in openhab/openhab-distro Mar 6, 2019

Update OH2 addons features dependency groupId/artifactId (#888)

c7f3c67

Related to openhab/openhab2-addons#5018 Signed-off-by: Wouter Born <github@maindrain.net>

maggu2810 deleted the bnd branch March 6, 2019 21:20

wborn mentioned this pull request Mar 7, 2019

Update CODEOWNERS for Astro and AVM Fritz bnd migrations #5061

Merged

wborn mentioned this pull request Mar 9, 2019

Use one reactor for bnd and tycho builds #5072

Merged

wborn mentioned this pull request Mar 9, 2019

gather-external-docs fails due to moved OH2 addons feature file openhab/openhab-docs#914

Closed

wborn added the infrastructure Build system and Karaf related issues and PRs label Mar 10, 2019

wborn added this to the 2.5 milestone Mar 10, 2019

wborn mentioned this pull request Mar 31, 2019

Get Releasebuild running again after ESH integration and BND migration openhab/openhab-distro#866

Closed

14 tasks

wborn removed this from the 2.5 milestone Dec 8, 2019

wborn added this to the 2.5 milestone Dec 16, 2019

add base to migrate to bnd and migrate astro #5018

add base to migrate to bnd and migrate astro #5018

Conversation

maggu2810 commented Mar 2, 2019

maggu2810 commented Mar 2, 2019

J-N-K commented Mar 2, 2019

maggu2810 commented Mar 2, 2019

wborn commented Mar 2, 2019

J-N-K left a comment

Choose a reason for hiding this comment

maggu2810 commented Mar 2, 2019

wborn commented Mar 2, 2019

J-N-K commented Mar 2, 2019

maggu2810 commented Mar 2, 2019 • edited Loading

maggu2810 commented Mar 2, 2019 • edited Loading

wborn commented Mar 2, 2019

wborn left a comment

Choose a reason for hiding this comment

maggu2810 commented Mar 2, 2019 • 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

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

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

maggu2810 Mar 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaikreuzer commented Mar 2, 2019

wborn commented Mar 2, 2019

maggu2810 commented Mar 2, 2019

maggu2810 commented Mar 3, 2019

cweitkamp commented Mar 4, 2019

kaikreuzer commented Mar 4, 2019

maggu2810 commented Mar 4, 2019

kaikreuzer commented Mar 4, 2019

maggu2810 commented Mar 4, 2019

davidgraeff left a comment

Choose a reason for hiding this comment

wborn commented Mar 4, 2019

openhab-bot commented Mar 6, 2019

kaikreuzer commented Mar 6, 2019

J-N-K commented Mar 8, 2019

maggu2810 commented Mar 8, 2019 • edited Loading

maggu2810 commented Mar 8, 2019 • edited Loading

wborn commented Mar 9, 2019

kaikreuzer commented Mar 9, 2019

wborn commented Mar 9, 2019

davidgraeff commented Mar 9, 2019

wborn commented Mar 9, 2019

maggu2810 commented Mar 2, 2019 •

edited

Loading

maggu2810 commented Mar 2, 2019 •

edited

Loading

maggu2810 commented Mar 2, 2019 •

edited

Loading

maggu2810 Mar 3, 2019 •

edited

Loading

maggu2810 commented Mar 8, 2019 •

edited

Loading

maggu2810 commented Mar 8, 2019 •

edited

Loading