VersionRange for root features is too tight when expanded version given

bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/ProductAction.java

@@ -276,7 +276,7 @@ protected File getExecutablesLocation() {
 		return null;
 	}
 
-	protected class RootFeatureAdvice implements IFilterAdvice {
+	protected class RootFeatureAdvice implements IFilterAdvice, IVersionRangeAdvice {
 
 		private final Collection<IVersionedId> rootFeatures;
 
@@ -322,5 +322,21 @@ private String getFilter(IInstallableUnit unit) {
 			}
 			return parameter.toString();
 		}
+
+		@Override
+		public Optional<VersionRange> getVersionRange(String namespace, String fid) {
+			if (NS_FEATURE.equals(namespace)) {
+				for (IVersionedId featureId : rootFeatures) {
+					if (fid.equals(featureId.getId())) {
+						Version fversion = featureId.getVersion();
+						if (fversion == null || Version.emptyVersion.equals(fversion)) {
+							return Optional.of(VersionRange.emptyRange);
+						}
+						return Optional.of(new VersionRange(fversion, true, Version.MAX_VERSION, true));
+					}
+				}
+			}
+			return Optional.empty();
+		}
 	}
 }

bundles/org.eclipse.equinox.p2.publisher/src/org/eclipse/equinox/p2/publisher/AbstractPublisherAction.java

