From c9ed6dfe012e8bb761e7a5c50cbd89814a47824b Mon Sep 17 00:00:00 2001 From: Hannes Wellmann Date: Sat, 18 Nov 2023 21:16:49 +0100 Subject: [PATCH] [TP] Support no version and version ranges in target definitions Fixes https://github.com/eclipse-pde/eclipse.pde/issues/757 --- .../core/target/IUBundleContainer.java | 165 +++++++++++++----- .../core/target/IULocationFactory.java | 7 +- .../internal/core/target/P2TargetUtils.java | 90 ++++++---- .../core/target/TargetPlatformService.java | 12 +- .../tests/target/IUBundleContainerTests.java | 81 ++++++++- .../ui/shared/target/EditIUContainerPage.java | 110 +++++++++++- .../internal/ui/shared/target/Messages.java | 2 + .../ui/shared/target/messages.properties | 2 + 8 files changed, 372 insertions(+), 97 deletions(-) diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IUBundleContainer.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IUBundleContainer.java index df07075d3f..9c61ab98bf 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IUBundleContainer.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IUBundleContainer.java @@ -15,6 +15,7 @@ * Manumitting Technologies Inc - bug 437726: wrong error messages opening target definition * Alexander Fedorov (ArSysOp) - Bug 542425, Bug 574629 * Christoph Läubrich - Bug 568865 - [target] add advanced editing capabilities for custom target platforms + * Hannes Wellmann - Support no version and version ranges in target definitions *******************************************************************************/ package org.eclipse.pde.internal.core.target; @@ -35,6 +36,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; @@ -54,9 +56,8 @@ import org.eclipse.equinox.p2.engine.IProfile; import org.eclipse.equinox.p2.metadata.IArtifactKey; import org.eclipse.equinox.p2.metadata.IInstallableUnit; -import org.eclipse.equinox.p2.metadata.IVersionedId; import org.eclipse.equinox.p2.metadata.Version; -import org.eclipse.equinox.p2.metadata.VersionedId; +import org.eclipse.equinox.p2.metadata.VersionRange; import org.eclipse.equinox.p2.query.IQuery; import org.eclipse.equinox.p2.query.IQueryResult; import org.eclipse.equinox.p2.query.IQueryable; @@ -79,6 +80,68 @@ */ public class IUBundleContainer extends AbstractBundleContainer { + record UnitDeclaration(String id, VersionRange version, boolean hasVersion) { + private static final String EMPTY_VERSION = Version.emptyVersion.toString(); + + static UnitDeclaration parse(String id, String version) { + version = version.strip(); + if (version.isEmpty() || version.equals(EMPTY_VERSION)) { + return new UnitDeclaration(id, VersionRange.emptyRange, !version.isEmpty()); + } else if (version.contains(",")) { //$NON-NLS-1$ + return new UnitDeclaration(id, VersionRange.create(version), true); + } else { + return create(id, Version.parseVersion(version)); + } + } + + static UnitDeclaration create(String id, Version version) { + VersionRange range = new VersionRange(version, true, version, true); + return new UnitDeclaration(id, range, true); + } + + UnitDeclaration { + Objects.requireNonNull(id); + Objects.requireNonNull(version); + } + + private boolean hasSingleVersion() { + return hasEmptyVersion() || (version.getMaximum().equals(version.getMinimum()) + && version.getIncludeMinimum() && version.getIncludeMaximum()); + } + + private boolean hasEmptyVersion() { + return version.equals(VersionRange.emptyRange); + } + + Version getSingleVersion() { + if (!hasSingleVersion()) { + throw new IllegalStateException("Declaration does not have a single version"); //$NON-NLS-1$ + } + return version.getMinimum(); + } + + IQuery createIUQuery() { + // For versions such as 0.0.0 or ranges, the IU query may return + // multiple IUs, so we check which is the latest version. + if (hasEmptyVersion()) { + return createLatestIUQuery(id); + } else if (hasSingleVersion()) { + return QueryUtil.createIUQuery(id, version.getMinimum()); + } else { // version range + return QueryUtil.createLatestQuery(QueryUtil.createIUQuery(id, version)); + } + } + + String versionString() { + return hasSingleVersion() ? version.getMinimum().toString() : version.toString(); + } + + @Override + public String toString() { + return hasEmptyVersion() ? id : id + '/' + versionString(); + } + } + /** * Constant describing the type of bundle container */ @@ -120,13 +183,10 @@ public class IUBundleContainer extends AbstractBundleContainer { public static final int FOLLOW_REPOSITORY_REFERENCES = 1 << 4; /** The list of id+version of all root units declared in this container. */ - private final Set fIUs; + private final Set fIUs; - /** - * Cached IU's referenced by this bundle container, or null if not - * resolved. - */ - private List fUnits = List.of(); + /** Cached IU's referenced by this bundle container . */ + private Map fUnits = Map.of(); /** * Repositories to consider, empty if default. @@ -166,7 +226,7 @@ public class IUBundleContainer extends AbstractBundleContainer { * {@link IUBundleContainer#INCLUDE_CONFIGURE_PHASE}, * {@link IUBundleContainer#FOLLOW_REPOSITORY_REFERENCES} */ - IUBundleContainer(List units, List repositories, int resolutionFlags) { + IUBundleContainer(List units, List repositories, int resolutionFlags) { fIUs = new LinkedHashSet<>(units); fFlags = resolutionFlags; fRepos = List.copyOf(repositories); @@ -264,16 +324,16 @@ protected TargetBundle[] resolveBundles(ITargetDefinition definition, IProgressM */ private void cacheIUs() throws CoreException { IProfile profile = fSynchronizer.getProfile(); - List result = new ArrayList<>(); + Map result = new LinkedHashMap<>(); MultiStatus status = new MultiStatus(PDECore.PLUGIN_ID, 0, Messages.IUBundleContainer_ProblemsLoadingRepositories); - for (IVersionedId unit : fIUs) { - queryIU(profile, unit, status).ifPresent(result::add); + for (UnitDeclaration unit : fIUs) { + queryIU(profile, unit, status).ifPresent(iu -> result.put(iu, unit)); } if (!status.isOK()) { fResolutionStatus = status; throw new CoreException(status); } - fUnits = List.copyOf(result); + fUnits = Collections.unmodifiableMap(result); } /** @@ -288,7 +348,7 @@ private void cacheBundles(ITargetDefinition target) throws CoreException { boolean onlyStrict = !fSynchronizer.getIncludeAllRequired(); IProfile metadata = fSynchronizer.getProfile(); PermissiveSlicer slicer = new PermissiveSlicer(metadata, new HashMap<>(), true, false, true, onlyStrict, false); - IQueryable slice = slicer.slice(fUnits, new NullProgressMonitor()); + IQueryable slice = slicer.slice(fUnits.keySet(), new NullProgressMonitor()); if (slicer.getStatus().getSeverity() == IStatus.ERROR) { // If the slicer has an error, report it instead of returning an empty set @@ -365,24 +425,31 @@ public synchronized IUBundleContainer update(Set toUpdate, IProgressMoni IQueryable source = P2TargetUtils.getQueryableMetadata(fRepos, isFollowRepositoryReferences(), progress.split(30)); boolean updated = false; - List updatedUnits = new ArrayList<>(fIUs); + List updatedUnits = new ArrayList<>(fIUs); SubMonitor loopProgress = progress.split(70).setWorkRemaining(updatedUnits.size()); for (int i = 0; i < updatedUnits.size(); i++) { - IVersionedId unit = updatedUnits.get(i); - if (!toUpdate.isEmpty() && !toUpdate.contains(unit.getId())) { + UnitDeclaration unit = updatedUnits.get(i); + if (!toUpdate.isEmpty() && !toUpdate.contains(unit.id())) { continue; } - IQuery query = QueryUtil.createLatestQuery(QueryUtil.createIUQuery(unit.getId())); + IQuery query = createLatestIUQuery(unit.id()); Optional queryResult = queryFirst(source, query, loopProgress.split(1)); Version updatedVersion = queryResult.map(IInstallableUnit::getVersion) // bail if the feature is no longer available. .orElseThrow(() -> new CoreException(Status.error(NLS.bind(Messages.IUBundleContainer_1, unit)))); // if the version is different from the spec (up or down), record the change. - if (!updatedVersion.equals(unit.getVersion())) { + if (!unit.hasSingleVersion()) { + updated = true; + // Don't update version ranges. They are usually intended to not + // use the latest version. Updating them is done explicitly. + continue; + } + Version declaredVersion = unit.getSingleVersion(); + if (!updatedVersion.equals(declaredVersion)) { updated = true; // if the spec was not specific (e.g., 0.0.0) the target def itself has changed. - if (!unit.getVersion().equals(Version.emptyVersion)) { - updatedUnits.set(i, new VersionedId(unit.getId(), updatedVersion)); + if (!declaredVersion.equals(Version.emptyVersion)) { + updatedUnits.set(i, UnitDeclaration.create(unit.id(), updatedVersion)); } } } @@ -486,7 +553,10 @@ public List getRepositories() { * @param unit unit to remove from the list of root IUs */ public synchronized void removeInstallableUnit(IInstallableUnit unit) { - fIUs.remove(new VersionedId(unit.getId(), unit.getVersion())); + UnitDeclaration description = fUnits.get(unit); + if (description != null) { + fIUs.remove(description); + } // Need to mark the container as unresolved clearResolutionStatus(); } @@ -571,12 +641,22 @@ public boolean isFollowRepositoryReferences() { * * @return the discovered IUs */ - public List getInstallableUnits() { - return fUnits; + public Collection getInstallableUnits() { + return fUnits.keySet(); + } + + public Map getInstallableUnitSpecifications() { + return fUnits.entrySet().stream().collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, e -> { + UnitDeclaration u = e.getValue(); + if (u.hasEmptyVersion() || !u.hasSingleVersion()) { + return u.version().toString(); + } + return ""; //$NON-NLS-1$ + })); } /** Returns the declared installable unit identifiers and versions. */ - Collection getDeclaredUnits() { + Collection getDeclaredUnits() { return Collections.unmodifiableSet(fIUs); } @@ -617,9 +697,12 @@ protected void associateWithTarget(ITargetDefinition target) { fSynchronizer.setFollowRepositoryReferences((fFlags & FOLLOW_REPOSITORY_REFERENCES) == FOLLOW_REPOSITORY_REFERENCES); } - private static final Comparator BY_ID_THEN_VERSION = Comparator // - .comparing(IVersionedId::getId) // - .thenComparing(IVersionedId::getVersion); + private static final Comparator BY_ID_THEN_VERSION = Comparator.comparing(UnitDeclaration::id) + // single versions first, then ranges + .thenComparing(u -> !u.hasSingleVersion()) // false unit.version().getMinimum()) + // for single versions lower-bound == upper-bound + .thenComparing(unit -> unit.version().getMaximum()); @Override public String serialize() { @@ -662,8 +745,10 @@ public String serialize() { // Generate a predictable order of the elements fIUs.stream().sorted(BY_ID_THEN_VERSION).forEach(iu -> { Element unit = document.createElement(TargetDefinitionPersistenceHelper.INSTALLABLE_UNIT); - unit.setAttribute(TargetDefinitionPersistenceHelper.ATTR_ID, iu.getId()); - unit.setAttribute(TargetDefinitionPersistenceHelper.ATTR_VERSION, iu.getVersion().toString()); + unit.setAttribute(TargetDefinitionPersistenceHelper.ATTR_ID, iu.id()); + if (iu.hasVersion()) { + unit.setAttribute(TargetDefinitionPersistenceHelper.ATTR_VERSION, iu.versionString()); + } containerElement.appendChild(unit); }); try { @@ -680,13 +765,14 @@ public String serialize() { } } - Collection getRootIUs(IProgressMonitor monitor) throws CoreException { + Map> getRootIUs(IProgressMonitor monitor) throws CoreException { IQueryable repos = P2TargetUtils.getQueryableMetadata(getRepositories(), isFollowRepositoryReferences(), monitor); MultiStatus status = new MultiStatus(PDECore.PLUGIN_ID, 0, Messages.IUBundleContainer_ProblemsLoadingRepositories); - List result = new ArrayList<>(); - for (IVersionedId iu : fIUs) { - queryIU(repos, iu, status).ifPresent(result::add); + Map> result = new HashMap<>(); + for (UnitDeclaration iu : fIUs) { + queryIU(repos, iu, status) + .ifPresent(u -> result.computeIfAbsent(u, __ -> new HashSet<>(1)).add(iu.version())); } if (!status.isOK()) { fResolutionStatus = status; @@ -695,12 +781,9 @@ Collection getRootIUs(IProgressMonitor monitor) throws CoreExc return result; } - private Optional queryIU(IQueryable queryable, IVersionedId iu, + private Optional queryIU(IQueryable queryable, UnitDeclaration iu, MultiStatus status) { - // For versions such as 0.0.0, the IU query may return multiple IUs, so - // we check which is the latest version - IQuery query = QueryUtil.createLatestQuery(QueryUtil.createIUQuery(iu)); - Optional queryResult = queryFirst(queryable, query, null); + Optional queryResult = queryFirst(queryable, iu.createIUQuery(), null); if (queryResult.isEmpty()) { status.add(Status.error(NLS.bind(Messages.IUBundleContainer_1, iu))); } @@ -711,6 +794,10 @@ static Optional queryFirst(IQueryable queryable, IQuery query, IPro return queryable.query(query, monitor).stream().findFirst(); } + private static IQuery createLatestIUQuery(String id) { + return QueryUtil.createLatestQuery(QueryUtil.createIUQuery(id)); + } + @Override public T getAdapter(Class adapter) { if (adapter.isInstance(fSynchronizer)) { diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IULocationFactory.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IULocationFactory.java index e160f40c99..e0c09b7f47 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IULocationFactory.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IULocationFactory.java @@ -72,11 +72,8 @@ public ITargetLocation getTargetLocation(String type, String serializedXML) thro if (element.getNodeName().equalsIgnoreCase(TargetDefinitionPersistenceHelper.INSTALLABLE_UNIT)) { String id = element.getAttribute(TargetDefinitionPersistenceHelper.ATTR_ID); if (id.length() > 0) { - String version = element.getAttribute(TargetDefinitionPersistenceHelper.ATTR_VERSION); - if (version.length() > 0) { - ids.add(id); - versions.add(version); - } + ids.add(id); + versions.add(element.getAttribute(TargetDefinitionPersistenceHelper.ATTR_VERSION)); } } else if (element.getNodeName().equalsIgnoreCase(TargetDefinitionPersistenceHelper.REPOSITORY)) { String loc = element.getAttribute(TargetDefinitionPersistenceHelper.LOCATION); diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java index d78735b9f9..e288dbee9a 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java @@ -73,10 +73,10 @@ import org.eclipse.equinox.p2.metadata.IInstallableUnit; import org.eclipse.equinox.p2.metadata.IProvidedCapability; import org.eclipse.equinox.p2.metadata.IRequirement; -import org.eclipse.equinox.p2.metadata.IVersionedId; import org.eclipse.equinox.p2.metadata.MetadataFactory; import org.eclipse.equinox.p2.metadata.MetadataFactory.InstallableUnitDescription; import org.eclipse.equinox.p2.metadata.Version; +import org.eclipse.equinox.p2.metadata.VersionRange; import org.eclipse.equinox.p2.planner.IPlanner; import org.eclipse.equinox.p2.planner.IProfileChangeRequest; import org.eclipse.equinox.p2.query.IQuery; @@ -96,9 +96,9 @@ import org.eclipse.pde.core.target.ITargetHandle; import org.eclipse.pde.core.target.ITargetLocation; import org.eclipse.pde.core.target.ITargetPlatformService; -import org.eclipse.pde.core.target.NameVersionDescriptor; import org.eclipse.pde.internal.core.ICoreConstants; import org.eclipse.pde.internal.core.PDECore; +import org.eclipse.pde.internal.core.target.IUBundleContainer.UnitDeclaration; import org.eclipse.pde.internal.core.util.CoreUtility; import org.osgi.framework.BundleContext; import org.osgi.framework.InvalidSyntaxException; @@ -143,6 +143,13 @@ public class P2TargetUtils { */ static final String PROP_INSTALLED_IU = PDECore.PLUGIN_ID + ".installed_iu"; //$NON-NLS-1$ + /** + * Installable unit property to store the version-specifications of + * root/installed IU's that are declared in the target container as a + * semicolon separated list. + */ + static final String PROP_IU_VERSION_DECLARATION = PDECore.PLUGIN_ID + ".iu_version_declaration"; //$NON-NLS-1$ + /** * Profile property that keeps track of provisioning mode for the target * (slice versus plan). @@ -563,31 +570,18 @@ private boolean checkProfile(ITargetDefinition target, final IProfile profile) { // Check if each installed/root IU can be matched with exactly one // IU-declaration. If not, the profile is not in sync anymore. - Set installedIUs = new HashSet<>(); + Map> installedIUs = new HashMap<>(); for (IInstallableUnit unit : queryResult) { - installedIUs.add(new NameVersionDescriptor(unit.getId(), unit.getVersion().toString())); - } - - Set emptyVersionIUs = new HashSet<>(); - for (IUBundleContainer iuContainer : iuContainers) { - for (IVersionedId iu : iuContainer.getDeclaredUnits()) { - // if there is something in a container but not in the profile, recreate - Version version = iu.getVersion(); - if (version.equals(Version.emptyVersion)) { - emptyVersionIUs.add(iu.getId()); - } else if (!installedIUs.remove(new NameVersionDescriptor(iu.getId(), version.toString()))) { - return false; - } - } - } - if (!emptyVersionIUs.isEmpty()) { - // match remaining installed IUs with IUs declared with empty - // version. It's a full match if for each IU declared with empty - // version exactly one IU remains. - installedIUs.removeIf(iu -> emptyVersionIUs.remove(iu.getId())); + Set declarations = installedIUs.computeIfAbsent(unit.getId(), id -> new HashSet<>(1)); + String declaredVersions = profile.getInstallableUnitProperty(unit, PROP_IU_VERSION_DECLARATION); + parseVersions(unit, declaredVersions).forEach(declarations::add); } - // If both are empty it's a full match and the profile checks out. - return emptyVersionIUs.isEmpty() && installedIUs.isEmpty(); + Map> declaredIUs = iuContainers.stream() // + .map(IUBundleContainer::getDeclaredUnits).flatMap(Collection::stream) // + .collect(Collectors.groupingBy(UnitDeclaration::id, + Collectors.mapping(UnitDeclaration::version, Collectors.toSet()))); + + return installedIUs.equals(declaredIUs); } private Stream iuBundleContainersOf(ITargetDefinition target) { @@ -1063,7 +1057,7 @@ private void resolveWithPlanner(ITargetDefinition target, IProfile profile, IPro SubMonitor subMonitor = SubMonitor.convert(monitor, Messages.IUBundleContainer_0, 220); // Get the root IUs for every relevant container in the target definition - Set units = getRootIUs(target, subMonitor.split(20)); + Map units = getRootIUs(target, subMonitor.split(20)); // create the provisioning plan IPlanner planner = getPlanner(); @@ -1071,10 +1065,11 @@ private void resolveWithPlanner(ITargetDefinition target, IProfile profile, IPro // first remove everything that was explicitly installed. Then add it back. This has the net effect of // removing everything that is no longer needed. computeRemovals(profile, request, getIncludeSource()); - request.addAll(units); - for (IInstallableUnit unit : units) { + request.addAll(units.keySet()); + units.forEach((unit, versionDeclarations) -> { request.setInstallableUnitProfileProperty(unit, PROP_INSTALLED_IU, Boolean.toString(true)); - } + request.setInstallableUnitProfileProperty(unit, PROP_IU_VERSION_DECLARATION, versionDeclarations); + }); List extraArtifactRepositories = new ArrayList<>(); List extraMetadataRepositories = new ArrayList<>(); @@ -1294,7 +1289,7 @@ private void resolveWithSlicer(ITargetDefinition target, IProfile profile, IProg SubMonitor subMonitor = SubMonitor.convert(monitor, Messages.IUBundleContainer_0, 110); // resolve IUs - Set units = getRootIUs(target, subMonitor.split(40)); + Map units = getRootIUs(target, subMonitor.split(40)); Collection repositories = getMetadataRepositories(target); if (repositories.isEmpty()) { @@ -1304,7 +1299,7 @@ private void resolveWithSlicer(ITargetDefinition target, IProfile profile, IProg subMonitor.split(5)); // do an initial slice to add everything the user requested - IQueryResult queryResult = slice(units, allMetadata, target, subMonitor.split(5)); + IQueryResult queryResult = slice(units.keySet(), allMetadata, target, subMonitor.split(5)); if (queryResult == null || queryResult.isEmpty()) { return; } @@ -1314,7 +1309,7 @@ private void resolveWithSlicer(ITargetDefinition target, IProfile profile, IProg if (getIncludeSource()) { // Build an IU that represents all the source bundles and slice again to add them in if available IInstallableUnit sourceIU = createSourceIU(queryResult, Version.createOSGi(1, 0, 0)); - List units2 = new ArrayList<>(units); + List units2 = new ArrayList<>(units.keySet()); units2.add(sourceIU); queryResult = slice(units2, allMetadata, target, subMonitor.split(5)); @@ -1336,9 +1331,10 @@ private void resolveWithSlicer(ITargetDefinition target, IProfile profile, IProg for (IInstallableUnit unit : newSet) { plan.addInstallableUnit(unit); } - for (IInstallableUnit unit : units) { + units.forEach((unit, versionDeclarations) -> { plan.setInstallableUnitProfileProperty(unit, PROP_INSTALLED_IU, Boolean.toString(true)); - } + plan.setInstallableUnitProfileProperty(unit, PROP_IU_VERSION_DECLARATION, versionDeclarations); + }); // remove all units that are in the current profile but not in the new slice Set toRemove = profile.query(QueryUtil.ALL_UNITS, null).toSet(); @@ -1526,20 +1522,23 @@ private void findProfileRepos(Set additionalRepos) { * @return the discovered IUs * @exception CoreException if unable to retrieve IU's */ - private Set getRootIUs(ITargetDefinition definition, IProgressMonitor monitor) + private Map getRootIUs(ITargetDefinition definition, IProgressMonitor monitor) throws CoreException { ITargetLocation[] containers = definition.getTargetLocations(); if (containers == null) { - return Set.of(); + return Map.of(); } SubMonitor subMonitor = SubMonitor.convert(monitor, Messages.IUBundleContainer_0, containers.length); MultiStatus status = new MultiStatus(PDECore.PLUGIN_ID, 0, Messages.IUBundleContainer_ProblemsLoadingRepositories); - Set result = new HashSet<>(); + Map result = new HashMap<>(); + // Collect all declared IUs and their version declaration. + // An IU may be declared multiple times with different versions for (ITargetLocation container : containers) { if (container instanceof IUBundleContainer iuContainer) { try { - result.addAll(iuContainer.getRootIUs(subMonitor.split(1))); + iuContainer.getRootIUs(subMonitor.split(1)) + .forEach((iu, versionDeclarations) -> addDeclaredVersions(result, iu, versionDeclarations)); } catch (CoreException e) { status.add(e.getStatus()); } @@ -1551,6 +1550,21 @@ private Set getRootIUs(ITargetDefinition definition, IProgress return result; } + private static final String VERSION_DECLARATION_SEPARATOR = ";"; //$NON-NLS-1$ + + private void addDeclaredVersions(Map result, IInstallableUnit iu, + Set versionDeclarations) { + String joindVersions = versionDeclarations.stream().map(VersionRange::toString) + .collect(Collectors.joining(VERSION_DECLARATION_SEPARATOR)); + result.merge(iu, joindVersions, (v1, v2) -> v1 + VERSION_DECLARATION_SEPARATOR + v2); + } + + private Stream parseVersions(IInstallableUnit unit, String versionList) { + return versionList == null // if null, a specific version was declared + ? Stream.of(new VersionRange(unit.getVersion(), true, unit.getVersion(), true)) + : Arrays.stream(versionList.split(VERSION_DECLARATION_SEPARATOR)).map(VersionRange::create); + } + /** * Returns the repositories to consider when resolving IU's (will return default set of * repositories if current repository settings are null). diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java index d99ca4b00b..a63ae635e6 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java @@ -61,8 +61,6 @@ import org.eclipse.e4.core.services.events.IEventBroker; import org.eclipse.equinox.frameworkadmin.BundleInfo; import org.eclipse.equinox.p2.metadata.IInstallableUnit; -import org.eclipse.equinox.p2.metadata.IVersionedId; -import org.eclipse.equinox.p2.metadata.VersionedId; import org.eclipse.osgi.service.datalocation.Location; import org.eclipse.osgi.util.NLS; import org.eclipse.pde.core.plugin.IPluginModelBase; @@ -78,6 +76,7 @@ import org.eclipse.pde.internal.core.PDEPreferencesManager; import org.eclipse.pde.internal.core.TargetDefinitionManager; import org.eclipse.pde.internal.core.TargetPlatformHelper; +import org.eclipse.pde.internal.core.target.IUBundleContainer.UnitDeclaration; import org.osgi.service.prefs.BackingStoreException; /** @@ -646,7 +645,8 @@ public IStatus compareWithTargetPlatform(ITargetDefinition target) throws CoreEx @Override public ITargetLocation newIULocation(IInstallableUnit[] units, URI[] repositories, int resolutionFlags) { - Stream ius = Arrays.stream(units).map(iu -> new VersionedId(iu.getId(), iu.getVersion())); + Stream ius = Arrays.stream(units) + .map(iu -> UnitDeclaration.create(iu.getId(), iu.getVersion())); return createIUBundleContainer(ius, repositories, resolutionFlags); } @@ -655,12 +655,12 @@ public ITargetLocation newIULocation(String[] unitIds, String[] versions, URI[] if (unitIds.length != versions.length) { throw new IllegalArgumentException("Units and versions must have the same length"); //$NON-NLS-1$ } - Stream ius = IntStream.range(0, unitIds.length) - .mapToObj(i -> new VersionedId(unitIds[i], versions[i])); + Stream ius = IntStream.range(0, unitIds.length) + .mapToObj(i -> UnitDeclaration.parse(unitIds[i], versions[i])); return createIUBundleContainer(ius, repositories, resolutionFlags); } - private ITargetLocation createIUBundleContainer(Stream ius, URI[] repos, int resolutionFlags) { + private ITargetLocation createIUBundleContainer(Stream ius, URI[] repos, int resolutionFlags) { return new IUBundleContainer(ius.toList(), repos == null ? List.of() : List.of(repos), resolutionFlags); } diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/IUBundleContainerTests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/IUBundleContainerTests.java index 637f129c4b..7b7fa0010d 100644 --- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/IUBundleContainerTests.java +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/IUBundleContainerTests.java @@ -190,6 +190,30 @@ public void testResolveSingleBundle() throws Exception { doResolutionTest(new String[]{"bundle.a1"}, bundles); } + @Test + public void testResolveUnitWithoutVersion() throws Exception { + URI uri = getURI("/tests/sites/site.a.b"); + String[] ids = { "feature.a.feature.group" }; + String[] versions = { "" }; + ITargetLocation container = getTargetService().newIULocation(ids, versions, new URI[] { uri }, + IUBundleContainer.INCLUDE_REQUIRED); + doResolutionTest(container, new String[] { "bundle.a1", "bundle.a2", "bundle.a3" }); + } + + @Test + public void testResolveUnitWithVersionRange() throws Exception { + URI uri = getURI("/tests/sites/site.a.b"); + String[] ids = { "feature.a.feature.group" }; + String[] versions = new String[] { "[1.0,1.1)" }; + ITargetLocation container = getTargetService().newIULocation(ids, versions, new URI[] { uri }, + IUBundleContainer.INCLUDE_REQUIRED); + doResolutionTest(container, new String[] { "bundle.a1", "bundle.a2", "bundle.a3" }); + versions = new String[] { "[2.0,3.0)" }; + ITargetLocation container2 = getTargetService().newIULocation(ids, versions, new URI[] { uri }, + IUBundleContainer.INCLUDE_REQUIRED); + doResolutionTest(container2, new String[] {}); + } + /** * Tests whether the in-memory artifact repository is correctly created from * a non-IU target location. @@ -302,8 +326,11 @@ public void testContentNotEqualNull() throws Exception { * @param bundleIds symbolic names of bundles that should be present after resolution */ protected void doResolutionTest(String[] unitIds, String[] bundleIds) throws Exception { + doResolutionTest(createContainer(unitIds), bundleIds); + } + + private void doResolutionTest(ITargetLocation container, String[] bundleIds) throws Exception { try { - IUBundleContainer container = createContainer(unitIds); ITargetDefinition target = getTargetService().newTarget(); target.setTargetLocations(new ITargetLocation[]{container}); List infos = getAllBundleInfos(target); @@ -314,9 +341,13 @@ protected void doResolutionTest(String[] unitIds, String[] bundleIds) throws Exc assertTrue("Missing: " + bundleId, names.contains(bundleId)); } List profiles = P2TargetUtils.cleanOrphanedTargetDefinitionProfiles(); - assertEquals(1, profiles.size()); - String id = profiles.get(0); - assertTrue("Unexpected profile GC'd", id.endsWith(target.getHandle().getMemento())); + if (bundleIds.length > 0) { + assertEquals(1, profiles.size()); + String id = profiles.get(0); + assertTrue("Unexpected profile GC'd", id.endsWith(target.getHandle().getMemento())); + } else { + assertEquals(0, profiles.size()); + } } finally { // Always clean any profiles, even if the test failed to prevent cascading failures P2TargetUtils.cleanOrphanedTargetDefinitionProfiles(); @@ -602,6 +633,48 @@ public void testSerialization3() throws Exception { deserializationTest(location); } + @Test + public void testSerializationVersionRange() throws Exception { + URI uri = getURI("/tests/sites/site.a.b"); + String[] unitIds = { "feature.b.feature.group" }; + String[] versions = { "[1.0,1.1)" }; + IUBundleContainer location = (IUBundleContainer) getTargetService().newIULocation(unitIds, versions, + new URI[] { uri }, 0); + String xml = location.serialize(); + assertToken(xml, "version=\"", "[1.0.0,1.1.0)"); + assertIncludeMode(xml, "slicer"); + deserializationTest(location); + } + + @Test + public void testSerializationNoVersion() throws Exception { + URI uri = getURI("/tests/sites/site.a.b"); + String[] unitIds = { "feature.b.feature.group" }; + String[] versions = { "" }; + IUBundleContainer location = (IUBundleContainer) getTargetService().newIULocation(unitIds, versions, + new URI[] { uri }, 0); + String xml = location.serialize(); + assertFalse("No version declaration expected", xml.contains("version=")); + assertIncludeMode(xml, "slicer"); + deserializationTest(location); + } + + @Test + public void testSerializationEmptyVersion() throws Exception { + // Ensure declared empty versions are preserved. If one day no version + // is supported for long enough, empty versions could be cleaned up to + // no version automatically. + URI uri = getURI("/tests/sites/site.a.b"); + String[] unitIds = { "feature.b.feature.group" }; + String[] versions = { "0.0.0" }; + IUBundleContainer location = (IUBundleContainer) getTargetService().newIULocation(unitIds, versions, + new URI[] { uri }, 0); + String xml = location.serialize(); + assertToken(xml, "version=\"", "0.0.0"); + assertIncludeMode(xml, "slicer"); + deserializationTest(location); + } + public void deserializationTest(IUBundleContainer location) throws Exception { ITargetDefinition td = getTargetService().newTarget(); td.setTargetLocations(new ITargetLocation[] {location}); diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/EditIUContainerPage.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/EditIUContainerPage.java index 3dc0f6f519..872db39625 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/EditIUContainerPage.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/EditIUContainerPage.java @@ -18,8 +18,11 @@ import java.net.URI; import java.net.URISyntaxException; +import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import org.eclipse.core.runtime.CoreException; @@ -28,14 +31,22 @@ import org.eclipse.equinox.internal.p2.ui.ProvUI; import org.eclipse.equinox.internal.p2.ui.ProvUIMessages; import org.eclipse.equinox.internal.p2.ui.dialogs.AvailableIUGroup; +import org.eclipse.equinox.internal.p2.ui.dialogs.ContainerCheckedTreeViewer; import org.eclipse.equinox.internal.p2.ui.dialogs.RepositorySelectionGroup; +import org.eclipse.equinox.internal.p2.ui.model.AvailableIUElement; import org.eclipse.equinox.p2.metadata.IInstallableUnit; import org.eclipse.equinox.p2.operations.ProvisioningSession; import org.eclipse.equinox.p2.ui.Policy; import org.eclipse.equinox.p2.ui.ProvisioningUI; import org.eclipse.jface.action.IAction; import org.eclipse.jface.dialogs.IDialogSettings; +import org.eclipse.jface.viewers.CellEditor; +import org.eclipse.jface.viewers.ColumnLabelProvider; +import org.eclipse.jface.viewers.EditingSupport; import org.eclipse.jface.viewers.StructuredSelection; +import org.eclipse.jface.viewers.StructuredViewer; +import org.eclipse.jface.viewers.TextCellEditor; +import org.eclipse.jface.viewers.TreeViewerColumn; import org.eclipse.jface.window.SameShellProvider; import org.eclipse.jface.wizard.WizardPage; import org.eclipse.osgi.util.NLS; @@ -45,6 +56,7 @@ import org.eclipse.pde.internal.core.PDECore; import org.eclipse.pde.internal.core.target.IUBundleContainer; import org.eclipse.pde.internal.core.target.P2TargetUtils; +import org.eclipse.pde.internal.core.util.VersionUtil; import org.eclipse.pde.internal.ui.IHelpContextIds; import org.eclipse.pde.internal.ui.PDEPlugin; import org.eclipse.pde.internal.ui.SWTFactory; @@ -58,6 +70,7 @@ import org.eclipse.swt.widgets.Text; import org.eclipse.swt.widgets.TreeItem; import org.eclipse.ui.PlatformUI; +import org.osgi.framework.Version; /** * Wizard page allowing users to select which IUs they would like to download @@ -80,6 +93,9 @@ public class EditIUContainerPage extends WizardPage implements IEditBundleContai private static final int REFRESH_INTERVAL = 4000; private static final int REFRESH_TRIES = 10; + private static final String EMPTY_VERSION = Version.emptyVersion.toString(); + private static final String LATEST_LABEL = Messages.EditIUContainerPage_Latest_Label; + /** * If the user is only downloading from a specific repository location, we store it here so it can be persisted in the target */ @@ -103,6 +119,7 @@ public class EditIUContainerPage extends WizardPage implements IEditBundleContai private RepositorySelectionGroup fRepoSelector; private AvailableIUGroup fAvailableIUGroup; + private Map versionSpecifications = new HashMap<>(); private Label fSelectionCount; private Button fPropertiesButton; private IAction fPropertyAction; @@ -158,8 +175,24 @@ public ITargetLocation getBundleContainer() { flags |= fIncludeSourceButton.getSelection() ? IUBundleContainer.INCLUDE_SOURCE : 0; flags |= fConfigurePhaseButton.getSelection() ? IUBundleContainer.INCLUDE_CONFIGURE_PHASE : 0; flags |= fFollowRepositoryReferencesButton.getSelection() ? IUBundleContainer.FOLLOW_REPOSITORY_REFERENCES : 0; - return service.newIULocation(fAvailableIUGroup.getCheckedLeafIUs(), - fRepoLocation != null ? new URI[] { fRepoLocation } : null, flags); + + IInstallableUnit[] selectedIUs = fAvailableIUGroup.getCheckedLeafIUs(); + URI[] repos = fRepoLocation != null ? new URI[] { fRepoLocation } : null; + versionSpecifications.values().removeIf(String::isBlank); + if (!versionSpecifications.isEmpty()) { + List ids = new ArrayList<>(selectedIUs.length); + List versions = new ArrayList<>(selectedIUs.length); + for (IInstallableUnit iu : selectedIUs) { + ids.add(iu.getId()); + String version = versionSpecifications.get(iu); + if (version == null || version.isBlank()) { + version = iu.getVersion().toString(); + } + versions.add(version); + } + return service.newIULocation(ids.toArray(String[]::new), versions.toArray(String[]::new), repos, flags); + } + return service.newIULocation(selectedIUs, repos, flags); } @Override @@ -237,7 +270,7 @@ private void refreshAvailableIUArea(final Composite parent) { final String pendingLabel = org.eclipse.ui.internal.progress.ProgressMessages.PendingUpdateAdapter_PendingLabel; if (children.length > 0 && !children[0].getText().equals(pendingLabel)) { fAvailableIUGroup.getCheckboxTreeViewer().expandAll(); - fAvailableIUGroup.setChecked(fEditContainer.getInstallableUnits().toArray()); + setInstallableUnits(fEditContainer); fAvailableIUGroup.getCheckboxTreeViewer().collapseAll(); loaded.set(true); } @@ -262,7 +295,69 @@ private void createAvailableIUArea(Composite parent) { if (!profileUI.getPolicy().getRepositoriesVisible()) { filterConstant = AvailableIUGroup.AVAILABLE_ALL; } - fAvailableIUGroup = new AvailableIUGroup(profileUI, parent, parent.getFont(), fQueryContext, null, filterConstant); + fAvailableIUGroup = new AvailableIUGroup(profileUI, parent, parent.getFont(), fQueryContext, null, + filterConstant) { + @Override + protected StructuredViewer createViewer(Composite parent) { + ContainerCheckedTreeViewer treeViewer = (ContainerCheckedTreeViewer) super.createViewer(parent); + TreeViewerColumn column = new TreeViewerColumn(treeViewer, SWT.NONE, getColumnConfig().length); + column.getColumn().setText(Messages.EditIUContainerPage_VersionSpecification_Label); + column.getColumn().setWidth(150); + column.getColumn().setResizable(true); + CellEditor versionSpecEditor = new TextCellEditor(treeViewer.getTree()); + versionSpecEditor.setValidator(this::validateVersionSpecification); + column.setEditingSupport(new EditingSupport(treeViewer) { + @Override + @SuppressWarnings("restriction") + protected void setValue(Object element, Object value) { + if (element instanceof AvailableIUElement iuElement && value instanceof String spec) { + spec = sanitizeVersionSpecification(spec); + versionSpecifications.put(iuElement.getIU(), spec); + treeViewer.update(iuElement, null); + } + } + + @Override + protected Object getValue(Object element) { + return getVersionSpecification(element); + } + + @Override + protected boolean canEdit(Object element) { + return element instanceof @SuppressWarnings("restriction") AvailableIUElement iuElement + && treeViewer.getChecked(iuElement); + } + + @Override + protected CellEditor getCellEditor(Object element) { + return versionSpecEditor; + } + }); + column.setLabelProvider(ColumnLabelProvider.createTextProvider(this::getVersionSpecification)); + return treeViewer; + } + + private static String sanitizeVersionSpecification(String spec) { + spec = spec.strip(); + return LATEST_LABEL.equals(spec) ? EMPTY_VERSION : spec; + } + + @SuppressWarnings("restriction") + private String getVersionSpecification(Object e) { + String spec = e instanceof AvailableIUElement iu // + ? versionSpecifications.getOrDefault(iu.getIU(), "") //$NON-NLS-1$ + : ""; //$NON-NLS-1$ + return EMPTY_VERSION.equals(spec) ? LATEST_LABEL : spec; + } + + private String validateVersionSpecification(Object value) { + if (LATEST_LABEL.equals(value)) { + return null; + } + IStatus result = VersionUtil.validateVersionRange((String) value); + return result.isOK() ? null : result.getMessage(); + } + }; fAvailableIUGroup.getCheckboxTreeViewer().addCheckStateListener(event -> { IInstallableUnit[] units = fAvailableIUGroup.getCheckedLeafIUs(); if (units.length > 0) { @@ -612,7 +707,7 @@ private void restoreWidgetState() { // Only able to check items if we don't have categories fQueryContext.setViewType(org.eclipse.equinox.internal.p2.ui.query.IUViewQueryContext.AVAILABLE_VIEW_FLAT); fAvailableIUGroup.updateAvailableViewState(); - fAvailableIUGroup.setChecked(fEditContainer.getInstallableUnits().toArray()); + setInstallableUnits(fEditContainer); // Make sure view is back in proper state updateViewContext(); IInstallableUnit[] units = fAvailableIUGroup.getCheckedLeafIUs(); @@ -625,4 +720,9 @@ private void restoreWidgetState() { fAvailableIUGroup.getCheckboxTreeViewer().collapseAll(); } } + + private void setInstallableUnits(IUBundleContainer iuContainer) { + versionSpecifications = new HashMap<>(iuContainer.getInstallableUnitSpecifications()); + fAvailableIUGroup.setChecked(versionSpecifications.keySet().toArray()); + } } \ No newline at end of file diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/Messages.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/Messages.java index c5e186604a..a245d5dbb5 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/Messages.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/Messages.java @@ -104,6 +104,8 @@ public class Messages extends NLS { public static String EditIUContainerPage_IncludeConfigurePhase; public static String EditIUContainerPage_itemSelected; public static String EditIUContainerPage_itemsSelected; + public static String EditIUContainerPage_VersionSpecification_Label; + public static String EditIUContainerPage_Latest_Label; public static String EditProfileContainerPage_1; public static String EditProfileContainerPage_2; public static String EditProfileContainerPage_3; diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/messages.properties b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/messages.properties index 85cd2ce029..671541550e 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/messages.properties +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/messages.properties @@ -99,6 +99,8 @@ EditIUContainerPage_9=The target platform service could not be acquired. EditIUContainerPage_IncludeConfigurePhase=Include configure phase EditIUContainerPage_itemSelected={0} item selected EditIUContainerPage_itemsSelected={0} items selected +EditIUContainerPage_VersionSpecification_Label=Version specification +EditIUContainerPage_Latest_Label=latest EditProfileContainerPage_1=Var&iables... EditProfileContainerPage_2=Select a configuration directory EditProfileContainerPage_3=Edit Installation