Skip to content

Commit

Permalink
Support no version and version ranges in target definitions
Browse files Browse the repository at this point in the history 
Fixes eclipse-pde#757
TODO: search for more issues?!
  • Loading branch information
@HannesWell
HannesWell committed Apr 21, 2024
1 parent c245bb9 commit 42ebff6
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 40 deletions.
128 changes: 104 additions & 24 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IUBundleContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,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;
Expand All @@ -78,6 +77,56 @@
*/
public class IUBundleContainer extends AbstractBundleContainer {

record UnitDescription(String id, VersionRange version) {

static UnitDescription parse(String id, String version) {
VersionRange range;
if (Version.emptyVersion.toString().equals(version)) {
range = VersionRange.emptyRange;
} else if (version.contains(",")) { //$NON-NLS-1$
range = VersionRange.create(version);
} else {
range = VersionRange.create('[' + version + ',' + version + ']');
}
return new UnitDescription(id, range);
}

static UnitDescription create(String id, Version version) {
VersionRange range = new VersionRange(version, true, version, true);
return new UnitDescription(id, range);
}

UnitDescription {
Objects.requireNonNull(id);
Objects.requireNonNull(version);
}

private boolean isSingleVersion() {
// TODO: also check bounds? if not both open, then it would be empty
// anyways...
return version.equals(VersionRange.emptyRange) || version.getMaximum().equals(version.getMinimum());
}

Optional<Version> getSingleVersion() {
return isSingleVersion() ? Optional.of(version.getMinimum()) : Optional.empty();
}

IQuery<IInstallableUnit> createIUQuery() {
return isSingleVersion() //
? QueryUtil.createIUQuery(id, version.getMinimum())
: QueryUtil.createIUQuery(id, version);
}

String versionString() {
return getSingleVersion().map(Version::toString).orElseGet(version()::toString);
}

@Override
public String toString() {
return VersionRange.emptyRange.equals(version) ? id : id + '/' + versionString();
}
}

/**
* Constant describing the type of bundle container
*/
Expand Down Expand Up @@ -114,7 +163,7 @@ public class IUBundleContainer extends AbstractBundleContainer {
public static final int INCLUDE_CONFIGURE_PHASE = 1 << 3;

/** The list of id+version of all root units declared in this container. */
private List<IVersionedId> fIUs;
private List<UnitDescription> fIUs;

/**
* Cached IU's referenced by this bundle container, or <code>null</code> if not
Expand Down Expand Up @@ -151,7 +200,7 @@ public class IUBundleContainer extends AbstractBundleContainer {
* @param repositories metadata repositories used to search for IU's or <code>null</code> for default set
* @param resolutionFlags bitmask of flags to control IU resolution, possible flags are {@link IUBundleContainer#INCLUDE_ALL_ENVIRONMENTS}, {@link IUBundleContainer#INCLUDE_REQUIRED}, {@link IUBundleContainer#INCLUDE_SOURCE}, {@link IUBundleContainer#INCLUDE_CONFIGURE_PHASE}
*/
IUBundleContainer(Collection<IVersionedId> units, List<URI> repositories, int resolutionFlags) {
IUBundleContainer(Collection<UnitDescription> units, List<URI> repositories, int resolutionFlags) {
fIUs = new ArrayList<>(units);
fFlags = resolutionFlags;
fRepos = List.copyOf(repositories);
Expand Down Expand Up @@ -251,8 +300,8 @@ private void cacheIUs() throws CoreException {
IProfile profile = fSynchronizer.getProfile();
List<IInstallableUnit> result = new ArrayList<>();
MultiStatus status = new MultiStatus(PDECore.PLUGIN_ID, 0, Messages.IUBundleContainer_ProblemsLoadingRepositories, null);
for (IVersionedId unit : fIUs) {
IQuery<IInstallableUnit> query = QueryUtil.createIUQuery(unit);
for (UnitDescription unit : fIUs) {
IQuery<IInstallableUnit> query = unit.createIUQuery();
addElement(profile, query, unit, result, status);
}
if (!status.isOK()) {
Expand All @@ -262,8 +311,8 @@ private void cacheIUs() throws CoreException {
fUnits = List.copyOf(result);
}

private void addElement(IQueryable<IInstallableUnit> queryable, IQuery<IInstallableUnit> query, IVersionedId iu,
List<IInstallableUnit> result, MultiStatus status) {
private void addElement(IQueryable<IInstallableUnit> queryable, IQuery<IInstallableUnit> query,
UnitDescription iu, List<IInstallableUnit> result, MultiStatus status) {
Optional<IInstallableUnit> queryResult = queryFirst(queryable, query, null);
if (queryResult.isEmpty()) {
status.add(Status.error(NLS.bind(Messages.IUBundleContainer_1, iu)));
Expand Down Expand Up @@ -367,25 +416,36 @@ public synchronized IUBundleContainer update(Set<String> toUpdate, IProgressMoni
SubMonitor progress = SubMonitor.convert(monitor, 100);
IQueryable<IInstallableUnit> source = P2TargetUtils.getQueryableMetadata(fRepos, progress.split(30));
boolean updated = false;
List<IVersionedId> updatedUnits = new ArrayList<>(fIUs);
List<UnitDescription> 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())) {
UnitDescription unit = updatedUnits.get(i);
if (!toUpdate.isEmpty() && !toUpdate.contains(unit.id())) {
continue;
}
IQuery<IInstallableUnit> query = QueryUtil.createLatestQuery(QueryUtil.createIUQuery(unit.getId()));
// TODO: why is createLatestQuery() used here but not in the other
// caller ?
IQuery<IInstallableUnit> query = QueryUtil.createLatestQuery(QueryUtil.createIUQuery(unit.id()));
Optional<IInstallableUnit> 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())) {
// FIXME: check the new logic here again?!
Optional<Version> singleVersion = unit.getSingleVersion();

if (singleVersion.isPresent() && !updatedVersion.equals(singleVersion.get())) {
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 (!singleVersion.get().equals(Version.emptyVersion)) {
updatedUnits.set(i, UnitDescription.create(unit.id(), updatedVersion));
}
} else {
// TODO: how to update version ranges properly. There are
// different strategies possible.
// E.g. compute its 'span' and apply it to the updated version
// as lower bound
throw new UnsupportedOperationException();
}
}
if (updated) {
Expand Down Expand Up @@ -488,7 +548,14 @@ public List<URI> 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()));
// TODO: TODO: maybe the units list can be converted to a map and map
// each unit to the declaring IU (if its not a dependency). Then we get
// an exact matched version for the range. Also consider this for the
// update method.
Optional<UnitDescription> toRemove = fIUs.stream()
.filter(iu -> iu.id().equals(unit.getId()) && iu.version().isIncluded(unit.getVersion())).findFirst();
toRemove.ifPresent(fIUs::remove);
// fIUs.remove(new VersionedId(unit.getId(), unit.getVersion()));
// Need to mark the container as unresolved
clearResolutionStatus();
}
Expand Down Expand Up @@ -565,7 +632,7 @@ public List<IInstallableUnit> getInstallableUnits() throws CoreException {
}

/** Returns installable unit identifiers and versions. */
List<IVersionedId> getUnits() {
List<UnitDescription> getUnits() {
return Collections.unmodifiableList(fIUs);
}

Expand Down Expand Up @@ -605,9 +672,17 @@ protected void associateWithTarget(ITargetDefinition target) {
fSynchronizer.setIncludeConfigurePhase((fFlags & INCLUDE_CONFIGURE_PHASE) == INCLUDE_CONFIGURE_PHASE);
}

private static final Comparator<IVersionedId> ID_FIRST_VERSION_SECOND = Comparator //
.comparing(IVersionedId::getId) //
.thenComparing(IVersionedId::getVersion);
private static final Comparator<UnitDescription> ID_FIRST_VERSION_SECOND = Comparator //
.comparing(UnitDescription::id) //
.thenComparing(unit -> {
Optional<Version> singleVersion = unit.getSingleVersion();
if (singleVersion.isEmpty()) {
return singleVersion.get();
} else {
// TODO: better solution? E.g. prefer version over ranges?
return unit.version().getMinimum();
}
});

@Override
public String serialize() {
Expand Down Expand Up @@ -636,8 +711,8 @@ public String serialize() {
// Generate a predictable order of the elements
fIUs.stream().sorted(ID_FIRST_VERSION_SECOND).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());
unit.setAttribute(TargetDefinitionPersistenceHelper.ATTR_VERSION, iu.versionString());
containerElement.appendChild(unit);
});
try {
Expand All @@ -659,9 +734,9 @@ IInstallableUnit[] getRootIUs(IProgressMonitor monitor) throws CoreException {
IQueryable<IInstallableUnit> repos = P2TargetUtils.getQueryableMetadata(getRepositories(), monitor);
MultiStatus status = new MultiStatus(PDECore.PLUGIN_ID, 0, Messages.IUBundleContainer_ProblemsLoadingRepositories, null);
List<IInstallableUnit> result = new ArrayList<>();
for (IVersionedId iu : fIUs) {
for (UnitDescription iu : fIUs) {
// For versions such as 0.0.0, the IU query may return multiple IUs, so we check which is the latest version
IQuery<IInstallableUnit> query = QueryUtil.createLatestQuery(QueryUtil.createIUQuery(iu));
IQuery<IInstallableUnit> query = QueryUtil.createLatestQuery(iu.createIUQuery());
addElement(repos, query, iu, result, status);
}
if (!status.isOK()) {
Expand All @@ -671,6 +746,11 @@ IInstallableUnit[] getRootIUs(IProgressMonitor monitor) throws CoreException {
return result.toArray(new IInstallableUnit[0]);
}

// // FIXME: check discrepancy?! Is this even necessary?! We have a
// // specific version why the latest? In case the version is 0.0.0! But
// // isn't that also interesting fpr the other caller?!
// IQuery<IInstallableUnit> query = QueryUtil.createLatestQuery(iuQuery);

@Override
public <T> T getAdapter(Class<T> adapter) {
if (adapter.isInstance(fSynchronizer)) {
Expand Down
11 changes: 6 additions & 5 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IULocationFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.Status;
import org.eclipse.equinox.p2.metadata.Version;
import org.eclipse.pde.core.target.ITargetLocation;
import org.eclipse.pde.core.target.ITargetLocationFactory;
import org.w3c.dom.Document;
Expand Down Expand Up @@ -72,10 +73,11 @@ public ITargetLocation getTargetLocation(String type, String serializedXML) thro
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);
if (version.isBlank()) {
version = Version.emptyVersion.toString();
}
ids.add(id);
versions.add(version);
}
} else if (element.getNodeName().equalsIgnoreCase(TargetDefinitionPersistenceHelper.REPOSITORY)) {
String loc = element.getAttribute(TargetDefinitionPersistenceHelper.LOCATION);
Expand All @@ -101,8 +103,7 @@ public ITargetLocation getTargetLocation(String type, String serializedXML) thro
flags |= Boolean.parseBoolean(includeAllPlatforms) ? IUBundleContainer.INCLUDE_ALL_ENVIRONMENTS : 0;
flags |= Boolean.parseBoolean(includeSource) ? IUBundleContainer.INCLUDE_SOURCE : 0;
flags |= Boolean.parseBoolean(includeConfigurePhase) ? IUBundleContainer.INCLUDE_CONFIGURE_PHASE : 0;
return TargetPlatformService.getDefault().newIULocation(iuIDs, iuVer, uris,
flags);
return TargetPlatformService.getDefault().newIULocation(iuIDs, iuVer, uris, flags);
}
return null;
}
Expand Down
18 changes: 12 additions & 6 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
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;
Expand All @@ -87,9 +86,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.UnitDescription;
import org.eclipse.pde.internal.core.util.CoreUtility;
import org.osgi.framework.BundleContext;
import org.osgi.framework.InvalidSyntaxException;
Expand Down Expand Up @@ -518,19 +517,26 @@ private boolean checkProfile(ITargetDefinition target, final IProfile profile) {
// still in the profile, we need to recreate (rather than uninstall)
IUProfilePropertyQuery propertyQuery = new IUProfilePropertyQuery(PROP_INSTALLED_IU, Boolean.toString(true));
IQueryResult<IInstallableUnit> queryResult = profile.query(propertyQuery, null);
Set<NameVersionDescriptor> installedIUs = new HashSet<>();
Map<String, List<IInstallableUnit>> installedIUs = new HashMap<>();
for (IInstallableUnit unit : queryResult) {
installedIUs.add(new NameVersionDescriptor(unit.getId(), unit.getVersion().toString()));
installedIUs.computeIfAbsent(unit.getId(), id -> new ArrayList<>(1)).add(unit);
}
ITargetLocation[] containers = target.getTargetLocations();
if (containers == null) {
return installedIUs.isEmpty();
}
for (ITargetLocation container : containers) {
if (container instanceof IUBundleContainer bc) {
for (IVersionedId iu : bc.getUnits()) {
for (UnitDescription iu : bc.getUnits()) {
// if there is something in a container but not in the profile, recreate
if (!installedIUs.remove(new NameVersionDescriptor(iu.getId(), iu.getVersion().toString()))) {
boolean[] removed = { false };
installedIUs.computeIfPresent(iu.id(), (id, units) -> {
// TODO: what happens if there are two ranges, one more
// specific and one wider that match different IUs?
removed[0] = units.removeIf(u -> iu.version().isIncluded(u.getVersion()));
return units.isEmpty() ? null : units;
});
if (!removed[0]) {
return false;
}
}
Expand Down
10 changes: 5 additions & 5 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -77,6 +75,7 @@
import org.eclipse.pde.internal.core.PDECore;
import org.eclipse.pde.internal.core.PDEPreferencesManager;
import org.eclipse.pde.internal.core.TargetDefinitionManager;
import org.eclipse.pde.internal.core.target.IUBundleContainer.UnitDescription;
import org.osgi.service.prefs.BackingStoreException;

/**
Expand Down Expand Up @@ -644,7 +643,8 @@ public IStatus compareWithTargetPlatform(ITargetDefinition target) throws CoreEx

@Override
public ITargetLocation newIULocation(IInstallableUnit[] units, URI[] repositories, int resolutionFlags) {
Stream<IVersionedId> ius = Arrays.stream(units).map(iu -> new VersionedId(iu.getId(), iu.getVersion()));
Stream<UnitDescription> ius = Arrays.stream(units)
.map(iu -> UnitDescription.create(iu.getId(), iu.getVersion()));
return new IUBundleContainer(ius.toList(), List.of(repositories), resolutionFlags);
}

Expand All @@ -653,8 +653,8 @@ 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<IVersionedId> ius = IntStream.range(0, unitIds.length)
.mapToObj(i -> new VersionedId(unitIds[i], versions[i]));
Stream<UnitDescription> ius = IntStream.range(0, unitIds.length)
.mapToObj(i -> UnitDescription.parse(unitIds[i], versions[i]));
return new IUBundleContainer(ius.toList(), List.of(repositories), resolutionFlags);
}

Expand Down

0 comments on commit 42ebff6

Please sign in to comment.