@@ -212,9 +212,7 @@ protected Collection<IRequirement> createIURequirements(Collection<? extends IVe
 				result.add(MetadataFactory.createRequirement(IInstallableUnit.NAMESPACE_IU_ID, iu.getId(), range,
 						iu.getFilter() == null ? null : iu.getFilter(), false, false));
 			} else {
-				Version version = next.getVersion();
-				VersionRange range = (version == null || Version.emptyVersion.equals(version)) ? VersionRange.emptyRange
-						: new VersionRange(version, true, version, true);
+				VersionRange range = getVersionRange(next);
 				IMatchExpression<IInstallableUnit> filter = getFilterAdvice(next);
 				result.add(MetadataFactory.createRequirement(IInstallableUnit.NAMESPACE_IU_ID, next.getId(), range,
 						filter, false, false));
@@ -223,6 +221,12 @@ protected Collection<IRequirement> createIURequirements(Collection<? extends IVe
 		return result;
 	}
 
+	protected VersionRange getVersionRange(IVersionedId versionedId) {
+		Version version = versionedId.getVersion();
+		return (version == null || Version.emptyVersion.equals(version)) ? VersionRange.emptyRange
+				: new VersionRange(version, true, version, true);
+	}
+
 	private IMatchExpression<IInstallableUnit> getFilterAdvice(IVersionedId name) {
 		if (info == null)
 			return null;

bundles/org.eclipse.equinox.p2.publisher/src/org/eclipse/equinox/p2/publisher/actions/IVersionRangeAdvice.java

@@ -0,0 +1,36 @@
+/*******************************************************************************
+ * Copyright (c) 2024 Christoph Läubrich and others.
+ *
+ * This program and the accompanying materials are made available under the terms of
+ * the Eclipse Public License 2.0 which accompanies this distribution, and is
+ * available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Christoph Läubrich - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.equinox.p2.publisher.actions;
+
+import java.util.Optional;
+import org.eclipse.equinox.p2.metadata.IInstallableUnit;
+import org.eclipse.equinox.p2.metadata.VersionRange;
+import org.eclipse.equinox.p2.publisher.IPublisherAdvice;
+import org.eclipse.equinox.spi.p2.publisher.PublisherHelper;
+
+public interface IVersionRangeAdvice extends IPublisherAdvice {
+
+	public static final String NS_FEATURE = PublisherHelper.NAMESPACE_ECLIPSE_TYPE + "." //$NON-NLS-1$
+			+ PublisherHelper.TYPE_ECLIPSE_FEATURE;
+	public static final String NS_IU = IInstallableUnit.NAMESPACE_IU_ID;
+
+	/**
+	 * Returns the {@link VersionRange} for the given id in the given namespace.
+	 *
+	 * @param namespace the namespace in which to look for advice
+	 * @param id        the id for the item in the given namespace
+	 * @return an {@link Optional} describing the {@link VersionRange} found.
+	 */
+	public Optional<VersionRange> getVersionRange(String namespace, String id);
+
+}

bundles/org.eclipse.equinox.p2.publisher/src/org/eclipse/equinox/p2/publisher/actions/RootIUAction.java

@@ -172,6 +172,15 @@ private InstallableUnitDescription createTopLevelIUDescription(Collection<? exte
 		return root;
 	}
 
+	@Override
+	protected VersionRange getVersionRange(IVersionedId versionedId) {
+		String namespace = versionedId.getId().endsWith(".feature.group") ? IVersionRangeAdvice.NS_FEATURE //$NON-NLS-1$
+				: IVersionRangeAdvice.NS_IU;
+		return info.getAdvice(null, true, versionedId.getId(), versionedId.getVersion(), IVersionRangeAdvice.class)
+				.stream().flatMap(advice -> advice.getVersionRange(namespace, versionedId.getId()).stream()).findFirst()
+				.orElseGet(() -> super.getVersionRange(versionedId));
+	}
+
 	private Version getVersionAdvice(String iuID) {
 		if (versionAdvice == null) {
 			versionAdvice = info.getAdvice(null, true, null, null, IVersionAdvice.class);

bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/publisher/actions/ProductActionTest.java

@@ -30,6 +30,7 @@
 import java.net.URI;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.List;
 import java.util.Optional;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.NullProgressMonitor;
@@ -42,7 +43,10 @@
 import org.eclipse.equinox.p2.metadata.IRequirement;
 import org.eclipse.equinox.p2.metadata.ITouchpointData;
 import org.eclipse.equinox.p2.metadata.IUpdateDescriptor;
+import org.eclipse.equinox.p2.metadata.IVersionedId;
 import org.eclipse.equinox.p2.metadata.Version;
+import org.eclipse.equinox.p2.metadata.VersionRange;
+import org.eclipse.equinox.p2.metadata.VersionedId;
 import org.eclipse.equinox.p2.metadata.expression.IMatchExpression;
 import org.eclipse.equinox.p2.publisher.AbstractPublisherAction;
 import org.eclipse.equinox.p2.publisher.IPublisherInfo;
@@ -440,7 +444,22 @@ public void testRequiredEEAsSpecified() throws Exception {
 
 	public void testInstallModeRootFeature() throws Exception {
 		ProductFile productFile = new ProductFile(
-				TestData.getFile("ProductActionTest", "brandedProduct/branded.product").toString());
+				TestData.getFile("ProductActionTest", "brandedProduct/branded.product").toString()) {
+			@Override
+			public List<IVersionedId> getFeatures(int options) {
+				if (options == ROOT_FEATURES) {
+					List<IVersionedId> features = super.getFeatures(options);
+					return features.stream().map(v -> {
+						// we simulate an expanded feature version here...
+						if ("org.example".equals(v.getId())) {
+							return new VersionedId(v.getId(), "1.2.3");
+						}
+						return v;
+					}).toList();
+				}
+				return super.getFeatures(options);
+			}
+		};
 		addContextIU("org.eclipse.platform.feature.group", "1.2.3");
 		addContextIU("org.example.feature.group", "1.0.0");
 		performProductAction(productFile);
@@ -452,11 +471,15 @@ public void testInstallModeRootFeature() throws Exception {
 				.map(IRequiredCapability.class::cast).filter(it -> it.getName().equals("org.example.feature.group"))
 				.findFirst();
 		assertTrue(installModeRootFeatureRequirement.isPresent());
-		IMatchExpression<IInstallableUnit> filter = installModeRootFeatureRequirement.get().getFilter();
+		IRequiredCapability capability = installModeRootFeatureRequirement.get();
+		IMatchExpression<IInstallableUnit> filter = capability.getFilter();
 		assertNotNull(filter);
 		String filterString = filter.getParameters()[0].toString();
 		assertEquals("(|(branded.product.install.mode.root=true)(org.eclipse.equinox.p2.install.mode.root=true))",
 				filterString);
+		VersionRange versionRange = capability.getRange();
+		assertTrue("Version Range is too strict: " + versionRange,
+				versionRange.isIncluded(Version.parseVersion("9999")));
 	}
 
 	public void testInstallModeRootFeatureWithFilter() throws Exception {