Description
Hi,
When we use the following features file:
<?xml version="1.0" encoding="UTF-8"?> <features name="dummy-1.0.0" xmlns="http://karaf.apache.org/xmlns/features/v1.0.0"> <feature name="framework" description="my framework" version="1.0.0" resolver="(obr)"> <bundle start-level="60">mvn:com.google.guava/guava/13.0.1</bundle> </feature> <feature name="app" description="my app" version="1.0.1" resolver="(obr)"> <feature version="1.0.0">framework</feature> <bundle start-level="60">mvn:com.google.guava/guava/13.0.1</bundle> </feature> </features>
first install the "framework" feature, the "guava" bundle is deployed.
If Karaf is restarted and the "app" feature is deployed, the "guava" is associated to the "app" feature and no longer to the "framework" feature.
The features persistence at "data/cache/bundle20/data/FeaturesServiceState.properties" contains:
features.framework_split_for_name_and_version_1.0.0= features.app_split_for_name_and_version_1.0.0=64
and should be (since the guava is required by both features):
features.framework_split_for_name_and_version_1.0.0=64 features.app_split_for_name_and_version_1.0.0=64
If the "app" feature is now uninstalled, the "guava" bundle is REMOVED! and the remaining "framework" feature is broken.
FIX proposal:
It seems the issue is located in "features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java"
The following patch doesn't recurse on already installed features, doesn't break internal states of those features and avoids the issue:
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java index 0c7982b..127b2eb 100644 --- a/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java +++ b/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java @@ -555,6 +555,14 @@ public class FeaturesServiceImpl implements FeaturesService { protected void doInstallFeature(InstallationState state, Feature feature, boolean verbose) throws Exception { if (feature != null) { + if (isInstalled(feature)) { + String msg = "Found installed feature " + feature.getName() + " " + feature.getVersion(); + LOGGER.info(msg); + if (verbose) { + System.out.println(msg); + } + return; + } String msg = "Installing feature " + feature.getName() + " " + feature.getVersion(); LOGGER.info(msg); if (verbose) { @@ -576,7 +584,7 @@ public class FeaturesServiceImpl implements FeaturesService { if (result.isNew) { state.installed.add(result.bundle); } - String msg2 = (result.isNew) ? "Found installed bundle: " + result.bundle : "Installing bundle " + bInfo.getLocation(); + String msg2 = (!result.isNew) ? "Found installed bundle: " + result.bundle : "Installing bundle " + bInfo.getLocation(); LOGGER.debug(msg2); if (verbose) { System.out.println(msg2); diff --git a/features/core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java b/features/core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java index b86565f..d23849e 100644 --- a/features/core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java +++ b/features/core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java @@ -340,8 +340,7 @@ public class FeaturesServiceTest extends TestBase { expect(bundleManager.getDataFile(EasyMock.<String>anyObject())).andReturn(dataFile).anyTimes(); expect(bundleManager.getBundleContext()).andReturn(null); expect(bundleManager.installBundleIfNeeded(bundleVer01Uri, 0, null)).andReturn(new BundleInstallerResult(bundleVer01, true)); - expect(bundleManager.installBundleIfNeeded(bundleVer01Uri, 0, null)).andReturn(new BundleInstallerResult(bundleVer01, false)); - expect(bundleManager.isBundleInstalled("bundle-0.1")).andReturn(bundleVer01).times(2); + expect(bundleManager.isBundleInstalled("bundle-0.1")).andReturn(bundleVer01); expect(bundleManager.getBundleContext()).andReturn(bundleContext); ignoreRefreshes(bundleManager); bundleManager.uninstall(Collections.EMPTY_LIST, true);
Michel, Fred & Frank